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

Prevent assert_changes with non-string message #1396

Open
joseph-carino opened this issue Dec 19, 2024 · 1 comment · May be fixed by #1401
Open

Prevent assert_changes with non-string message #1396

joseph-carino opened this issue Dec 19, 2024 · 1 comment · May be fixed by #1401

Comments

@joseph-carino
Copy link

Is your feature request related to a problem? Please describe.

ActiveSupport's assert_changes is often misused to check if an expression's value has changed by a specific delta during the execution of a block. For example:

assert_changes(-> { value }, 1) do
  value += 1
end

However, the second argument to assert_changes is actually the message emitted on assertion failure, NOT the expected change delta. assert_changes uses from: and to: named arguments to accept specific change values.

It may be difficult to identify this mistake, as the assertion will pass as long as the expression changes by ANY value.

FYI assert_difference is probably what developers are looking for - it actually does accept an expected delta.

Describe the solution you'd like

Cop should prevent use of non-string message parameters for assert_changes.

@joseph-carino
Copy link
Author

Working on this!

joseph-carino added a commit to joseph-carino/rubocop-rails that referenced this issue Dec 23, 2024
@joseph-carino joseph-carino linked a pull request Dec 23, 2024 that will close this issue
9 tasks
joseph-carino added a commit to joseph-carino/rubocop-rails that referenced this issue Dec 23, 2024
joseph-carino added a commit to joseph-carino/rubocop-rails that referenced this issue Dec 23, 2024
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 a pull request may close this issue.

1 participant