From e5f1bce4b823cc24718287a3707b29d73982ac3c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 8 May 2023 21:14:08 -0400 Subject: [PATCH] Implement --extend-fixable --- crates/ruff/src/settings/configuration.rs | 9 +++- crates/ruff/src/settings/mod.rs | 51 ++++++++++++++++++----- crates/ruff/src/settings/options.rs | 18 ++++++++ crates/ruff_cli/src/args.rs | 34 ++++++++++++++- crates/ruff_wasm/src/lib.rs | 2 + docs/configuration.md | 2 + ruff.schema.json | 10 +++++ 7 files changed, 114 insertions(+), 12 deletions(-) diff --git a/crates/ruff/src/settings/configuration.rs b/crates/ruff/src/settings/configuration.rs index 59a611746eed4..957436f0a0639 100644 --- a/crates/ruff/src/settings/configuration.rs +++ b/crates/ruff/src/settings/configuration.rs @@ -33,6 +33,7 @@ pub struct RuleSelection { pub extend_select: Vec, pub fixable: Option>, pub unfixable: Vec, + pub extend_fixable: Vec, } #[derive(Debug, Default)] @@ -104,7 +105,13 @@ impl Configuration { .collect(), extend_select: options.extend_select.unwrap_or_default(), fixable: options.fixable, - unfixable: options.unfixable.unwrap_or_default(), + unfixable: options + .unfixable + .into_iter() + .flatten() + .chain(options.extend_unfixable.into_iter().flatten()) + .collect(), + extend_fixable: options.extend_fixable.unwrap_or_default(), }], allowed_confusables: options.allowed_confusables, builtins: options.builtins, diff --git a/crates/ruff/src/settings/mod.rs b/crates/ruff/src/settings/mod.rs index 4f83bb43491d3..4da221a3d803d 100644 --- a/crates/ruff/src/settings/mod.rs +++ b/crates/ruff/src/settings/mod.rs @@ -264,16 +264,11 @@ impl From<&Configuration> for RuleTable { // across config files (which otherwise wouldn't be possible since ruff // only has `extended` but no `extended-by`). let mut carryover_ignores: Option<&[RuleSelector]> = None; + let mut carryover_unfixables: Option<&[RuleSelector]> = None; let mut redirects = FxHashMap::default(); for selection in &config.rule_selections { - // We do not have an extend-fixable option, so fixable and unfixable - // selectors can simply be applied directly to fixable_set. - if selection.fixable.is_some() { - fixable_set.clear(); - } - // If a selection only specifies extend-select we cannot directly // apply its rule selectors to the select_set because we firstly have // to resolve the effectively selected rules within the current rule selection @@ -282,10 +277,13 @@ impl From<&Configuration> for RuleTable { // We do this via the following HashMap where the bool indicates // whether to enable or disable the given rule. let mut select_map_updates: FxHashMap = FxHashMap::default(); + let mut fixable_map_updates: FxHashMap = FxHashMap::default(); let carriedover_ignores = carryover_ignores.take(); + let carriedover_unfixables = carryover_unfixables.take(); for spec in Specificity::iter() { + // Iterate over rule selectors in order of specificity. for selector in selection .select .iter() @@ -307,17 +305,26 @@ impl From<&Configuration> for RuleTable { select_map_updates.insert(rule, false); } } - if let Some(fixable) = &selection.fixable { - fixable_set - .extend(fixable.iter().filter(|s| s.specificity() == spec).flatten()); + // Apply the same logic to `fixable` and `unfixable`. + for selector in selection + .fixable + .iter() + .flatten() + .chain(selection.extend_fixable.iter()) + .filter(|s| s.specificity() == spec) + { + for rule in selector { + fixable_map_updates.insert(rule, true); + } } for selector in selection .unfixable .iter() + .chain(carriedover_unfixables.into_iter().flatten()) .filter(|s| s.specificity() == spec) { for rule in selector { - fixable_set.remove(rule); + fixable_map_updates.insert(rule, false); } } } @@ -347,6 +354,29 @@ impl From<&Configuration> for RuleTable { } } + // Apply the same logic to `fixable` and `unfixable`. + if let Some(fixable) = &selection.fixable { + fixable_set = fixable_map_updates + .into_iter() + .filter_map(|(rule, enabled)| enabled.then_some(rule)) + .collect(); + + if fixable.is_empty() + && selection.extend_fixable.is_empty() + && !selection.unfixable.is_empty() + { + carryover_unfixables = Some(&selection.unfixable); + } + } else { + for (rule, enabled) in fixable_map_updates { + if enabled { + fixable_set.insert(rule); + } else { + fixable_set.remove(rule); + } + } + } + // We insert redirects into the hashmap so that we // can warn the users about remapped rule codes. for selector in selection @@ -357,6 +387,7 @@ impl From<&Configuration> for RuleTable { .chain(selection.ignore.iter()) .chain(selection.extend_select.iter()) .chain(selection.unfixable.iter()) + .chain(selection.extend_fixable.iter()) { if let RuleSelector::Prefix { prefix, diff --git a/crates/ruff/src/settings/options.rs b/crates/ruff/src/settings/options.rs index b9248c6eeb7e8..1533aca091d22 100644 --- a/crates/ruff/src/settings/options.rs +++ b/crates/ruff/src/settings/options.rs @@ -177,6 +177,24 @@ pub struct Options { /// A list of rule codes or prefixes to enable, in addition to those /// specified by `select`. pub extend_select: Option>, + #[option( + default = r#"[]"#, + value_type = "list[RuleSelector]", + example = r#" + # Enable autofix for flake8-bugbear (`B`), on top of any rules specified by `fixable`. + extend-fixable = ["B"] + "# + )] + /// A list of rule codes or prefixes to consider autofixable, in addition to those + /// specified by `fixable`. + pub extend_fixable: Option>, + /// A list of rule codes or prefixes to consider non-auto-fixable, in addition to those + /// specified by `unfixable`. + /// + /// This option has been **deprecated** in favor of `unfixable` since its usage is now + /// interchangeable with `unfixable`. + #[schemars(skip)] + pub extend_unfixable: Option>, #[option( default = "[]", value_type = "list[str]", diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs index 6236f4a2e49c9..22841c761dbe2 100644 --- a/crates/ruff_cli/src/args.rs +++ b/crates/ruff_cli/src/args.rs @@ -189,6 +189,27 @@ pub struct CheckArgs { hide_possible_values = true )] pub unfixable: Option>, + /// Like --fixable, but adds additional rule codes on top of the fixable + /// ones. + #[arg( + long, + value_delimiter = ',', + value_name = "RULE_CODE", + value_parser = parse_rule_selector, + help_heading = "Rule selection", + hide_possible_values = true + )] + pub extend_fixable: Option>, + /// Like --unfixable. (Deprecated: You can just use --unfixable instead.) + #[arg( + long, + value_delimiter = ',', + value_name = "RULE_CODE", + value_parser = parse_rule_selector, + help_heading = "Rule selection", + hide = true + )] + pub extend_unfixable: Option>, /// Respect file exclusions via `.gitignore` and other standard ignore /// files. #[arg( @@ -381,8 +402,10 @@ impl CheckArgs { dummy_variable_rgx: self.dummy_variable_rgx, exclude: self.exclude, extend_exclude: self.extend_exclude, + extend_fixable: self.extend_fixable, extend_ignore: self.extend_ignore, extend_select: self.extend_select, + extend_unfixable: self.extend_unfixable, fixable: self.fixable, ignore: self.ignore, line_length: self.line_length, @@ -449,8 +472,10 @@ pub struct Overrides { pub dummy_variable_rgx: Option, pub exclude: Option>, pub extend_exclude: Option>, + pub extend_fixable: Option>, pub extend_ignore: Option>, pub extend_select: Option>, + pub extend_unfixable: Option>, pub fixable: Option>, pub ignore: Option>, pub line_length: Option, @@ -501,7 +526,14 @@ impl ConfigProcessor for &Overrides { .collect(), extend_select: self.extend_select.clone().unwrap_or_default(), fixable: self.fixable.clone(), - unfixable: self.unfixable.clone().unwrap_or_default(), + unfixable: self + .unfixable + .iter() + .cloned() + .chain(self.extend_unfixable.iter().cloned()) + .flatten() + .collect(), + extend_fixable: self.extend_fixable.clone().unwrap_or_default(), }); if let Some(format) = &self.format { config.format = Some(*format); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index a2b54e1a8627c..741f4ebbfb807 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -97,8 +97,10 @@ pub fn defaultSettings() -> Result { allowed_confusables: Some(Vec::default()), builtins: Some(Vec::default()), dummy_variable_rgx: Some(defaults::DUMMY_VARIABLE_RGX.as_str().to_string()), + extend_fixable: Some(Vec::default()), extend_ignore: Some(Vec::default()), extend_select: Some(Vec::default()), + extend_unfixable: Some(Vec::default()), external: Some(Vec::default()), ignore: Some(Vec::default()), line_length: Some(defaults::LINE_LENGTH), diff --git a/docs/configuration.md b/docs/configuration.md index 545faaecd06c8..f6f0d059d6dc7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -239,6 +239,8 @@ Rule selection: List of rule codes to treat as eligible for autofix. Only applicable when autofix itself is enabled (e.g., via `--fix`) --unfixable List of rule codes to treat as ineligible for autofix. Only applicable when autofix itself is enabled (e.g., via `--fix`) + --extend-fixable + Like --fixable, but adds additional rule codes on top of the fixable ones File selection: --exclude List of paths, used to omit files and/or directories from analysis diff --git a/ruff.schema.json b/ruff.schema.json index b78d6161fb6a1..40f22f1b6c961 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -66,6 +66,16 @@ "type": "string" } }, + "extend-fixable": { + "description": "A list of rule codes or prefixes to consider autofixable, in addition to those specified by `fixable`.", + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/RuleSelector" + } + }, "extend-include": { "description": "A list of file patterns to include when linting, in addition to those specified by `include`.\n\nInclusion are based on globs, and should be single-path patterns, like `*.pyw`, to include any file with the `.pyw` extension.\n\nFor more information on the glob syntax, refer to the [`globset` documentation](https://docs.rs/globset/latest/globset/#syntax).", "type": [