From c86dbf7018117589af5baa16ad3fd07b32247a56 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 22 Mar 2024 10:43:33 +0100 Subject: [PATCH] [Fix #1155] Add new `Rails/AssertNotPredicate` This converts `assert_not(obj.one?)` into `assert_not_predicate(obj, :one?)` These methods are rails-specific and not present by default in minitest. For users that prefer the `refute` method calls, `Rails/RefuteMethods` will correct this if configured that way. Both the code and tests are pretty much a one-to-one copy from rubocop-minitest.`RefutePredicate` --- changelog/new_rails_assert_not_predicate.md | 1 + config/default.yml | 7 ++ lib/rubocop/cop/rails/assert_not_predicate.rb | 74 ++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/assert_not_predicate_spec.rb | 88 +++++++++++++++++++ 5 files changed, 171 insertions(+) create mode 100644 changelog/new_rails_assert_not_predicate.md create mode 100644 lib/rubocop/cop/rails/assert_not_predicate.rb create mode 100644 spec/rubocop/cop/rails/assert_not_predicate_spec.rb diff --git a/changelog/new_rails_assert_not_predicate.md b/changelog/new_rails_assert_not_predicate.md new file mode 100644 index 0000000000..e0e9350d31 --- /dev/null +++ b/changelog/new_rails_assert_not_predicate.md @@ -0,0 +1 @@ +* [#1155](https://github.com/rubocop/rubocop-rails/issues/1155): Add new `Rails/AssertNotPredicate` to convert `assert_not(obj.foo?)` into `assert_not_predicate(obj, :foo?)`. ([@earlopain][]) diff --git a/config/default.yml b/config/default.yml index 37291c46bf..d794d950fe 100644 --- a/config/default.yml +++ b/config/default.yml @@ -227,6 +227,13 @@ Rails/AssertNot: Include: - '**/test/**/*' +Rails/AssertNotPredicate: + Description: 'Prefer `assert_not_predicate` instead of `assert_not` with predicate method.' + Enabled: pending + VersionAdded: '<>' + Include: + - '**/test/**/*' + Rails/AttributeDefaultBlockValue: Description: 'Pass method call in block for attribute option `default`.' Enabled: pending diff --git a/lib/rubocop/cop/rails/assert_not_predicate.rb b/lib/rubocop/cop/rails/assert_not_predicate.rb new file mode 100644 index 0000000000..b981a16757 --- /dev/null +++ b/lib/rubocop/cop/rails/assert_not_predicate.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Prefer assert_not_predicate(obj, :foo?) over assert_not(obj.foo?) + # + # @example + # # bad + # assert_not(obj.one?) + # assert_not(obj.one?, 'message') + # + # # good + # assert_not_predicate(obj, :one?) + # assert_not_predicate(obj, :one?, 'message') + # + class AssertNotPredicate < Base + # NOTE: Code lifted from rubocop-minitest `PredicateAssertionHandleable` + extend AutoCorrector + + MSG = 'Prefer using `%s_predicate(%s)`.' + RESTRICT_ON_SEND = [:assert_not].freeze + + def assertion_type + :assert_not + end + + def on_send(node) + return unless node.first_argument + return if node.first_argument.block_type? || node.first_argument.numblock_type? + return unless predicate_method?(node.first_argument) + return unless node.first_argument.arguments.count.zero? + + add_offense(node, message: offense_message(node.arguments)) do |corrector| + autocorrect(corrector, node, node.arguments) + end + end + + def autocorrect(corrector, node, arguments) + corrector.replace(node.loc.selector, "#{assertion_type}_predicate") + + new_arguments = new_arguments(arguments).join(', ') + + corrector.replace(node.first_argument, new_arguments) + end + + private + + def predicate_method?(first_argument) + first_argument.respond_to?(:predicate_method?) && first_argument.predicate_method? + end + + def offense_message(arguments) + message_argument = arguments.last if arguments.first != arguments.last + + new_arguments = [new_arguments(arguments), message_argument&.source].flatten.compact.join(', ') + + format(MSG, assertion_type: assertion_type, new_arguments: new_arguments) + end + + def new_arguments(arguments) + receiver = correct_receiver(arguments.first.receiver) + method_name = arguments.first.method_name + + [receiver, ":#{method_name}"] + end + + def correct_receiver(receiver) + receiver ? receiver.source : 'self' + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 0d92ff4ca5..121513a0fb 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -26,6 +26,7 @@ require_relative 'rails/application_record' require_relative 'rails/arel_star' require_relative 'rails/assert_not' +require_relative 'rails/assert_not_predicate' require_relative 'rails/attribute_default_block_value' require_relative 'rails/belongs_to' require_relative 'rails/blank' diff --git a/spec/rubocop/cop/rails/assert_not_predicate_spec.rb b/spec/rubocop/cop/rails/assert_not_predicate_spec.rb new file mode 100644 index 0000000000..1721dee8b6 --- /dev/null +++ b/spec/rubocop/cop/rails/assert_not_predicate_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::AssertNotPredicate, :config do + it 'registers an offense when using assert_not with predicate method' do + expect_offense(<<~RUBY) + assert_not(obj.one?) + ^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_not_predicate(obj, :one?)`. + RUBY + + expect_correction(<<~RUBY) + assert_not_predicate(obj, :one?) + RUBY + end + + it 'registers an offense when using assert_not with predicate method and message' do + expect_offense(<<~RUBY) + assert_not(obj.one?, 'message') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_not_predicate(obj, :one?, 'message')`. + RUBY + + expect_correction(<<~RUBY) + assert_not_predicate(obj, :one?, 'message') + RUBY + end + + it 'registers an offense when using assert_not with predicate method and heredoc message' do + expect_offense(<<~RUBY) + assert_not(obj.one?, <<~MESSAGE) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_not_predicate(obj, :one?, <<~MESSAGE)`. + message + MESSAGE + RUBY + + expect_correction(<<~RUBY) + assert_not_predicate(obj, :one?, <<~MESSAGE) + message + MESSAGE + RUBY + end + + it 'registers and offense when using assert_not with receiver omitted predicate method' do + expect_offense(<<~RUBY) + assert_not(one?) + ^^^^^^^^^^^^^^^^ Prefer using `assert_not_predicate(self, :one?)`. + RUBY + + expect_correction(<<~RUBY) + assert_not_predicate(self, :one?) + RUBY + end + + it 'registers no offense when using assert_not_predicate method' do + expect_no_offenses(<<~RUBY) + assert_not_predicate(obj, :one?) + RUBY + end + + it 'registers no offense when using assert_not with non predicate method' do + expect_no_offenses(<<~RUBY) + assert_not(obj.do_something) + RUBY + end + + it 'registers no offense when using assert_not with local variable' do + expect_no_offenses(<<~RUBY) + obj = create_obj + assert_not(obj) + RUBY + end + + it 'registers no offense when using assert_not_predicate with predicate method and arguments' do + expect_no_offenses(<<~RUBY) + assert_not(obj.foo?(arg)) + RUBY + end + + it 'registers no offense when using assert_not with predicate method and numbered arguments' do + expect_no_offenses(<<~RUBY) + assert_not([1, 2, 3].any? { some_filter_function _1 }) + RUBY + end + + it 'raises no error when using assert_not with block' do + expect_no_offenses(<<~RUBY) + assert_not { false } + RUBY + end +end