From 8ff3182f88206374238cda77f49f7e741fc3ec78 Mon Sep 17 00:00:00 2001 From: Elijah Potter Date: Fri, 8 Mar 2024 11:27:23 -0700 Subject: [PATCH] feat: swapped out `LintSet` for `LintGroup` and made linters configurable --- Cargo.lock | 13 +++ harper-core/benches/parse_demo.rs | 15 +--- harper-core/src/lexing/url.rs | 4 - harper-core/src/lib.rs | 2 +- harper-core/src/linting/lint_group.rs | 107 ++++++++++++++++++++++++ harper-core/src/linting/lint_set.rs | 113 -------------------------- harper-core/src/linting/mod.rs | 4 +- harper-ls/Cargo.toml | 1 + harper-ls/README.md | 26 +++++- harper-ls/src/backend.rs | 58 +++++++++---- harper-ls/src/config.rs | 15 +++- harper-serve/src/main.rs | 5 +- harper-wasm/src/lib.rs | 10 ++- web/src/lib/analysis.ts | 2 +- 14 files changed, 217 insertions(+), 158 deletions(-) create mode 100644 harper-core/src/linting/lint_group.rs delete mode 100644 harper-core/src/linting/lint_set.rs diff --git a/Cargo.lock b/Cargo.lock index e26b63c2..313f5af8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -428,6 +428,7 @@ checksum = "645c6916888f6cb6350d2550b80fb63e734897a8498abe35cfb732b6487804b0" dependencies = [ "futures-channel", "futures-core", + "futures-executor", "futures-io", "futures-sink", "futures-task", @@ -450,6 +451,17 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dfc6580bb841c5a68e9ef15c77ccc837b40a7504914d52e47b8b0e9bbda25a1d" +[[package]] +name = "futures-executor" +version = "0.3.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a576fc72ae164fca6b9db127eaa9a9dda0d61316034f33a0a0d4eda41f02b01d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.30" @@ -548,6 +560,7 @@ dependencies = [ "anyhow", "clap", "dirs 5.0.1", + "futures", "harper-core", "itertools 0.12.1", "once_cell", diff --git a/harper-core/benches/parse_demo.rs b/harper-core/benches/parse_demo.rs index f049133b..a31c2075 100644 --- a/harper-core/benches/parse_demo.rs +++ b/harper-core/benches/parse_demo.rs @@ -1,5 +1,5 @@ use divan::{black_box, AllocProfiler, Bencher}; -use harper_core::{Document, FullDictionary, LintSet, Linter}; +use harper_core::{Document, FullDictionary, LintGroup, LintGroupConfig, Linter}; #[global_allocator] static ALLOC: AllocProfiler = AllocProfiler::system(); @@ -13,19 +13,10 @@ fn parse_demo(bencher: Bencher) { }); } -#[divan::bench] -fn create_lint_set(bencher: Bencher) { - let dictionary = FullDictionary::create_from_curated(); - - bencher.bench_local(|| { - let _lint_set = LintSet::new().with_standard(dictionary.clone()); - }); -} - #[divan::bench] fn lint_demo(bencher: Bencher) { let dictionary = FullDictionary::create_from_curated(); - let mut lint_set = LintSet::new().with_standard(dictionary); + let mut lint_set = LintGroup::new(Default::default(), dictionary); let document = Document::new_markdown(black_box(DEMO)); bencher.bench_local(|| { @@ -37,7 +28,7 @@ fn lint_demo(bencher: Bencher) { fn lint_demo_uncached(bencher: Bencher) { let dictionary = FullDictionary::create_from_curated(); bencher.bench_local(|| { - let mut lint_set = LintSet::new().with_standard(dictionary.clone()); + let mut lint_set = LintGroup::new(LintGroupConfig::default(), dictionary.clone()); let document = Document::new_markdown(black_box(DEMO)); lint_set.lint(&document); diff --git a/harper-core/src/lexing/url.rs b/harper-core/src/lexing/url.rs index 83a51365..95a829d7 100644 --- a/harper-core/src/lexing/url.rs +++ b/harper-core/src/lexing/url.rs @@ -36,8 +36,6 @@ fn lex_ip_schemepart(source: &[char]) -> Option { // Parse endpoint path while cursor != rest.len() { - dbg!(&rest[cursor..]); - if rest[cursor] != '/' { break; } @@ -84,8 +82,6 @@ fn lex_login(source: &[char]) -> Option { fn lex_hostport(source: &[char]) -> Option { let hostname_end = lex_hostname(source)?; - dbg!(&source[..hostname_end]); - if source.get(hostname_end) == Some(&':') { Some( source diff --git a/harper-core/src/lib.rs b/harper-core/src/lib.rs index 5db04dd6..23152d67 100644 --- a/harper-core/src/lib.rs +++ b/harper-core/src/lib.rs @@ -9,7 +9,7 @@ mod spell; mod token; pub use document::Document; -pub use linting::{Lint, LintKind, LintSet, Linter, Suggestion}; +pub use linting::{Lint, LintGroup, LintGroupConfig, LintKind, Linter, Suggestion}; pub use span::Span; pub use spell::{Dictionary, FullDictionary, MergedDictionary}; pub use token::{FatToken, Punctuation, Token, TokenKind, TokenStringExt}; diff --git a/harper-core/src/linting/lint_group.rs b/harper-core/src/linting/lint_group.rs new file mode 100644 index 00000000..730c374f --- /dev/null +++ b/harper-core/src/linting/lint_group.rs @@ -0,0 +1,107 @@ +use paste::paste; +use serde::{Deserialize, Serialize}; + +use super::an_a::AnA; +use super::long_sentences::LongSentences; +use super::matcher::Matcher; +use super::repeated_words::RepeatedWords; +use super::sentence_capitalization::SentenceCapitalization; +use super::spaces::Spaces; +use super::spell_check::SpellCheck; +use super::spelled_numbers::SpelledNumbers; +use super::unclosed_quotes::UnclosedQuotes; +use super::wrong_quotes::WrongQuotes; +use super::{Lint, Linter}; +use crate::{Dictionary, Document}; + +macro_rules! create_lint_group_config { + ($($linter:ident => $default:expr),*) => { + paste! { + #[derive(Debug, Clone, Copy, Serialize, Deserialize, Default)] + pub struct LintGroupConfig { + $( + #[doc = "Configures the use of the `" $linter "` linter. + If set to [`None`], the default configuration will be used."] + pub [<$linter:snake>]: Option, + )* + pub spell_check: Option + } + + impl LintGroupConfig { + /// Fills the [`None`] values in the configuration with the default values. + pub fn fill_default_values(&mut self){ + $( + if self.[<$linter:snake>].is_none() { + self.[<$linter:snake>] = Some($default); + } + )* + + if self.spell_check.is_none() { + self.spell_check = Some(true); + } + } + } + + pub struct LintGroup { + $( + [<$linter:snake>]: $linter, + )* + spell_check: SpellCheck, + pub config: LintGroupConfig + } + + + impl LintGroup { + pub fn new(config: LintGroupConfig, dictionary: T) -> Self { + Self { + $( + [<$linter:snake>]: $linter::default(), + )* + spell_check: SpellCheck::new(dictionary), + config, + } + } + } + + impl Linter for LintGroup { + fn lint(&mut self, document: &Document) -> Vec{ + let mut lints = Vec::new(); + + let mut config = self.config.clone(); + config.fill_default_values(); + + $( + if config.[<$linter:snake>].unwrap() { + lints.append(&mut self.[<$linter:snake>].lint(document)); + } + )* + + if config.spell_check.unwrap() { + lints.append(&mut self.spell_check.lint(document)); + } + + + lints + } + } + } + }; +} + +create_lint_group_config!( + SpelledNumbers => false, + AnA => true, + SentenceCapitalization => true, + UnclosedQuotes => true, + WrongQuotes => true, + LongSentences => true, + RepeatedWords => true, + Spaces => true, + Matcher => true +); + +impl Default for LintGroup { + fn default() -> Self { + Self::new(LintGroupConfig::default(), T::default()) + } +} diff --git a/harper-core/src/linting/lint_set.rs b/harper-core/src/linting/lint_set.rs deleted file mode 100644 index 6ecb8852..00000000 --- a/harper-core/src/linting/lint_set.rs +++ /dev/null @@ -1,113 +0,0 @@ -use paste::paste; - -use super::an_a::AnA; -use super::long_sentences::LongSentences; -use super::matcher::Matcher; -use super::repeated_words::RepeatedWords; -use super::sentence_capitalization::SentenceCapitalization; -use super::spaces::Spaces; -use super::spell_check::SpellCheck; -use super::spelled_numbers::SpelledNumbers; -use super::unclosed_quotes::UnclosedQuotes; -use super::wrong_quotes::WrongQuotes; -use super::Linter; -use crate::{Dictionary, Document, Lint}; - -pub struct LintSet { - pub(super) linters: Vec> -} - -impl Linter for LintSet { - fn lint(&mut self, document: &Document) -> Vec { - let mut lints = Vec::new(); - - for linter in &mut self.linters { - lints.append(&mut linter.lint(document)); - } - - lints.sort_by_key(|lint| lint.span.start); - - lints - } -} - -impl LintSet { - pub fn new() -> Self { - Self { - linters: Vec::new() - } - } - - pub fn add_standard(&mut self, dictionary: impl Dictionary + 'static) -> &mut Self { - self.add_repeated_words() - .add_spelled_numbers() - .add_an_a() - .add_long_sentences() - .add_unclosed_quotes() - .add_sentence_capitalization() - .add_matcher() - .add_spell_check(dictionary) - .add_spaces(); - self - } - - pub fn with_standard(mut self, dictionary: impl Dictionary + 'static) -> Self { - self.add_standard(dictionary); - self - } - - pub fn add_spell_check(&mut self, dictionary: impl Dictionary + 'static) -> &mut Self { - self.linters.push(Box::new(SpellCheck::new(dictionary))); - self - } - - pub fn with_spell_check(mut self, dictionary: impl Dictionary + 'static) -> Self { - self.add_spell_check(dictionary); - self - } -} - -impl Default for LintSet { - fn default() -> Self { - Self::new() - } -} - -/// Create builder methods for the linters that do not take any arguments. -macro_rules! create_simple_builder_methods { - ($($linter:ident),*) => { - impl LintSet { - paste! { - $( - #[doc = "Modifies self, adding the `" $linter "` linter to the set."] - pub fn [](&mut self) -> &mut Self{ - self.linters.push(Box::<$linter>::default()); - self - } - )* - } - - paste! { - $( - #[doc = "Consumes self, adding the `" $linter "` linter to the set."] - pub fn [](mut self) -> Self{ - self.[](); - self - } - )* - } - } - }; -} - -create_simple_builder_methods!( - SpelledNumbers, - AnA, - SentenceCapitalization, - UnclosedQuotes, - WrongQuotes, - LongSentences, - RepeatedWords, - Spaces, - Matcher -); diff --git a/harper-core/src/linting/mod.rs b/harper-core/src/linting/mod.rs index 38928367..34e7ca33 100644 --- a/harper-core/src/linting/mod.rs +++ b/harper-core/src/linting/mod.rs @@ -1,6 +1,6 @@ mod an_a; mod lint; -mod lint_set; +mod lint_group; mod long_sentences; mod matcher; mod repeated_words; @@ -12,7 +12,7 @@ mod unclosed_quotes; mod wrong_quotes; pub use lint::{Lint, LintKind, Suggestion}; -pub use lint_set::LintSet; +pub use lint_group::{LintGroup, LintGroupConfig}; use crate::Document; diff --git a/harper-ls/Cargo.toml b/harper-ls/Cargo.toml index 68a659e5..7665eadf 100644 --- a/harper-ls/Cargo.toml +++ b/harper-ls/Cargo.toml @@ -34,3 +34,4 @@ tracing = "0.1.40" tracing-subscriber = "0.3.18" resolve-path = "0.1.0" open = "5.1.1" +futures = "0.3.30" diff --git a/harper-ls/README.md b/harper-ls/README.md index 4f5619d3..7eb54cea 100644 --- a/harper-ls/README.md +++ b/harper-ls/README.md @@ -12,7 +12,7 @@ Refer to the linked documentation for more information. If you happen to use [`mason.nvim`](https://github.com/williamboman/mason.nvim), installation will be pretty straightforward. `harper-ls` is in the official Mason registry, so you can install it the same way you install anything through Mason. -If you __don't__ install your LSPs through Mason, we also have binary releases are available on [GitHub](https://github.com/elijah-potter/harper/releases). +If you __don't__ install your LSPs through Mason, we also have binary releases available on [GitHub](https://github.com/elijah-potter/harper/releases). Finally, if you have [Rust installed](https://www.rust-lang.org/tools/install), you're in luck! To install `harper-ls`, simply run: @@ -54,6 +54,30 @@ lspconfig.harper_ls.setup { } ``` +You can also toggle any particular linter. +The default values are shown below: + +```lua +lspconfig.harper_ls.setup { + settings = { + ["harper-ls"] = { + linters = { + spell_check = true, + spelled_numbers = false, + an_a = true, + sentence_capitalization = true, + unclosed_quotes = true, + wrong_quotes = true, + long_sentences = true, + repeated_words = true, + spaces = true, + matcher = true + } + } + }, +} +``` + ### File-Local Dictionary Sometimes, you'll encounter a word (or name) that is only valid within the context of a specific file. diff --git a/harper-ls/src/backend.rs b/harper-ls/src/backend.rs index 768ce43d..cedeb32b 100644 --- a/harper-ls/src/backend.rs +++ b/harper-ls/src/backend.rs @@ -7,14 +7,14 @@ use harper_core::{ Dictionary, Document, FullDictionary, - LintSet, + LintGroup, Linter, MergedDictionary, Token, TokenKind }; use serde_json::Value; -use tokio::sync::Mutex; +use tokio::sync::{Mutex, RwLock}; use tower_lsp::jsonrpc::Result; use tower_lsp::lsp_types::notification::{PublishDiagnostics, ShowMessage}; use tower_lsp::lsp_types::{ @@ -46,7 +46,7 @@ use tower_lsp::lsp_types::{ Url }; use tower_lsp::{Client, LanguageServer}; -use tracing::{error, info}; +use tracing::{error, info, instrument}; use crate::config::Config; use crate::diagnostics::{lint_to_code_actions, lints_to_diagnostics}; @@ -58,14 +58,14 @@ use crate::tree_sitter_parser::TreeSitterParser; struct DocumentState { document: Document, ident_dict: Arc, - linter: LintSet + linter: LintGroup> } /// Deallocate pub struct Backend { client: Client, static_dictionary: Arc, - config: Mutex, + config: RwLock, doc_state: Mutex> } @@ -77,7 +77,7 @@ impl Backend { client, static_dictionary: dictionary.into(), doc_state: Mutex::new(HashMap::new()), - config: Mutex::new(config) + config: RwLock::new(config) } } @@ -95,12 +95,15 @@ impl Backend { rewritten.into() } + /// Get the location of the file's specific dictionary + #[instrument(skip(self))] async fn get_file_dict_path(&self, url: &Url) -> PathBuf { - let config = self.config.lock().await; + let config = self.config.read().await; config.file_dict_path.join(Self::file_dict_name(url)) } + /// Load a specific file's dictionary async fn load_file_dictionary(&self, url: &Url) -> FullDictionary { match load_dict(self.get_file_dict_path(url).await).await { Ok(dict) => dict, @@ -112,12 +115,13 @@ impl Backend { } } + #[instrument(skip(self, dict))] async fn save_file_dictionary(&self, url: &Url, dict: impl Dictionary) -> anyhow::Result<()> { Ok(save_dict(self.get_file_dict_path(url).await, dict).await?) } async fn load_user_dictionary(&self) -> FullDictionary { - let config = self.config.lock().await; + let config = self.config.read().await; info!( "Loading user dictionary from `{}`", @@ -138,8 +142,9 @@ impl Backend { } } + #[instrument(skip_all)] async fn save_user_dictionary(&self, dict: impl Dictionary) -> anyhow::Result<()> { - let config = self.config.lock().await; + let config = self.config.read().await; info!( "Saving user dictionary to `{}`", @@ -153,6 +158,7 @@ impl Backend { Ok(save_dict(&config.user_dict_path, dict).await?) } + #[instrument(skip(self))] async fn generate_global_dictionary(&self) -> anyhow::Result> { let mut dict = MergedDictionary::new(); dict.add_dictionary(self.static_dictionary.clone()); @@ -161,6 +167,7 @@ impl Backend { Ok(dict) } + #[instrument(skip(self))] async fn generate_file_dictionary( &self, url: &Url @@ -176,6 +183,7 @@ impl Backend { Ok(global_dictionary) } + #[instrument(skip(self))] async fn update_document_from_file(&self, url: &Url) -> anyhow::Result<()> { let content = match tokio::fs::read_to_string(url.path()).await { Ok(content) => content, @@ -188,13 +196,18 @@ impl Backend { self.update_document(url, &content).await } + #[instrument(skip(self, text))] async fn update_document(&self, url: &Url, text: &str) -> anyhow::Result<()> { - let mut lock = self.doc_state.lock().await; + let mut doc_lock = self.doc_state.lock().await; + let config_lock = self.config.read().await; // TODO: Only reset linter when underlying dictionaries change let mut doc_state = DocumentState { - linter: LintSet::new().with_standard(self.generate_file_dictionary(url).await?), + linter: LintGroup::new( + config_lock.lint_config, + self.generate_file_dictionary(url).await? + ), ..Default::default() }; @@ -212,7 +225,7 @@ impl Backend { let mut merged = self.generate_file_dictionary(url).await?; merged.add_dictionary(new_dict); - doc_state.linter = LintSet::new().with_standard(merged); + doc_state.linter = LintGroup::new(config_lock.lint_config, merged); } } @@ -224,11 +237,12 @@ impl Backend { Document::new(text, Box::new(Markdown)) }; - lock.insert(url.clone(), doc_state); + doc_lock.insert(url.clone(), doc_state); Ok(()) } + #[instrument(skip(self))] async fn generate_code_actions( &self, url: &Url, @@ -269,6 +283,7 @@ impl Backend { Ok(actions) } + #[instrument(skip(self))] async fn generate_diagnostics(&self, url: &Url) -> Vec { let mut doc_states = self.doc_state.lock().await; let Some(doc_state) = doc_states.get_mut(url) else { @@ -280,6 +295,7 @@ impl Backend { lints_to_diagnostics(doc_state.document.get_full_content(), &lints) } + #[instrument(skip(self))] async fn publish_diagnostics(&self, url: &Url) { let client = self.client.clone(); @@ -465,7 +481,7 @@ impl LanguageServer for Backend { async fn did_change_configuration(&self, params: DidChangeConfigurationParams) { info!("Changing user configuration."); - let new_config = match Config::from_json_value(params.settings) { + let new_config = match Config::from_lsp_config(params.settings) { Ok(new_config) => new_config, Err(err) => { error!("Unable to change config: {}", err); @@ -473,8 +489,18 @@ impl LanguageServer for Backend { } }; - let mut config = self.config.lock().await; - *config = new_config; + { + let mut config = self.config.write().await; + *config = new_config; + } + + let keys: Vec = { + let doc_state = self.doc_state.lock().await; + + doc_state.keys().cloned().collect() + }; + + futures::future::join_all(keys.iter().map(|key| self.update_document_from_file(key))).await; } async fn code_action(&self, params: CodeActionParams) -> Result> { diff --git a/harper-ls/src/config.rs b/harper-ls/src/config.rs index 54f91fae..d5579160 100644 --- a/harper-ls/src/config.rs +++ b/harper-ls/src/config.rs @@ -1,17 +1,19 @@ use std::path::PathBuf; use dirs::{config_dir, data_local_dir}; +use harper_core::LintGroupConfig; use resolve_path::PathResolveExt; use serde_json::Value; #[derive(Debug, Clone)] pub struct Config { pub user_dict_path: PathBuf, - pub file_dict_path: PathBuf + pub file_dict_path: PathBuf, + pub lint_config: LintGroupConfig } impl Config { - pub fn from_json_value(value: Value) -> anyhow::Result { + pub fn from_lsp_config(value: Value) -> anyhow::Result { let mut base = Config::default(); let Value::Object(value) = value else { @@ -32,6 +34,12 @@ impl Config { } } + if let Some(v) = value.get("linters") { + dbg!(v); + base.lint_config = serde_json::from_value(v.clone())?; + dbg!(base.lint_config); + } + Ok(base) } } @@ -42,7 +50,8 @@ impl Default for Config { user_dict_path: config_dir().unwrap().join("harper-ls/dictionary.txt"), file_dict_path: data_local_dir() .unwrap() - .join("harper-ls/file_dictionaries/") + .join("harper-ls/file_dictionaries/"), + lint_config: LintGroupConfig::default() } } } diff --git a/harper-serve/src/main.rs b/harper-serve/src/main.rs index e1803b2d..96cc21a9 100644 --- a/harper-serve/src/main.rs +++ b/harper-serve/src/main.rs @@ -8,7 +8,7 @@ use axum::middleware::{self, Next}; use axum::response::Response; use axum::routing::post; use axum::{Json, Router}; -use harper_core::{Document, FatToken, FullDictionary, Lint, LintSet, Linter, Span, Suggestion}; +use harper_core::{Document, FatToken, FullDictionary, Lint, LintGroup, Linter, Span, Suggestion}; use serde::{Deserialize, Serialize}; use tokio::time::Instant; use tracing::{info, Level}; @@ -90,7 +90,8 @@ async fn lint(Json(payload): Json) -> (StatusCode, Json> = - Lazy::new(|| Mutex::new(LintSet::new().with_standard(FullDictionary::create_from_curated()))); +static LINTER: Lazy>> = Lazy::new(|| { + Mutex::new(LintGroup::new( + Default::default(), + FullDictionary::create_from_curated() + )) +}); /// Create the serializer that preserves types across the JavaScript barrier fn glue_serializer() -> serde_wasm_bindgen::Serializer { diff --git a/web/src/lib/analysis.ts b/web/src/lib/analysis.ts index 182d70f3..48b034f2 100644 --- a/web/src/lib/analysis.ts +++ b/web/src/lib/analysis.ts @@ -1,4 +1,4 @@ -const defaultUseWasm = true; +const defaultUseWasm = false; export interface ParseResponse { tokens: Token[];