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

Implement --extend-fixable option #4297

Merged
merged 1 commit into from
May 19, 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
9 changes: 8 additions & 1 deletion crates/ruff/src/settings/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct RuleSelection {
pub extend_select: Vec<RuleSelector>,
pub fixable: Option<Vec<RuleSelector>>,
pub unfixable: Vec<RuleSelector>,
pub extend_fixable: Vec<RuleSelector>,
}

#[derive(Debug, Default)]
Expand Down Expand Up @@ -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,
Expand Down
51 changes: 41 additions & 10 deletions crates/ruff/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<Rule, bool> = FxHashMap::default();
let mut fixable_map_updates: FxHashMap<Rule, bool> = 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()
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions crates/ruff/src/settings/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<RuleSelector>>,
#[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<Vec<RuleSelector>>,
/// 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<Vec<RuleSelector>>,
#[option(
default = "[]",
value_type = "list[str]",
Expand Down
34 changes: 33 additions & 1 deletion crates/ruff_cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,27 @@ pub struct CheckArgs {
hide_possible_values = true
)]
pub unfixable: Option<Vec<RuleSelector>>,
/// 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<Vec<RuleSelector>>,
/// 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<Vec<RuleSelector>>,
/// Respect file exclusions via `.gitignore` and other standard ignore
/// files.
#[arg(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -449,8 +472,10 @@ pub struct Overrides {
pub dummy_variable_rgx: Option<Regex>,
pub exclude: Option<Vec<FilePattern>>,
pub extend_exclude: Option<Vec<FilePattern>>,
pub extend_fixable: Option<Vec<RuleSelector>>,
pub extend_ignore: Option<Vec<RuleSelector>>,
pub extend_select: Option<Vec<RuleSelector>>,
pub extend_unfixable: Option<Vec<RuleSelector>>,
pub fixable: Option<Vec<RuleSelector>>,
pub ignore: Option<Vec<RuleSelector>>,
pub line_length: Option<usize>,
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ pub fn defaultSettings() -> Result<JsValue, JsValue> {
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),
Expand Down
2 changes: 2 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <RULE_CODE>
List of rule codes to treat as ineligible for autofix. Only applicable when autofix itself is enabled (e.g., via `--fix`)
--extend-fixable <RULE_CODE>
Like --fixable, but adds additional rule codes on top of the fixable ones

File selection:
--exclude <FILE_PATTERN> List of paths, used to omit files and/or directories from analysis
Expand Down
10 changes: 10 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.