diff --git a/changelog/fix_false_positive_for_rails_redundant_presence_validation_on_belongs_to.md b/changelog/fix_false_positive_for_rails_redundant_presence_validation_on_belongs_to.md new file mode 100644 index 0000000000..2af6991677 --- /dev/null +++ b/changelog/fix_false_positive_for_rails_redundant_presence_validation_on_belongs_to.md @@ -0,0 +1 @@ +* [#1319](https://github.com/rubocop/rubocop-rails/issues/1319): Fix a false positive for `Rails/RedundantPresenceValidationOnBelongsTo` when removing `presence` would leave other non-validation options like `allow_blank` without validations. ([@earlopain][]) diff --git a/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb b/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb index c6a656eea7..c745c27064 100644 --- a/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb +++ b/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb @@ -39,6 +39,9 @@ class RedundantPresenceValidationOnBelongsTo < Base MSG = 'Remove explicit presence validation for %s.' RESTRICT_ON_SEND = %i[validates].freeze + # From https://github.com/rails/rails/blob/7a0bf93b9dd291c7f61121a41b3a813ac8857e6a/activemodel/lib/active_model/validations/validates.rb#L157-L159 + NON_VALIDATION_OPTIONS = %i[if unless on allow_blank allow_nil strict].freeze + minimum_target_rails_version 5.0 # @!method presence_validation?(node) @@ -170,6 +173,12 @@ class RedundantPresenceValidationOnBelongsTo < Base def on_send(node) presence_validation?(node) do |all_keys, options, presence| + # If presence is the only validation option and other non-validation options + # are present, removing it will cause rails to error. + used_option_keys = options.keys.select(&:sym_type?).map(&:value) + remaining_validations = used_option_keys - NON_VALIDATION_OPTIONS - [:presence] + return if remaining_validations.none? && options.keys.length > 1 + keys = non_optional_belongs_to(node.parent, all_keys) return if keys.none? diff --git a/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb b/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb index e7d8d66e7c..9ab0f940d2 100644 --- a/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb +++ b/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb @@ -278,6 +278,13 @@ class Profile RUBY end + it 'does not register an offense with `if` and other validation option' do + expect_no_offenses(<<~RUBY) + belongs_to :user + validates :user, presence: true, if: -> { condition }, numericality: true + RUBY + end + it 'does not register an offense with `unless` option' do expect_no_offenses(<<~RUBY) belongs_to :user @@ -298,6 +305,13 @@ class Profile validates :user, presence: true, strict: MissingUserError RUBY end + + it 'does not register an offense when other options are present but none are validations' do + expect_no_offenses(<<~RUBY) + belongs_to :user + validates :user, presence: true, allow_blank: false + RUBY + end end context 'Rails < 5.0', :rails42 do