Skip to content
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.

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

[Fix #255] Add new Minitest/AssertOperator and Minitest/RefuteOperator cops #260

Conversation

koic
Copy link
Member

@koic koic commented Sep 21, 2023

Fixes #255.

This PR adds new Minitest/AssertOperator and Minitest/RefuteOperator cops.

Minitest/AssertOperator cop

This cop enforces the use of assert_operator(expected, :<, actual) over assert(expected < actual).

# bad
assert(expected < actual)

# good
assert_operator(expected, :<, actual)

Minitest/RefuteOperator cop

This cop enforces the use of refute_operator(expected, :<, actual) over refute(expected < actual).

# bad
refute(expected < actual)

# good
refute_operator(expected, :<, actual)

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • 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.

…uteOperator` cops

Fixes rubocop#255.

This PR adds new `Minitest/AssertOperator` and `Minitest/RefuteOperator` cops.

## `Minitest/AssertOperator` cop

This cop enforces the use of `assert_operator(expected, :<, actual)` over `assert(expected < actual)`.

```ruby
# bad
assert(expected < actual)

# good
assert_operator(expected, :<, actual)
```

## `Minitest/RefuteOperator` cop

This cop enforces the use of `refute_operator(expected, :<, actual)` over `refute(expected < actual)`.

```ruby
# bad
refute(expected < actual)

# good
refute_operator(expected, :<, actual)
```
@koic koic merged commit 71f70d7 into rubocop:master Sep 22, 2023
10 checks passed
@koic koic deleted the add_new_minitest_assert_operator_and_refute_operator_cops branch September 22, 2023 14:05
@GUI
Copy link

GUI commented Sep 25, 2023

@koic: Is it intentional that this triggers for hash access too? I understand the benefits for math operators like in these examples and in the style guide, but these suggestions for hashes seem less idiomatic to me:

[Correctable] Minitest/RefuteOperator: Prefer using refute_operator(response.headers, :[], "content-length").
    refute(response.headers["content-length"])

[Correctable] Minitest/AssertOperator: Prefer using assert_operator(data["headers"], :[], "authorization").
    assert(data["headers"]["authorization"])

Since I didn't see this type of thing called out in the style guide, I just wasn't sure if this new behavior was intentional or not. Thanks!

@flavorjones
Copy link
Contributor

@GUI you may be interested in #264

@koic
Copy link
Member Author

koic commented Sep 27, 2023

I've released 0.32.2 with that change. Thank you!
https://github.com/rubocop/rubocop-minitest/releases/tag/v0.32.2

@TSMMark
Copy link
Contributor

TSMMark commented Sep 28, 2023

This is awesome thanks so much @koic for adding this!

I just ran this across our codebase and discovered an autocorrect which I found interesting:

refute(something == something_else)

corrected to:

refute_operator(something, :==, something_else)

, which is expected, of course.

However, I'm surprised the Minitest/RefuteEqual cop didn't already report this, and correct to:

refute_equal(something, something_else)

Is that a missing feature of Minitest/RefuteEqual? If so, would you like me to open an issue?

Thanks again

koic added a commit to koic/rubocop-minitest that referenced this pull request Sep 28, 2023
Follow up rubocop#260 (comment)

This PR makes `Minitest/RefuteEqual` aware of `refute(expected == actual)`.
@koic
Copy link
Member Author

koic commented Sep 28, 2023

Thank you for the feedback. I've opened #265. After this I'll open a PR that will make some more adjustments.

koic added a commit to koic/rubocop-minitest that referenced this pull request Nov 27, 2023
Follow up rubocop#260 (comment)

This PR makes `Minitest/RefuteEqual` aware of `refute(expected == actual)`.
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.

Enforce assert_operator instead of assert with operator expression
4 participants