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.

Sign up for GitHub

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

Jump to bottom

Rails/TransactionExitStatement false positive? #669

Closed
modosc opened this issue Mar 16, 2022 · 2 comments · Fixed by #674 or #696
Closed

Rails/TransactionExitStatement false positive? #669

modosc opened this issue Mar 16, 2022 · 2 comments · Fixed by #674 or #696
Labels
bug Something isn't working

Comments

@modosc
Copy link

modosc commented Mar 16, 2022

Rails/TransactionExitStatement is catching this code but i don't think it's correct:

Foo.transaction do 
  # stuff
rescue => e
  return false
end

i believe in this case return is being called outside of the transaction and therefore isn't exiting the transaction (at this point the txn already exited by raising an error).

Expected behavior

this code should be ignored.

Actual behavior

this code is flagged.

Steps to reproduce the problem

see above code.

RuboCop version

$ rubocop -V
1.26.0 (using Parser 3.1.1.0, rubocop-ast 1.16.0, running on ruby 3.1.1 x86_64-darwin21)
  - rubocop-performance 1.13.3
  - rubocop-rails 2.14.0
  - rubocop-rspec 2.9.0
@koic koic added the bug Something isn't working label Mar 17, 2022
koic added a commit to koic/rubocop-rails that referenced this issue Mar 17, 2022
@koic
[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStat…
d9ec02d 
…ement`

Fixes rubocop#669.

This PR fixes a false positive for `Rails/TransactionExitStatement`
when `return` is used in `rescue`.
@koic koic mentioned this issue Mar 17, 2022
Merged
10 tasks
@koic koic closed this as completed in #674 Mar 17, 2022
koic added a commit that referenced this issue Mar 17, 2022
@koic
Merge pull request #674 from koic/fix_false_positive_for_rails_transa…
10e478f 
…ction_exit_statement

[Fix #669] Fix a false positive for `Rails/TransactionExitStatement`
@modosc
Copy link
Author

modosc commented Mar 17, 2022

thanks!

koic added a commit to koic/rubocop-rails that referenced this issue Apr 23, 2022
@koic
Fix a false negative for Rails/TransactionExitStatement
fc8ae80 
Follow up rubocop#674 (comment)

Revert "[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStatement`"

This reverts commit d9ec02d.

So this PR fixes a false negative for `Rails/TransactionExitStatement` when
`return` is used in `rescue`.
@koic koic mentioned this issue Apr 23, 2022
Merged
10 tasks
@koic
Copy link
Member

koic commented Apr 23, 2022

JFYI, this behavior is not a false positive and will be changed by #696. Please see below for the reason:
#674 (comment)

koic added a commit to koic/rubocop-rails that referenced this issue Apr 23, 2022
@koic
Fix a false negative for Rails/TransactionExitStatement
f229951 
Follow up rubocop#674 (comment)

Revert "[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStatement`"

This reverts commit d9ec02d and tweak a spec.

So this PR fixes a false negative for `Rails/TransactionExitStatement` when
`return` is used in `rescue`.
koic added a commit to koic/rubocop-rails that referenced this issue Apr 27, 2022
@koic
Fix a false negative for Rails/TransactionExitStatement
d5dfc68 
Follow up rubocop#674 (comment)

Revert "[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStatement`"

This reverts commit d9ec02d and add an example, tweak a spec.

So this PR fixes a false negative for `Rails/TransactionExitStatement` when
`return` is used in `rescue`.
koic added a commit to koic/rubocop-rails that referenced this issue Apr 27, 2022
@koic
Fix a false negative for Rails/TransactionExitStatement
6866080 
Follow up rubocop#674 (comment)

Revert "[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStatement`"

This reverts commit d9ec02d and add an example, tweak a spec.

So this PR fixes a false negative for `Rails/TransactionExitStatement` when
`return` is used in `rescue`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #669] Fix a false positive for Rails/TransactionExitStatement koic/rubocop-rails
Fix a false negative for Rails/TransactionExitStatement koic/rubocop-rails
2 participants
@koic @modosc