Add RuboCop for `assert_not` over `assert !`

We added `assert_not` in f75addd "to replace warty 'assert !foo'".
fa8d35b agrees that it is warty, and so do I. This custom Rubocop rule
turns the wart into a violation.

As with my last custom cop, https://github.com/rails/rails/pull/32441,
I want to make sure this looks right on code climate before pushing
another commit to autocorrect everything.

@toshimaru I just noticed
https://github.com/toshimaru/rubocop-rails/pull/26
Is there a better way to add these custom cops, or were you saying we
shouldn't have custom cops at all?
This commit is contained in:
Daniel Colson 2018-04-17 08:37:35 -04:00
parent ef2af628a9
commit 087e0ccb72
6 changed files with 101 additions and 10 deletions

View File

@ -15,6 +15,11 @@ CustomCops/RefuteNot:
Include:
- '**/*_test.rb'
# Prefer assert_not over assert !
CustomCops/AssertNot:
Include:
- '**/*_test.rb'
# Prefer &&/|| over and/or.
Style/AndOr:
Enabled: true

View File

@ -1,3 +1,4 @@
# frozen_string_literal: true
require_relative "custom_cops/refute_not"
require_relative "custom_cops/assert_not"

View File

@ -0,0 +1,40 @@
# frozen_string_literal: true
module CustomCops
# Enforces the use of `assert_not` over `assert !`.
#
# @example
# # bad
# assert !x
# assert ! x
#
# # good
# assert_not x
#
class AssertNot < RuboCop::Cop::Cop
MSG = "Prefer `assert_not` over `assert !`"
def_node_matcher :offensive?, "(send nil? :assert (send ... :!))"
def on_send(node)
add_offense(node) if offensive?(node)
end
def autocorrect(node)
expression = node.loc.expression
->(corrector) do
corrector.replace(
expression,
corrected_source(expression.source)
)
end
end
private
def corrected_source(source)
source.gsub(/^assert(\(| ) *! */, "assert_not\\1")
end
end
end

View File

@ -0,0 +1,42 @@
# frozen_string_literal: true
require "support/cop_helper"
require_relative "../../lib/custom_cops/assert_not"
class AssertNotTest < ActiveSupport::TestCase
include CopHelper
setup do
@cop = CustomCops::AssertNot.new
end
test "rejects 'assert !'" do
inspect_source @cop, "assert !x"
assert_offense @cop, "^^^^^^^^^ Prefer `assert_not` over `assert !`"
end
test "rejects 'assert !' with a complex value" do
inspect_source @cop, "assert !a.b(c)"
assert_offense @cop, "^^^^^^^^^^^^^^ Prefer `assert_not` over `assert !`"
end
test "autocorrects `assert !`" do
corrected = autocorrect_source(@cop, "assert !false")
assert_equal "assert_not false", corrected
end
test "autocorrects `assert !` with extra spaces" do
corrected = autocorrect_source(@cop, "assert ! false")
assert_equal "assert_not false", corrected
end
test "autocorrects `assert !` with parentheses" do
corrected = autocorrect_source(@cop, "assert(!false)")
assert_equal "assert_not(false)", corrected
end
test "accepts `assert_not`" do
inspect_source @cop, "assert_not x"
assert_empty @cop.offenses
end
end

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
require "support/cop_helper"
require "./lib/custom_cops/refute_not"
require_relative "../../lib/custom_cops/refute_not"
class RefuteNotTest < ActiveSupport::TestCase
include CopHelper
@ -59,15 +59,6 @@ class RefuteNotTest < ActiveSupport::TestCase
private
def assert_offense(cop, expected_message)
assert_not_empty cop.offenses
offense = cop.offenses.first
carets = "^" * offense.column_length
assert_equal expected_message, "#{carets} #{offense.message}"
end
def offense_message(refute_method, assert_method)
carets = "^" * refute_method.to_s.length
"#{carets} Prefer `#{assert_method}` over `#{refute_method}`"

View File

@ -16,6 +16,18 @@ module CopHelper
rewrite(cop, processed_source)
end
def assert_offense(cop, expected_message)
assert_not_empty(
cop.offenses,
"Expected offense with message \"#{expected_message}\", but got no offense"
)
offense = cop.offenses.first
carets = "^" * offense.column_length
assert_equal expected_message, "#{carets} #{offense.message}"
end
private
TARGET_RUBY_VERSION = 2.4