Add LSP preflight, CLI override, and examples

haasonsaas · haasonsaas · commit 7ab95a5ef3b5 · 2025-12-25T09:33:00.000-08:00
diff --git a/FEATURES.md b/FEATURES.md
@@ -96,4 +96,10 @@ diffscope smart-review --old-file src/old.py --new-file src/new.py
 4. **Prioritized Workflow**: Issues ranked by severity and effort for efficient fixing
 5. **Professional Reporting**: Executive summaries suitable for team reviews
 
-This enhanced system provides professional-grade code review capabilities while maintaining the simplicity and flexibility of the original diffscope architecture.
+This enhanced system provides professional-grade code review capabilities while maintaining the simplicity and flexibility of the original diffscope architecture.
+
+## Symbol Indexing
+
+- Regex and LSP-backed symbol indexing with auto-detection when `symbol_index_lsp_command` is omitted.
+- LSP preflight command: `diffscope lsp-check` validates binaries, language IDs, and unmapped extensions.
+- Ready-to-copy LSP configs in `examples/lsp/` and setup guide in `docs/lsp.md`.
diff --git a/README.md b/README.md
@@ -223,7 +223,7 @@ exclude_patterns:
   - "**/__pycache__/**"
 ```
 
-Set `symbol_index_provider: lsp` to use a language server; it falls back to regex indexing if the LSP binary is missing. Configure `symbol_index_lsp_languages` and `symbol_index_lsp_command` to match your server (for example, `typescript-language-server --stdio` with `ts`/`tsx` language IDs). If you omit `symbol_index_lsp_command`, diffscope will try to auto-detect a server based on installed binaries and the file types in your repo.
+Set `symbol_index_provider: lsp` to use a language server; it falls back to regex indexing if the LSP binary is missing. Configure `symbol_index_lsp_languages` and `symbol_index_lsp_command` to match your server (for example, `typescript-language-server --stdio` with `ts`/`tsx` language IDs). If you omit `symbol_index_lsp_command`, diffscope will try to auto-detect a server based on installed binaries and the file types in your repo. You can also force a server for a single run with `--lsp-command`.
 
 ### LSP Symbol Index Examples (All Common Languages)
 
@@ -300,7 +300,13 @@ symbol_index_lsp_languages:
 
 ### LSP Setup Notes (Install + Command)
 
-For detailed install commands, OS-specific package manager options, and troubleshooting, see `docs/lsp.md`.
+For detailed install commands, OS-specific package manager options, and troubleshooting, see `docs/lsp.md`. For ready-made configs per language, see `examples/lsp/`.
+
+You can validate your setup with:
+
+```
+diffscope lsp-check
+```
 
 ## Plugin Development
 
@@ -627,6 +633,9 @@ diffscope pr [--number N] [--post-comments] [--summary]
 # Repository check (uncommitted changes at path)
 diffscope check [path]
 
+# LSP preflight checks
+diffscope lsp-check [path]
+
 # File comparison
 diffscope compare --old-file old.py --new-file new.py
 
diff --git a/docs/lsp.md b/docs/lsp.md
@@ -10,6 +10,12 @@ If `symbol_index_provider: lsp` and `symbol_index_lsp_command` is not set, diffs
 
 If it picks the wrong server, set `symbol_index_lsp_command` explicitly.
 
+## CLI Overrides
+
+- Force a command for this run: `diffscope --lsp-command "typescript-language-server --stdio" smart-review --diff diff.patch`
+- Preflight your setup: `diffscope lsp-check` (prints the detected command, whether it is available on PATH, and any unmapped extensions)
+- Ready-to-copy configs live in `examples/lsp/`.
+
 ## Package Manager Matrix (brew/apt/choco)
 
 Use these when you prefer OS-level packages. If a cell says "manual", use the language-specific install section below.
@@ -72,3 +78,4 @@ bin/jdtls -configuration /path/to/config -data /path/to/workspace
 - LSP binary not found: check PATH or use an absolute path in `symbol_index_lsp_command`.
 - Server expects stdio: use the `--stdio` form when required (e.g., TypeScript, Solargraph).
 - JDT LS needs a unique `-data` directory per workspace; configure a stable path.
+- Use `diffscope lsp-check` to see unmapped extensions and missing binaries.
diff --git a/examples/lsp/cpp.yml b/examples/lsp/cpp.yml
@@ -0,0 +1,11 @@
+# Diffscope LSP config for C/C++
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: clangd
+symbol_index_lsp_languages:
+  c: c
+  h: c
+  cc: cpp
+  cpp: cpp
+  cxx: cpp
+  hpp: cpp
diff --git a/examples/lsp/csharp.yml b/examples/lsp/csharp.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for C#
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: csharp-ls
+symbol_index_lsp_languages:
+  cs: csharp
diff --git a/examples/lsp/go.yml b/examples/lsp/go.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for Go
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: gopls
+symbol_index_lsp_languages:
+  go: go
diff --git a/examples/lsp/java.yml b/examples/lsp/java.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for Java
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: "jdtls -configuration /path/to/config -data /path/to/workspace"
+symbol_index_lsp_languages:
+  java: java
diff --git a/examples/lsp/kotlin.yml b/examples/lsp/kotlin.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for Kotlin
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: kotlin-lsp
+symbol_index_lsp_languages:
+  kt: kotlin
diff --git a/examples/lsp/php.yml b/examples/lsp/php.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for PHP
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: "phpactor language-server"
+symbol_index_lsp_languages:
+  php: php
diff --git a/examples/lsp/python.yml b/examples/lsp/python.yml
@@ -0,0 +1,7 @@
+# Diffscope LSP config for Python
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: pylsp
+symbol_index_lsp_languages:
+  py: python
+  pyi: python
diff --git a/examples/lsp/ruby.yml b/examples/lsp/ruby.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for Ruby
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: "solargraph stdio"
+symbol_index_lsp_languages:
+  rb: ruby
diff --git a/examples/lsp/rust.yml b/examples/lsp/rust.yml
@@ -0,0 +1,6 @@
+# Diffscope LSP config for Rust
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: rust-analyzer
+symbol_index_lsp_languages:
+  rs: rust
diff --git a/examples/lsp/typescript.yml b/examples/lsp/typescript.yml
@@ -0,0 +1,9 @@
+# Diffscope LSP config for TypeScript/JavaScript
+symbol_index: true
+symbol_index_provider: lsp
+symbol_index_lsp_command: "typescript-language-server --stdio"
+symbol_index_lsp_languages:
+  ts: typescript
+  tsx: typescriptreact
+  js: javascript
+  jsx: javascriptreact
diff --git a/src/core/symbol_index.rs b/src/core/symbol_index.rs
@@ -107,6 +107,34 @@ impl SymbolIndex {
         choose_lsp_command(&extension_counts, &enabled_extensions)
     }
 
+    pub fn scan_extension_counts<F>(
+        repo_root: &Path,
+        max_files: usize,
+        should_exclude: F,
+    ) -> HashMap<String, usize>
+    where
+        F: Fn(&PathBuf) -> bool,
+    {
+        if max_files == 0 {
+            return HashMap::new();
+        }
+        let enabled_extensions = HashSet::new();
+        collect_extension_counts(
+            repo_root,
+            max_files.min(LSP_DETECT_MAX_FILES),
+            &enabled_extensions,
+            should_exclude,
+        )
+    }
+
+    pub fn lsp_command_available(command: &str) -> bool {
+        let program = match command_program(command) {
+            Some(program) => program,
+            None => return false,
+        };
+        is_program_available(&program)
+    }
+
     pub fn build<F>(
         repo_root: &Path,
         max_files: usize,
@@ -437,6 +465,15 @@ fn choose_lsp_command(
     best_command.map(|command| command.to_string())
 }
 
+fn split_command(command: &str) -> Result<Vec<String>> {
+    shell_words::split(command).map_err(|err| anyhow::anyhow!(err.to_string()))
+}
+
+fn command_program(command: &str) -> Option<String> {
+    let parts = split_command(command).ok()?;
+    parts.first().cloned()
+}
+
 fn is_program_available(program: &str) -> bool {
     if program.trim().is_empty() {
         return false;
@@ -649,7 +686,7 @@ struct LspClient {
 
 impl LspClient {
     fn spawn(command: &str, root: &Path) -> Result<Self> {
-        let parts = shell_words::split(command).map_err(|err| anyhow::anyhow!(err.to_string()))?;
+        let parts = split_command(command)?;
         let (program, args) = parts
             .split_first()
             .ok_or_else(|| anyhow::anyhow!("Empty LSP command"))?;
diff --git a/src/main.rs b/src/main.rs
@@ -8,7 +8,7 @@ use clap::{Parser, Subcommand};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::io::IsTerminal;
 use std::path::{Path, PathBuf};
 use tracing::{info, warn};
@@ -47,6 +47,13 @@ struct Cli {
 
     #[arg(short, long, global = true)]
     verbose: bool,
+
+    #[arg(
+        long,
+        global = true,
+        help = "Force an LSP command for symbol indexing (enables LSP provider)"
+    )]
+    lsp_command: Option<String>,
 }
 
 #[derive(Subcommand)]
@@ -119,6 +126,11 @@ enum Commands {
         )]
         output: Option<PathBuf>,
     },
+    #[command(about = "Preflight LSP setup and configuration")]
+    LspCheck {
+        #[arg(default_value = ".")]
+        path: PathBuf,
+    },
     Feedback {
         #[arg(
             long,
@@ -184,6 +196,11 @@ async fn main() -> Result<()> {
     if let Some(flag) = cli.openai_responses {
         config.openai_use_responses = Some(flag);
     }
+    if let Some(command) = cli.lsp_command {
+        config.symbol_index = true;
+        config.symbol_index_provider = "lsp".to_string();
+        config.symbol_index_lsp_command = Some(command);
+    }
     config.normalize();
 
     match cli.command {
@@ -230,6 +247,9 @@ async fn main() -> Result<()> {
         } => {
             changelog_command(from, to, release, output).await?;
         }
+        Commands::LspCheck { path } => {
+            lsp_check_command(path, config).await?;
+        }
         Commands::Feedback {
             accept,
             reject,
@@ -459,6 +479,132 @@ async fn check_command(path: PathBuf, config: config::Config, format: OutputForm
     review_diff_content_with_repo(&diff_content, config, format, &repo_root).await
 }
 
+async fn lsp_check_command(path: PathBuf, config: config::Config) -> Result<()> {
+    let repo_root = core::GitIntegration::new(&path)
+        .ok()
+        .and_then(|git| git.workdir())
+        .unwrap_or(path);
+
+    println!("LSP health check");
+    println!("repo: {}", repo_root.display());
+    println!(
+        "symbol_index: {}",
+        if config.symbol_index {
+            "enabled"
+        } else {
+            "disabled"
+        }
+    );
+    println!("symbol_index_provider: {}", config.symbol_index_provider);
+    if !config.symbol_index {
+        println!("note: symbol_index is disabled (set symbol_index: true)");
+    }
+    if config.symbol_index_provider != "lsp" {
+        println!("note: symbol_index_provider is not lsp (set symbol_index_provider: lsp)");
+    }
+
+    let configured_command = config.symbol_index_lsp_command.clone();
+    let detected_command = if configured_command.is_none() {
+        core::SymbolIndex::detect_lsp_command(
+            &repo_root,
+            config.symbol_index_max_files,
+            &config.symbol_index_lsp_languages,
+            |path| config.should_exclude(path),
+        )
+    } else {
+        None
+    };
+
+    if let Some(command) = &configured_command {
+        println!("configured LSP command: {}", command);
+    }
+    if let Some(command) = &detected_command {
+        println!("detected LSP command: {}", command);
+    }
+
+    let effective_command = configured_command.or(detected_command);
+    if let Some(command) = &effective_command {
+        let available = core::SymbolIndex::lsp_command_available(command);
+        println!("effective LSP command: {}", command);
+        println!(
+            "command available: {}",
+            if available { "yes" } else { "no" }
+        );
+    } else {
+        println!("effective LSP command: <none>");
+        println!("command available: no");
+    }
+
+    let mut normalized_languages = HashMap::new();
+    let mut invalid_mappings = Vec::new();
+    for (ext, language) in &config.symbol_index_lsp_languages {
+        let ext = ext.trim().to_ascii_lowercase();
+        let language = language.trim().to_string();
+        if ext.is_empty() || language.is_empty() {
+            invalid_mappings.push(format!("{}:{}", ext, language));
+            continue;
+        }
+        normalized_languages.insert(ext, language);
+    }
+
+    if normalized_languages.is_empty() {
+        println!("language map: empty (set symbol_index_lsp_languages)");
+    } else {
+        println!("language map entries: {}", normalized_languages.len());
+    }
+    if !invalid_mappings.is_empty() {
+        println!(
+            "invalid language map entries: {}",
+            invalid_mappings.join(", ")
+        );
+    }
+
+    let extension_counts = core::SymbolIndex::scan_extension_counts(
+        &repo_root,
+        config.symbol_index_max_files,
+        |path| config.should_exclude(path),
+    );
+    if extension_counts.is_empty() {
+        println!("repo extensions: none detected (check path or excludes)");
+        return Ok(());
+    }
+
+    let mut extension_list: Vec<_> = extension_counts.iter().collect();
+    extension_list.sort_by(|(a_ext, a_count), (b_ext, b_count)| {
+        b_count.cmp(a_count).then_with(|| a_ext.cmp(b_ext))
+    });
+    let top_extensions: Vec<String> = extension_list
+        .iter()
+        .take(10)
+        .map(|(ext, count)| format!("{}({})", ext, count))
+        .collect();
+    println!("top extensions: {}", top_extensions.join(", "));
+
+    let mut unmapped = Vec::new();
+    for ext in extension_counts.keys() {
+        if !normalized_languages.contains_key(ext) {
+            unmapped.push(ext.clone());
+        }
+    }
+    unmapped.sort();
+    if !unmapped.is_empty() {
+        println!("unmapped repo extensions: {}", unmapped.join(", "));
+    }
+
+    let mut unused = Vec::new();
+    for ext in normalized_languages.keys() {
+        if !extension_counts.contains_key(ext) {
+            unused.push(ext.clone());
+        }
+    }
+    unused.sort();
+    if !unused.is_empty() {
+        println!("unused language map entries: {}", unused.join(", "));
+    }
+
+    Ok(())
+}
+
 async fn git_command(
     command: GitCommands,
     config: config::Config,
