From c568fa832740931247b805dbc97c730f4548d9eb Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Thu, 21 Mar 2024 13:35:45 +0100 Subject: [PATCH] [Fix #1230] Fix a false positive for `Rails/SaveBang` if `persisted?` is checked on parenthesised expression. --- .../fix_false_positive_for_rails_save_bang.md | 1 + lib/rubocop/cop/rails/save_bang.rb | 2 ++ spec/rubocop/cop/rails/save_bang_spec.rb | 32 +++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 changelog/fix_false_positive_for_rails_save_bang.md diff --git a/changelog/fix_false_positive_for_rails_save_bang.md b/changelog/fix_false_positive_for_rails_save_bang.md new file mode 100644 index 0000000000..98325ae32d --- /dev/null +++ b/changelog/fix_false_positive_for_rails_save_bang.md @@ -0,0 +1 @@ +* [#1230](https://github.com/rubocop/rubocop-rails/issues/1230): Fix a false positive for `Rails/SaveBang` if `persisted?` is checked on parenthesised expression. ([@earlopain][]) diff --git a/lib/rubocop/cop/rails/save_bang.rb b/lib/rubocop/cop/rails/save_bang.rb index 133dcc97ee..8f57a0c502 100644 --- a/lib/rubocop/cop/rails/save_bang.rb +++ b/lib/rubocop/cop/rails/save_bang.rb @@ -196,6 +196,8 @@ def persisted_referenced?(assignment) end def call_to_persisted?(node) + node = node.parent.condition if node.parenthesized_call? && node.parent.if_type? + node.send_type? && node.method?(:persisted?) end diff --git a/spec/rubocop/cop/rails/save_bang_spec.rb b/spec/rubocop/cop/rails/save_bang_spec.rb index b19701aee1..8514fb1bcc 100644 --- a/spec/rubocop/cop/rails/save_bang_spec.rb +++ b/spec/rubocop/cop/rails/save_bang_spec.rb @@ -638,6 +638,38 @@ def whatever RUBY end + it "when using persisted? on the result of #{method} in if assignment" do + expect_no_offenses(<<~RUBY) + if (user = User.#{method}).persisted? + foo(user) + else + bar(user) + end + RUBY + end + + it "when not using persisted? on the result of #{method} in if assignment" do + expect_offense(<<~RUBY, method: method) + if (user = User.#{method}) + ^{method} Use `#{method}!` instead of `#{method}` if the return value is not checked. Or check `persisted?` on model returned from `#{method}`. + foo(user) + else + bar(user) + end + RUBY + end + + it "when using persisted? on the result of #{method} in elsif assignment" do + expect_no_offenses(<<~RUBY) + if something + elsif (user = User.#{method}).persisted? + foo(user) + else + bar(user) + end + RUBY + end + it "when using #{method} with `||`" do expect_no_offenses(<<~RUBY) def find_or_create(**opts)