From 003a36071d6714383db7f66c0541f1fe01580d13 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 1 Feb 2024 10:46:36 -0600 Subject: [PATCH] Add tests for redirected rules (#9754) Extends https://github.com/astral-sh/ruff/pull/9752 adding internal test rules for redirection Fixes a bug where we did not see warnings for exact codes that are redirected (just prefixes) --- crates/ruff/tests/integration_test.rs | 65 +++++++++- crates/ruff_linter/src/codes.rs | 6 + crates/ruff_linter/src/linter.rs | 9 ++ crates/ruff_linter/src/rule_redirects.rs | 6 + .../src/rules/ruff/rules/test_rules.rs | 111 ++++++++++++++++++ crates/ruff_workspace/src/configuration.rs | 4 + 6 files changed, 195 insertions(+), 6 deletions(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 864df1f2b8804..109239da4d96a 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -826,7 +826,8 @@ fn nursery_prefix() { -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF920 Hey this is a deprecated test rule. -:1:1: RUF921 Hey this is another deprecated test rule. - Found 6 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- @@ -850,7 +851,8 @@ fn nursery_all() { -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF920 Hey this is a deprecated test rule. -:1:1: RUF921 Hey this is another deprecated test rule. - Found 7 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 8 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- @@ -929,7 +931,8 @@ fn preview_enabled_prefix() { -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF911 Hey this is a preview test rule. -:1:1: RUF912 Hey this is a nursery test rule. - Found 6 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- @@ -953,7 +956,8 @@ fn preview_enabled_all() { -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF911 Hey this is a preview test rule. -:1:1: RUF912 Hey this is a nursery test rule. - Found 8 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 9 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- @@ -1088,7 +1092,8 @@ fn preview_enabled_group_ignore() { -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF911 Hey this is a preview test rule. -:1:1: RUF912 Hey this is a nursery test rule. - Found 6 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- @@ -1145,6 +1150,53 @@ fn removed_indirect() { "###); } +#[test] +fn redirect_direct() { + // Selection of a redirected rule directly should use the new rule and warn + let mut cmd = RuffCheck::default().args(["--select", "RUF940"]).build(); + assert_cmd_snapshot!(cmd, @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 1 error. + + ----- stderr ----- + warning: `RUF940` has been remapped to `RUF950`. + "###); +} + +#[test] +fn redirect_indirect() { + // Selection _including_ a redirected rule without matching should not fail + // nor should the rule be used + let mut cmd = RuffCheck::default().args(["--select", "RUF94"]).build(); + assert_cmd_snapshot!(cmd, @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + "###); +} + +#[test] +fn redirect_prefix() { + // Selection using a redirected prefix should switch to all rules in the + // new prefix + let mut cmd = RuffCheck::default().args(["--select", "RUF96"]).build(); + assert_cmd_snapshot!(cmd, @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 1 error. + + ----- stderr ----- + warning: `RUF96` has been remapped to `RUF95`. + "###); +} + #[test] fn deprecated_direct() { // Selection of a deprecated rule without preview enabled should still work @@ -1770,7 +1822,8 @@ extend-safe-fixes = ["RUF9"] -:1:1: RUF903 Hey this is a stable test rule with a display only fix. -:1:1: RUF920 Hey this is a deprecated test rule. -:1:1: RUF921 Hey this is another deprecated test rule. - Found 6 errors. + -:1:1: RUF950 Hey this is a test rule that was redirected from another. + Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). ----- stderr ----- diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index e54d7eff4f592..f318e6e7a3ed0 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -959,6 +959,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "930") => (RuleGroup::Removed, rules::ruff::rules::RemovedTestRule), #[cfg(feature = "test-rules")] (Ruff, "931") => (RuleGroup::Removed, rules::ruff::rules::AnotherRemovedTestRule), + #[cfg(feature = "test-rules")] + (Ruff, "940") => (RuleGroup::Removed, rules::ruff::rules::RedirectedFromTestRule), + #[cfg(feature = "test-rules")] + (Ruff, "950") => (RuleGroup::Stable, rules::ruff::rules::RedirectedToTestRule), + #[cfg(feature = "test-rules")] + (Ruff, "960") => (RuleGroup::Removed, rules::ruff::rules::RedirectedFromPrefixTestRule), // flake8-django diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 0ce297b596481..0196aeb933628 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -246,6 +246,15 @@ pub fn check_path( Rule::AnotherRemovedTestRule => { test_rules::AnotherRemovedTestRule::diagnostic(locator, indexer) } + Rule::RedirectedToTestRule => { + test_rules::RedirectedToTestRule::diagnostic(locator, indexer) + } + Rule::RedirectedFromTestRule => { + test_rules::RedirectedFromTestRule::diagnostic(locator, indexer) + } + Rule::RedirectedFromPrefixTestRule => { + test_rules::RedirectedFromPrefixTestRule::diagnostic(locator, indexer) + } _ => unreachable!("All test rules must have an implementation"), }; if let Some(diagnostic) = diagnostic { diff --git a/crates/ruff_linter/src/rule_redirects.rs b/crates/ruff_linter/src/rule_redirects.rs index 12408c56bd2df..87330b9ae3fc2 100644 --- a/crates/ruff_linter/src/rule_redirects.rs +++ b/crates/ruff_linter/src/rule_redirects.rs @@ -100,5 +100,11 @@ static REDIRECTS: Lazy> = Lazy::new(|| { ("T004", "FIX004"), ("RUF011", "B035"), ("TCH006", "TCH010"), + // Test redirect by exact code + #[cfg(feature = "test-rules")] + ("RUF940", "RUF950"), + // Test redirect by prefix + #[cfg(feature = "test-rules")] + ("RUF96", "RUF95"), ]) }); diff --git a/crates/ruff_linter/src/rules/ruff/rules/test_rules.rs b/crates/ruff_linter/src/rules/ruff/rules/test_rules.rs index f846f9ff3aaee..d148dff835d95 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/test_rules.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/test_rules.rs @@ -43,6 +43,9 @@ pub(crate) const TEST_RULES: &[Rule] = &[ Rule::AnotherDeprecatedTestRule, Rule::RemovedTestRule, Rule::AnotherRemovedTestRule, + Rule::RedirectedFromTestRule, + Rule::RedirectedToTestRule, + Rule::RedirectedFromPrefixTestRule, ]; pub(crate) trait TestRule { @@ -438,3 +441,111 @@ impl TestRule for AnotherRemovedTestRule { )) } } + +/// ## What it does +/// Fake rule for testing. +/// +/// ## Why is this bad? +/// Tests must pass! +/// +/// ## Example +/// ```python +/// foo +/// ``` +/// +/// Use instead: +/// ```python +/// bar +/// ``` +#[violation] +pub struct RedirectedFromTestRule; + +impl Violation for RedirectedFromTestRule { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Hey this is a test rule that was redirected to another.") + } +} + +impl TestRule for RedirectedFromTestRule { + fn diagnostic(_locator: &Locator, _indexer: &Indexer) -> Option { + Some(Diagnostic::new( + RedirectedFromTestRule, + ruff_text_size::TextRange::default(), + )) + } +} + +/// ## What it does +/// Fake rule for testing. +/// +/// ## Why is this bad? +/// Tests must pass! +/// +/// ## Example +/// ```python +/// foo +/// ``` +/// +/// Use instead: +/// ```python +/// bar +/// ``` +#[violation] +pub struct RedirectedToTestRule; + +impl Violation for RedirectedToTestRule { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Hey this is a test rule that was redirected from another.") + } +} + +impl TestRule for RedirectedToTestRule { + fn diagnostic(_locator: &Locator, _indexer: &Indexer) -> Option { + Some(Diagnostic::new( + RedirectedToTestRule, + ruff_text_size::TextRange::default(), + )) + } +} + +/// ## What it does +/// Fake rule for testing. +/// +/// ## Why is this bad? +/// Tests must pass! +/// +/// ## Example +/// ```python +/// foo +/// ``` +/// +/// Use instead: +/// ```python +/// bar +/// ``` +#[violation] +pub struct RedirectedFromPrefixTestRule; + +impl Violation for RedirectedFromPrefixTestRule { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Hey this is a test rule that was redirected to another by prefix.") + } +} + +impl TestRule for RedirectedFromPrefixTestRule { + fn diagnostic(_locator: &Locator, _indexer: &Indexer) -> Option { + Some(Diagnostic::new( + RedirectedFromPrefixTestRule, + ruff_text_size::TextRange::default(), + )) + } +} diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 4bb0996f81da3..ea0abf4322fe8 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -940,6 +940,10 @@ impl LintConfiguration { if let RuleSelector::Prefix { prefix, redirected_from: Some(redirect_from), + } + | RuleSelector::Rule { + prefix, + redirected_from: Some(redirect_from), } = selector { redirects.insert(redirect_from, prefix);