Skip to content

fix: find clashing linter names when part of merged linter#3244

Open
hippietrail wants to merge 10 commits into
Automattic:masterfrom
hippietrail:find-clashing-merged-linter-names-3241
Open

fix: find clashing linter names when part of merged linter#3244
hippietrail wants to merge 10 commits into
Automattic:masterfrom
hippietrail:find-clashing-merged-linter-names-3241

Conversation

@hippietrail
Copy link
Copy Markdown
Collaborator

@hippietrail hippietrail commented Apr 25, 2026

Issues

Closes #3241

Description

🚨 NOTE: THE CI IS EXPECTED TO FAIL AS THIS PR FINDS A CLASHING LINTER NAME!! 🚨

It was found by accident that the name of a standalone linter is the same as one of the component linters that make up a merged linter. The previous code that found clashing linter names did not pick up on this. Probably due to expecting that merged linters used LintGroups, which is not the case.

After trying several ways to attack this using the Windsurf built-in AI I was able to learn enough to see a much more minimal solution. The AI then implemented it the way I suggested. I then simplified the code, renamed some variables, and went over every line manually.

I then discovered that the code only went one level deep and did not cover when a merged linter is composed of other merged linters.

I then iterated between the AI trying code to handle this, me reworking its code, getting it to check my code, etc. So it very much warrants being critiqued by a human who understands this part of the codebase.

Demo

---- linting::lint_group::tests::no_linter_names_clash stdout ----

thread 'linting::lint_group::tests::no_linter_names_clash' (97628475) panicked at harper-core/src/linting/lint_group/mod.rs:1042:17:
⚠️ Found 1 clashing linter names: TooTo (part of: ToTwoToo)


failures:
    linting::lint_group::structured_config::tests::curated_default_config_lists_every_registered_rule
    linting::lint_group::tests::no_linter_names_clash

test result: FAILED. 5159 passed; 2 failed; 252 ignored; 0 measured; 0 filtered out; finished in 10.48s

How Has This Been Tested?

Manually.

Checklist

  • I have performed a self-review of my own code
  • I have added tests to cover my changes
  • I have considered splitting this into smaller pull requests.

@hippietrail
Copy link
Copy Markdown
Collaborator Author

hippietrail commented Apr 26, 2026

The initial version of the PR didn't find clashes when a merged linter contained merged linters, as is the case with NounVerbConfusion

@hippietrail
Copy link
Copy Markdown
Collaborator Author

I keep finding a surprising number of edge cases that make the code more complex. Turning it into a draft while I continue to work through it all.

@hippietrail hippietrail marked this pull request as draft April 30, 2026 11:10
@hippietrail hippietrail marked this pull request as ready for review May 4, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clashing linters named TooTo are not detected by no_linter_names_clash()

1 participant