Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for redirected rules #9754

Merged
merged 3 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 59 additions & 6 deletions crates/ruff/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 -----
Expand All @@ -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 -----
Expand Down Expand Up @@ -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 -----
Expand All @@ -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 -----
Expand Down Expand Up @@ -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 -----
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 -----
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/rule_redirects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,11 @@ static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = 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"),
])
});
111 changes: 111 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/test_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Diagnostic> {
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<Diagnostic> {
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<Diagnostic> {
Some(Diagnostic::new(
RedirectedFromPrefixTestRule,
ruff_text_size::TextRange::default(),
))
}
}
4 changes: 4 additions & 0 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down