From d31a079bd68a9f3d4ac57eb7d5f1b48c4ad289eb Mon Sep 17 00:00:00 2001 From: Mika Vilpas Date: Sun, 9 Jun 2024 10:44:38 +0300 Subject: [PATCH 1/2] feat: code action to add a misspelling to the config file https://github.com/tekumara/typos-lsp/issues/12 --- Cargo.lock | 1 + crates/typos-lsp/Cargo.toml | 1 + crates/typos-lsp/src/lsp.rs | 115 ++++++++++++++++-- crates/typos-lsp/src/state.rs | 5 + crates/typos-lsp/src/typos.rs | 40 +++++- .../src/typos/config_file_location.rs | 49 ++++++++ .../src/typos/config_file_suggestions.rs | 15 +++ 7 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 crates/typos-lsp/src/typos/config_file_location.rs create mode 100644 crates/typos-lsp/src/typos/config_file_suggestions.rs diff --git a/Cargo.lock b/Cargo.lock index f5c3c01..0f3e3c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1522,6 +1522,7 @@ dependencies = [ "similar-asserts", "test-log", "tokio", + "toml", "tower-lsp", "tracing", "tracing-subscriber", diff --git a/crates/typos-lsp/Cargo.toml b/crates/typos-lsp/Cargo.toml index a8015e7..b449eb7 100644 --- a/crates/typos-lsp/Cargo.toml +++ b/crates/typos-lsp/Cargo.toml @@ -21,6 +21,7 @@ matchit = "0.8.4" shellexpand = "3.1.0" regex = "1.10.6" once_cell = "1.19.0" +toml = "0.8.12" [dev-dependencies] test-log = { version = "0.2.16", features = ["trace"] } diff --git a/crates/typos-lsp/src/lsp.rs b/crates/typos-lsp/src/lsp.rs index 7151797..25dc13a 100644 --- a/crates/typos-lsp/src/lsp.rs +++ b/crates/typos-lsp/src/lsp.rs @@ -2,13 +2,14 @@ use matchit::Match; use std::borrow::Cow; use std::collections::HashMap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Mutex; use serde_json::{json, to_string}; use tower_lsp::lsp_types::*; use tower_lsp::*; use tower_lsp::{Client, LanguageServer}; +use typos_cli::config::DictConfig; use typos_cli::policy; use crate::state::{url_path_sanitised, BackendState}; @@ -21,6 +22,16 @@ pub struct Backend<'s, 'p> { #[derive(Debug, serde::Serialize, serde::Deserialize)] struct DiagnosticData<'c> { corrections: Vec>, + typo: Cow<'c, str>, +} + +#[derive(Debug, serde::Serialize, serde::Deserialize)] +struct IgnoreInProjectCommandArguments { + typo: String, + /// The file that contains the typo to ignore + typo_file_path: String, + /// The configuration file that should be modified to ignore the typo + config_file_path: String, } #[tower_lsp::async_trait] @@ -97,6 +108,11 @@ impl LanguageServer for Backend<'static, 'static> { resolve_provider: None, }, )), + execute_command_provider: Some(ExecuteCommandOptions { + // TODO this magic string should be a constant + commands: vec!["ignore-in-project".to_string()], + work_done_progress_options: WorkDoneProgressOptions::default(), + }), workspace: Some(WorkspaceServerCapabilities { workspace_folders: Some(WorkspaceFoldersServerCapabilities { supported: Some(true), @@ -150,6 +166,8 @@ impl LanguageServer for Backend<'static, 'static> { .await; } + /// Called by the editor to request displaying a list of code actions and commands for a given + /// position in the current file. async fn code_action( &self, params: CodeActionParams, @@ -163,10 +181,10 @@ impl LanguageServer for Backend<'static, 'static> { .filter(|diag| diag.source == Some("typos".to_string())) .flat_map(|diag| match &diag.data { Some(data) => { - if let Ok(DiagnosticData { corrections }) = + if let Ok(DiagnosticData { corrections, typo }) = serde_json::from_value::(data.clone()) { - corrections + let mut suggestions: Vec<_> = corrections .iter() .map(|c| { CodeActionOrCommand::CodeAction(CodeAction { @@ -191,7 +209,44 @@ impl LanguageServer for Backend<'static, 'static> { ..CodeAction::default() }) }) - .collect() + .collect(); + + if let Ok(Match { value, .. }) = self + .state + .lock() + .unwrap() + .router + .at(params.text_document.uri.to_file_path().unwrap().to_str().unwrap()) + { + let typo_file: &Url = ¶ms.text_document.uri; + let config_files = + value.config_files_in_project(Path::new(typo_file.as_str())); + + suggestions.push(CodeActionOrCommand::Command(Command { + title: format!("Ignore `{}` in the project", typo), + command: "ignore-in-project".to_string(), + arguments: Some( + [serde_json::to_value(IgnoreInProjectCommandArguments { + typo: typo.to_string(), + typo_file_path: typo_file.to_string(), + config_file_path: config_files + .project_root + .path + .to_string_lossy() + .to_string(), + }) + .unwrap()] + .into(), + ), + })); + } else { + tracing::warn!( + "code_action: Cannot create a code action for ignoring a typo in the project. Reason: No route found for file '{}'", + params.text_document.uri + ); + } + + suggestions } else { tracing::error!( "Deserialization failed: received {:?} as diagnostic data", @@ -210,6 +265,51 @@ impl LanguageServer for Backend<'static, 'static> { Ok(Some(actions)) } + /// Called by the editor to execute a server side command, such as ignoring a typo. + async fn execute_command( + &self, + raw_params: ExecuteCommandParams, + ) -> jsonrpc::Result> { + tracing::debug!( + "execute_command: {:?}", + to_string(&raw_params).unwrap_or_default() + ); + + // TODO reduce the nesting + if raw_params.command == "ignore-in-project" { + let argument = raw_params + .arguments + .into_iter() + .next() + .expect("no arguments for ignore-in-project command"); + + if let Ok(IgnoreInProjectCommandArguments { + typo, + config_file_path, + .. + }) = serde_json::from_value::(argument) + { + let mut config = typos_cli::config::Config::from_file(Path::new(&config_file_path)) + .ok() + .flatten() + .unwrap_or_default(); + + config.default.dict.update(&DictConfig { + extend_words: HashMap::from([(typo.clone().into(), typo.into())]), + ..Default::default() + }); + + std::fs::write( + &config_file_path, + toml::to_string_pretty(&config).expect("cannot serialize config"), + ) + .unwrap_or_else(|_| panic!("Cannot write to {}", config_file_path)); + }; + } + + Ok(None) + } + async fn did_change_workspace_folders(&self, params: DidChangeWorkspaceFoldersParams) { tracing::debug!( "did_change_workspace_folders: {:?}", @@ -271,9 +371,10 @@ impl<'s, 'p> Backend<'s, 'p> { }, // store corrections for retrieval during code_action data: match typo.corrections { - typos::Status::Corrections(corrections) => { - Some(json!(DiagnosticData { corrections })) - } + typos::Status::Corrections(corrections) => Some(json!(DiagnosticData { + corrections, + typo: typo.typo + })), _ => None, }, ..Diagnostic::default() diff --git a/crates/typos-lsp/src/state.rs b/crates/typos-lsp/src/state.rs index 3d5ec69..de71022 100644 --- a/crates/typos-lsp/src/state.rs +++ b/crates/typos-lsp/src/state.rs @@ -8,8 +8,13 @@ use crate::typos::Instance; #[derive(Default)] pub(crate) struct BackendState<'s> { pub severity: Option, + /// The path to the configuration file given to the LSP server. Settings in this configuration + /// file override the typos.toml settings. pub config: Option, pub workspace_folders: Vec, + + /// Maps routes (file system paths) to TyposCli instances, so that we can quickly find the + /// correct instance for a given file path pub router: Router>, } diff --git a/crates/typos-lsp/src/typos.rs b/crates/typos-lsp/src/typos.rs index 1eeaf1e..ac40272 100644 --- a/crates/typos-lsp/src/typos.rs +++ b/crates/typos-lsp/src/typos.rs @@ -1,11 +1,18 @@ -use std::path::Path; +mod config_file_location; +mod config_file_suggestions; + +use std::path::{Path, PathBuf}; use bstr::ByteSlice; use ignore::overrides::{Override, OverrideBuilder}; use typos_cli::policy; pub struct Instance<'s> { + /// File path rules to ignore pub ignores: Override, pub engine: policy::ConfigEngine<'s>, + + /// The path where the LSP server was started + pub project_root: PathBuf, } impl Instance<'_> { @@ -46,10 +53,41 @@ impl Instance<'_> { let ignore = ignores.build()?; Ok(Instance { + project_root: path.to_path_buf(), ignores: ignore, engine, }) } + + /// Returns the typos_cli configuration files that are relevant for the given path. Note that + /// all config files are read by typos_cli, and the settings are applied in precedence order: + /// + /// + pub fn config_files_in_project( + &self, + starting_path: &Path, + ) -> config_file_suggestions::ConfigFileSuggestions { + // limit the search to the project root, never search above it + let project_path = self.project_root.as_path(); + + let mut suggestions = config_file_suggestions::ConfigFileSuggestions { + project_root: config_file_location::ConfigFileLocation::from_dir_or_default( + self.project_root.as_path(), + ), + config_files: vec![], + }; + starting_path + .ancestors() + .filter(|path| path.starts_with(project_path)) + .filter(|path| *path != self.project_root.as_path()) + .for_each(|path| { + let config_location = + config_file_location::ConfigFileLocation::from_dir_or_default(path); + suggestions.config_files.push(config_location); + }); + + suggestions + } } // mimics typos_cli::file::FileChecker::check_file diff --git a/crates/typos-lsp/src/typos/config_file_location.rs b/crates/typos-lsp/src/typos/config_file_location.rs new file mode 100644 index 0000000..643fab9 --- /dev/null +++ b/crates/typos-lsp/src/typos/config_file_location.rs @@ -0,0 +1,49 @@ +use std::path::Path; + +use std::path::PathBuf; + +/// Represents a path to a typos_cli config file and, if it contains a configuration file, the file +/// contents +pub struct ConfigFileLocation { + pub path: PathBuf, + pub config: Option, +} + +impl ConfigFileLocation { + pub fn from_dir_or_default(path: &Path) -> ConfigFileLocation { + let directory = if path.is_dir() { + path + } else { + path.parent().unwrap() + }; + ConfigFileLocation::from_dir(directory).unwrap_or_else(|_| ConfigFileLocation { + path: path.to_path_buf(), + config: None, + }) + } + + // copied from typos_cli::config::Config::from_dir with the difference that it shows which + // config file was found of the supported ones. This information is useful when we want to + // modify the config file later on. + pub fn from_dir(dir: &Path) -> anyhow::Result { + assert!( + dir.is_dir(), + "Expected a directory that might contain a configuration file" + ); + + for file in typos_cli::config::SUPPORTED_FILE_NAMES { + let path = dir.join(file); + if let Ok(Some(config)) = typos_cli::config::Config::from_file(path.as_path()) { + return Ok(ConfigFileLocation { + path, + config: Some(config), + }); + } + } + + Err(anyhow::anyhow!( + "No typos_cli config file found starting from {:?}", + dir + )) + } +} diff --git a/crates/typos-lsp/src/typos/config_file_suggestions.rs b/crates/typos-lsp/src/typos/config_file_suggestions.rs new file mode 100644 index 0000000..09abbfd --- /dev/null +++ b/crates/typos-lsp/src/typos/config_file_suggestions.rs @@ -0,0 +1,15 @@ +use config_file_location::ConfigFileLocation; + +use super::config_file_location; + +/// Represents the paths to typos_cli config files that could be used when adding a new ignore +/// rule. The config files may or may not exist. +pub struct ConfigFileSuggestions { + /// The path to a (possible) configuration file in the directory where the LSP server was + /// started. This is always included as the default suggestion. + pub project_root: ConfigFileLocation, + + /// Other configuration files that currently exist in the project. The order is from the closest + /// to the currently open file to the project root. Only existing files are included. + pub config_files: Vec, +} From 9872ae8508821fd3f4be2649ca2cade90baa52c0 Mon Sep 17 00:00:00 2001 From: Mika Vilpas Date: Fri, 21 Jun 2024 11:59:05 +0300 Subject: [PATCH 2/2] fixup! feat: code action to add a misspelling to the config file --- Cargo.lock | 51 +++++++- crates/typos-lsp/Cargo.toml | 3 +- crates/typos-lsp/src/lsp.rs | 60 +++++----- .../typos-lsp/src/lsp/ignore_typo_action.rs | 111 ++++++++++++++++++ crates/typos-lsp/src/typos.rs | 60 ++++------ .../src/typos/config_file_location.rs | 83 +++++++++++-- .../src/typos/config_file_suggestions.rs | 11 +- crates/typos-lsp/tests/integration_test.rs | 94 ++++++++++++--- 8 files changed, 374 insertions(+), 99 deletions(-) create mode 100644 crates/typos-lsp/src/lsp/ignore_typo_action.rs diff --git a/Cargo.lock b/Cargo.lock index 0f3e3c6..3fd91f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -480,6 +480,22 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" +[[package]] +name = "errno" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" +dependencies = [ + "libc", + "windows-sys 0.52.0", +] + +[[package]] +name = "fastrand" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8c02a5121d4ea3eb16a80748c74f5549a5665e4c21333c6098f283870fbdea6" + [[package]] name = "fnv" version = "1.0.7" @@ -737,6 +753,12 @@ dependencies = [ "libc", ] +[[package]] +name = "linux-raw-sys" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" + [[package]] name = "lock_api" version = "0.4.12" @@ -995,6 +1017,19 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" +[[package]] +name = "rustix" +version = "0.38.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f55e80d50763938498dd5ebb18647174e0c76dc38c5505294bb224624f30f36" +dependencies = [ + "bitflags 2.6.0", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.52.0", +] + [[package]] name = "ryu" version = "1.0.18" @@ -1166,6 +1201,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04cbcdd0c794ebb0d4cf35e88edd2f7d2c4c3e9a5a6dab322839b321c6a87a64" +dependencies = [ + "cfg-if", + "fastrand", + "once_cell", + "rustix", + "windows-sys 0.59.0", +] + [[package]] name = "test-log" version = "0.2.16" @@ -1520,9 +1568,10 @@ dependencies = [ "serde_json", "shellexpand", "similar-asserts", + "tempfile", "test-log", "tokio", - "toml", + "toml_edit", "tower-lsp", "tracing", "tracing-subscriber", diff --git a/crates/typos-lsp/Cargo.toml b/crates/typos-lsp/Cargo.toml index b449eb7..b2a4083 100644 --- a/crates/typos-lsp/Cargo.toml +++ b/crates/typos-lsp/Cargo.toml @@ -21,9 +21,10 @@ matchit = "0.8.4" shellexpand = "3.1.0" regex = "1.10.6" once_cell = "1.19.0" -toml = "0.8.12" +toml_edit = "0.22.14" [dev-dependencies] test-log = { version = "0.2.16", features = ["trace"] } httparse = "1.9" similar-asserts = "1.4" +tempfile = "3.10.1" diff --git a/crates/typos-lsp/src/lsp.rs b/crates/typos-lsp/src/lsp.rs index 25dc13a..316ef86 100644 --- a/crates/typos-lsp/src/lsp.rs +++ b/crates/typos-lsp/src/lsp.rs @@ -1,15 +1,15 @@ +use ignore_typo_action::IGNORE_IN_PROJECT; use matchit::Match; use std::borrow::Cow; use std::collections::HashMap; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::Mutex; use serde_json::{json, to_string}; use tower_lsp::lsp_types::*; use tower_lsp::*; use tower_lsp::{Client, LanguageServer}; -use typos_cli::config::DictConfig; use typos_cli::policy; use crate::state::{url_path_sanitised, BackendState}; @@ -19,6 +19,8 @@ pub struct Backend<'s, 'p> { default_policy: policy::Policy<'p, 'p, 'p>, } +mod ignore_typo_action; + #[derive(Debug, serde::Serialize, serde::Deserialize)] struct DiagnosticData<'c> { corrections: Vec>, @@ -28,8 +30,6 @@ struct DiagnosticData<'c> { #[derive(Debug, serde::Serialize, serde::Deserialize)] struct IgnoreInProjectCommandArguments { typo: String, - /// The file that contains the typo to ignore - typo_file_path: String, /// The configuration file that should be modified to ignore the typo config_file_path: String, } @@ -109,8 +109,7 @@ impl LanguageServer for Backend<'static, 'static> { }, )), execute_command_provider: Some(ExecuteCommandOptions { - // TODO this magic string should be a constant - commands: vec!["ignore-in-project".to_string()], + commands: vec![IGNORE_IN_PROJECT.to_string()], work_done_progress_options: WorkDoneProgressOptions::default(), }), workspace: Some(WorkspaceServerCapabilities { @@ -218,27 +217,38 @@ impl LanguageServer for Backend<'static, 'static> { .router .at(params.text_document.uri.to_file_path().unwrap().to_str().unwrap()) { - let typo_file: &Url = ¶ms.text_document.uri; - let config_files = - value.config_files_in_project(Path::new(typo_file.as_str())); + let config_files = value.config_files_in_project(); suggestions.push(CodeActionOrCommand::Command(Command { title: format!("Ignore `{}` in the project", typo), - command: "ignore-in-project".to_string(), + command: IGNORE_IN_PROJECT.to_string(), arguments: Some( [serde_json::to_value(IgnoreInProjectCommandArguments { typo: typo.to_string(), - typo_file_path: typo_file.to_string(), config_file_path: config_files .project_root - .path .to_string_lossy() .to_string(), }) - .unwrap()] - .into(), + .unwrap()] + .into(), ), })); + + if let Some(explicit_config) = &config_files.explicit { + suggestions.push(CodeActionOrCommand::Command(Command { + title: format!("Ignore `{}` in the configuration file", typo), + command: IGNORE_IN_PROJECT.to_string(), + arguments: Some( + [serde_json::to_value(IgnoreInProjectCommandArguments { + typo: typo.to_string(), + config_file_path: explicit_config.to_string_lossy().to_string(), + }) + .unwrap()] + .into(), + ), + })); + } } else { tracing::warn!( "code_action: Cannot create a code action for ignoring a typo in the project. Reason: No route found for file '{}'", @@ -275,8 +285,7 @@ impl LanguageServer for Backend<'static, 'static> { to_string(&raw_params).unwrap_or_default() ); - // TODO reduce the nesting - if raw_params.command == "ignore-in-project" { + if raw_params.command == IGNORE_IN_PROJECT { let argument = raw_params .arguments .into_iter() @@ -289,21 +298,12 @@ impl LanguageServer for Backend<'static, 'static> { .. }) = serde_json::from_value::(argument) { - let mut config = typos_cli::config::Config::from_file(Path::new(&config_file_path)) - .ok() - .flatten() - .unwrap_or_default(); - - config.default.dict.update(&DictConfig { - extend_words: HashMap::from([(typo.clone().into(), typo.into())]), - ..Default::default() - }); - - std::fs::write( - &config_file_path, - toml::to_string_pretty(&config).expect("cannot serialize config"), + ignore_typo_action::ignore_typo_in_config_file( + PathBuf::from(config_file_path), + typo, ) - .unwrap_or_else(|_| panic!("Cannot write to {}", config_file_path)); + .unwrap(); + self.state.lock().unwrap().update_router().unwrap(); }; } diff --git a/crates/typos-lsp/src/lsp/ignore_typo_action.rs b/crates/typos-lsp/src/lsp/ignore_typo_action.rs new file mode 100644 index 0000000..288f774 --- /dev/null +++ b/crates/typos-lsp/src/lsp/ignore_typo_action.rs @@ -0,0 +1,111 @@ +use std::{ + fs::read_to_string, + path::{Path, PathBuf}, +}; + +use anyhow::{anyhow, Context}; +use toml_edit::DocumentMut; + +pub(super) const IGNORE_IN_PROJECT: &str = "ignore-in-project"; + +pub(super) fn ignore_typo_in_config_file(config_file: PathBuf, typo: String) -> anyhow::Result<()> { + let input = read_to_string(&config_file) + .with_context(|| anyhow!("Cannot read config file at {}", config_file.display())) + .unwrap_or("".to_string()); + + let document = add_typo(input, typo, &config_file)?; + + std::fs::write(&config_file, document.to_string()) + .with_context(|| anyhow!("Cannot write config file to {}", config_file.display()))?; + + Ok(()) +} + +fn add_typo( + input: String, + typo: String, + config_file_path: &Path, +) -> Result { + // preserve comments and formatting + let mut document = input + .parse::() + .with_context(|| anyhow!("Cannot parse config file at {}", config_file_path.display()))?; + let extend_words = document + .entry("default") + .or_insert(toml_edit::table()) + .as_table_mut() + .context("Cannot get 'default' table")? + .entry("extend-words") + .or_insert(toml_edit::table()) + .as_table_mut() + .context("Cannot get 'extend-words' table")?; + extend_words[typo.as_str()] = toml_edit::value(typo.clone()); + Ok(document) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_add_typo_to_empty_file() { + let empty_file = ""; + let document = add_typo( + empty_file.to_string(), + "typo".to_string(), + PathBuf::from("test.toml").as_path(), + ) + .unwrap(); + + similar_asserts::assert_eq!( + document.to_string(), + [ + "[default]", + "", + "[default.extend-words]", + "typo = \"typo\"", + "" + ] + .join("\n") + ); + } + + #[test] + fn test_add_typo_to_existing_file() -> anyhow::Result<()> { + // should preserve comments and formatting + + let existing_file = [ + "[files] # comment", + "# comment", + "extend-exclude = [\"CHANGELOG.md\", \"crates/typos-lsp/tests/integration_test.rs\"]", + ] + .join("\n"); + + // make sure the config is valid (so the test makes sense) + let _ = typos_cli::config::Config::from_toml(&existing_file)?; + + let document = add_typo( + existing_file.to_string(), + "typo".to_string(), + PathBuf::from("test.toml").as_path(), + )?; + + similar_asserts::assert_eq!( + document.to_string(), + [ + "[files] # comment", + "# comment", + "extend-exclude = [\"CHANGELOG.md\", \"crates/typos-lsp/tests/integration_test.rs\"]", + "", + "[default]", + "", + "[default.extend-words]", + "typo = \"typo\"", + "" + ] + .join("\n") + ); + + Ok(()) + } +} diff --git a/crates/typos-lsp/src/typos.rs b/crates/typos-lsp/src/typos.rs index ac40272..91b6e90 100644 --- a/crates/typos-lsp/src/typos.rs +++ b/crates/typos-lsp/src/typos.rs @@ -1,9 +1,11 @@ mod config_file_location; mod config_file_suggestions; -use std::path::{Path, PathBuf}; +use std::path::Path; use bstr::ByteSlice; +use config_file_location::ConfigFileLocation; +use config_file_suggestions::ConfigFileSuggestions; use ignore::overrides::{Override, OverrideBuilder}; use typos_cli::policy; pub struct Instance<'s> { @@ -12,7 +14,10 @@ pub struct Instance<'s> { pub engine: policy::ConfigEngine<'s>, /// The path where the LSP server was started - pub project_root: PathBuf, + pub project_root: ConfigFileLocation, + + /// The explicit configuration file that was given to the LSP server at startup + pub explicit_config: Option, } impl Instance<'_> { @@ -27,13 +32,17 @@ impl Instance<'_> { // TODO: currently mimicking typos here but do we need to create and update // a default config? + let mut c = typos_cli::config::Config::default(); - if let Some(config_path) = config { - let custom = typos_cli::config::Config::from_file(config_path)?; - if let Some(custom) = custom { - c.update(&custom); - engine.set_overrides(c); - } + let explicit_config = config.map(ConfigFileLocation::from_file_path_or_default); + + if let Some(ConfigFileLocation { + config: Some(ref config), + .. + }) = explicit_config + { + c.update(config); + engine.set_overrides(c); } // initialise an engine and overrides using the config file from path or its parent @@ -53,40 +62,21 @@ impl Instance<'_> { let ignore = ignores.build()?; Ok(Instance { - project_root: path.to_path_buf(), + explicit_config, + project_root: ConfigFileLocation::from_dir_or_default(path), ignores: ignore, engine, }) } - /// Returns the typos_cli configuration files that are relevant for the given path. Note that - /// all config files are read by typos_cli, and the settings are applied in precedence order: + /// Returns the typos_cli configuration files that are relevant for the current project. /// /// - pub fn config_files_in_project( - &self, - starting_path: &Path, - ) -> config_file_suggestions::ConfigFileSuggestions { - // limit the search to the project root, never search above it - let project_path = self.project_root.as_path(); - - let mut suggestions = config_file_suggestions::ConfigFileSuggestions { - project_root: config_file_location::ConfigFileLocation::from_dir_or_default( - self.project_root.as_path(), - ), - config_files: vec![], - }; - starting_path - .ancestors() - .filter(|path| path.starts_with(project_path)) - .filter(|path| *path != self.project_root.as_path()) - .for_each(|path| { - let config_location = - config_file_location::ConfigFileLocation::from_dir_or_default(path); - suggestions.config_files.push(config_location); - }); - - suggestions + pub fn config_files_in_project(&self) -> ConfigFileSuggestions { + ConfigFileSuggestions { + explicit: self.explicit_config.as_ref().map(|c| c.path.clone()), + project_root: self.project_root.path.to_path_buf(), + } } } diff --git a/crates/typos-lsp/src/typos/config_file_location.rs b/crates/typos-lsp/src/typos/config_file_location.rs index 643fab9..3843a58 100644 --- a/crates/typos-lsp/src/typos/config_file_location.rs +++ b/crates/typos-lsp/src/typos/config_file_location.rs @@ -3,13 +3,28 @@ use std::path::Path; use std::path::PathBuf; /// Represents a path to a typos_cli config file and, if it contains a configuration file, the file -/// contents +/// contents. +/// +/// When reading a config from a directory, many configuration files are supported, and only one is +/// chosen in a given order. Shows the name of the config file that is used ("typos.toml", +/// "_typos.toml", ".typos.toml", "pyproject.toml"). This information is useful when we want to +/// modify the config file later on. +#[derive(Debug, PartialEq, Clone)] pub struct ConfigFileLocation { pub path: PathBuf, pub config: Option, } impl ConfigFileLocation { + pub fn from_file_path_or_default(path: &Path) -> ConfigFileLocation { + let config = typos_cli::config::Config::from_file(path).ok().flatten(); + + ConfigFileLocation { + config, + path: path.to_path_buf(), + } + } + pub fn from_dir_or_default(path: &Path) -> ConfigFileLocation { let directory = if path.is_dir() { path @@ -17,18 +32,17 @@ impl ConfigFileLocation { path.parent().unwrap() }; ConfigFileLocation::from_dir(directory).unwrap_or_else(|_| ConfigFileLocation { - path: path.to_path_buf(), + path: path.join("typos.toml").to_path_buf(), config: None, }) } - // copied from typos_cli::config::Config::from_dir with the difference that it shows which - // config file was found of the supported ones. This information is useful when we want to - // modify the config file later on. - pub fn from_dir(dir: &Path) -> anyhow::Result { + // copied from typos_cli::config::Config::from_dir + fn from_dir(dir: &Path) -> anyhow::Result { assert!( dir.is_dir(), - "Expected a directory that might contain a configuration file" + "Expected a directory that might contain a configuration file, got {:?}", + dir.is_dir() ); for file in typos_cli::config::SUPPORTED_FILE_NAMES { @@ -47,3 +61,58 @@ impl ConfigFileLocation { )) } } + +#[cfg(test)] +mod tests { + use std::fs::File; + use std::io::Write; + use tempfile::tempdir; + + use super::ConfigFileLocation; + + #[test] + fn test_from_dir_or_default_with_exact_path() -> anyhow::Result<()> { + // when given a path to a configuration file, should resolve it to the same file + + // create a temporary directory on disk + let dir = tempdir()?; + + let file_path = dir.path().join("typos.toml"); + let mut file = File::create(&file_path)?; + writeln!(file, "#")?; + let config_file_location = ConfigFileLocation::from_dir_or_default(&file_path); + + assert_eq!( + config_file_location, + ConfigFileLocation { + path: file_path.to_path_buf(), + config: Some(typos_cli::config::Config::default()), + } + ); + + Ok(()) + } + + #[test] + fn test_from_dir_or_default_with_directory() -> anyhow::Result<()> { + // when given a path to a directory, should resolve it to the first configuration file + // found in the directory. This should support all of the supported file names, although + // this test only tests one of them. + + // NOTE when `dir` is dropped, the temporary directory is deleted from disk + let dir = tempdir()?; + let dir_path = dir.path(); + + let config_file_location = ConfigFileLocation::from_dir_or_default(dir.path()); + + assert_eq!( + config_file_location, + ConfigFileLocation { + path: dir_path.join("typos.toml").to_path_buf(), + config: None, + } + ); + + Ok(()) + } +} diff --git a/crates/typos-lsp/src/typos/config_file_suggestions.rs b/crates/typos-lsp/src/typos/config_file_suggestions.rs index 09abbfd..e20abfd 100644 --- a/crates/typos-lsp/src/typos/config_file_suggestions.rs +++ b/crates/typos-lsp/src/typos/config_file_suggestions.rs @@ -1,15 +1,12 @@ -use config_file_location::ConfigFileLocation; - -use super::config_file_location; +use std::path::PathBuf; /// Represents the paths to typos_cli config files that could be used when adding a new ignore /// rule. The config files may or may not exist. pub struct ConfigFileSuggestions { /// The path to a (possible) configuration file in the directory where the LSP server was /// started. This is always included as the default suggestion. - pub project_root: ConfigFileLocation, + pub project_root: PathBuf, - /// Other configuration files that currently exist in the project. The order is from the closest - /// to the currently open file to the project root. Only existing files are included. - pub config_files: Vec, + /// The explicit configuration file that was given to the LSP server at startup. + pub explicit: Option, } diff --git a/crates/typos-lsp/tests/integration_test.rs b/crates/typos-lsp/tests/integration_test.rs index 5e90e92..6d78847 100644 --- a/crates/typos-lsp/tests/integration_test.rs +++ b/crates/typos-lsp/tests/integration_test.rs @@ -20,6 +20,11 @@ async fn test_initialize_e2e() { "codeActionKinds": ["quickfix"], "workDoneProgress": false }, + "executeCommandProvider": { + "commands": [ + "ignore-in-project", + ], + }, "positionEncoding": "utf-16", "textDocumentSync": 1, "workspace": { @@ -48,7 +53,7 @@ async fn test_code_action() { }, "range": range(1, 0, 2), "context": { - "diagnostics": [ diag("`fo` should be `of`, `for`", 1, 0, 2) ], + "diagnostics": [ diag("`fo` should be `of`, `for`", "fo", 1, 0, 2) ], "only": ["quickfix"], "triggerKind": 1 } @@ -68,7 +73,7 @@ async fn test_code_action() { }, "range": range(0, 11, 21), "context": { - "diagnostics": [ diag("`apropriate` should be `appropriate`", 0, 11, 21) ], + "diagnostics": [ diag("`apropriate` should be `appropriate`", "apropriate", 0, 11, 21) ], "only": ["quickfix"], "triggerKind": 1 } @@ -84,8 +89,20 @@ async fn test_code_action() { similar_asserts::assert_eq!( server.request(&did_open).await, publish_diagnostics(&[ - diag("`apropriate` should be `appropriate`", 0, 11, 21), - diag("`fo` should be `of`, `for`, `do`, `go`, `to`", 1, 0, 2) + diag( + "`apropriate` should be `appropriate`", + "apropriate", + 0, + 11, + 21 + ), + diag( + "`fo` should be `of`, `for`, `do`, `go`, `to`", + "fo", + 1, + 0, + 2 + ) ]) ); @@ -96,7 +113,7 @@ async fn test_code_action() { "jsonrpc": "2.0", "result": [ { - "diagnostics": [ diag("`fo` should be `of`, `for`", 1, 0, 2) ], + "diagnostics": [ diag("`fo` should be `of`, `for`", "fo", 1, 0, 2) ], "edit": { "changes": { "file:///C%3A/diagnostics.txt": [ @@ -111,7 +128,7 @@ async fn test_code_action() { "title": "of" }, { - "diagnostics": [ diag("`fo` should be `of`, `for`", 1, 0, 2) ], + "diagnostics": [ diag("`fo` should be `of`, `for`", "fo", 1, 0, 2) ], "edit": { "changes": { "file:///C%3A/diagnostics.txt": [ @@ -124,7 +141,17 @@ async fn test_code_action() { }, "kind": "quickfix", "title": "for" - } + }, + { + "arguments": [ + { + "config_file_path": "/typos.toml", + "typo": "fo", + }, + ], + "command": "ignore-in-project", + "title": "Ignore `fo` in the project", + }, ], "id": 2 } @@ -138,7 +165,7 @@ async fn test_code_action() { "jsonrpc": "2.0", "result": [ { - "diagnostics": [ diag("`apropriate` should be `appropriate`", 0, 11, 21) ], + "diagnostics": [ diag("`apropriate` should be `appropriate`", "apropriate", 0, 11, 21) ], "edit": { "changes": { "file:///C%3A/diagnostics.txt": [ @@ -152,7 +179,17 @@ async fn test_code_action() { "isPreferred": true, "kind": "quickfix", "title": "appropriate" - } + }, + { + "arguments": [ + { + "config_file_path": "/typos.toml", + "typo": "apropriate", + }, + ], + "command": "ignore-in-project", + "title": "Ignore `apropriate` in the project", + }, ], "id": 3 } @@ -180,7 +217,10 @@ async fn test_config_file() { // check "fo" is corrected to "of" because of default.extend-words similar_asserts::assert_eq!( server.request(&did_open_diag_txt).await, - publish_diagnostics_with(&[diag("`fo` should be `of`", 0, 0, 2)], Some(&diag_txt)) + publish_diagnostics_with( + &[diag("`fo` should be `of`", "fo", 0, 0, 2)], + Some(&diag_txt) + ) ); // check changelog is excluded because of files.extend-exclude @@ -221,7 +261,10 @@ async fn test_custom_config_file() { // in custom_typos.toml which overrides typos.toml similar_asserts::assert_eq!( server.request(&did_open_diag_txt).await, - publish_diagnostics_with(&[diag("`fo` should be `go`", 0, 0, 2)], Some(&diag_txt)) + publish_diagnostics_with( + &[diag("`fo` should be `go`", "fo", 0, 0, 2)], + Some(&diag_txt) + ) ); } @@ -248,7 +291,10 @@ async fn test_custom_config_no_workspace_folder() { // in custom_typos.toml which overrides typos.toml similar_asserts::assert_eq!( server.request(&did_open_diag_txt).await, - publish_diagnostics_with(&[diag("`fo` should be `go`", 0, 0, 2)], Some(&diag_txt)) + publish_diagnostics_with( + &[diag("`fo` should be `go`", "fo", 0, 0, 2)], + Some(&diag_txt) + ) ); } @@ -265,7 +311,13 @@ async fn test_non_file_uri() { similar_asserts::assert_eq!( server.request(&did_open_diag_txt).await, publish_diagnostics_with( - &[diag("`apropriate` should be `appropriate`", 0, 0, 10)], + &[diag( + "`apropriate` should be `appropriate`", + "apropriate", + 0, + 0, + 10 + )], Some(&term) ) ); @@ -284,7 +336,13 @@ async fn test_empty_file_uri() { similar_asserts::assert_eq!( server.request(&did_open_diag_txt).await, publish_diagnostics_with( - &[diag("`apropriate` should be `appropriate`", 0, 0, 10)], + &[diag( + "`apropriate` should be `appropriate`", + "apropriate", + 0, + 0, + 10 + )], Some(&term) ) ); @@ -299,14 +357,14 @@ async fn test_position_with_unicode_text() { let unicode_text = did_open("¿Qué hace él?"); similar_asserts::assert_eq!( server.request(&unicode_text).await, - publish_diagnostics(&[diag("`hace` should be `have`", 0, 5, 9)]) + publish_diagnostics(&[diag("`hace` should be `have`", "hace", 0, 5, 9)]) ); // ẽ has two code points U+0065 U+0303 (latin small letter e, combining tilde) let unicode_text = did_open("ẽ hace"); similar_asserts::assert_eq!( server.request(&unicode_text).await, - publish_diagnostics(&[diag("`hace` should be `have`", 0, 3, 7)]) + publish_diagnostics(&[diag("`hace` should be `have`", "hace", 0, 3, 7)]) ); } @@ -377,7 +435,7 @@ fn did_open_with(text: &str, uri: Option<&Url>) -> String { .to_string() } -fn diag(message: &str, line: u32, start: u32, end: u32) -> Value { +fn diag(message: &str, typo: &str, line: u32, start: u32, end: u32) -> Value { static RE: Lazy = Lazy::new(|| Regex::new(r"`[^`]+` should be (.*)").unwrap()); let caps = RE.captures(message).unwrap(); @@ -385,7 +443,7 @@ fn diag(message: &str, line: u32, start: u32, end: u32) -> Value { let corrections: Vec<&str> = caps[1].split(", ").map(|s| s.trim_matches('`')).collect(); json!({ - "data": { "corrections": corrections }, + "data": { "corrections": corrections, "typo": typo }, "message": message, "range": range(line,start,end), "severity": 2,