ISSUE-1742 Replace kwalify with dry-schema for schema validation

The current validator (Kwalify) seems outdated and lacks good documentation.

A recent issue showed that we could improve the schema validation to also
check and warn against missing configurations (See issue: #1734)

dry-schema provide good documentation and it looks like it also provides
the features we require.

Structural validation where key presence can be verified separately from
values. This removes ambiguity related to "presence" validation where you
don't know if value is indeed nil or if a key is missing in the input
hash.

Changes include:
- Update changelog
- Add a validation section to How-To-Write-New-Detectors
- Add schema specs
- Update feature specs
- Catch schema validation error in lib/reek/configuration/configuration_file_finder.rb,
  so we have access to the config file path for the displayed error message.

See: #1742
See: #1734

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change log
 
+## [Unreleased]
+
+* (fbuys) Replace the config schema validator [Kwalify](https://github.com/sunaku/kwalify) with [dry-schema](https://github.com/dry-rb/dry-schema)
+
 ## 6.1.4 (2023-01-13)
 
 * (mvz) Update parser dependency to the 3.2.x series

docs/How-To-Write-New-Detectors.md

@@ -124,6 +124,18 @@ end
 
 The following examples should then cover the detector specific features.
 
+### Schema validation
+
+We make use of [dry-schema](https://github.com/dry-rb/dry-schema) to validate the
+the reek configuration file (usually named `.reek.yml` in the root of a project).
+The validation will warn reek users of missing or incorrectly defined configuration.
+
+If you add or modify a detector you will need to make sure that you update the 
+[schema](lib/reek/configuration/schema.rb) file. For help with the schema syntax
+you can refer to the [dry-schema documentation](https://dry-rb.org/gems/dry-schema).
+You can also take a look at the existing schema rules for they all look fairly
+similar.
+
 ### Cucumber features
 
 We are trying to write as few Cucumber features as possible.

features/configuration_files/directory_specific_directives.feature

@@ -200,7 +200,8 @@ Feature: Directory directives
     Then the exit status indicates an error
     And stderr reports:
     """
-    Error: We found some problems with your configuration file: [/directories/dummy_directory/IteratorsNested] key 'IteratorsNested:' is undefined.
+    Error: Invalid configuration file config.reek, error is 
+    [/directories/dummy_directory/IteratorsNested/enabled] is not allowed.
     """
 
   Scenario: Abort on file as directory directive

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: We found some problems with your configuration file: [/] 'Not a valid configuration file': not a mapping.
+    Error: Invalid configuration file corrupt.reek, error is unrecognized configuration data
     """
     And the exit status indicates an error
     And it reports nothing

features/configuration_files/schema_validation.feature

@@ -55,5 +55,6 @@ Feature: Validate schema
     Then the exit status indicates an error
     And stderr reports:
     """
-    Error: We found some problems with your configuration file: [/detectors/DoesNotExist] key 'DoesNotExist:' is undefined.
+    Error: Invalid configuration file config.reek, error is 
+    [/detectors/DoesNotExist/enabled] is not allowed.
     """

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
 

lib/reek/configuration/schema.rb

@@ -0,0 +1,177 @@
+# frozen_string_literal: true
+
+require 'dry/schema'
+
+module Reek
+  module Configuration
+    #
+    # Configuration schema constants.
+    #
+    class Schema
+      # Enable the :info extension so we can introspect
+      # your keys and types
+      Dry::Schema.load_extensions(:info)
+
+      # rubocop:disable Metrics/BlockLength
+      ALL_DETECTORS_SCHEMA = Dry::Schema.Params do
+        optional(:Attribute).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:BooleanParameter).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:ClassVariable).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:ControlParameter).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)
+          optional(:max_copies).filled(:integer)
+          optional(:min_clump_size).filled(:integer)
+        end
+        optional(:DuplicateMethodCall).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_calls).filled(:integer)
+          optional(:allow_calls).array(:string)
+        end
+        optional(:FeatureEnvy).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:InstanceVariableAssumption).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:IrresponsibleModule).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:LongParameterList).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_params).filled(:integer)
+          optional(:overrides).filled(:hash) do
+            required(:initialize).filled(:hash) do
+              required(:max_params).filled(:integer)
+            end
+          end
+        end
+        optional(:LongYieldList).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_params).filled(:integer)
+        end
+        optional(:ManualDispatch).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:MissingSafeMethod).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:ModuleInitialize).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:NestedIterators).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_allowed_nesting).filled(:integer)
+          optional(:ignore_iterators) { array(:string) & filled? }
+        end
+        optional(:NilCheck).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:RepeatedConditional).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_ifs).filled(:integer)
+        end
+        optional(:SubclassedFromCoreClass).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:TooManyConstants).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_constants).filled(:integer)
+        end
+        optional(:TooManyInstanceVariables).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_instance_variables).filled(:integer)
+        end
+        optional(:TooManyMethods).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_methods).filled(:integer)
+        end
+        optional(:TooManyStatements).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:max_statements).filled(:integer)
+        end
+        optional(:UncommunicativeMethodName).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:reject).array(:string)
+          optional(:accept).array(:string)
+        end
+        optional(:UncommunicativeModuleName).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:reject).array(:string)
+          optional(:accept).array(:string)
+        end
+        optional(:UncommunicativeParameterName).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:reject).array(:string)
+          optional(:accept).array(:string)
+        end
+        optional(:UncommunicativeVariableName).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:reject).array(:string)
+          optional(:accept).array(:string)
+        end
+        optional(:UnusedParameters).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:UnusedPrivateMethod).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+        end
+        optional(:UtilityFunction).filled(:hash) do
+          optional(:enabled).filled(:bool)
+          optional(:exclude).array(:string)
+          optional(:public_methods_only).filled(:bool)
+        end
+      end
+      # rubocop:enable Metrics/BlockLength
+
+      # @quality :reek:TooManyStatements { max_statements: 7 }
+      def self.schema(directories = [])
+        Dry::Schema.Params do
+          config.validate_keys = true
+
+          optional(:detectors).filled(ALL_DETECTORS_SCHEMA)
+          optional(:directories).filled(:hash) do
+            directories.each { |dir| optional(dir.to_sym).filled(ALL_DETECTORS_SCHEMA) }
+          end
+          optional(:exclude_paths).array(:string)
+        end
+      end
+    end
+  end
+end