diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 05937829d1..69b1c5d9d3 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -1228,8 +1228,50 @@ x = eval(input("Enter a number: ")) fn deprecated_indirect_redirect() { // e.g. `PGH001` is deprecated and redirected, we see a violation in stable // without any warnings + let mut cmd = RuffCheck::default().args(["--select", "PGH"]).build(); + assert_cmd_snapshot!(cmd + .pass_stdin(r###" +x = eval(input("Enter a number: ")) +"###), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:2:5: S307 Use of possibly insecure function; consider using `ast.literal_eval` + Found 1 error. + + ----- stderr ----- + warning: Selection `PGH` includes deprecated rule `PGH001` and will be removed in a future release; use `S307` instead. + warning: Selection `PGH` includes deprecated rule `PGH002` and will be removed in a future release; use `G010` instead. + "###); +} + +#[test] +fn deprecated_indirect_redirect_select_new_code() { + // e.g. `PGH001` is deprecated and redirected + // but the user has moved to the new code so we should not see a warning let mut cmd = RuffCheck::default() - .args(["--select", "PGH", "--preview"]) + .args(["--select", "PGH", "--select", "S307"]) + .build(); + assert_cmd_snapshot!(cmd + .pass_stdin(r###" +x = eval(input("Enter a number: ")) +"###), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:2:5: S307 Use of possibly insecure function; consider using `ast.literal_eval` + Found 1 error. + + ----- stderr ----- + warning: Selection `PGH` includes deprecated rule `PGH001` and will be removed in a future release; use `S307` instead. + warning: Selection `PGH` includes deprecated rule `PGH002` and will be removed in a future release; use `G010` instead. + "###); +} + +#[test] +fn deprecated_indirect_redirect_ignore_new_code() { + let mut cmd = RuffCheck::default() + .args(["--select", "PGH", "--ignore", "S307"]) .build(); assert_cmd_snapshot!(cmd .pass_stdin(r###" @@ -1240,8 +1282,30 @@ x = eval(input("Enter a number: ")) ----- stdout ----- ----- stderr ----- + warning: Selection `PGH` includes deprecated rule `PGH001` and will be removed in a future release; use `S307` instead. + warning: Selection `PGH` includes deprecated rule `PGH002` and will be removed in a future release; use `G010` instead. "###); } + +#[test] +fn deprecated_indirect_redirect_ignore_old_code() { + let mut cmd = RuffCheck::default() + .args(["--select", "PGH", "--ignore", "PGH001"]) + .build(); + assert_cmd_snapshot!(cmd + .pass_stdin(r###" +x = eval(input("Enter a number: ")) +"###), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: Selection `PGH` includes deprecated rule `PGH001` and will be removed in a future release; use `S307` instead. + warning: Selection `PGH` includes deprecated rule `PGH002` and will be removed in a future release; use `G010` instead. + "###); +} + #[test] fn deprecated_direct_preview_enabled() { // Direct selection of a deprecated rule in preview should fail diff --git a/crates/ruff_linter/src/lib.rs b/crates/ruff_linter/src/lib.rs index 857c7fadae..518b8d9c86 100644 --- a/crates/ruff_linter/src/lib.rs +++ b/crates/ruff_linter/src/lib.rs @@ -34,7 +34,7 @@ pub mod packaging; pub mod pyproject_toml; pub mod registry; mod renamer; -mod rule_redirects; +pub mod rule_redirects; pub mod rule_selector; pub mod rules; pub mod settings; diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 21499e9608..b523506a85 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -31,7 +31,7 @@ pub enum FromCodeError { Unknown, } -#[derive(EnumIter, Debug, PartialEq, Eq, Clone, Hash, RuleNamespace)] +#[derive(EnumIter, Debug, PartialEq, Eq, Clone, Hash, Copy, RuleNamespace)] pub enum Linter { /// [Pyflakes](https://pypi.org/project/pyflakes/) #[prefix = "F"] diff --git a/crates/ruff_linter/src/rule_redirects.rs b/crates/ruff_linter/src/rule_redirects.rs index 7e74bdafd8..793d8a78a3 100644 --- a/crates/ruff_linter/src/rule_redirects.rs +++ b/crates/ruff_linter/src/rule_redirects.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use once_cell::sync::Lazy; /// Returns the redirect target for the given code. -pub(crate) fn get_redirect_target(code: &str) -> Option<&'static str> { +pub fn get_redirect_target(code: &str) -> Option<&'static str> { REDIRECTS.get(code).copied() } @@ -21,9 +21,6 @@ static REDIRECTS: Lazy> = Lazy::new(|| { ("C9", "C90"), ("T1", "T10"), ("T2", "T20"), - // The PGH category is deprecated - ("PGH001", "S307"), - ("PGH002", "G010"), // TODO(charlie): Remove by 2023-02-01. ("R", "RET"), ("R5", "RET5"), diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index 36edde5c63..7deabb21eb 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -8,7 +8,7 @@ use strum_macros::EnumIter; use crate::codes::RuleCodePrefix; use crate::codes::RuleIter; use crate::registry::{Linter, Rule, RuleNamespace}; -use crate::rule_redirects::get_redirect; +use crate::rule_redirects::{get_redirect, get_redirect_target}; use crate::settings::types::PreviewMode; #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 95e4adf5ca..9b71608603 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -22,6 +22,7 @@ use ruff_formatter::IndentStyle; use ruff_linter::line_width::{IndentWidth, LineLength}; use ruff_linter::registry::RuleNamespace; use ruff_linter::registry::{Rule, RuleSet, INCOMPATIBLE_CODES}; +use ruff_linter::rule_redirects::get_redirect_target; use ruff_linter::rule_selector::{PreviewOptions, Specificity}; use ruff_linter::rules::pycodestyle; use ruff_linter::settings::rule_table::RuleTable; @@ -756,7 +757,7 @@ impl LintConfiguration { let mut redirects = FxHashMap::default(); let mut deprecated_nursery_selectors = FxHashSet::default(); let mut deprecated_selectors = FxHashSet::default(); - let mut deprecated_rules = FxHashSet::default(); + let mut deprecated_redirected_rules = FxHashSet::default(); let mut deprecated_redirected_selectors = FxHashSet::default(); let mut removed_selectors = FxHashSet::default(); let mut ignored_preview_selectors = FxHashSet::default(); @@ -790,7 +791,15 @@ impl LintConfiguration { .chain(selection.extend_select.iter()) .filter(|s| s.specificity() == spec) { - for rule in selector.rules(&preview) { + for rule in selector.rules(&preview).map(|rule| { + let code = &rule.noqa_code().to_string(); + if let Some(target) = get_redirect_target(code) { + Rule::from_code(target) + .expect("Redirect targets must be valid rule codes") + } else { + rule + } + }) { select_map_updates.insert(rule, true); if spec == Specificity::Rule { @@ -931,11 +940,16 @@ impl LintConfiguration { .insert((redirected_selector, selector)); } } - } else { + } else if !matches!(selector, RuleSelector::All) { // For non-exact selections note all of the deprecated rules that have been renamed + // Unless it is ALL, in which case this is a bit overzealous for rule in selector.rules(&preview) { if rule.is_deprecated() { - deprecated_rules.insert((rule, selector)); + if let Some(target) = + get_redirect_target(&rule.noqa_code().to_string()) + { + deprecated_redirected_rules.insert((rule, target, selector)); + } } } } @@ -1070,13 +1084,30 @@ impl LintConfiguration { } } - for (rule, selection) in deprecated_rules { - let (prefix, code) = selection.prefix_and_code(); + for (rule, redirected_to, from_selection) in deprecated_redirected_rules + .iter() + .sorted_by_key(|(rule, ..)| rule) + { + let (prefix, code) = from_selection.prefix_and_code(); let rule_code = rule.noqa_code(); - // TODO(zanieb): Determine if we should warn here - // warn_user!( - // "Selection `{prefix}{code}` includes deprecated rule `{rule_code}` which will be removed in a future release.", - // ); + let redirected_to_rule = Rule::from_code(redirected_to) + .expect("Redirected rules should always be valid codes"); + + // if !(select_set.contains(*rule) || select_set.contains(redirected_to_rule)) { + // continue; + // } + + // If the user has selected the new rule as well, do not warn + // Due to limitations in our redirection infrastructure, we also must remove the deprecated rule from + // the selection or the user will see a violation for both the new and old rule. + // if select_set.contains(redirected_to_rule) { + // select_set.remove(*rule); + // continue; + // } + + warn_user!( + "Selection `{prefix}{code}` includes deprecated rule `{rule_code}` and will be removed in a future release; use `{redirected_to}` instead.", + ); } for selection in ignored_preview_selectors {