diff --git a/README.md b/README.md index 2b65ba7..7bb523f 100644 --- a/README.md +++ b/README.md @@ -82,8 +82,9 @@ Protols is configured using a `protols.toml` file, which you can place in any di include_paths = ["foobar", "bazbaaz"] # Include paths to look for protofiles during parsing disable_parse_diagnostics = true # Disable diagnostics for parsing -[config.experimental] # Experimental configuration; this should be considered unsafe and not fully tested -use_protoc_diagnostics = true # Use diagnostics from protoc +[config.experimental] # experimental configuration; this should be considered unsafe and not fully tested +use_protoc_diagnostics = true # use diagnostics from protoc +protoc_path = "protoc" # Path to proto compiler (protoc) [formatter] # Formatter specific configuration clang_format_path = "/usr/bin/clang-format" # clang-format binary to execute in formatting @@ -96,13 +97,14 @@ clang_format_path = "/usr/bin/clang-format" # clang-format binary to execute in The `[config]` section contains stable settings that should generally remain unchanged. - `include_paths`: Directories to search for `.proto` files. Absolute or relative to LSP workspace root. -- `disable_parse_diagnostics`: Set to `true` to disable diagnostics during parsing. +- `disable_parse_diagnostics`: Set to `true` to disable tree-sitter parse diagnostics during parsing. #### Experimental Configuration The `[config.experimental]` section contains settings that are in development or not fully tested. - `use_protoc_diagnostics`: Enable diagnostics from the `protoc` compiler when set to `true`. +- `protoc_path`: Uses protoc from this path for diagnostics #### Formatter Configuration diff --git a/protols.toml b/protols.toml index bc012f2..1bd0e47 100644 --- a/protols.toml +++ b/protols.toml @@ -1,2 +1,5 @@ [config] include_paths = ["src/workspace/input"] + +[config.experimental] # experimental configuration; this should be considered unsafe and not fully tested +use_protoc_diagnostics = true # use diagnostics from protoc diff --git a/sample/simple.proto b/sample/simple.proto index 7e157d3..69c80d9 100644 --- a/sample/simple.proto +++ b/sample/simple.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package com.book; +import "google/protobuf/any.proto"; + // This is a book represeted by some comments that we like to address in the // review message Book { @@ -24,6 +26,7 @@ message Book { } enum BookState { + UNSPECIFIED = 0; HARD_COVER = 1; SOFT_COVER = 2; } @@ -52,10 +55,10 @@ service BookService { message BookStore { reserved 1; - Book book = 0; - string name = 1; - map books = 2; - EnumSample sample = 3; + Book book = 5; + string name = 2; + map books = 3; + EnumSample sample = 4; } // These are enum options representing some operation in the proto diff --git a/src/config/mod.rs b/src/config/mod.rs index 8c5e6f7..0f9c09f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -6,6 +6,10 @@ fn default_clang_format_path() -> String { "clang-format".to_string() } +fn default_protoc_path() -> String { + "protoc".to_string() +} + #[derive(Serialize, Deserialize, Debug, Clone, Default)] #[serde(default)] pub struct ProtolsConfig { @@ -28,10 +32,11 @@ pub struct Config { pub experimental: ExperimentalConfig, } -#[derive(Serialize, Deserialize, Debug, Clone, Default)] +#[derive(Serialize, Deserialize, Debug, Clone)] #[serde(default)] pub struct ExperimentalConfig { pub use_protoc_diagnostics: bool, + pub protoc_path: String, } impl Default for FormatterConfig { @@ -41,3 +46,12 @@ impl Default for FormatterConfig { } } } + +impl Default for ExperimentalConfig { + fn default() -> Self { + Self { + protoc_path: default_protoc_path(), + use_protoc_diagnostics: false, + } + } +} diff --git a/src/config/workspace.rs b/src/config/workspace.rs index 3a41724..08e3d9d 100644 --- a/src/config/workspace.rs +++ b/src/config/workspace.rs @@ -21,15 +21,18 @@ pub struct WorkspaceProtoConfigs { impl WorkspaceProtoConfigs { pub fn new() -> Self { + // Try to find protobuf library and get its include paths + let protoc_include_prefix = Config::new() + .atleast_version("3.0.0") + .probe("protobuf") + .map(|lib| lib.include_paths) + .unwrap_or_default(); + Self { - workspaces: Default::default(), - formatters: Default::default(), - protoc_include_prefix: Config::new() - .atleast_version("3.0.0") - .probe("protobuf") - .map(|l| l.include_paths) - .unwrap_or_default(), - configs: Default::default(), + workspaces: HashSet::new(), + formatters: HashMap::new(), + configs: HashMap::new(), + protoc_include_prefix, } } diff --git a/src/lsp.rs b/src/lsp.rs index 53ff242..917ebaa 100644 --- a/src/lsp.rs +++ b/src/lsp.rs @@ -392,7 +392,26 @@ impl LanguageServer for ProtoLanguageServer { Box::pin(async move { Ok(response) }) } - fn did_save(&mut self, _: DidSaveTextDocumentParams) -> Self::NotifyResult { + fn did_save(&mut self, params: DidSaveTextDocumentParams) -> Self::NotifyResult { + let uri = params.text_document.uri; + let content = self.state.get_content(&uri); + + let Some(ipath) = self.configs.get_include_paths(&uri) else { + return ControlFlow::Continue(()); + }; + + let Some(pconf) = self.configs.get_config_for_uri(&uri) else { + return ControlFlow::Continue(()); + }; + + if let Some(diagnostics) = self + .state + .upsert_file(&uri, content, &ipath, 8, &pconf.config) + { + if let Err(e) = self.client.publish_diagnostics(diagnostics) { + error!(error=%e, "failed to publish diagnostics") + } + } ControlFlow::Continue(()) } @@ -408,15 +427,14 @@ impl LanguageServer for ProtoLanguageServer { return ControlFlow::Continue(()); }; - let Some(diagnostics) = self.state.upsert_file(&uri, content.clone(), &ipath, 8) else { - return ControlFlow::Continue(()); - }; - let Some(pconf) = self.configs.get_config_for_uri(&uri) else { return ControlFlow::Continue(()); }; - if !pconf.config.disable_parse_diagnostics { + if let Some(diagnostics) = self + .state + .upsert_file(&uri, content, &ipath, 8, &pconf.config) + { if let Err(e) = self.client.publish_diagnostics(diagnostics) { error!(error=%e, "failed to publish diagnostics") } @@ -432,20 +450,21 @@ impl LanguageServer for ProtoLanguageServer { return ControlFlow::Continue(()); }; - let Some(diagnostics) = self.state.upsert_file(&uri, content, &ipath, 2) else { + let Some(pconf) = self.configs.get_config_for_uri(&uri) else { return ControlFlow::Continue(()); }; - let Some(ws) = self.configs.get_config_for_uri(&uri) else { - return ControlFlow::Continue(()); - }; - - if !ws.config.disable_parse_diagnostics { + // override config to disable protoc diagnostics during change + let mut pconf = pconf.config.clone(); + pconf.experimental.use_protoc_diagnostics = false; + if let Some(diagnostics) = self + .state + .upsert_file(&uri, content, &ipath, 8, &pconf) + { if let Err(e) = self.client.publish_diagnostics(diagnostics) { error!(error=%e, "failed to publish diagnostics") } } - ControlFlow::Continue(()) } diff --git a/src/main.rs b/src/main.rs index cdfcba4..684fb5e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ mod formatter; mod lsp; mod nodekind; mod parser; +mod protoc; mod server; mod state; mod utils; diff --git a/src/protoc.rs b/src/protoc.rs new file mode 100644 index 0000000..fb16eb9 --- /dev/null +++ b/src/protoc.rs @@ -0,0 +1,85 @@ +use crate::utils::ts_to_lsp_position; +use async_lsp::lsp_types::{Diagnostic, DiagnosticSeverity, Range}; +use std::process::Command; +use tree_sitter::Point; + +pub struct ProtocDiagnostics {} + +impl ProtocDiagnostics { + pub fn new() -> Self { + Self {} + } + + pub fn collect_diagnostics( + &self, + protoc_path: &str, + file_path: &str, + include_paths: &[String], + ) -> Vec { + let mut cmd = Command::new(protoc_path); + + // Add include paths + for path in include_paths { + cmd.arg("-I").arg(path); + } + + // Generate descriptor but discard its output + cmd.arg("-o") + .arg(if cfg!(windows) { "NUL" } else { "/dev/null" }); + + // Add the file to check + cmd.arg(file_path); + + // Run protoc and capture output + match cmd.output() { + Ok(output) => { + if !output.status.success() { + let error = String::from_utf8_lossy(&output.stderr); + self.parse_protoc_output(&error) + } else { + Vec::new() + } + } + Err(e) => { + tracing::error!(error=%e, "failed to run protoc"); + Vec::new() + } + } + } + + fn parse_protoc_output(&self, output: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for line in output.lines() { + // Parse protoc error format: file:line:column: message + if let Some((file_info, message)) = line.split_once(": ") { + let parts: Vec<&str> = file_info.split(':').collect(); + if parts.len() >= 3 { + if let (Ok(line), Ok(col)) = (parts[1].parse::(), parts[2].parse::()) + { + let point = Point { + row: (line - 1) as usize, + column: (col - 1) as usize, + }; + let diagnostic = Diagnostic { + range: Range { + start: ts_to_lsp_position(&point), + end: ts_to_lsp_position(&Point { + row: point.row, + column: point.column + 1, + }), + }, + severity: Some(DiagnosticSeverity::ERROR), + source: Some("protoc".to_string()), + message: message.to_string(), + ..Default::default() + }; + diagnostics.push(diagnostic); + } + } + } + } + + diagnostics + } +} diff --git a/src/state.rs b/src/state.rs index c11eba6..d7bf43c 100644 --- a/src/state.rs +++ b/src/state.rs @@ -12,15 +12,19 @@ use tree_sitter::Node; use walkdir::WalkDir; use crate::{ + config::Config, nodekind::NodeKind, parser::{ParsedTree, ProtoParser}, }; +use crate::protoc::ProtocDiagnostics; + pub struct ProtoLanguageState { documents: Arc>>, trees: Arc>>, parser: Arc>, parsed_workspaces: Arc>>, + protoc_diagnostics: Arc>, } impl ProtoLanguageState { @@ -30,6 +34,7 @@ impl ProtoLanguageState { trees: Default::default(), parser: Arc::new(Mutex::new(ProtoParser::new())), parsed_workspaces: Arc::new(RwLock::new(HashSet::new())), + protoc_diagnostics: Arc::new(Mutex::new(ProtocDiagnostics::new())), } } @@ -217,13 +222,34 @@ impl ProtoLanguageState { content: String, ipath: &[PathBuf], depth: usize, + config: &Config, ) -> Option { info!(%uri, %depth, "upserting file"); let diag = self.upsert_content(uri, content.clone(), ipath, depth); self.get_tree(uri).map(|tree| { - let diag = tree.collect_import_diagnostics(content.as_ref(), diag); - let mut d = tree.collect_parse_diagnostics(); - d.extend(diag); + let mut d = vec![]; + if !config.disable_parse_diagnostics { + d.extend(tree.collect_parse_diagnostics()); + } + d.extend(tree.collect_import_diagnostics(content.as_ref(), diag)); + + // Add protoc diagnostics if enabled + if config.experimental.use_protoc_diagnostics { + if let Ok(protoc_diagnostics) = self.protoc_diagnostics.lock() { + if let Some(file_path) = uri.to_file_path().ok() { + let protoc_diags = protoc_diagnostics.collect_diagnostics( + &config.experimental.protoc_path, + file_path.to_str().unwrap_or_default(), + &ipath + .iter() + .map(|p| p.to_str().unwrap_or_default().to_string()) + .collect::>(), + ); + d.extend(protoc_diags); + } + } + } + PublishDiagnosticsParams { uri: tree.uri.clone(), diagnostics: d, diff --git a/src/workspace/definition.rs b/src/workspace/definition.rs index 3a47242..474f87f 100644 --- a/src/workspace/definition.rs +++ b/src/workspace/definition.rs @@ -52,8 +52,8 @@ mod test { use insta::assert_yaml_snapshot; + use crate::config::Config; use crate::state::ProtoLanguageState; - #[test] fn workspace_test_definition() { let ipath = vec![PathBuf::from("src/workspace/input")]; @@ -66,9 +66,9 @@ mod test { let c = include_str!("input/c.proto"); let mut state: ProtoLanguageState = ProtoLanguageState::new(); - state.upsert_file(&a_uri, a.to_owned(), &ipath, 2); - state.upsert_file(&b_uri, b.to_owned(), &ipath, 2); - state.upsert_file(&c_uri, c.to_owned(), &ipath, 2); + state.upsert_file(&a_uri, a.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&b_uri, b.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&c_uri, c.to_owned(), &ipath, 2, &Config::default()); assert_yaml_snapshot!(state.definition( &ipath, diff --git a/src/workspace/hover.rs b/src/workspace/hover.rs index 53c5f41..2d3bd3b 100644 --- a/src/workspace/hover.rs +++ b/src/workspace/hover.rs @@ -664,9 +664,9 @@ Included from {}"#, mod test { use insta::assert_yaml_snapshot; + use crate::config::Config; use crate::context::hoverable::Hoverables; use crate::state::ProtoLanguageState; - #[test] fn workspace_test_hover() { let ipath = vec![std::env::current_dir().unwrap().join("src/workspace/input")]; @@ -679,9 +679,9 @@ mod test { let c = include_str!("input/c.proto"); let mut state: ProtoLanguageState = ProtoLanguageState::new(); - state.upsert_file(&a_uri, a.to_owned(), &ipath, 3); - state.upsert_file(&b_uri, b.to_owned(), &ipath, 2); - state.upsert_file(&c_uri, c.to_owned(), &ipath, 2); + state.upsert_file(&a_uri, a.to_owned(), &ipath, 3, &Config::default()); + state.upsert_file(&b_uri, b.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&c_uri, c.to_owned(), &ipath, 2, &Config::default()); assert_yaml_snapshot!(state.hover( &ipath, diff --git a/src/workspace/rename.rs b/src/workspace/rename.rs index 9ee7783..fb341d6 100644 --- a/src/workspace/rename.rs +++ b/src/workspace/rename.rs @@ -70,6 +70,7 @@ mod test { use insta::assert_yaml_snapshot; + use crate::config::Config; use crate::state::ProtoLanguageState; #[test] @@ -84,9 +85,9 @@ mod test { let c = include_str!("input/c.proto"); let mut state: ProtoLanguageState = ProtoLanguageState::new(); - state.upsert_file(&a_uri, a.to_owned(), &ipath, 2); - state.upsert_file(&b_uri, b.to_owned(), &ipath, 2); - state.upsert_file(&c_uri, c.to_owned(), &ipath, 2); + state.upsert_file(&a_uri, a.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&b_uri, b.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&c_uri, c.to_owned(), &ipath, 2, &Config::default()); assert_yaml_snapshot!(state.rename_fields( "com.workspace", @@ -123,9 +124,9 @@ mod test { let c = include_str!("input/c.proto"); let mut state: ProtoLanguageState = ProtoLanguageState::new(); - state.upsert_file(&a_uri, a.to_owned(), &ipath, 2); - state.upsert_file(&b_uri, b.to_owned(), &ipath, 2); - state.upsert_file(&c_uri, c.to_owned(), &ipath, 2); + state.upsert_file(&a_uri, a.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&b_uri, b.to_owned(), &ipath, 2, &Config::default()); + state.upsert_file(&c_uri, c.to_owned(), &ipath, 2, &Config::default()); assert_yaml_snapshot!(state.reference_fields( "com.workspace",