Skip to content

feat: publish some linter priorities that other linters refer to - #4255

Open
hippietrail wants to merge 4 commits into
Automattic:masterfrom
hippietrail:public_linter_priorities
Open

feat: publish some linter priorities that other linters refer to#4255
hippietrail wants to merge 4 commits into
Automattic:masterfrom
hippietrail:public_linter_priorities

Conversation

@hippietrail

Copy link
Copy Markdown
Collaborator

Issues

N/A

Description

We already had a couple of linters that declared their priorities had to be higher or lower than a certain other linter. This was only noted in comments.

While investigating why "unvisible" was flagged by SpellCheck instead of WrongNegative, I wanted to do this again. So instead I've made doing this a bit more robust by publishing the priorities of two linters and the other linters can bump those up or down in logic rather than just in comments.

One closely related problem is that lower priority numbers mean higher priority. So one of the existing ones may have had this backward. I kept its logic as it was but added a comment stating this possibility.

Update: I added a test to make sure WrongNegative gets a shot at linting an error before SpellCheck. In doing so I discovered that assert_lint_message only failed if there was a lint message but it didn't match. I now also fails if there's no lint message at all. This revealed that two long-standing tests have been silently failing: ves_the_right_message_for_canadian_english in out_of_the_window.rs and not_confident_proc_should_be_pro_c in split_words.

I removed the one from split_words which tested that "proc" got corrected to "pro c" since splits resulting in single letters was specifically removed in #3857 - so I moved "proc" to that test.

The one in out_of_the_window had silently been backwards and is now correct.

One gotcha was that to test multiple linters on the same mistake, you have to manually call remove_overlaps(), which was not obvious to me.

How Has This Been Tested?

cargo test

AI Disclosure

  • I am a human and didn't use any AI.
  • I used LLM features of my editor, but not an agent.
  • I consulted one or more coding AIs, but didn't use an agent.
  • I used an AI agent interactively.
  • I am an agent or I got an agent to do the work autonomously.

If Your PR Implements or Enhances a Linter

  • I made up the sentences in the unit tests.
  • The sentences in the unit tests were generated by an AI.
  • I'm using examples from the bug report / feature request.
  • I collected real-world sentences for the unit tests.

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.

- `assert_lint_message` only failed when the message didn't match - not when no message was generated at all
- Due to the above, a test in `out_of_the_window` had been failing silently.
- Due to the above, a test in `split_words` had been silently failing since a logic update a month ago.
- Add a test to make sure that when `SpellCheck` and `WrongNegative` lints are both enabled, `WrongNegative` gets first stab at fixing errors relevant to both.
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.

1 participant