From c072f876347abbd8acbffd0a1649b79eca46fbeb Mon Sep 17 00:00:00 2001 From: Abhay Nikam Date: Wed, 23 Oct 2019 12:02:07 +0530 Subject: [PATCH] Adds cop to enforce assert_instance_of over assert(object.instance_of?(Class)) --- CHANGELOG.md | 1 + config/default.yml | 6 ++ .../cop/minitest/assert_instance_of.rb | 60 +++++++++++++++++++ lib/rubocop/cop/minitest_cops.rb | 1 + manual/cops.md | 1 + manual/cops_minitest.md | 25 ++++++++ .../cop/minitest/assert_instance_of_test.rb | 57 ++++++++++++++++++ 7 files changed, 151 insertions(+) create mode 100644 lib/rubocop/cop/minitest/assert_instance_of.rb create mode 100644 test/rubocop/cop/minitest/assert_instance_of_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b82678bc..677a340c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#29](https://github.com/rubocop-hq/rubocop-minitest/pull/29): Add new `Minitest/RefuteRespondTo` cop. ([@herwinw][]) * [#31](https://github.com/rubocop-hq/rubocop-minitest/pull/31): Add new `Minitest/AssertEqual` cop. ([@herwinw][]) +* [#34](https://github.com/rubocop-hq/rubocop-minitest/pull/34): Add new `Minitest/AssertInstanceOf` cop. ([@abhaynikam][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 69949507..c5db0b70 100644 --- a/config/default.yml +++ b/config/default.yml @@ -21,6 +21,12 @@ Minitest/AssertIncludes: Enabled: true VersionAdded: '0.2' +Minitest/AssertInstanceOf: + Description: 'This cop enforces the test to use `assert_instance_of(Class, object)` over `assert(object.instance_of?(Class))`' + StyleGuide: 'https://github.com/rubocop-hq/minitest-style-guide#assert-instance-of' + Enabled: true + VersionAdded: '0.4' + Minitest/AssertNil: Description: 'This cop enforces the test to use `assert_nil` instead of using `assert_equal(nil, something)`.' StyleGuide: 'https://github.com/rubocop-hq/minitest-style-guide#assert-nil' diff --git a/lib/rubocop/cop/minitest/assert_instance_of.rb b/lib/rubocop/cop/minitest/assert_instance_of.rb new file mode 100644 index 00000000..fdc013c7 --- /dev/null +++ b/lib/rubocop/cop/minitest/assert_instance_of.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Minitest + # This cop enforces the test to use `assert_instance_of(Class, object)` + # over `assert(object.instance_of?(Class))`. + # + # @example + # # bad + # assert(object.instance_of?(Class)) + # assert(object.instance_of?(Class), 'the message') + # + # # good + # assert_instance_of(Class, object) + # assert_instance_of(Class, object, 'the message') + # + class AssertInstanceOf < Cop + MSG = 'Prefer using `assert_instance_of(%s)` over ' \ + '`assert(%s)`.' + + def_node_matcher :assert_with_instance_of, <<~PATTERN + (send nil? :assert $(send $_ :instance_of? $_) $...) + PATTERN + + def on_send(node) + assert_with_instance_of(node) do + |first_receiver_arg, object, method, rest_args| + + message = rest_args.first + arguments = node_arguments(object, method, message) + receiver = [first_receiver_arg.source, message&.source].compact.join(', ') + + offense_message = format(MSG, arguments: arguments, receiver: receiver) + + add_offense(node, message: offense_message) + end + end + + def autocorrect(node) + lambda do |corrector| + assert_with_instance_of(node) do |_, object, method, rest_args| + message = rest_args.first + arguments = node_arguments(object, method, message) + + replacement = "assert_instance_of(#{arguments})" + corrector.replace(node.loc.expression, replacement) + end + end + end + + private + + def node_arguments(object, method, message) + [method, object, message].compact.map(&:source).join(', ') + end + end + end + end +end diff --git a/lib/rubocop/cop/minitest_cops.rb b/lib/rubocop/cop/minitest_cops.rb index d079a944..96901c2b 100644 --- a/lib/rubocop/cop/minitest_cops.rb +++ b/lib/rubocop/cop/minitest_cops.rb @@ -4,6 +4,7 @@ require_relative 'minitest/assert_equal' require_relative 'minitest/assert_nil' require_relative 'minitest/assert_includes' +require_relative 'minitest/assert_instance_of' require_relative 'minitest/assert_respond_to' require_relative 'minitest/assert_truthy' require_relative 'minitest/refute_empty' diff --git a/manual/cops.md b/manual/cops.md index 5656ea60..d05e77a9 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -4,6 +4,7 @@ * [Minitest/AssertEmpty](cops_minitest.md#minitestassertempty) * [Minitest/AssertEqual](cops_minitest.md#minitestassertequal) * [Minitest/AssertIncludes](cops_minitest.md#minitestassertincludes) +* [Minitest/AssertInstanceOf](cops_minitest.md#minitestassertinstanceof) * [Minitest/AssertNil](cops_minitest.md#minitestassertnil) * [Minitest/AssertRespondTo](cops_minitest.md#minitestassertrespondto) * [Minitest/AssertTruthy](cops_minitest.md#minitestasserttruthy) diff --git a/manual/cops_minitest.md b/manual/cops_minitest.md index 8e04fa5f..20b92c3c 100644 --- a/manual/cops_minitest.md +++ b/manual/cops_minitest.md @@ -73,6 +73,31 @@ assert_includes(collection, object, 'the message') * [https://github.com/rubocop-hq/minitest-style-guide#assert-includes](https://github.com/rubocop-hq/minitest-style-guide#assert-includes) +## Minitest/AssertInstanceOf + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | Yes | Yes | 0.4 | - + +This cop enforces the test to use `assert_instance_of(Class, object)` +over `assert(object.instance_of?(Class))`. + +### Examples + +```ruby +# bad +assert(object.instance_of?(Class)) +assert(object.instance_of?(Class), 'the message') + +# good +assert_instance_of(Class, object) +assert_instance_of(Class, object, 'the message') +``` + +### References + +* [https://github.com/rubocop-hq/minitest-style-guide#assert-instance-of](https://github.com/rubocop-hq/minitest-style-guide#assert-instance-of) + ## Minitest/AssertNil Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/test/rubocop/cop/minitest/assert_instance_of_test.rb b/test/rubocop/cop/minitest/assert_instance_of_test.rb new file mode 100644 index 00000000..6d34a9a9 --- /dev/null +++ b/test/rubocop/cop/minitest/assert_instance_of_test.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'test_helper' + +class AssertInstanceOfTest < Minitest::Test + def setup + @cop = RuboCop::Cop::Minitest::AssertInstanceOf.new + end + + def test_registers_offense_when_using_assert_with_instance_of + assert_offense(<<~RUBY, @cop) + class FooTest < Minitest::Test + def test_do_something + assert(object.instance_of?(SomeClass)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, object)` over `assert(object.instance_of?(SomeClass))`. + end + end + RUBY + + assert_correction(<<~RUBY, @cop) + class FooTest < Minitest::Test + def test_do_something + assert_instance_of(SomeClass, object) + end + end + RUBY + end + + def test_registers_offense_when_using_assert_with_instance_of_and_message + assert_offense(<<~RUBY, @cop) + class FooTest < Minitest::Test + def test_do_something + assert(object.instance_of?(SomeClass), 'the message') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, object, 'the message')` over `assert(object.instance_of?(SomeClass), 'the message')`. + end + end + RUBY + + assert_correction(<<~RUBY, @cop) + class FooTest < Minitest::Test + def test_do_something + assert_instance_of(SomeClass, object, 'the message') + end + end + RUBY + end + + def test_does_not_register_offense_when_using_assert_instance_of_method + assert_no_offenses(<<~RUBY, @cop) + class FooTest < Minitest::Test + def test_do_something + assert_instance_of(SomeClass, object) + end + end + RUBY + end +end