-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(forge
): detect invalid inline config keys
#9363
Conversation
return Err(InlineConfigError { | ||
line, | ||
source: InlineConfigParserError::InvalidConfigKey( | ||
wrong_key, | ||
format!("{VALID_CONFIG_KEYS:?}"), | ||
), | ||
}); |
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.
I think a warning would be sufficient here given that the key is simply ignored when continuing
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.
Warnings won't be shown if --quiet
is enabled.
Moreover, currently we error out on invalid property keys like fuzz.runssss
. This keeps it consistent with that i.e err thrown on invalid keys.
/// Used to detect invalid configuration keys in the inline config. | ||
/// | ||
/// # Example | ||
/// | ||
/// forge-config: default.fuzz.runs = 100 - `fuzz` is a valid key | ||
/// forge-config: default.wrong.runs = 100 - `wrong` is an invalid key | ||
const VALID_CONFIG_KEYS: [&str; 2] = ["fuzz", "invariant"]; |
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.
For now this is sufficient I think, assuming this functionality will be updated when we have full support for in-line configuration
/// | ||
/// forge-config: default.fuzz.runs = 100 - `fuzz` is a valid key | ||
/// forge-config: default.wrong.runs = 100 - `wrong` is an invalid key | ||
const VALID_CONFIG_KEYS: [&str; 2] = ["fuzz", "invariant"]; |
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.
I wonder if this isn't too simplistic?
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.
It is for now. But it won't be needed when we have full support for inline config.
@grandizzy can we merge this? |
Superseded by #9414 |
Motivation
Closes #5371
Solution
const VALID_CONFIG_KEYS
VALID_CONFIG_KEYS
.InvalidConfigKey
error if it does.