Skip to content

fix: warn about clashing linter names #2374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hippietrail wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix: warn about clashing linter names #2374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4633d74
fix: warn about clashing linter names
hippietrail Dec 24, 2025
dfd6dde
Merge branch 'master' into linter-clash-2343
elijah-potter Jan 6, 2026
e570f5f
Merge branch 'master' of https://github.com/Automattic/harper into li…
hippietrail Jan 7, 2026
31a07a9
refactor: clash linter name dection is now a test
hippietrail Jan 7, 2026
25609b5
Merge branch 'linter-clash-2343' of https://github.com/hippietrail/ha…
hippietrail Jan 7, 2026
8991055
Merge branch 'master' of https://github.com/Automattic/harper into li…
hippietrail Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions harper-cli/src/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,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(
Expand Down
149 changes: 99 additions & 50 deletions harper-core/src/linting/lint_group.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ pub struct LintGroup {
/// of the key.
chunk_expr_cache: LruCache<(CharString, u64), BTreeMap<String, Vec<Lint>>>,
hasher_builder: RandomState,
clashing_linter_names: Option<Vec<String>>,
}

impl LintGroup {
Expand All @@ -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,
}
}

Expand All @@ -375,6 +377,14 @@ impl LintGroup {
/// If it returns `false`, it is because a linter with that key already existed in the group.
pub fn add_boxed(&mut self, name: impl AsRef<str>, linter: Box<dyn Linter>) -> bool {
if self.contains_key(&name) {
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.insert(name.as_ref().to_string(), linter);
Expand All @@ -394,6 +404,14 @@ impl LintGroup {
linter: impl ExprLinter<Unit = Chunk> + 'static,
) -> bool {
if self.contains_key(&name) {
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
Expand All @@ -408,9 +426,33 @@ 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))
{
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);

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))
{
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);
}

Expand Down Expand Up @@ -469,7 +511,7 @@ impl LintGroup {
pub fn new_curated(dictionary: Arc<impl Dictionary + 'static>, 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());
Expand All @@ -478,7 +520,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 {
Expand All @@ -489,6 +549,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 weir_rules::lint_group());
out.merge_from(&mut phrase_set_corrections::lint_group());
out.merge_from(&mut proper_noun_capitalization_linters::lint_group(
Expand All @@ -510,6 +579,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);
Expand Down Expand Up @@ -541,6 +611,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);
Expand All @@ -565,6 +636,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);
Expand All @@ -573,6 +645,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);
Expand All @@ -586,6 +660,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);
Expand Down Expand Up @@ -613,6 +688,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);
Expand All @@ -623,6 +699,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);
Expand All @@ -635,13 +712,15 @@ 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);
insert_expr_rule!(RollerSkated, true);
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);
Expand Down Expand Up @@ -672,8 +751,10 @@ 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_struct_rule_with_dict!(UseTitleCase, true);
insert_expr_rule!(VerbToAdjective, true);
insert_expr_rule!(VeryUnique, true);
insert_expr_rule!(ViceVersa, true);
Expand All @@ -690,54 +771,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
}

Expand Down Expand Up @@ -902,4 +935,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(", ")
);
}
}
}
}
Loading