From f88f5a457cb95dc22519aa33b527a73f198e92e8 Mon Sep 17 00:00:00 2001
From: Daniel Colson <danieljamescolson@gmail.com>
Date: Tue, 3 Apr 2018 20:50:00 -0400
Subject: 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.
---
ci/custom_cops/bin/test | 5 ++
ci/custom_cops/lib/custom_cops.rb | 3 +
ci/custom_cops/lib/custom_cops/refute_not.rb | 71 ++++++++++++++++++++
ci/custom_cops/test/custom_cops/refute_not_test.rb | 75 ++++++++++++++++++++++
ci/custom_cops/test/support/cop_helper.rb | 35 ++++++++++
5 files changed, 189 insertions(+)
create mode 100755 ci/custom_cops/bin/test
create mode 100644 ci/custom_cops/lib/custom_cops.rb
create mode 100644 ci/custom_cops/lib/custom_cops/refute_not.rb
create mode 100644 ci/custom_cops/test/custom_cops/refute_not_test.rb
create mode 100644 ci/custom_cops/test/support/cop_helper.rb
(limited to 'ci/custom_cops')
diff --git a/ci/custom_cops/bin/test b/ci/custom_cops/bin/test
new file mode 100755
index 0000000000..495ffec83a
--- /dev/null
+++ b/ci/custom_cops/bin/test
@@ -0,0 +1,5 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+COMPONENT_ROOT = File.expand_path("..", __dir__)
+require_relative "../../../tools/test"
diff --git a/ci/custom_cops/lib/custom_cops.rb b/ci/custom_cops/lib/custom_cops.rb
new file mode 100644
index 0000000000..d5d17f8856
--- /dev/null
+++ b/ci/custom_cops/lib/custom_cops.rb
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+require_relative "custom_cops/refute_not"
diff --git a/ci/custom_cops/lib/custom_cops/refute_not.rb b/ci/custom_cops/lib/custom_cops/refute_not.rb
new file mode 100644
index 0000000000..3e89e0fd32
--- /dev/null
+++ b/ci/custom_cops/lib/custom_cops/refute_not.rb
@@ -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
diff --git a/ci/custom_cops/test/custom_cops/refute_not_test.rb b/ci/custom_cops/test/custom_cops/refute_not_test.rb
new file mode 100644
index 0000000000..5dbd8bf32a
--- /dev/null
+++ b/ci/custom_cops/test/custom_cops/refute_not_test.rb
@@ -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
diff --git a/ci/custom_cops/test/support/cop_helper.rb b/ci/custom_cops/test/support/cop_helper.rb
new file mode 100644
index 0000000000..d259154df5
--- /dev/null
+++ b/ci/custom_cops/test/support/cop_helper.rb
@@ -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
--
cgit v1.2.3