From f04b7b07c8139f2e48c4994532035ae0aeee92f4 Mon Sep 17 00:00:00 2001 From: Zanie Date: Wed, 27 Sep 2023 09:53:13 -0500 Subject: [PATCH] Add `explicit-preview-rules` to toggle explicit selection of preview rules --- crates/flake8_to_ruff/src/plugin.rs | 4 +- crates/ruff_linter/src/rule_selector.rs | 19 +++- crates/ruff_linter/src/settings/mod.rs | 5 +- crates/ruff_workspace/src/configuration.rs | 117 ++++++++++++++++----- crates/ruff_workspace/src/options.rs | 13 +++ docs/preview.md | 25 ++++- ruff.schema.json | 14 +++ 7 files changed, 159 insertions(+), 38 deletions(-) diff --git a/crates/flake8_to_ruff/src/plugin.rs b/crates/flake8_to_ruff/src/plugin.rs index 37c8795f57989..5b2fc585dc850 100644 --- a/crates/flake8_to_ruff/src/plugin.rs +++ b/crates/flake8_to_ruff/src/plugin.rs @@ -4,7 +4,7 @@ use std::str::FromStr; use anyhow::anyhow; use ruff_linter::registry::Linter; -use ruff_linter::settings::types::PreviewMode; +use ruff_linter::rule_selector::PreviewOptions; use ruff_linter::RuleSelector; #[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] @@ -332,7 +332,7 @@ pub(crate) fn infer_plugins_from_codes(selectors: &HashSet) -> Vec .filter(|plugin| { for selector in selectors { if selector - .rules(PreviewMode::Disabled) + .rules(&PreviewOptions::default()) .any(|rule| Linter::from(plugin).rules().any(|r| r == rule)) { return true; diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index 910cac9cb1fa2..ea83c058ed014 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -198,16 +198,19 @@ impl RuleSelector { } } - /// Returns rules matching the selector, taking into account whether preview mode is enabled. - pub fn rules(&self, preview: PreviewMode) -> impl Iterator + '_ { + /// Returns rules matching the selector, taking into account preview options enabled. + pub fn rules<'a>(&'a self, preview: &PreviewOptions) -> impl Iterator + 'a { + let preview_enabled = preview.mode.is_enabled(); + let preview_require_explicit = preview.require_explicit; #[allow(deprecated)] self.all_rules().filter(move |rule| { // Always include rules that are not in preview or the nursery !(rule.is_preview() || rule.is_nursery()) // Backwards compatibility allows selection of nursery rules by exact code or dedicated group || ((matches!(self, RuleSelector::Rule { .. }) || matches!(self, RuleSelector::Nursery { .. })) && rule.is_nursery()) - // Enabling preview includes all preview or nursery rules - || preview.is_enabled() + // Enabling preview includes all preview or nursery rules unless explicit selection + // is turned on + || (preview_enabled && (matches!(self, RuleSelector::Rule { .. }) || !preview_require_explicit)) }) } } @@ -232,6 +235,14 @@ impl Iterator for RuleSelectorIter { } } +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct PreviewOptions { + pub mode: PreviewMode, + /// If true, preview rule selection requires explicit codes e.g. not prefixes. + /// Generally this should be derived from the user-facing `explicit-preview-rules` option. + pub require_explicit: bool, +} + #[cfg(feature = "schemars")] mod schema { use itertools::Itertools; diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 6ced3e3be0608..48fdf907f8db0 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -30,6 +30,7 @@ use super::line_width::{LineLength, TabSize}; use self::rule_table::RuleTable; use self::types::PreviewMode; +use crate::rule_selector::PreviewOptions; pub mod flags; pub mod rule_table; @@ -44,6 +45,7 @@ pub struct LinterSettings { pub target_version: PythonVersion, pub preview: PreviewMode, + pub explicit_preview_rules: bool, // Rule-specific settings pub allowed_confusables: FxHashSet, @@ -121,7 +123,7 @@ impl LinterSettings { project_root: project_root.to_path_buf(), rules: PREFIXES .iter() - .flat_map(|selector| selector.rules(PreviewMode::default())) + .flat_map(|selector| selector.rules(&PreviewOptions::default())) .collect(), allowed_confusables: FxHashSet::from_iter([]), @@ -168,6 +170,7 @@ impl LinterSettings { pylint: pylint::settings::Settings::default(), pyupgrade: pyupgrade::settings::Settings::default(), preview: PreviewMode::default(), + explicit_preview_rules: false, } } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 89d941bb8dbec..ad350a3346355 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -20,7 +20,7 @@ use ruff_formatter::{IndentStyle, LineWidth}; use ruff_linter::line_width::{LineLength, TabSize}; use ruff_linter::registry::RuleNamespace; use ruff_linter::registry::{Rule, RuleSet, INCOMPATIBLE_CODES}; -use ruff_linter::rule_selector::Specificity; +use ruff_linter::rule_selector::{PreviewOptions, Specificity}; use ruff_linter::settings::rule_table::RuleTable; use ruff_linter::settings::types::{ FilePattern, FilePatternSet, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat, @@ -180,6 +180,7 @@ impl Configuration { .collect(), )?, src: self.src.unwrap_or_else(|| vec![project_root.to_path_buf()]), + explicit_preview_rules: lint.explicit_preview_rules.unwrap_or_default(), task_tags: lint .task_tags @@ -442,6 +443,7 @@ pub struct LintConfiguration { pub extend_per_file_ignores: Vec, pub per_file_ignores: Option>, pub rule_selections: Vec, + pub explicit_preview_rules: Option, // Global lint settings pub allowed_confusables: Option>, @@ -519,6 +521,7 @@ impl LintConfiguration { .unwrap_or_default(), external: options.external, ignore_init_module_imports: options.ignore_init_module_imports, + explicit_preview_rules: options.explicit_preview_rules, per_file_ignores: options.per_file_ignores.map(|per_file_ignores| { per_file_ignores .into_iter() @@ -559,14 +562,19 @@ impl LintConfiguration { } fn as_rule_table(&self, preview: PreviewMode) -> RuleTable { + let preview = PreviewOptions { + mode: preview, + require_explicit: self.explicit_preview_rules.unwrap_or_default(), + }; + // The select_set keeps track of which rules have been selected. let mut select_set: RuleSet = PREFIXES .iter() - .flat_map(|selector| selector.rules(preview)) + .flat_map(|selector| selector.rules(&preview)) .collect(); // The fixable set keeps track of which rules are fixable. - let mut fixable_set: RuleSet = RuleSelector::All.rules(preview).collect(); + let mut fixable_set: RuleSet = RuleSelector::All.rules(&preview).collect(); // Ignores normally only subtract from the current set of selected // rules. By that logic the ignore in `select = [], ignore = ["E501"]` @@ -605,7 +613,7 @@ impl LintConfiguration { .chain(selection.extend_select.iter()) .filter(|s| s.specificity() == spec) { - for rule in selector.rules(preview) { + for rule in selector.rules(&preview) { select_map_updates.insert(rule, true); } } @@ -615,7 +623,7 @@ impl LintConfiguration { .chain(carriedover_ignores.into_iter().flatten()) .filter(|s| s.specificity() == spec) { - for rule in selector.rules(preview) { + for rule in selector.rules(&preview) { select_map_updates.insert(rule, false); } } @@ -627,7 +635,7 @@ impl LintConfiguration { .chain(selection.extend_fixable.iter()) .filter(|s| s.specificity() == spec) { - for rule in selector.rules(preview) { + for rule in selector.rules(&preview) { fixable_map_updates.insert(rule, true); } } @@ -637,7 +645,7 @@ impl LintConfiguration { .chain(carriedover_unfixables.into_iter().flatten()) .filter(|s| s.specificity() == spec) { - for rule in selector.rules(preview) { + for rule in selector.rules(&preview) { fixable_map_updates.insert(rule, false); } } @@ -704,16 +712,16 @@ impl LintConfiguration { { #[allow(deprecated)] if matches!(selector, RuleSelector::Nursery) { - let suggestion = if preview.is_disabled() { + let suggestion = if preview.mode.is_disabled() { " Use the `--preview` flag instead." } else { // We have no suggested alternative since there is intentionally no "PREVIEW" selector "" }; warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}"); - } + }; - if preview.is_disabled() { + if preview.mode.is_disabled() { if let RuleSelector::Rule { prefix, .. } = selector { if prefix.rules().any(|rule| rule.is_nursery()) { deprecated_nursery_selectors.insert(selector); @@ -721,7 +729,7 @@ impl LintConfiguration { } // Check if the selector is empty because preview mode is disabled - if selector.rules(PreviewMode::Disabled).next().is_none() { + if selector.rules(&PreviewOptions::default()).next().is_none() { ignored_preview_selectors.insert(selector); } } @@ -810,6 +818,9 @@ impl LintConfiguration { .or(config.ignore_init_module_imports), logger_objects: self.logger_objects.or(config.logger_objects), per_file_ignores: self.per_file_ignores.or(config.per_file_ignores), + explicit_preview_rules: self + .explicit_preview_rules + .or(config.explicit_preview_rules), task_tags: self.task_tags.or(config.task_tags), typing_modules: self.typing_modules.or(config.typing_modules), // Plugins @@ -932,13 +943,13 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result> #[cfg(test)] mod tests { + use crate::configuration::{LintConfiguration, RuleSelection}; use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb}; use ruff_linter::registry::{Linter, Rule, RuleSet}; + use ruff_linter::rule_selector::PreviewOptions; use ruff_linter::settings::types::PreviewMode; use ruff_linter::RuleSelector; - use crate::configuration::{LintConfiguration, RuleSelection}; - const NURSERY_RULES: &[Rule] = &[ Rule::MissingCopyrightNotice, Rule::IndentationWithInvalidMultiple, @@ -999,13 +1010,14 @@ mod tests { #[allow(clippy::needless_pass_by_value)] fn resolve_rules( selections: impl IntoIterator, - preview: Option, + preview: Option, ) -> RuleSet { LintConfiguration { rule_selections: selections.into_iter().collect(), + explicit_preview_rules: preview.as_ref().map(|preview| preview.require_explicit), ..LintConfiguration::default() } - .as_rule_table(preview.unwrap_or_default()) + .as_rule_table(preview.map(|preview| preview.mode).unwrap_or_default()) .iter_enabled() // Filter out rule gated behind `#[cfg(feature = "unreachable-code")]`, which is off-by-default .filter(|rule| rule.noqa_code() != "RUF014") @@ -1241,7 +1253,10 @@ mod tests { select: Some(vec![RuleSelector::All]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); @@ -1250,7 +1265,10 @@ mod tests { select: Some(vec![RuleSelector::All]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), ); assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); } @@ -1262,7 +1280,10 @@ mod tests { select: Some(vec![Linter::Flake8Copyright.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1272,7 +1293,10 @@ mod tests { select: Some(vec![Linter::Flake8Copyright.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); @@ -1285,7 +1309,10 @@ mod tests { select: Some(vec![Flake8Copyright::_0.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1295,7 +1322,10 @@ mod tests { select: Some(vec![Flake8Copyright::_0.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); @@ -1303,12 +1333,16 @@ mod tests { #[test] fn select_rule_preview() { + // Test inclusion when toggling preview on and off let actual = resolve_rules( [RuleSelection { select: Some(vec![Refurb::_145.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1318,7 +1352,24 @@ mod tests { select: Some(vec![Refurb::_145.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), + ); + let expected = RuleSet::from_rule(Rule::SliceCopy); + assert_eq!(actual, expected); + + // Test inclusion when preview is on but explicit codes are required + let actual = resolve_rules( + [RuleSelection { + select: Some(vec![Refurb::_145.into()]), + ..RuleSelection::default() + }], + Some(PreviewOptions { + mode: PreviewMode::Enabled, + require_explicit: true, + }), ); let expected = RuleSet::from_rule(Rule::SliceCopy); assert_eq!(actual, expected); @@ -1333,7 +1384,10 @@ mod tests { select: Some(vec![Flake8Copyright::_001.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); @@ -1343,7 +1397,10 @@ mod tests { select: Some(vec![Flake8Copyright::_001.into()]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); @@ -1359,7 +1416,10 @@ mod tests { select: Some(vec![RuleSelector::Nursery]), ..RuleSelection::default() }], - Some(PreviewMode::Disabled), + Some(PreviewOptions { + mode: PreviewMode::Disabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rules(NURSERY_RULES); assert_eq!(actual, expected); @@ -1369,7 +1429,10 @@ mod tests { select: Some(vec![RuleSelector::Nursery]), ..RuleSelection::default() }], - Some(PreviewMode::Enabled), + Some(PreviewOptions { + mode: PreviewMode::Enabled, + ..PreviewOptions::default() + }), ); let expected = RuleSet::from_rules(NURSERY_RULES); assert_eq!(actual, expected); diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index bff96a404d07b..366cf41720fc2 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -570,6 +570,19 @@ pub struct LintOptions { )] pub select: Option>, + /// Whether to require exact codes to select preview rules. When enabled, + /// preview rules will not be selected by prefixes — the full code of each + /// preview rule will be required to enable the rule. + #[option( + default = "false", + value_type = "bool", + example = r#" + # Require explicit selection of preview rules + explicit-preview-rules = true + "# + )] + pub explicit_preview_rules: Option, + /// A list of task tags to recognize (e.g., "TODO", "FIXME", "XXX"). /// /// Comments starting with these tags will be ignored by commented-out code diff --git a/docs/preview.md b/docs/preview.md index 58b892ba10986..d3573a3615afd 100644 --- a/docs/preview.md +++ b/docs/preview.md @@ -47,11 +47,28 @@ Or, if you provided the `--preview` CLI flag. To see which rules are currently in preview, visit the [rules reference](rules.md). +## Selecting single preview rules + +When preview mode is enabled, selecting rule categories or prefixes will include all preview rules that match. +If you would prefer to opt-in to each preview rule individually, you can toggle the `explicit-preview-rules` +setting in your `pyproject.toml`: + +```toml +[tool.ruff] +preview = true +explicit-preview-rules = true +``` + +In our previous example, `--select` with `ALL` `HYP`, `HYP0`, or `HYP00` would not enable `HYP001`. Each preview +rule will need to be selected with its exact code, e.g. `--select ALL,HYP001`. + +If preview mode is not enabled, this setting has no effect. + ## Legacy behavior Before the preview mode was introduced, new rules were added in a "nursery" category that required selection of -rules with their exact code. +rules with their exact codes — similar to if `explicit-preview-rules` is enabled. -The nursery category has been deprecated and all rules in the nursery are now considered to be in preview. For backwards -compatibility, nursery rules are selectable with their exact codes without enabling preview mode but a warning will -be displayed. +The nursery category has been deprecated and all rules in the nursery are now considered to be in preview. +For backwards compatibility, nursery rules are selectable with their exact codes without enabling preview mode. +However, this behavior will display a warning and support will be removed in a future release. diff --git a/ruff.schema.json b/ruff.schema.json index 1b0d1db27eca8..fac8933e475d3 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -50,6 +50,13 @@ "type": "string" } }, + "explicit-preview-rules": { + "description": "Whether to require exact codes to select preview rules. When enabled, preview rules will not be selected by prefixes — the full code of each preview rule will be required to enable the rule.", + "type": [ + "boolean", + "null" + ] + }, "extend": { "description": "A path to a local `pyproject.toml` file to merge into this configuration. User home directory and environment variables will be expanded.\n\nTo resolve the current `pyproject.toml` file, Ruff will first resolve this base configuration file, then merge in any properties defined in the current configuration file.", "type": [ @@ -1569,6 +1576,13 @@ "null" ] }, + "explicit-preview-rules": { + "description": "Whether to require exact codes to select preview rules. When enabled, preview rules will not be selected by prefixes — the full code of each preview rule will be required to enable the rule.", + "type": [ + "boolean", + "null" + ] + }, "extend-fixable": { "description": "A list of rule codes or prefixes to consider autofixable, in addition to those specified by `fixable`.", "type": [