forked from rails/rails
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add custom RuboCop for
assert_not
over refute
Since at least cf4afc4 we have preferred `assert_not` methods over `refute` methods. I have seen plenty of comments in PRs about this, and we have tried to fix it a few times (5294ad8, e45f176, 8910f12, 41f50be, d4cfd54, 48a183e, and 211adb4), but the `refute` methods keep sneaking back in. This custom RuboCop will take care of enforcing this preference, so we don't have to think about it again. I suspect there are other similar stylistic preferences that could be solved with some custom RuboCops, so I will definitely keep my eyes open. `assert_not` over `assert !` might be a good candidate, for example. I wasn't totally sure if `ci/custom_cops` was the best place to put this, but nothing else seemed quite right. At one point I had it set up as a gem, but I think custom cops like this would have limited value in another context. I want to see how code climate handles the new cops before autocorrecting the existing violations. If things go as expected, I will push another commit with those corrections.
- Loading branch information
1 parent
09b2348
commit f88f5a4
Showing
6 changed files
with
196 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#!/usr/bin/env ruby | ||
# frozen_string_literal: true | ||
|
||
COMPONENT_ROOT = File.expand_path("..", __dir__) | ||
require_relative "../../../tools/test" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# frozen_string_literal: true | ||
|
||
require_relative "custom_cops/refute_not" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# frozen_string_literal: true | ||
|
||
module CustomCops | ||
# Enforces the use of `#assert_not` methods over `#refute` methods. | ||
# | ||
# @example | ||
# # bad | ||
# refute false | ||
# refute_empty [1, 2, 3] | ||
# refute_equal true, false | ||
# | ||
# # good | ||
# assert_not false | ||
# assert_not_empty [1, 2, 3] | ||
# assert_not_equal true, false | ||
# | ||
class RefuteNot < RuboCop::Cop::Cop | ||
MSG = "Prefer `%<assert_method>s` over `%<refute_method>s`" | ||
|
||
CORRECTIONS = { | ||
refute: "assert_not", | ||
refute_empty: "assert_not_empty", | ||
refute_equal: "assert_not_equal", | ||
refute_in_delta: "assert_not_in_delta", | ||
refute_in_epsilon: "assert_not_in_epsilon", | ||
refute_includes: "assert_not_includes", | ||
refute_instance_of: "assert_not_instance_of", | ||
refute_kind_of: "assert_not_kind_of", | ||
refute_nil: "assert_not_nil", | ||
refute_operator: "assert_not_operator", | ||
refute_predicate: "assert_not_predicate", | ||
refute_respond_to: "assert_not_respond_to", | ||
refute_same: "assert_not_same", | ||
refute_match: "assert_no_match" | ||
}.freeze | ||
|
||
OFFENSIVE_METHODS = CORRECTIONS.keys.freeze | ||
|
||
def_node_matcher :offensive?, "(send nil? #offensive_method? ...)" | ||
|
||
def on_send(node) | ||
return unless offensive?(node) | ||
|
||
message = offense_message(node.method_name) | ||
add_offense(node, location: :selector, message: message) | ||
end | ||
|
||
def autocorrect(node) | ||
->(corrector) do | ||
corrector.replace( | ||
node.loc.selector, | ||
CORRECTIONS[node.method_name] | ||
) | ||
end | ||
end | ||
|
||
private | ||
|
||
def offensive_method?(method_name) | ||
OFFENSIVE_METHODS.include?(method_name) | ||
end | ||
|
||
def offense_message(method_name) | ||
format( | ||
MSG, | ||
refute_method: method_name, | ||
assert_method: CORRECTIONS[method_name] | ||
) | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
# frozen_string_literal: true | ||
|
||
require "support/cop_helper" | ||
require "./lib/custom_cops/refute_not" | ||
|
||
class RefuteNotTest < ActiveSupport::TestCase | ||
include CopHelper | ||
|
||
setup do | ||
@cop = CustomCops::RefuteNot.new | ||
end | ||
|
||
{ | ||
refute: :assert_not, | ||
refute_empty: :assert_not_empty, | ||
refute_equal: :assert_not_equal, | ||
refute_in_delta: :assert_not_in_delta, | ||
refute_in_epsilon: :assert_not_in_epsilon, | ||
refute_includes: :assert_not_includes, | ||
refute_instance_of: :assert_not_instance_of, | ||
refute_kind_of: :assert_not_kind_of, | ||
refute_nil: :assert_not_nil, | ||
refute_operator: :assert_not_operator, | ||
refute_predicate: :assert_not_predicate, | ||
refute_respond_to: :assert_not_respond_to, | ||
refute_same: :assert_not_same, | ||
refute_match: :assert_no_match | ||
}.each do |refute_method, assert_method| | ||
test "rejects `#{refute_method}` with a single argument" do | ||
inspect_source(@cop, "#{refute_method} a") | ||
assert_offense @cop, offense_message(refute_method, assert_method) | ||
end | ||
|
||
test "rejects `#{refute_method}` with multiple arguments" do | ||
inspect_source(@cop, "#{refute_method} a, b, c") | ||
assert_offense @cop, offense_message(refute_method, assert_method) | ||
end | ||
|
||
test "autocorrects `#{refute_method}` with a single argument" do | ||
corrected = autocorrect_source(@cop, "#{refute_method} a") | ||
assert_equal "#{assert_method} a", corrected | ||
end | ||
|
||
test "autocorrects `#{refute_method}` with multiple arguments" do | ||
corrected = autocorrect_source(@cop, "#{refute_method} a, b, c") | ||
assert_equal "#{assert_method} a, b, c", corrected | ||
end | ||
|
||
test "accepts `#{assert_method}` with a single argument" do | ||
inspect_source(@cop, "#{assert_method} a") | ||
assert_empty @cop.offenses | ||
end | ||
|
||
test "accepts `#{assert_method}` with multiple arguments" do | ||
inspect_source(@cop, "#{assert_method} a, b, c") | ||
assert_empty @cop.offenses | ||
end | ||
end | ||
|
||
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}`" | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# frozen_string_literal: true | ||
|
||
require "rubocop" | ||
|
||
module CopHelper | ||
def inspect_source(cop, source) | ||
processed_source = parse_source(source) | ||
raise "Error parsing example code" unless processed_source.valid_syntax? | ||
investigate(cop, processed_source) | ||
processed_source | ||
end | ||
|
||
def autocorrect_source(cop, source) | ||
cop.instance_variable_get(:@options)[:auto_correct] = true | ||
processed_source = inspect_source(cop, source) | ||
rewrite(cop, processed_source) | ||
end | ||
|
||
private | ||
TARGET_RUBY_VERSION = 2.4 | ||
|
||
def parse_source(source) | ||
RuboCop::ProcessedSource.new(source, TARGET_RUBY_VERSION) | ||
end | ||
|
||
def rewrite(cop, processed_source) | ||
RuboCop::Cop::Corrector.new(processed_source.buffer, cop.corrections) | ||
.rewrite | ||
end | ||
|
||
def investigate(cop, processed_source) | ||
RuboCop::Cop::Commissioner.new([cop], [], raise_error: true) | ||
.investigate(processed_source) | ||
end | ||
end |