diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 101135facc..a6b08f2b93 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-07-20 17:51:45 +0900 using RuboCop version 0.72.0. +# on 2019-09-13 11:45:19 -0400 using RuboCop version 0.74.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -14,16 +14,16 @@ InternalAffairs/NodeDestructuring: - 'lib/rubocop/cop/rails/relative_date_constant.rb' - 'lib/rubocop/cop/rails/time_zone.rb' -# Offense count: 9 +# Offense count: 11 Metrics/AbcSize: Max: 17 # Offense count: 4 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 164 + Max: 169 -# Offense count: 23 +# Offense count: 25 # Configuration parameters: CountComments, ExcludedMethods. Metrics/MethodLength: Max: 14 @@ -34,7 +34,7 @@ Metrics/MethodLength: RSpec/ContextWording: Enabled: false -# Offense count: 261 +# Offense count: 277 # Configuration parameters: Max. RSpec/ExampleLength: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d926a55a2..67c0084c94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ * [#123](https://github.com/rubocop-hq/rubocop-rails/pull/123): Add new `Rails/ApplicationController` and `Rails/ApplicationMailer` cops. ([@eugeneius][]) +### Bug fixes + +* [#120](https://github.com/rubocop-hq/rubocop-rails/issues/120): Fix message for `Rails/SaveBang` when the save is in the body of a conditional. ([@jas14][]) + ## 2.3.2 (2019-09-01) ### Bug fixes @@ -85,3 +89,4 @@ [@prathamesh-sonpatki]: https://github.com/prathamesh-sonpatki [@MaximeLaurenty]: https://github.com/MaximeLaurenty [@eugeneius]: https://github.com/eugeneius +[@jas14]: https://github.com/jas14 diff --git a/lib/rubocop/cop/rails/save_bang.rb b/lib/rubocop/cop/rails/save_bang.rb index e301023469..436aa9dd09 100644 --- a/lib/rubocop/cop/rails/save_bang.rb +++ b/lib/rubocop/cop/rails/save_bang.rb @@ -141,7 +141,7 @@ def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity return unless persist_method?(node) return if return_value_assigned?(node) return if implicit_return?(node) - return if check_used_in_conditional(node) + return if check_used_in_condition_or_compound_boolean(node) return if argument?(node) return if explicit_return?(node) @@ -211,8 +211,8 @@ def array_parent(node) array end - def check_used_in_conditional(node) - return false unless conditional?(node) + def check_used_in_condition_or_compound_boolean(node) + return false unless in_condition_or_compound_boolean?(node) unless MODIFY_PERSIST_METHODS.include?(node.method_name) add_offense_for_node(node, CREATE_CONDITIONAL_MSG) @@ -221,15 +221,21 @@ def check_used_in_conditional(node) true end - def conditional?(node) # rubocop:disable Metrics/CyclomaticComplexity + def in_condition_or_compound_boolean?(node) node = node.block_node || node + parent = node.parent + return false unless parent - condition = node.parent - return false unless condition + operator_or_single_negative?(parent) || + (conditional?(parent) && node == parent.condition) + end + + def operator_or_single_negative?(node) + node.or_type? || node.and_type? || single_negative?(node) + end - condition.if_type? || condition.case_type? || - condition.or_type? || condition.and_type? || - single_negative?(condition) + def conditional?(parent) + parent.if_type? || parent.case_type? end def allowed_receiver?(node) diff --git a/spec/rubocop/cop/rails/save_bang_spec.rb b/spec/rubocop/cop/rails/save_bang_spec.rb index ccb92cc4dd..e78e3f5ca0 100644 --- a/spec/rubocop/cop/rails/save_bang_spec.rb +++ b/spec/rubocop/cop/rails/save_bang_spec.rb @@ -223,6 +223,28 @@ end end + it "when using #{method} in the body of a oneline if" do + inspect_source("object.#{method} if false") + + expect(cop.messages) + .to eq(["Use `#{method}!` instead of `#{method}` " \ + 'if the return value is not checked.']) + end + + it "when using #{method} in the body of an else" do + inspect_source(<<~RUBY) + if condition + puts "true" + else + object.#{method} + end + RUBY + + expect(cop.messages) + .to eq(["Use `#{method}!` instead of `#{method}` " \ + 'if the return value is not checked.']) + end + it "when using #{method} with a bunch of hashes & arrays" do inspect_source(<<~RUBY) return [{ success: object.#{method} }, true]