From 73210b0f9ede4cd02013de49919b7b205a6c9929 Mon Sep 17 00:00:00 2001 From: Samyak Jain Date: Tue, 20 Apr 2021 22:44:26 +0530 Subject: [PATCH] Tweak the AST so that it checks the usage of File.expand_path used with lib Update tests according to the new AST Signed-off-by: Samyak Jain --- .../cop/packaging/require_hardcoding_lib.rb | 17 +- .../packaging/require_hardcoding_lib_spec.rb | 155 ++++-------------- 2 files changed, 40 insertions(+), 132 deletions(-) diff --git a/lib/rubocop/cop/packaging/require_hardcoding_lib.rb b/lib/rubocop/cop/packaging/require_hardcoding_lib.rb index af9f0a5..c49088d 100644 --- a/lib/rubocop/cop/packaging/require_hardcoding_lib.rb +++ b/lib/rubocop/cop/packaging/require_hardcoding_lib.rb @@ -40,15 +40,14 @@ class RequireHardcodingLib < Base # This is the message that will be displayed when RuboCop::Packaging # finds an offense of using `require` with relative path to lib. - MSG = "Avoid using `require` with relative path to `lib/`. " \ - "Use `require` with absolute path instead." + MSG = "Avoid using relative path to `lib/`. " \ + "Use absolute path instead." - def_node_matcher :require?, <<~PATTERN - {(send nil? :require (str #falls_in_lib?)) - (send nil? :require (send (const nil? :File) :expand_path (str #falls_in_lib?) (send nil? :__dir__))) - (send nil? :require (send (const nil? :File) :expand_path (str #falls_in_lib_using_file?) (str _))) - (send nil? :require (send (send (const nil? :File) :dirname {(str _) (send nil? _)}) :+ (str #falls_in_lib_with_file_dirname_plus_str?))) - (send nil? :require (dstr (begin (send (const nil? :File) :dirname {(str _) (send nil? _)})) (str #falls_in_lib_with_file_dirname_plus_str?)))} + def_node_matcher :File?, <<~PATTERN + {(send (const nil? :File) :expand_path (str #falls_in_lib?) (send nil? :__dir__)) + (send (const nil? :File) :expand_path (str #falls_in_lib_using_file?) (str _)) + (send (send (const nil? :File) :dirname {(str _) (send nil? _)}) :+ (str #falls_in_lib_with_file_dirname_plus_str?)) + (dstr (begin (send (const nil? :File) :dirname {(str _) (send nil? _)})) (str #falls_in_lib_with_file_dirname_plus_str?))} PATTERN # Extended from the Base class. @@ -65,7 +64,7 @@ def on_new_investigation # More about the `#on_send` method can be found here: # https://github.com/rubocop-hq/rubocop-ast/blob/08d0f49a47af1e9a30a6d8f67533ba793c843d67/lib/rubocop/ast/traversal.rb#L112 def on_send(node) - return unless require?(node) + return unless File?(node) add_offense(node) do |corrector| corrector.replace(node, good_require_call) diff --git a/spec/rubocop/cop/packaging/require_hardcoding_lib_spec.rb b/spec/rubocop/cop/packaging/require_hardcoding_lib_spec.rb index a32afac..ac8ec18 100644 --- a/spec/rubocop/cop/packaging/require_hardcoding_lib_spec.rb +++ b/spec/rubocop/cop/packaging/require_hardcoding_lib_spec.rb @@ -5,45 +5,9 @@ let(:project_root) { RuboCop::ConfigLoader.project_root } - context "when `require` call lies outside spec/" do - let(:filename) { "#{project_root}/spec/foo_spec.rb" } - let(:source) { "require '../lib/foo.rb'" } - - it "registers an offense" do - expect_offense(<<~RUBY, filename) - #{source} - #{"^" * source.length} #{message} - RUBY - - expect_correction(<<~RUBY) - require "foo.rb" - RUBY - end - end - - context "when `require` call after using `unshift` lies outside spec/" do - let(:filename) { "#{project_root}/tests/foo/bar.rb" } - let(:source) { <<~RUBY.chomp } - $:.unshift('../../lib') - require '../../lib/foo/bar' - RUBY - - it "registers an offense" do - expect_offense(<<~RUBY, filename) - #{source} - #{"^" * 27} #{message} - RUBY - - expect_correction(<<~RUBY) - $:.unshift('../../lib') - require "foo/bar" - RUBY - end - end - - context "when `require` call uses File#expand_path method with __FILE__" do + context "when use of File#expand_path method with __FILE__" do let(:filename) { "#{project_root}/spec/foo.rb" } - let(:source) { "require File.expand_path('../../lib/foo', __FILE__)" } + let(:source) { "File.expand_path('../../lib/foo', __FILE__)" } it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -57,9 +21,9 @@ end end - context "when `require` call uses File#expand_path method with __dir__" do + context "when use of File#expand_path method with __dir__" do let(:filename) { "#{project_root}/test/foo/bar/qux_spec.rb" } - let(:source) { "require File.expand_path('../../../lib/foo/bar/baz/qux', __dir__)" } + let(:source) { "File.expand_path('../../../lib/foo/bar/baz/qux', __dir__)" } it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -73,9 +37,9 @@ end end - context "when `require` call uses File#dirname method with __FILE__" do + context "when use of File#dirname method with __FILE__" do let(:filename) { "#{project_root}/specs/baz/qux_spec.rb" } - let(:source) { "require File.dirname(__FILE__) + '/../../lib/baz/qux'" } + let(:source) { "File.dirname(__FILE__) + '/../../lib/baz/qux'" } it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -89,9 +53,9 @@ end end - context "when `require` call uses File#dirname method with __FILE__ with interpolation" do + context "when use of File#dirname method with __FILE__ with interpolation" do let(:filename) { "#{project_root}/specs/baz/qux_spec.rb" } - let(:source) { 'require "#{File.dirname(__FILE__)}/../../lib/baz/qux"' } # rubocop:disable Lint/InterpolationCheck + let(:source) { '"#{File.dirname(__FILE__)}/../../lib/baz/qux"' } # rubocop:disable Lint/InterpolationCheck it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -105,9 +69,9 @@ end end - context "when `require` call uses File#dirname method with __dir__" do + context "when use of File#dirname method with __dir__" do let(:filename) { "#{project_root}/spec/foo.rb" } - let(:source) { "require File.dirname(__dir__) + '/../lib/foo'" } + let(:source) { "File.dirname(__dir__) + '/../lib/foo'" } it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -121,9 +85,9 @@ end end - context "when `require` call uses File#dirname method with __dir__ with interpolation" do + context "when use of File#dirname method with __dir__ with interpolation" do let(:filename) { "#{project_root}/spec/foo.rb" } - let(:source) { 'require "#{File.dirname(__dir__)}/../lib/foo"' } # rubocop:disable Lint/InterpolationCheck + let(:source) { '"#{File.dirname(__dir__)}/../lib/foo"' } # rubocop:disable Lint/InterpolationCheck it "registers an offense" do expect_offense(<<~RUBY, filename) @@ -137,64 +101,9 @@ end end - context "when the `require` call doesn't use relative path" do - let(:filename) { "#{project_root}/spec/bar_spec.rb" } - let(:source) { "require 'bar'" } - - it "does not register an offense" do - expect_no_offenses(<<~RUBY, filename) - #{source} - RUBY - end - end - - context "when the `require` call lies inside test/" do - let(:filename) { "#{project_root}/test/bar/foo_spec.rb" } - let(:source) { "require '../foo'" } - - it "does not register an offense" do - expect_no_offenses(<<~RUBY, filename) - #{source} - RUBY - end - end - - context "when the `require` call is made to lib/ but it lies under spec/" do - let(:filename) { "#{project_root}/spec/lib/baz/qux_spec.rb" } - let(:source) { "require '../../lib/foo'" } - - it "does not register an offense" do - expect_no_offenses(<<~RUBY, filename) - #{source} - RUBY - end - end - - context "when the `require` call is made to lib/ from inside lib/ itself" do - let(:filename) { "#{project_root}/lib/foo/bar.rb" } - let(:source) { "require '../foo'" } - - it "does not register an offense" do - expect_no_offenses(<<~RUBY, filename) - #{source} - RUBY - end - end - - context "when the `require` call is made from the gemspec file" do - let(:filename) { "#{project_root}/foo.gemspec" } - let(:source) { "require 'lib/foo/version'" } - - it "does not register an offense" do - expect_no_offenses(<<~RUBY, filename) - #{source} - RUBY - end - end - - context "when `require` call uses File#expand_path method with __FILE__ but lies inside lib/" do + context "when use of File#expand_path method with __FILE__ but lies inside lib/" do let(:filename) { "#{project_root}/lib/foo/bar.rb" } - let(:source) { "require File.expand_path('../../foo', __FILE__)" } + let(:source) { "File.expand_path('../../foo', __FILE__)" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -203,9 +112,9 @@ end end - context "when `require` call uses File#expand_path method with __FILE__ and is made from the gemspec file" do + context "when use of File#expand_path method with __FILE__ and is made from the gemspec file" do let(:filename) { "#{project_root}/bar.gemspec" } - let(:source) { "require File.expand_path('../lib/foo/version', __FILE__)" } + let(:source) { "File.expand_path('../lib/foo/version', __FILE__)" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -214,9 +123,9 @@ end end - context "when `require` call uses File#expand_path method with __dir__ but lies inside lib/" do + context "when use of File#expand_path method with __dir__ but lies inside lib/" do let(:filename) { "#{project_root}/lib/foo/bar/baz/qux.rb" } - let(:source) { "require File.expand_path('../../foo', __dir__)" } + let(:source) { "File.expand_path('../../foo', __dir__)" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -225,9 +134,9 @@ end end - context "when `require` call uses File#expand_path method with __dir__ and is made from the gemspec file" do + context "when use of File#expand_path method with __dir__ and is made from the gemspec file" do let(:filename) { "#{project_root}/qux.gemspec" } - let(:source) { "require File.expand_path('lib/qux/version', __dir__)" } + let(:source) { "File.expand_path('lib/qux/version', __dir__)" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -236,9 +145,9 @@ end end - context "when the `require` call uses File#dirname with __FILE__ but lies inside tests/" do + context "when use of File#dirname with __FILE__ but lies inside tests/" do let(:filename) { "#{project_root}/tests/foo/bar_spec.rb" } - let(:source) { "require File.dirname(__FILE__) + '/../lib/bar'" } + let(:source) { "File.dirname(__FILE__) + '/../lib/bar'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -247,9 +156,9 @@ end end - context "when the `require` call uses File#dirname with __FILE__ but lies inside lib/" do + context "when use of File#dirname with __FILE__ but lies inside lib/" do let(:filename) { "#{project_root}/lib/baz/qux.rb" } - let(:source) { "require File.dirname(__FILE__) + '/../baz'" } + let(:source) { "File.dirname(__FILE__) + '/../baz'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -258,9 +167,9 @@ end end - context "when the `require` call uses File#dirname with __FILE__ and is made from the gemspec file" do + context "when use of File#dirname with __FILE__ and is made from the gemspec file" do let(:filename) { "#{project_root}/bar.gemspec" } - let(:source) { "require File.dirname(__FILE__) + '/../lib/bar/version'" } + let(:source) { "File.dirname(__FILE__) + '/../lib/bar/version'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -269,9 +178,9 @@ end end - context "when the `require` call uses File#dirname with __dir__ but lies inside spec/" do + context "when use of File#dirname with __dir__ but lies inside spec/" do let(:filename) { "#{project_root}/spec/foo/bar_spec.rb" } - let(:source) { "require File.dirname(__dir__) + '/../lib/bar'" } + let(:source) { "File.dirname(__dir__) + '/../lib/bar'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -280,9 +189,9 @@ end end - context "when the `require` call uses File#dirname with __dir__ but lies inside lib/" do + context "when use of File#dirname with __dir__ but lies inside lib/" do let(:filename) { "#{project_root}/lib/baz/qux.rb" } - let(:source) { "require File.dirname(__dir__) + '/../baz'" } + let(:source) { "File.dirname(__dir__) + '/../baz'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename) @@ -291,9 +200,9 @@ end end - context "when the `require` call uses File#dirname with __dir__ and is made from the gemspec file" do + context "when use of File#dirname with __dir__ and is made from the gemspec file" do let(:filename) { "#{project_root}/baz.gemspec" } - let(:source) { "require File.dirname(__dir__) + '/lib/baz/version'" } + let(:source) { "File.dirname(__dir__) + '/lib/baz/version'" } it "does not register an offense" do expect_no_offenses(<<~RUBY, filename)