From a63371c22152bb5bc5be7999b0192a06fd7c8424 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Wed, 1 Jul 2020 12:21:16 +0300 Subject: [PATCH] Add new `Minitest/AssertPathExists` and `Minitest/RefutePathExists` cops --- CHANGELOG.md | 1 + config/default.yml | 12 ++++ docs/modules/ROOT/pages/cops.adoc | 2 + docs/modules/ROOT/pages/cops_minitest.adoc | 64 ++++++++++++++++++ .../cop/minitest/assert_path_exists.rb | 67 +++++++++++++++++++ .../cop/minitest/refute_path_exists.rb | 67 +++++++++++++++++++ lib/rubocop/cop/minitest_cops.rb | 2 + .../cop/minitest/assert_path_exists_test.rb | 53 +++++++++++++++ .../cop/minitest/refute_path_exists_test.rb | 53 +++++++++++++++ 9 files changed, 321 insertions(+) create mode 100644 lib/rubocop/cop/minitest/assert_path_exists.rb create mode 100644 lib/rubocop/cop/minitest/refute_path_exists.rb create mode 100644 test/rubocop/cop/minitest/assert_path_exists_test.rb create mode 100644 test/rubocop/cop/minitest/refute_path_exists_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc15b38..d439c19e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### New features +* [#83](https://github.com/rubocop-hq/rubocop-minitest/pull/83): New cops `AssertPathExists` and `RefutePathExists` check for use of `assert_path_exists`/`refute_path_exists` instead of `assert(File.exist?(path))`/`refute(File.exist?(path))`. ([@fatkodima][]) * [#84](https://github.com/rubocop-hq/rubocop-minitest/pull/84): New cops `AssertKindOf` and `RefuteKindOf` check for use of `assert_kind_of`/`refute_kind_of` instead of `assert(foo.kind_of?(Class))`/`refute(foo.kind_of?(Class))`. ([@fatkodima][]) ### Changes diff --git a/config/default.yml b/config/default.yml index f37f2a01..29037607 100644 --- a/config/default.yml +++ b/config/default.yml @@ -51,6 +51,12 @@ Minitest/AssertNil: Enabled: true VersionAdded: '0.1' +Minitest/AssertPathExists: + Description: 'This cop enforces the test to use `assert_path_exists` instead of using `assert(File.exist?(path))`.' + StyleGuide: 'https://github.com/rubocop-hq/minitest-style-guide#assert-path-exists' + Enabled: 'pending' + VersionAdded: '0.10' + Minitest/AssertRespondTo: Description: 'This cop enforces the test to use `assert_respond_to(object, :do_something)` over `assert(object.respond_to?(:do_something))`.' StyleGuide: 'https://minitest.rubystyle.guide#assert-responds-to-method' @@ -117,6 +123,12 @@ Minitest/RefuteNil: Enabled: true VersionAdded: '0.2' +Minitest/RefutePathExists: + Description: 'This cop enforces the test to use `refute_path_exists` instead of using `refute(File.exist?(path))`.' + StyleGuide: 'https://github.com/rubocop-hq/minitest-style-guide#refute-path-exists' + Enabled: 'pending' + VersionAdded: '0.10' + Minitest/RefuteRespondTo: Description: 'This cop enforces the test to use `refute_respond_to(object, :do_something)` over `refute(object.respond_to?(:do_something))`.' StyleGuide: 'https://minitest.rubystyle.guide#refute-respond-to' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 0a077247..a2253b85 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -10,6 +10,7 @@ * xref:cops_minitest.adoc#minitestassertkindof[Minitest/AssertKindOf] * xref:cops_minitest.adoc#minitestassertmatch[Minitest/AssertMatch] * xref:cops_minitest.adoc#minitestassertnil[Minitest/AssertNil] +* xref:cops_minitest.adoc#minitestassertpathexists[Minitest/AssertPathExists] * xref:cops_minitest.adoc#minitestassertrespondto[Minitest/AssertRespondTo] * xref:cops_minitest.adoc#minitestasserttruthy[Minitest/AssertTruthy] * xref:cops_minitest.adoc#minitestglobalexpectations[Minitest/GlobalExpectations] @@ -21,6 +22,7 @@ * xref:cops_minitest.adoc#minitestrefutekindof[Minitest/RefuteKindOf] * xref:cops_minitest.adoc#minitestrefutematch[Minitest/RefuteMatch] * xref:cops_minitest.adoc#minitestrefutenil[Minitest/RefuteNil] +* xref:cops_minitest.adoc#minitestrefutepathexists[Minitest/RefutePathExists] * xref:cops_minitest.adoc#minitestrefuterespondto[Minitest/RefuteRespondTo] // END_COP_LIST diff --git a/docs/modules/ROOT/pages/cops_minitest.adoc b/docs/modules/ROOT/pages/cops_minitest.adoc index 73dc42ec..35f618c9 100644 --- a/docs/modules/ROOT/pages/cops_minitest.adoc +++ b/docs/modules/ROOT/pages/cops_minitest.adoc @@ -249,6 +249,38 @@ assert_nil(actual, 'message') * https://minitest.rubystyle.guide#assert-nil +== Minitest/AssertPathExists + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.10 +| - +|=== + +This cop enforces the test to use `assert_path_exists` +instead of using `assert(File.exist?(path))`. + +=== Examples + +[source,ruby] +---- +# bad +assert(File.exist?(path)) +assert(File.exist?(path), 'message') + +# good +assert_path_exists(path) +assert_path_exists(path, 'message') +---- + +=== References + +* https://github.com/rubocop-hq/minitest-style-guide#assert-path-exists + == Minitest/AssertRespondTo |=== @@ -607,6 +639,38 @@ refute_nil(actual, 'message') * https://minitest.rubystyle.guide#refute-nil +== Minitest/RefutePathExists + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.10 +| - +|=== + +This cop enforces the test to use `refute_path_exists` +instead of using `refute(File.exist?(path))`. + +=== Examples + +[source,ruby] +---- +# bad +refute(File.exist?(path)) +refute(File.exist?(path), 'message') + +# good +refute_path_exists(path) +refute_path_exists(path, 'message') +---- + +=== References + +* https://github.com/rubocop-hq/minitest-style-guide#refute-path-exists + == Minitest/RefuteRespondTo |=== diff --git a/lib/rubocop/cop/minitest/assert_path_exists.rb b/lib/rubocop/cop/minitest/assert_path_exists.rb new file mode 100644 index 00000000..150feed5 --- /dev/null +++ b/lib/rubocop/cop/minitest/assert_path_exists.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Minitest + # This cop enforces the test to use `assert_path_exists` + # instead of using `assert(File.exist?(path))`. + # + # @example + # # bad + # assert(File.exist?(path)) + # assert(File.exist?(path), 'message') + # + # # good + # assert_path_exists(path) + # assert_path_exists(path, 'message') + # + class AssertPathExists < Cop + MSG = 'Prefer using `%s` over `%s`.' + + def_node_matcher :assert_file_exists, <<~PATTERN + (send nil? :assert + (send + (const _ :File) ${:exist? :exists?} $_) + $...) + PATTERN + + def on_send(node) + assert_file_exists(node) do |method_name, path, message| + message = message.first + + add_offense(node, + message: format(MSG, good_method: build_good_method(path, message), + bad_method: build_bad_method(method_name, path, message))) + end + end + + def autocorrect(node) + assert_file_exists(node) do |_method_name, path, message| + message = message.first + + lambda do |corrector| + replacement = build_good_method(path, message) + corrector.replace(node, replacement) + end + end + end + + private + + def build_good_method(path, message) + args = [path.source, message&.source].compact.join(', ') + "assert_path_exists(#{args})" + end + + def build_bad_method(method_name, path, message) + path_arg = "File.#{method_name}(#{path.source})" + if message + "assert(#{path_arg}, #{message.source})" + else + "assert(#{path_arg})" + end + end + end + end + end +end diff --git a/lib/rubocop/cop/minitest/refute_path_exists.rb b/lib/rubocop/cop/minitest/refute_path_exists.rb new file mode 100644 index 00000000..64366c5b --- /dev/null +++ b/lib/rubocop/cop/minitest/refute_path_exists.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Minitest + # This cop enforces the test to use `refute_path_exists` + # instead of using `refute(File.exist?(path))`. + # + # @example + # # bad + # refute(File.exist?(path)) + # refute(File.exist?(path), 'message') + # + # # good + # refute_path_exists(path) + # refute_path_exists(path, 'message') + # + class RefutePathExists < Cop + MSG = 'Prefer using `%s` over `%s`.' + + def_node_matcher :refute_file_exists, <<~PATTERN + (send nil? :refute + (send + (const _ :File) ${:exist? :exists?} $_) + $...) + PATTERN + + def on_send(node) + refute_file_exists(node) do |method_name, path, message| + message = message.first + + add_offense(node, + message: format(MSG, good_method: build_good_method(path, message), + bad_method: build_bad_method(method_name, path, message))) + end + end + + def autocorrect(node) + refute_file_exists(node) do |_method_name, path, message| + message = message.first + + lambda do |corrector| + replacement = build_good_method(path, message) + corrector.replace(node, replacement) + end + end + end + + private + + def build_good_method(path, message) + args = [path.source, message&.source].compact.join(', ') + "refute_path_exists(#{args})" + end + + def build_bad_method(method_name, path, message) + path_arg = "File.#{method_name}(#{path.source})" + if message + "refute(#{path_arg}, #{message.source})" + else + "refute(#{path_arg})" + end + end + end + end + end +end diff --git a/lib/rubocop/cop/minitest_cops.rb b/lib/rubocop/cop/minitest_cops.rb index 7b3e3b5d..b2d367ed 100644 --- a/lib/rubocop/cop/minitest_cops.rb +++ b/lib/rubocop/cop/minitest_cops.rb @@ -10,6 +10,7 @@ require_relative 'minitest/assert_includes' require_relative 'minitest/assert_instance_of' require_relative 'minitest/assert_match' +require_relative 'minitest/assert_path_exists' require_relative 'minitest/assert_respond_to' require_relative 'minitest/assert_truthy' require_relative 'minitest/global_expectations' @@ -21,4 +22,5 @@ require_relative 'minitest/refute_includes' require_relative 'minitest/refute_match' require_relative 'minitest/refute_instance_of' +require_relative 'minitest/refute_path_exists' require_relative 'minitest/refute_respond_to' diff --git a/test/rubocop/cop/minitest/assert_path_exists_test.rb b/test/rubocop/cop/minitest/assert_path_exists_test.rb new file mode 100644 index 00000000..4333219e --- /dev/null +++ b/test/rubocop/cop/minitest/assert_path_exists_test.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'test_helper' + +class AssertPathExistsTest < Minitest::Test + def test_registers_offense_when_using_assert_file_exist + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert(File.exist?(path)) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_path_exists(path)` over `assert(File.exist?(path))`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_path_exists(path) + end + end + RUBY + end + + def test_registers_offense_when_using_assert_file_exist_and_message + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert(File.exist?(path), 'message') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_path_exists(path, 'message')` over `assert(File.exist?(path), 'message')`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_path_exists(path, 'message') + end + end + RUBY + end + + def test_does_not_register_offense_when_using_assert_path_exists_method + assert_no_offenses(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_path_exists(path) + end + end + RUBY + end +end diff --git a/test/rubocop/cop/minitest/refute_path_exists_test.rb b/test/rubocop/cop/minitest/refute_path_exists_test.rb new file mode 100644 index 00000000..b59aaba4 --- /dev/null +++ b/test/rubocop/cop/minitest/refute_path_exists_test.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'test_helper' + +class RefutePathExistsTest < Minitest::Test + def test_registers_offense_when_using_refute_file_exist + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute(File.exist?(path)) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_path_exists(path)` over `refute(File.exist?(path))`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute_path_exists(path) + end + end + RUBY + end + + def test_registers_offense_when_using_refute_file_exist_and_message + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute(File.exist?(path), 'message') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_path_exists(path, 'message')` over `refute(File.exist?(path), 'message')`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute_path_exists(path, 'message') + end + end + RUBY + end + + def test_does_not_register_offense_when_using_refute_path_exists_method + assert_no_offenses(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute_path_exists(path) + end + end + RUBY + end +end