From d12312744e4e64af960224873872aab065b5f16a Mon Sep 17 00:00:00 2001 From: Francois Buys Date: Wed, 1 Nov 2023 00:35:15 +0200 Subject: [PATCH] Address PR feedback --- .../directory_specific_directives.feature | 2 +- features/configuration_files/masking_smells.feature | 2 +- features/configuration_files/schema_validation.feature | 2 +- lib/reek/configuration/configuration_file_finder.rb | 2 +- lib/reek/configuration/schema.rb | 5 ----- lib/reek/configuration/schema_validator.rb | 7 +++---- spec/reek/configuration/configuration_file_finder_spec.rb | 6 ++++++ spec/reek/configuration/schema_validator_spec.rb | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/features/configuration_files/directory_specific_directives.feature b/features/configuration_files/directory_specific_directives.feature index 95e0944ba..328fff330 100644 --- a/features/configuration_files/directory_specific_directives.feature +++ b/features/configuration_files/directory_specific_directives.feature @@ -200,7 +200,7 @@ Feature: Directory directives Then the exit status indicates an error And stderr reports: """ - Error: We found some problems with your configuration file: + Error: Invalid configuration file config.reek, error is [/directories/dummy_directory/IteratorsNested/enabled] is not allowed. """ diff --git a/features/configuration_files/masking_smells.feature b/features/configuration_files/masking_smells.feature index ec17e274d..026b5235a 100644 --- a/features/configuration_files/masking_smells.feature +++ b/features/configuration_files/masking_smells.feature @@ -9,7 +9,7 @@ Feature: Masking smells using config files When I run reek -c corrupt.reek smelly.rb And stderr reports: """ - Error: Invalid configuration file at .reek.yml. + Error: Invalid configuration file corrupt.reek, error is unrecognized configuration data """ And the exit status indicates an error And it reports nothing diff --git a/features/configuration_files/schema_validation.feature b/features/configuration_files/schema_validation.feature index 22435951e..0d63aa589 100644 --- a/features/configuration_files/schema_validation.feature +++ b/features/configuration_files/schema_validation.feature @@ -55,6 +55,6 @@ Feature: Validate schema Then the exit status indicates an error And stderr reports: """ - Error: We found some problems with your configuration file: + Error: Invalid configuration file config.reek, error is [/detectors/DoesNotExist/enabled] is not allowed. """ diff --git a/lib/reek/configuration/configuration_file_finder.rb b/lib/reek/configuration/configuration_file_finder.rb index 2668d9ed9..b6d6a025e 100644 --- a/lib/reek/configuration/configuration_file_finder.rb +++ b/lib/reek/configuration/configuration_file_finder.rb @@ -58,11 +58,11 @@ def load_from_file(path) begin configuration = YAML.load_file(path) || {} + SchemaValidator.new(configuration).validate rescue StandardError => error raise Errors::ConfigFileError, "Invalid configuration file #{path}, error is #{error}" end - SchemaValidator.new(configuration).validate ConfigurationConverter.new(configuration).convert end diff --git a/lib/reek/configuration/schema.rb b/lib/reek/configuration/schema.rb index 57f91ad1a..8c09fac52 100644 --- a/lib/reek/configuration/schema.rb +++ b/lib/reek/configuration/schema.rb @@ -30,10 +30,6 @@ class Schema optional(:enabled).filled(:bool) optional(:exclude).array(:string) end - optional(:DataClump).filled(:hash) do - optional(:enabled).filled(:bool) - optional(:exclude).array(:string) - end optional(:DataClump).filled(:hash) do optional(:enabled).filled(:bool) optional(:exclude).array(:string) @@ -168,7 +164,6 @@ class Schema def self.schema(directories = []) Dry::Schema.Params do config.validate_keys = true - # config.messages.load_paths << "lib/reek/configuration/schema_errors.yml" optional(:detectors).filled(ALL_DETECTORS_SCHEMA) optional(:directories).filled(:hash) do diff --git a/lib/reek/configuration/schema_validator.rb b/lib/reek/configuration/schema_validator.rb index c05659195..9406eeb48 100644 --- a/lib/reek/configuration/schema_validator.rb +++ b/lib/reek/configuration/schema_validator.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require_relative '../cli/silencer' require_relative '../errors/config_file_error' require_relative 'schema' @@ -17,12 +16,12 @@ def initialize(configuration) end def validate - result = CLI::Silencer.without_warnings { @validator.call(@configuration) } + result = @validator.call(@configuration) return if result.success? raise Errors::ConfigFileError, error_message(result.errors) rescue NoMethodError - raise Errors::ConfigFileError, "Invalid configuration file at #{Reek::DEFAULT_CONFIGURATION_FILE_NAME}." + raise Errors::ConfigFileError, 'unrecognized configuration data' end private @@ -32,7 +31,7 @@ def error_message(errors) messages = errors.map do |error| "[/#{error.path.join('/')}] #{error.text}." end.join("\n") - "We found some problems with your configuration file:\n#{messages}" + "\n#{messages}" end end end diff --git a/spec/reek/configuration/configuration_file_finder_spec.rb b/spec/reek/configuration/configuration_file_finder_spec.rb index 4ff3887c3..bdfa99b33 100644 --- a/spec/reek/configuration/configuration_file_finder_spec.rb +++ b/spec/reek/configuration/configuration_file_finder_spec.rb @@ -120,6 +120,12 @@ end end + it 'raises an error for unrecogized configuration data' do + path = CONFIGURATION_DIR.join('corrupt.reek') + expect { described_class.load_from_file(path) }. + to raise_error(Reek::Errors::ConfigFileError, /Invalid configuration file #{path}/) + end + context 'with exclude, accept and reject settings' do context 'when configuring top level detectors' do let(:configuration) do diff --git a/spec/reek/configuration/schema_validator_spec.rb b/spec/reek/configuration/schema_validator_spec.rb index 7f055f06e..2d2e63e37 100644 --- a/spec/reek/configuration/schema_validator_spec.rb +++ b/spec/reek/configuration/schema_validator_spec.rb @@ -25,7 +25,7 @@ let(:configuration) { 'Not a valid configuration file' } it 'raises an error' do - message = "Invalid configuration file at #{Reek::DEFAULT_CONFIGURATION_FILE_NAME}." + message = 'unrecognized configuration data' expect { validator.validate }.to raise_error(Reek::Errors::ConfigFileError, message) end end