-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add configuration validation #709
Conversation
042573d
to
c997622
Compare
The first time `options` method is hit, we validate the Tapioca configuration. The validation makes sure that all top level keys match to some command, all command options match to one of the flags for that command and that all values for command options are of the expected type.
c997622
to
dd3d128
Compare
Thor::CoreExt::HashWithIndifferentAccess.new(config[command_name] || {}) | ||
end | ||
|
||
sig { params(config_file: String, config: T::Hash[T.untyped, T.untyped]).void } | ||
def validate_config!(config_file, config) | ||
# To ensure that this is not re-entered, we mark during validation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect config_options
to be called more than once in the same execution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, say_error
(or say
) end up calling options[:quiet]
to decide to print or not which ends up reentering the options
method, before we've memoized the merged options hash. That causes an infinite loop. In general, any Thor code we call from this method could end up doing another options
lookup, so I am protecting this method.
Motivation
Fixes #704
Implementation
The first time
options
method is hit, we validate the Tapioca configuration. The validation makes sure that all top level keys match to some command, all command options match to one of the flags for that command and that all values for command options are of the expected type.Tests
Add new tests for configuration failures.