Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
12 changes: 12 additions & 0 deletions docs/How-To-Write-New-Detectors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion features/configuration_files/masking_smells.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion features/configuration_files/schema_validation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
2 changes: 1 addition & 1 deletion lib/reek/configuration/configuration_file_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ def load_from_file(path)

begin
configuration = YAML.load_file(path) || {}
SchemaValidator.new(configuration).validate
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving it here so we can catch configuration file errors raised by the validator.
The benefit is that we have access to the config file path here.

If this is not ideal, I can revert to the way we had but won't have access to the config file path.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense 👍

rescue StandardError => error
raise Errors::ConfigFileError, "Invalid configuration file #{path}, error is #{error}"
end

SchemaValidator.new(configuration).validate
ConfigurationConverter.new(configuration).convert
end

Expand Down
177 changes: 177 additions & 0 deletions lib/reek/configuration/schema.rb
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file replaces the old schema.yml

Original file line number Diff line number Diff line change
@@ -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
Loading