From cce63a872dc4853561b9bac55d92ad136f3b924f Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 30 Jan 2024 16:04:05 -0600 Subject: [PATCH 1/5] Fix bug where selection included deprecated rules during preview # Conflicts: # crates/ruff/tests/integration_test.rs # crates/ruff_linter/src/rule_selector.rs --- crates/ruff_linter/src/rule_selector.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index 4dec3ebc0e927..c21c89ed6717a 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -172,7 +172,7 @@ impl Visitor<'_> for SelectorVisitor { } impl RuleSelector { - /// Return all matching rules, regardless of whether they're in preview. + /// Return all matching rules, regardless of rule group filters like preview and deprecated. pub fn all_rules(&self) -> impl Iterator + '_ { match self { RuleSelector::All => RuleSelectorIter::All(Rule::iter()), @@ -198,7 +198,7 @@ impl RuleSelector { } } - /// Returns rules matching the selector, taking into account preview options enabled. + /// Returns rules matching the selector, taking into account rule groups like preview and deprecated. 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; @@ -210,7 +210,7 @@ impl RuleSelector { || ((matches!(self, RuleSelector::Rule { .. }) || matches!(self, RuleSelector::Nursery { .. })) && rule.is_nursery()) // Enabling preview includes all preview or nursery rules unless explicit selection // is turned on - || (preview_enabled && (matches!(self, RuleSelector::Rule { .. }) || !preview_require_explicit)) + || ((rule.is_preview() || rule.is_nursery()) && preview_enabled && (self.is_exact() || !preview_require_explicit)) // Deprecated rules are excluded in preview mode unless explicitly selected || (rule.is_deprecated() && (!preview_enabled || matches!(self, RuleSelector::Rule { .. }))) // Removed rules are included if explicitly selected but will error downstream From 7749abcc6115c7d8d21fb43914fc12b0185d38ec Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 30 Jan 2024 15:33:55 -0600 Subject: [PATCH 2/5] Refactor # Conflicts: # crates/ruff/tests/integration_test.rs # crates/ruff_linter/src/rule_selector.rs # crates/ruff_workspace/src/configuration.rs --- crates/ruff_linter/src/rule_selector.rs | 11 ++++++++--- crates/ruff_workspace/src/configuration.rs | 16 +++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index c21c89ed6717a..b7e0745d903bc 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -207,16 +207,21 @@ impl RuleSelector { // Always include stable rules rule.is_stable() // Backwards compatibility allows selection of nursery rules by exact code or dedicated group - || ((matches!(self, RuleSelector::Rule { .. }) || matches!(self, RuleSelector::Nursery { .. })) && rule.is_nursery()) + || ((self.is_exact() || matches!(self, RuleSelector::Nursery { .. })) && rule.is_nursery()) // Enabling preview includes all preview or nursery rules unless explicit selection // is turned on || ((rule.is_preview() || rule.is_nursery()) && preview_enabled && (self.is_exact() || !preview_require_explicit)) // Deprecated rules are excluded in preview mode unless explicitly selected - || (rule.is_deprecated() && (!preview_enabled || matches!(self, RuleSelector::Rule { .. }))) + || (rule.is_deprecated() && (!preview_enabled || self.is_exact())) // Removed rules are included if explicitly selected but will error downstream - || (rule.is_removed() && matches!(self, RuleSelector::Rule { .. })) + || (rule.is_removed() && self.is_exact()) }) } + + /// Returns true if this selector is exact i.e. selects a single rule by code + pub fn is_exact(&self) -> bool { + matches!(self, Self::Rule { .. }) + } } pub enum RuleSelectorIter { diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 47079922eef43..74c661699d118 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -902,8 +902,8 @@ impl LintConfiguration { // Unstable rules if preview.mode.is_disabled() && kind.is_enable() { - if let RuleSelector::Rule { prefix, .. } = selector { - if prefix.rules().any(|rule| rule.is_nursery()) { + if selector.is_exact() { + if selector.all_rules().all(|rule| rule.is_nursery()) { deprecated_nursery_selectors.insert(selector); } } @@ -915,17 +915,15 @@ impl LintConfiguration { } // Deprecated rules - if kind.is_enable() { - if let RuleSelector::Rule { prefix, .. } = selector { - if prefix.rules().any(|rule| rule.is_deprecated()) { - deprecated_selectors.insert(selector); - } + if kind.is_enable() && selector.is_exact() { + if selector.all_rules().all(|rule| rule.is_deprecated()) { + deprecated_selectors.insert(selector.clone()); } } // Removed rules - if let RuleSelector::Rule { prefix, .. } = selector { - if prefix.rules().any(|rule| rule.is_removed()) { + if selector.is_exact() { + if selector.all_rules().all(|rule| rule.is_removed()) { removed_selectors.insert(selector); } } From 4e02152f2387290c13b0eadbb02901dc1f466294 Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 30 Jan 2024 15:36:18 -0600 Subject: [PATCH 3/5] Change phrasing # Conflicts: # crates/ruff/tests/integration_test.rs # crates/ruff_workspace/src/configuration.rs --- crates/ruff/tests/integration_test.rs | 8 ++++---- crates/ruff_workspace/src/configuration.rs | 8 +++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 1b541ae330b83..f045dc968aa5b 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -1006,7 +1006,7 @@ fn preview_disabled_direct() { ----- stdout ----- ----- stderr ----- - warning: Selection `FURB145` has no effect because the `--preview` flag was not included. + warning: Selection `FURB145` has no effect because preview is not enabled. "###); } @@ -1021,7 +1021,7 @@ fn preview_disabled_prefix_empty() { ----- stdout ----- ----- stderr ----- - warning: Selection `CPY` has no effect because the `--preview` flag was not included. + warning: Selection `CPY` has no effect because preview is not enabled. "###); } @@ -1224,7 +1224,7 @@ def reciprocal(n): ----- stderr ----- ruff failed - Cause: Selection of deprecated rule `TRY200` is not allowed when preview mode is enabled. + Cause: Selection of deprecated rule `TRY200` is not allowed when preview is enabled. "###); } @@ -1271,7 +1271,7 @@ def reciprocal(n): ----- stderr ----- ruff failed - Cause: Selection of deprecated rules is not allowed when preview mode is enabled. Remove selection of: + Cause: Selection of deprecated rules is not allowed when preview is enabled. Remove selection of: - ANN102 - ANN101 diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 74c661699d118..1138e80ba4bcf 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -1005,10 +1005,10 @@ impl LintConfiguration { [] => (), [selection] => { let (prefix, code) = selection.prefix_and_code(); - return Err(anyhow!("Selection of deprecated rule `{prefix}{code}` is not allowed when preview mode is enabled.")); + return Err(anyhow!("Selection of deprecated rule `{prefix}{code}` is not allowed when preview is enabled.")); } [..] => { - let mut message = "Selection of deprecated rules is not allowed when preview mode is enabled. Remove selection of:".to_string(); + let mut message = "Selection of deprecated rules is not allowed when preview is enabled. Remove selection of:".to_string(); for selection in deprecated_selectors { let (prefix, code) = selection.prefix_and_code(); message.push_str("\n\t- "); @@ -1023,9 +1023,7 @@ impl LintConfiguration { for selection in ignored_preview_selectors { let (prefix, code) = selection.prefix_and_code(); - warn_user!( - "Selection `{prefix}{code}` has no effect because the `--preview` flag was not included.", - ); + warn_user!("Selection `{prefix}{code}` has no effect because preview is not enabled.",); } let mut rules = RuleTable::empty(); From b0a4b87667155e9ed050f8a258b0f818b66e4dd6 Mon Sep 17 00:00:00 2001 From: Zanie Date: Wed, 31 Jan 2024 11:41:24 -0600 Subject: [PATCH 4/5] Fix TRY test cases --- crates/ruff/tests/integration_test.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index f045dc968aa5b..81af0a2ca8814 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -1139,11 +1139,13 @@ def reciprocal(n): try: return 1 / n except ZeroDivisionError: - raise ValueError + raise ValueError() "###), @r###" - success: true - exit_code: 0 + success: false + exit_code: 1 ----- stdout ----- + -:6:9: TRY200 Use `raise from` to specify exception cause + Found 1 error. ----- stderr ----- warning: Rule `TRY200` is deprecated and will be removed in a future release. @@ -1216,7 +1218,7 @@ def reciprocal(n): try: return 1 / n except ZeroDivisionError: - raise ValueError + raise ValueError() "###), @r###" success: false exit_code: 2 @@ -1240,7 +1242,7 @@ def reciprocal(n): try: return 1 / n except ZeroDivisionError: - raise ValueError + raise ValueError() "###), @r###" success: true exit_code: 0 @@ -1263,7 +1265,7 @@ def reciprocal(n): try: return 1 / n except ZeroDivisionError: - raise ValueError + raise ValueError() "###), @r###" success: false exit_code: 2 From cdba7ea469a5c64c8a91705f781943e0ac19b057 Mon Sep 17 00:00:00 2001 From: Zanie Date: Wed, 31 Jan 2024 11:42:16 -0600 Subject: [PATCH 5/5] Fix `TRY200` example in docs --- .../src/rules/tryceratops/rules/reraise_no_cause.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/reraise_no_cause.rs b/crates/ruff_linter/src/rules/tryceratops/rules/reraise_no_cause.rs index 866894d1d4695..19317109888a5 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/reraise_no_cause.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/reraise_no_cause.rs @@ -25,7 +25,7 @@ use crate::checkers::ast::Checker; /// try: /// return 1 / n /// except ZeroDivisionError: -/// raise ValueError +/// raise ValueError() /// ``` /// /// Use instead: @@ -34,7 +34,7 @@ use crate::checkers::ast::Checker; /// try: /// return 1 / n /// except ZeroDivisionError as exc: -/// raise ValueError from exc +/// raise ValueError() from exc /// ``` /// /// ## References