Make `UselssAssertion` autocorrect `contextual`

rubocop / rubocop-minitest

Code style checking for Minitest files.

https://docs.rubocop.org/rubocop-minitest

MIT License

144 stars 44 forks source link

Make `UselssAssertion` autocorrect `contextual` #308

Closed Earlopain closed 8 months ago

Earlopain commented 8 months ago

This is an annoying autocorrect to happen while typing.

Before submitting the PR make sure the following are checked:

[x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
[x] Wrote good commit messages.
[ ] Commit message starts with [Fix #issue-number] (if the related issue exists).
[x] Feature branch is up-to-date with master (if not - rebase it).
[ ] Squashed related commits together.
[ ] Added tests.
[x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
[ ] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

koic commented 8 months ago

Hm, I don't see the use case for setting this cop to AutoCorrect: contextual.

Earlopain commented 8 months ago

Here's how it goes for me usually

Screencast_20240407_173508.webm

koic commented 8 months ago

Thank you for making the video. However, I didn't quite get it. Can you provide some additional explanation in text?

Earlopain commented 8 months ago

Sure, I can try. When I write tests I tend to do this:

assert_equal([], some_method)
# Expected: []
# Actual: ["foo", "bar"]

I let the tests run, get the data from the failure, and copy it over to the first argument to make it pass. With the autocorrect I have an extra step where I need to restore the intended assert_equal to make it work. I guess I could write anything else than [] but when I know it'll be an array I just do it automatically.

Hope that makes sense.

andyw8 commented 8 months ago

I understand the annoyance for your workflow, but I think this shouldn't be marked as contextual. I think there would be many genuine occurrences that won't be caught until a full RuboCop check is run, e.g. after pushing to CI.

koic commented 8 months ago

I agree with @andyw8. So, applying AutoCorrect: contextual for cases clearly written with unintended arguments would mean that most cops in RuboCop Minitest would no longer be AutoCorrect: always (default). I will close this PR, but thank you for the video and the explanation.