From 4633d74913a2b53f11df952c7d98231eec8e0f7c Mon Sep 17 00:00:00 2001 From: hippietrail Date: Wed, 24 Dec 2025 21:00:03 +0700 Subject: [PATCH 1/2] fix: warn about clashing linter names --- harper-cli/src/lint.rs | 8 ++++++++ harper-core/src/linting/lint_group.rs | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/harper-cli/src/lint.rs b/harper-cli/src/lint.rs index 8576a4551..f49a57fce 100644 --- a/harper-cli/src/lint.rs +++ b/harper-cli/src/lint.rs @@ -386,6 +386,14 @@ fn configure_lint_group( .iter() .for_each(|rule| lint_group.config.set_rule_enabled(rule, false)); } + + // Have all rules been disabled somehow? + if !lint_group + .iter_keys() + .any(|rule| lint_group.config.is_rule_enabled(rule)) + { + eprintln!("Warning: No rules are enabled."); + } } fn count_lint_kinds_and_rules( diff --git a/harper-core/src/linting/lint_group.rs b/harper-core/src/linting/lint_group.rs index 5f9949048..d3deebacb 100644 --- a/harper-core/src/linting/lint_group.rs +++ b/harper-core/src/linting/lint_group.rs @@ -363,6 +363,10 @@ impl LintGroup { /// If it returns `false`, it is because a linter with that key already existed in the group. pub fn add(&mut self, name: impl AsRef, linter: impl Linter + 'static) -> bool { if self.contains_key(&name) { + eprintln!( + "⚠️ [LintGroup::add] Linter with name `{}` already exists", + name.as_ref() + ); false } else { self.linters @@ -383,6 +387,10 @@ impl LintGroup { linter: impl ExprLinter + 'static, ) -> bool { if self.contains_key(&name) { + eprintln!( + "⚠️ [LintGroup::add_chunk_expr_linter] Linter with name `{}` already exists", + name.as_ref() + ); false } else { self.chunk_expr_linters @@ -397,9 +405,23 @@ impl LintGroup { self.config.merge_from(&mut other.config); let other_linters = std::mem::take(&mut other.linters); + if let Some((conflicting_key, _)) = other_linters.iter().find(|(k, _)| self.contains_key(k)) + { + eprintln!( + "⚠️ [LintGroup::merge_from] Linter with name `{conflicting_key}` already exists" + ); + } self.linters.extend(other_linters); let other_expr_linters = std::mem::take(&mut other.chunk_expr_linters); + if let Some((conflicting_key, _)) = other_expr_linters + .iter() + .find(|(k, _)| self.contains_key(k)) + { + eprintln!( + "⚠️ [LintGroup::merge_from] ExprLinter with name `{conflicting_key}` already exists" + ); + } self.chunk_expr_linters.extend(other_expr_linters); } From 31a07a964caaec6b4004ff0fedab9bff10722115 Mon Sep 17 00:00:00 2001 From: hippietrail Date: Wed, 7 Jan 2026 17:04:01 +0700 Subject: [PATCH 2/2] refactor: clash linter name dection is now a test --- harper-core/src/linting/lint_group.rs | 155 +++++++++++++++----------- 1 file changed, 91 insertions(+), 64 deletions(-) diff --git a/harper-core/src/linting/lint_group.rs b/harper-core/src/linting/lint_group.rs index b5c4fe276..2a7d77902 100644 --- a/harper-core/src/linting/lint_group.rs +++ b/harper-core/src/linting/lint_group.rs @@ -346,6 +346,7 @@ pub struct LintGroup { /// of the key. chunk_expr_cache: LruCache<(CharString, u64), BTreeMap>>, hasher_builder: RandomState, + clashing_linter_names: Option>, } impl LintGroup { @@ -356,6 +357,7 @@ impl LintGroup { chunk_expr_linters: BTreeMap::new(), chunk_expr_cache: LruCache::new(NonZero::new(1000).unwrap()), hasher_builder: RandomState::default(), + clashing_linter_names: None, } } @@ -369,10 +371,14 @@ impl LintGroup { /// If it returns `false`, it is because a linter with that key already existed in the group. pub fn add(&mut self, name: impl AsRef, linter: impl Linter + 'static) -> bool { if self.contains_key(&name) { - eprintln!( - "⚠️ [LintGroup::add] Linter with name `{}` already exists", - name.as_ref() - ); + if self.clashing_linter_names.is_none() { + self.clashing_linter_names = Some(vec![name.as_ref().to_string()]); + } else { + self.clashing_linter_names + .as_mut() + .unwrap() + .push(name.as_ref().to_string()); + } false } else { self.linters @@ -393,10 +399,14 @@ impl LintGroup { linter: impl ExprLinter + 'static, ) -> bool { if self.contains_key(&name) { - eprintln!( - "⚠️ [LintGroup::add_chunk_expr_linter] Linter with name `{}` already exists", - name.as_ref() - ); + if self.clashing_linter_names.is_none() { + self.clashing_linter_names = Some(vec![name.as_ref().to_string()]); + } else { + self.clashing_linter_names + .as_mut() + .unwrap() + .push(name.as_ref().to_string()); + } false } else { self.chunk_expr_linters @@ -413,9 +423,14 @@ impl LintGroup { let other_linters = std::mem::take(&mut other.linters); if let Some((conflicting_key, _)) = other_linters.iter().find(|(k, _)| self.contains_key(k)) { - eprintln!( - "⚠️ [LintGroup::merge_from] Linter with name `{conflicting_key}` already exists" - ); + if self.clashing_linter_names.is_none() { + self.clashing_linter_names = Some(vec![conflicting_key.clone()]); + } else { + self.clashing_linter_names + .as_mut() + .unwrap() + .push(conflicting_key.clone()); + } } self.linters.extend(other_linters); @@ -424,9 +439,14 @@ impl LintGroup { .iter() .find(|(k, _)| self.contains_key(k)) { - eprintln!( - "⚠️ [LintGroup::merge_from] ExprLinter with name `{conflicting_key}` already exists" - ); + if self.clashing_linter_names.is_none() { + self.clashing_linter_names = Some(vec![conflicting_key.clone()]); + } else { + self.clashing_linter_names + .as_mut() + .unwrap() + .push(conflicting_key.clone()); + } } self.chunk_expr_linters.extend(other_expr_linters); } @@ -486,7 +506,7 @@ impl LintGroup { pub fn new_curated(dictionary: Arc, dialect: Dialect) -> Self { let mut out = Self::empty(); - /// Add a `Linter` to the group, setting it to be enabled by default. + /// Add a `Linter` to the group, setting it to be enabled or disabled. macro_rules! insert_struct_rule { ($rule:ident, $default_config:expr) => { out.add(stringify!($rule), $rule::default()); @@ -495,7 +515,25 @@ impl LintGroup { }; } - /// Add a chunk-based `ExprLinter` to the group, setting it to be enabled by default. + /// Add a `Linter` that requires a `Dictionary` to the group, setting it to be enabled or disabled. + macro_rules! insert_struct_rule_with_dict { + ($rule:ident, $default_config:expr) => { + out.add(stringify!($rule), $rule::new(dictionary.clone())); + out.config + .set_rule_enabled(stringify!($rule), $default_config); + }; + } + + /// Add a `Linter` that requires a `Dialect` to the group, setting it to be enabled or disabled. + macro_rules! insert_struct_rule_with_dialect { + ($rule:ident, $default_config:expr) => { + out.add(stringify!($rule), $rule::new(dialect)); + out.config + .set_rule_enabled(stringify!($rule), $default_config); + }; + } + + /// Add a chunk-based `ExprLinter` to the group, setting it to be enabled or disabled. /// While you _can_ pass an `ExprLinter` to `insert_struct_rule`, using this macro instead /// will allow it to use more aggressive caching strategies. macro_rules! insert_expr_rule { @@ -506,6 +544,15 @@ impl LintGroup { }; } + /// Add a chunk-based `ExprLinter` that requires a `Dictionary` to the group, setting it to be enabled or disabled. + macro_rules! insert_expr_rule_with_dict { + ($rule:ident, $default_config:expr) => { + out.add_chunk_expr_linter(stringify!($rule), $rule::new(dictionary.clone())); + out.config + .set_rule_enabled(stringify!($rule), $default_config); + }; + } + out.merge_from(&mut phrase_corrections::lint_group()); out.merge_from(&mut phrase_set_corrections::lint_group()); out.merge_from(&mut proper_noun_capitalization_linters::lint_group( @@ -527,6 +574,7 @@ impl LintGroup { insert_expr_rule!(AllowTo, true); insert_expr_rule!(AmInTheMorning, true); insert_expr_rule!(AmountsFor, true); + insert_struct_rule_with_dialect!(AnA, true); insert_expr_rule!(AndIn, true); insert_expr_rule!(AndTheLike, true); insert_expr_rule!(AnotherThingComing, true); @@ -558,6 +606,7 @@ impl LintGroup { insert_expr_rule!(DespiteOf, true); insert_expr_rule!(Didnt, true); insert_struct_rule!(DiscourseMarkers, true); + insert_expr_rule_with_dict!(DisjointPrefixes, true); insert_expr_rule!(DotInitialisms, true); insert_expr_rule!(DoubleClick, true); insert_expr_rule!(DoubleModal, true); @@ -582,6 +631,7 @@ impl LintGroup { insert_expr_rule!(GoodAt, true); insert_expr_rule!(Handful, true); insert_expr_rule!(HavePronoun, true); + insert_struct_rule_with_dialect!(HaveTakeALook, true); insert_expr_rule!(Hedging, true); insert_expr_rule!(HelloGreeting, true); insert_expr_rule!(Hereby, true); @@ -590,6 +640,8 @@ impl LintGroup { insert_expr_rule!(HyphenateNumberDay, true); insert_expr_rule!(IAmAgreement, true); insert_expr_rule!(IfWouldve, true); + insert_struct_rule_with_dialect!(InOnTheCards, true); + insert_struct_rule_with_dict!(InflectedVerbAfterTo, true); insert_expr_rule!(InterestedIn, true); insert_expr_rule!(ItLooksLikeThat, true); insert_struct_rule!(ItsContraction, true); @@ -603,6 +655,7 @@ impl LintGroup { insert_expr_rule!(Likewise, true); insert_struct_rule!(LongSentences, true); insert_expr_rule!(LookingForwardTo, true); + insert_struct_rule_with_dict!(MassNouns, true); insert_struct_rule!(MergeWords, true); insert_expr_rule!(MissingPreposition, true); insert_expr_rule!(MissingTo, true); @@ -630,6 +683,7 @@ impl LintGroup { insert_expr_rule!(OnFloor, true); insert_expr_rule!(OnceOrTwice, true); insert_expr_rule!(OneAndTheSame, true); + insert_expr_rule_with_dict!(OneOfTheSingular, true); insert_expr_rule!(OpenCompounds, true); insert_expr_rule!(OpenTheLight, true); insert_expr_rule!(OrthographicConsistency, true); @@ -640,6 +694,7 @@ impl LintGroup { insert_struct_rule!(PhrasalVerbAsCompoundNoun, true); insert_expr_rule!(PiqueInterest, true); insert_expr_rule!(PluralWrongWordOfPhrase, true); + insert_struct_rule_with_dict!(PossessiveNoun, false); insert_expr_rule!(PossessiveYour, true); insert_expr_rule!(ProgressiveNeedsBe, true); insert_expr_rule!(PronounAre, true); @@ -652,6 +707,7 @@ impl LintGroup { insert_struct_rule!(QuoteSpacing, true); insert_expr_rule!(RedundantAcronyms, true); insert_expr_rule!(RedundantAdditiveAdverbs, true); + insert_struct_rule_with_dialect!(Regionalisms, true); insert_struct_rule!(RepeatedWords, true); insert_expr_rule!(Respond, true); insert_expr_rule!(RightClick, true); @@ -659,6 +715,7 @@ impl LintGroup { insert_expr_rule!(SafeToSave, true); insert_expr_rule!(SaveToSafe, true); insert_expr_rule!(SemicolonApostrophe, true); + insert_struct_rule_with_dict!(SentenceCapitalization, true); insert_expr_rule!(ShootOneselfInTheFoot, true); insert_expr_rule!(SimplePastToPastParticiple, true); insert_expr_rule!(SinceDuration, true); @@ -688,9 +745,11 @@ impl LintGroup { insert_expr_rule!(ToAdverb, true); insert_struct_rule!(ToTwoToo, true); insert_expr_rule!(Touristic, true); + insert_expr_rule_with_dict!(TransposedSpace, true); insert_struct_rule!(UnclosedQuotes, true); insert_expr_rule!(UpdatePlaceNames, true); insert_expr_rule!(UseGenitive, false); + insert_struct_rule_with_dict!(UseTitleCase, true); insert_expr_rule!(VerbToAdjective, true); insert_expr_rule!(VeryUnique, true); insert_expr_rule!(ViceVersa, true); @@ -707,54 +766,6 @@ impl LintGroup { out.add("SpellCheck", SpellCheck::new(dictionary.clone(), dialect)); out.config.set_rule_enabled("SpellCheck", true); - out.add( - "InflectedVerbAfterTo", - InflectedVerbAfterTo::new(dictionary.clone()), - ); - out.config.set_rule_enabled("InflectedVerbAfterTo", true); - - out.add("InOnTheCards", InOnTheCards::new(dialect)); - out.config.set_rule_enabled("InOnTheCards", true); - - out.add( - "SentenceCapitalization", - SentenceCapitalization::new(dictionary.clone()), - ); - out.config.set_rule_enabled("SentenceCapitalization", true); - - out.add("PossessiveNoun", PossessiveNoun::new(dictionary.clone())); - out.config.set_rule_enabled("PossessiveNoun", false); - - out.add("Regionalisms", Regionalisms::new(dialect)); - out.config.set_rule_enabled("Regionalisms", true); - - out.add("HaveTakeALook", HaveTakeALook::new(dialect)); - out.config.set_rule_enabled("HaveTakeALook", true); - - out.add("MassNouns", MassNouns::new(dictionary.clone())); - out.config.set_rule_enabled("MassNouns", true); - - out.add("UseTitleCase", UseTitleCase::new(dictionary.clone())); - out.config.set_rule_enabled("UseTitleCase", true); - - out.add_chunk_expr_linter( - "DisjointPrefixes", - DisjointPrefixes::new(dictionary.clone()), - ); - out.config.set_rule_enabled("DisjointPrefixes", true); - - out.add_chunk_expr_linter("TransposedSpace", TransposedSpace::new(dictionary.clone())); - out.config.set_rule_enabled("TransposedSpace", true); - - out.add_chunk_expr_linter( - "OneOfTheSingular", - OneOfTheSingular::new(dictionary.clone()), - ); - out.config.set_rule_enabled("OneOfTheSingular", true); - - out.add("AnA", AnA::new(dialect)); - out.config.set_rule_enabled("AnA", true); - out } @@ -919,4 +930,20 @@ mod tests { } } } + + #[test] + fn no_linter_names_clash() { + let group = + LintGroup::new_curated(Arc::new(MutableDictionary::default()), Dialect::American); + + if let Some(names) = &group.clashing_linter_names { + if !names.is_empty() { + panic!( + "⚠️ Found {} clashing linter names: {}", + names.len(), + names.join(", ") + ); + } + } + } }