From 765d34f209deafc41c03438ef34e8e7504976496 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Sun, 21 Jan 2024 15:25:50 +0000 Subject: [PATCH] Check that `Include` and `Exclude` paths exist (#33) * Check that `Include` and `Exclude` paths exist - This adds a `PathCleanup` class that removes any paths in the `Include` and `Exclude` lists that don't exist (relative to the `.rubocop.yml`, ignoring paths with globs). - This is useful for when you have a `.rubocop.yml` file that has been around for a while and has lots of paths that people forget to clean up when they delete the files, so doing so manually would be tedious. * More comprehensive `path_cleanup` test * Remove `Symbol` from `PERMITTED_CLASSES` - This was needed in an earlier iteration of the code I wrote, but it seems to work without it now on my test repos. * Rename method for clarity * The YAML parsing makes string checks redundant * Use `filter_map` instead of `each_with_index` and an intermediate array - This is very nice! --- lib/ruboclean.rb | 1 + lib/ruboclean/path_cleanup.rb | 36 ++++++++++++++ lib/ruboclean/rubocop_configuration.rb | 9 ++++ lib/ruboclean/rubocop_configuration_path.rb | 4 +- lib/ruboclean/runner.rb | 2 +- test/fixtures/file_exists.rb | 2 + test/fixtures/other_file_exists.rb | 2 + test/ruboclean/path_cleanup_test.rb | 50 ++++++++++++++++++++ test/ruboclean/rubocop_configuration_test.rb | 21 ++++++++ 9 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 lib/ruboclean/path_cleanup.rb create mode 100644 test/fixtures/file_exists.rb create mode 100644 test/fixtures/other_file_exists.rb create mode 100644 test/ruboclean/path_cleanup_test.rb diff --git a/lib/ruboclean.rb b/lib/ruboclean.rb index 7ea3dd3..084dad3 100644 --- a/lib/ruboclean.rb +++ b/lib/ruboclean.rb @@ -6,6 +6,7 @@ require "ruboclean/rubocop_configuration_path" require "ruboclean/runner" require "ruboclean/orderer" +require "ruboclean/path_cleanup" # Ruboclean entry point module Ruboclean diff --git a/lib/ruboclean/path_cleanup.rb b/lib/ruboclean/path_cleanup.rb new file mode 100644 index 0000000..d0fa867 --- /dev/null +++ b/lib/ruboclean/path_cleanup.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Ruboclean + # Cleans up any `Include` or `Exclude` paths that don't exist. + # The `Include` and `Exclude` paths are relative to the directory + # where the `.rubocop.yml` file is located. If a path includes a + # wildcard, it's assumed to be valid. + class PathCleanup + def initialize(config_hash) + @config_hash = config_hash + end + + def cleanup + %i[Include Exclude].each do |kind| + select_stanzas(kind).each do |cop| + paths = @config_hash.dig(cop, kind) + paths&.select! { |path| regexp_or_wildcard?(path) || File.exist?(path) } + end + end + + @config_hash + end + + def select_stanzas(kind) + @config_hash.filter_map do |cop, value| + next unless value.is_a?(Hash) + + cop if value.key?(kind) + end + end + + def regexp_or_wildcard?(path) + path.is_a?(Regexp) || path.include?("*") + end + end +end diff --git a/lib/ruboclean/rubocop_configuration.rb b/lib/ruboclean/rubocop_configuration.rb index a668565..b38c7ab 100644 --- a/lib/ruboclean/rubocop_configuration.rb +++ b/lib/ruboclean/rubocop_configuration.rb @@ -11,6 +11,15 @@ def order Ruboclean::Orderer.new(@config_hash).order end + def path_cleanup + Ruboclean::PathCleanup.new(@config_hash).cleanup + end + + def perform + @config_hash = order + path_cleanup + end + def nil? @config_hash.nil? end diff --git a/lib/ruboclean/rubocop_configuration_path.rb b/lib/ruboclean/rubocop_configuration_path.rb index 95c557a..1e9e741 100644 --- a/lib/ruboclean/rubocop_configuration_path.rb +++ b/lib/ruboclean/rubocop_configuration_path.rb @@ -6,7 +6,7 @@ module Ruboclean # Interface for reading and writing the `.rubocop.yml` file class RubocopConfigurationPath - PERMITTED_CLASSED = [Regexp].freeze + PERMITTED_CLASSES = [Regexp].freeze # Thrown if given path is invalid class InvalidPathError < StandardError @@ -44,7 +44,7 @@ def sanitize_yaml(data) end def load_yaml - YAML.safe_load(source_yaml, permitted_classes: PERMITTED_CLASSED) + YAML.safe_load(source_yaml, permitted_classes: PERMITTED_CLASSES) end def source_yaml diff --git a/lib/ruboclean/runner.rb b/lib/ruboclean/runner.rb index 36bedaf..4b0ddb4 100644 --- a/lib/ruboclean/runner.rb +++ b/lib/ruboclean/runner.rb @@ -13,7 +13,7 @@ def run! return if rubocop_configuration.nil? - rubocop_configuration_path.write(rubocop_configuration.order, preserve_comments: arguments.preserve_comments?) + rubocop_configuration_path.write(rubocop_configuration.perform, preserve_comments: arguments.preserve_comments?) end private diff --git a/test/fixtures/file_exists.rb b/test/fixtures/file_exists.rb new file mode 100644 index 0000000..ac70bc7 --- /dev/null +++ b/test/fixtures/file_exists.rb @@ -0,0 +1,2 @@ +# frozen_string_literal: true +# test for path cleanup diff --git a/test/fixtures/other_file_exists.rb b/test/fixtures/other_file_exists.rb new file mode 100644 index 0000000..ac70bc7 --- /dev/null +++ b/test/fixtures/other_file_exists.rb @@ -0,0 +1,2 @@ +# frozen_string_literal: true +# test for path cleanup diff --git a/test/ruboclean/path_cleanup_test.rb b/test/ruboclean/path_cleanup_test.rb new file mode 100644 index 0000000..38b677e --- /dev/null +++ b/test/ruboclean/path_cleanup_test.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "test_helper" +require "ruboclean/path_cleanup" + +module Ruboclean + class PathCleanupTest < BaseTest + def test_path_cleanup_includes + input = { SomeCop: { Include: ["some_other_file.rb", "test/fixtures/file_exists.rb", "lib/**/*.rb"] } } + output = { SomeCop: { Include: ["test/fixtures/file_exists.rb", "lib/**/*.rb"] } } + + assert_equal output, Ruboclean::PathCleanup.new(input).cleanup + end + + def test_path_cleanup_excludes + input = { SomeCop: { Exclude: ["config/**/*.rb", "test/fixtures/other_file_exists.rb", + "some_other_non_existent_file.rb", "test/fixtures/not_here.rb"] } } + output = { SomeCop: { Exclude: ["config/**/*.rb", "test/fixtures/other_file_exists.rb"] } } + + assert_equal output, Ruboclean::PathCleanup.new(input).cleanup + end + + # rubocop:disable Metrics/MethodLength + def test_path_cleanup_include_and_exclude + input = { + SomeCop: { + Include: ["lib/**/*.rb", "test/fixtures/not_here.rb"], + Exclude: ["config/**/*.rb", "test/fixtures/other_file_exists.rb", "some_other_non_existent_file.rb"], + EnforcedStyle: "something" + }, + SomeOtherCop: { + EnforcedStyle: "something" + } + } + output = { + SomeCop: { + Include: ["lib/**/*.rb"], + Exclude: ["config/**/*.rb", "test/fixtures/other_file_exists.rb"], + EnforcedStyle: "something" + }, + SomeOtherCop: { + EnforcedStyle: "something" + } + } + + assert_equal output, Ruboclean::PathCleanup.new(input).cleanup + end + # rubocop:enable Metrics/MethodLength + end +end diff --git a/test/ruboclean/rubocop_configuration_test.rb b/test/ruboclean/rubocop_configuration_test.rb index 16ea7a1..1d59d41 100644 --- a/test/ruboclean/rubocop_configuration_test.rb +++ b/test/ruboclean/rubocop_configuration_test.rb @@ -14,6 +14,27 @@ def test_order end end + def test_path_cleanup + input = { SomeCop: { Include: ["some_other_file.rb", "test/fixtures/file_exists.rb", "lib/**/*.rb"] } } + output = { SomeCop: { Include: ["test/fixtures/file_exists.rb", "lib/**/*.rb"] } } + + Ruboclean::RubocopConfiguration.new(input).path_cleanup.tap do |cleaned_output| + assert_instance_of Hash, cleaned_output + assert_equal output.to_a, cleaned_output.to_a + end + end + + def test_perform + input = { "Rails" => { Exclude: ["does_not_exist.rb", "test/fixtures/file_exists.rb"] }, + "AllCops" => { Enabled: true } } + output = { "AllCops" => { Enabled: true }, "Rails" => { Exclude: ["test/fixtures/file_exists.rb"] } } + + Ruboclean::RubocopConfiguration.new(input).perform.tap do |cleaned_output| + assert_instance_of Hash, cleaned_output + assert_equal output.to_a, cleaned_output.to_a + end + end + def test_nil? assert_nil Ruboclean::RubocopConfiguration.new(nil) end