Skip to content

Commit

Permalink
Explicitly ban overriding extend as part of a --config flag (astral…
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood authored and nkxxll committed Mar 4, 2024
1 parent ed3d808 commit fc2e637
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 40 deletions.
96 changes: 56 additions & 40 deletions crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,38 +745,34 @@ fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
}
}

/// Enumeration of various ways in which a --config CLI flag
/// could be invalid
#[derive(Debug)]
enum TomlParseFailureKind {
SyntaxError,
UnknownOption,
enum InvalidConfigFlagReason {
InvalidToml(toml::de::Error),
/// It was valid TOML, but not a valid ruff config file.
/// E.g. the user tried to select a rule that doesn't exist,
/// or tried to enable a setting that doesn't exist
ValidTomlButInvalidRuffSchema(toml::de::Error),
/// It was a valid ruff config file, but the user tried to pass a
/// value for `extend` as part of the config override.
// `extend` is special, because it affects which config files we look at
/// in the first place. We currently only parse --config overrides *after*
/// we've combined them with all the arguments from the various config files
/// that we found, so trying to override `extend` as part of a --config
/// override is forbidden.
ExtendPassedViaConfigFlag,
}

impl std::fmt::Display for TomlParseFailureKind {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let display = match self {
Self::SyntaxError => "The supplied argument is not valid TOML",
Self::UnknownOption => {
impl InvalidConfigFlagReason {
const fn description(&self) -> &'static str {
match self {
Self::InvalidToml(_) => "The supplied argument is not valid TOML",
Self::ValidTomlButInvalidRuffSchema(_) => {
"Could not parse the supplied argument as a `ruff.toml` configuration option"
}
};
write!(f, "{display}")
}
}

#[derive(Debug)]
struct TomlParseFailure {
kind: TomlParseFailureKind,
underlying_error: toml::de::Error,
}

impl std::fmt::Display for TomlParseFailure {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let TomlParseFailure {
kind,
underlying_error,
} = self;
let display = format!("{kind}:\n\n{underlying_error}");
write!(f, "{}", display.trim_end())
Self::ExtendPassedViaConfigFlag => "Cannot include `extend` in a --config flag value",
}
}
}

Expand Down Expand Up @@ -827,18 +823,19 @@ impl TypedValueParser for ConfigArgumentParser {
.to_str()
.ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?;

let toml_parse_error = match toml::Table::from_str(value) {
Ok(table) => match table.try_into() {
Ok(option) => return Ok(SingleConfigArgument::SettingsOverride(Arc::new(option))),
Err(underlying_error) => TomlParseFailure {
kind: TomlParseFailureKind::UnknownOption,
underlying_error,
},
},
Err(underlying_error) => TomlParseFailure {
kind: TomlParseFailureKind::SyntaxError,
underlying_error,
let config_parse_error = match toml::Table::from_str(value) {
Ok(table) => match table.try_into::<Options>() {
Ok(option) => {
if option.extend.is_none() {
return Ok(SingleConfigArgument::SettingsOverride(Arc::new(option)));
}
InvalidConfigFlagReason::ExtendPassedViaConfigFlag
}
Err(underlying_error) => {
InvalidConfigFlagReason::ValidTomlButInvalidRuffSchema(underlying_error)
}
},
Err(underlying_error) => InvalidConfigFlagReason::InvalidToml(underlying_error),
};

let mut new_error = clap::Error::new(clap::error::ErrorKind::ValueValidation).with_cmd(cmd);
Expand All @@ -853,6 +850,21 @@ impl TypedValueParser for ConfigArgumentParser {
clap::error::ContextValue::String(value.to_string()),
);

let underlying_error = match &config_parse_error {
InvalidConfigFlagReason::ExtendPassedViaConfigFlag => {
let tip = config_parse_error.description().into();
new_error.insert(
clap::error::ContextKind::Suggested,
clap::error::ContextValue::StyledStrs(vec![tip]),
);
return Err(new_error);
}
InvalidConfigFlagReason::InvalidToml(underlying_error)
| InvalidConfigFlagReason::ValidTomlButInvalidRuffSchema(underlying_error) => {
underlying_error
}
};

// small hack so that multiline tips
// have the same indent on the left-hand side:
let tip_indent = " ".repeat(" tip: ".len());
Expand Down Expand Up @@ -881,12 +893,16 @@ The path `{value}` does not exist"
));
}
} else if value.contains('=') {
tip.push_str(&format!("\n\n{toml_parse_error}"));
tip.push_str(&format!(
"\n\n{}:\n\n{underlying_error}",
config_parse_error.description()
));
}
let tip = tip.trim_end().to_owned().into();

new_error.insert(
clap::error::ContextKind::Suggested,
clap::error::ContextValue::StyledStrs(vec![tip.into()]),
clap::error::ContextValue::StyledStrs(vec![tip]),
);

Err(new_error)
Expand Down
18 changes: 18 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,24 @@ fn too_many_config_files() -> Result<()> {
Ok(())
}

#[test]
fn extend_passed_via_config_argument() {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args(["--config", "extend = 'foo.toml'", "."]), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: invalid value 'extend = 'foo.toml'' for '--config <CONFIG_OPTION>'
tip: Cannot include `extend` in a --config flag value
For more information, try '--help'.
"###);
}

#[test]
fn config_file_and_isolated() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down

0 comments on commit fc2e637

Please sign in to comment.