WIP: Nightmare scenario warning on redirected deprecated rules

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

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;

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"]

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<HashMap<&'static str, &'static str>> = 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"),

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)]

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 {
+        for (rule, redirected_to, from_selection) in deprecated_redirected_rules
-            let (prefix, code) = selection.prefix_and_code();
+            .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
+            let redirected_to_rule = Rule::from_code(redirected_to)
-            // warn_user!(
+                .expect("Redirected rules should always be valid codes");
-            //     "Selection `{prefix}{code}` includes deprecated rule `{rule_code}` which will be removed in a future release.",
+
-            // );
+            // 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 {