Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
203a514
Create rule RUF102 -- Invalid Rule Code -- in preview
maxmynter Apr 2, 2025
437680d
Add tests for RUF102
maxmynter Apr 2, 2025
d0eadbe
Update Schema
maxmynter Apr 2, 2025
223ae54
Integrate RUF102 into ruleset
maxmynter Apr 2, 2025
7a15b89
Apply RUF102 check in noqa handling
maxmynter Apr 2, 2025
1ed6d76
Satisfy Clippy
maxmynter Apr 2, 2025
f7014db
(refactor) Simplify `all_codes_invalid` handling
maxmynter Apr 2, 2025
5cc8305
(tests) Test comment preceeding codes
maxmynter Apr 2, 2025
3f50765
(fixup) Respect external config in invalid rule codes rule
maxmynter Apr 2, 2025
e734d3d
(test) Respect external config in invalid rule codes
maxmynter Apr 2, 2025
c8a17fd
(tests) Add snapshot for External code in ignore invalid codes
maxmynter Apr 2, 2025
2be49a0
(refactor) Use 'split_at' for inline slicing
maxmynter Apr 2, 2025
e3f4e07
(fixup) Use find, not takewhile, to handle multibyte chars
maxmynter Apr 2, 2025
52abe14
Implement two passes and only collect valid codes first pass
maxmynter Apr 2, 2025
d34476f
Move applicable parts RUF102 to general rules test
maxmynter Apr 2, 2025
e044bf9
Remove stale snapshot
maxmynter Apr 2, 2025
22b774b
(fixup) Fix heading level in invalid noqa docs
maxmynter Apr 2, 2025
3050304
Make test name more expressive
maxmynter Apr 3, 2025
2fe49fb
Add external reference to docstring
maxmynter Apr 3, 2025
e9de061
Check common case first in external rule check
maxmynter Apr 3, 2025
979cf98
Use code_str variable
maxmynter Apr 3, 2025
ba56357
Update cago insta snapshot
maxmynter Apr 4, 2025
921f066
Refactor rule fix handling
maxmynter Apr 4, 2025
f9d4cf3
(chore) Wrestling with Clippy
maxmynter Apr 4, 2025
9397c2b
Update / fix test cases
maxmynter Apr 4, 2025
c8a9fe7
(chore) Remove unneccessary ;
maxmynter Apr 4, 2025
e1998a1
Simplifications
MichaReiser Apr 4, 2025
636a364
Delete commented out code
MichaReiser Apr 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF102.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Invalid code
import os # noqa: INVALID123
# External code
import re # noqa: V123
# Valid noqa
import sys # noqa: E402
from functools import cache # Preceeding comment # noqa: F401, INVALID456
from itertools import product # Preceeding comment # noqa: INVALID789
# Succeeding comment
import math # noqa: INVALID000 # Succeeding comment
# Mixed valid and invalid
from typing import List # noqa: F401, INVALID123
# Test for multiple invalid
from collections import defaultdict # noqa: INVALID100, INVALID200, F401
# Test for preserving valid codes when fixing
from itertools import chain # noqa: E402, INVALID300, F401
# Test for mixed code types
import json # noqa: E402, INVALID400, V100
7 changes: 7 additions & 0 deletions crates/ruff_linter/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,13 @@ pub(crate) fn check_noqa(
);
}

if settings.rules.enabled(Rule::InvalidRuleCode)
&& !per_file_ignores.contains(Rule::InvalidRuleCode)
&& !exemption.enumerates(Rule::InvalidRuleCode)
{
ruff::rules::invalid_noqa_code(diagnostics, &noqa_directives, locator, &settings.external);
}

ignored_diagnostics.sort_unstable();
ignored_diagnostics
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "059") => (RuleGroup::Preview, rules::ruff::rules::UnusedUnpackedVariable),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
(Ruff, "102") => (RuleGroup::Preview, rules::ruff::rules::InvalidRuleCode),

(Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
#[cfg(any(feature = "test-rules", test))]
Expand Down
15 changes: 15 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ mod tests {
#[test_case(Rule::UnusedUnpackedVariable, Path::new("RUF059_3.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
#[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down Expand Up @@ -314,6 +315,20 @@ mod tests {
Ok(())
}

#[test]
fn invalid_rule_code_external_rules() -> Result<()> {
let diagnostics = test_path(
Path::new("ruff/RUF102.py"),
&settings::LinterSettings {
external: vec!["V".to_string()],
..settings::LinterSettings::for_rule(Rule::InvalidRuleCode)
},
)?;

assert_messages!(diagnostics);
Ok(())
}

#[test]
fn ruff_per_file_ignores() -> Result<()> {
let diagnostics = test_path(
Expand Down
160 changes: 160 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
use crate::noqa::{Code, Directive};
use crate::registry::Rule;
use crate::Locator;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use crate::noqa::{Codes, NoqaDirectives};

/// ## What it does
/// Checks for `noqa` codes that are invalid.
///
/// ## Why is this bad?
/// Invalid rule codes serve no purpose and may indicate outdated code suppressions.
///
/// ## Example
/// ```python
/// import os # noqa: XYZ999
/// ```
///
/// Use instead:
/// ```python
/// import os
/// ```
///
/// Or if there are still valid codes needed:
/// ```python
/// import os # noqa: E402
/// ```
///
/// ## Options
/// - `lint.external`
#[derive(ViolationMetadata)]
pub(crate) struct InvalidRuleCode {
pub(crate) rule_code: String,
}

impl AlwaysFixableViolation for InvalidRuleCode {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid rule code in `# noqa`: {}", self.rule_code)
}

fn fix_title(&self) -> String {
"Remove the rule code".to_string()
}
}

/// RUF102 for invalid noqa codes
pub(crate) fn invalid_noqa_code(
diagnostics: &mut Vec<Diagnostic>,
noqa_directives: &NoqaDirectives,
locator: &Locator,
external: &[String],
) {
for line in noqa_directives.lines() {
let Directive::Codes(directive) = &line.directive else {
continue;
};

let all_valid = directive.iter().all(|code| code_is_valid(code, external));

if all_valid {
continue;
}

let (valid_codes, invalid_codes): (Vec<_>, Vec<_>) = directive
.iter()
.partition(|&code| code_is_valid(code, external));

if valid_codes.is_empty() {
diagnostics.push(all_codes_invalid_diagnostic(directive, invalid_codes));
} else {
diagnostics.extend(invalid_codes.into_iter().map(|invalid_code| {
some_codes_are_invalid_diagnostic(directive, invalid_code, locator)
}));
}
}
}

fn code_is_valid(code: &Code, external: &[String]) -> bool {
let code_str = code.as_str();
Rule::from_code(code_str).is_ok() || external.iter().any(|ext| code_str.starts_with(ext))
}

fn all_codes_invalid_diagnostic(
directive: &Codes<'_>,
invalid_codes: Vec<&Code<'_>>,
) -> Diagnostic {
Diagnostic::new(
InvalidRuleCode {
rule_code: invalid_codes
.into_iter()
.map(Code::as_str)
.collect::<Vec<_>>()
.join(", "),
},
directive.range(),
)
.with_fix(Fix::safe_edit(Edit::range_deletion(directive.range())))
}

fn some_codes_are_invalid_diagnostic(
codes: &Codes,
invalid_code: &Code,
locator: &Locator,
) -> Diagnostic {
let diagnostic = Diagnostic::new(
InvalidRuleCode {
rule_code: invalid_code.to_string(),
},
invalid_code.range(),
);
diagnostic.with_fix(Fix::safe_edit(remove_invalid_noqa(
codes,
invalid_code,
locator,
)))
}

fn remove_invalid_noqa(codes: &Codes, invalid_code: &Code, locator: &Locator) -> Edit {
// Is this the first code after the `:` that needs to get deleted
// For the first element, delete from after the `:` to the next comma (including)
// For any other element, delete from the previous comma (including) to the next comma (excluding)
let mut first = false;

// Find the index of the previous comma or colon.
let start = locator
.slice(TextRange::new(codes.start(), invalid_code.start()))
.rmatch_indices([',', ':'])
.next()
.map(|(offset, text)| {
let offset = codes.start() + TextSize::try_from(offset).unwrap();
if text == ":" {
first = true;
// Don't include the colon in the deletion range, or the noqa comment becomes invalid
offset + ':'.text_len()
} else {
offset
}
})
.unwrap_or(invalid_code.start());

// Find the index of the trailing comma (if any)
let end = locator
.slice(TextRange::new(invalid_code.end(), codes.end()))
.find(',')
.map(|offset| {
let offset = invalid_code.end() + TextSize::try_from(offset).unwrap();

if first {
offset + ','.text_len()
} else {
offset
}
})
.unwrap_or(invalid_code.end());

Edit::range_deletion(TextRange::new(start, end))
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub(crate) use invalid_assert_message_literal_argument::*;
pub(crate) use invalid_formatter_suppression_comment::*;
pub(crate) use invalid_index_type::*;
pub(crate) use invalid_pyproject_toml::*;
pub(crate) use invalid_rule_code::*;
pub(crate) use map_int_version_parsing::*;
pub(crate) use missing_fstring_syntax::*;
pub(crate) use mutable_class_default::*;
Expand Down Expand Up @@ -78,6 +79,7 @@ mod invalid_assert_message_literal_argument;
mod invalid_formatter_suppression_comment;
mod invalid_index_type;
mod invalid_pyproject_toml;
mod invalid_rule_code;
mod map_int_version_parsing;
mod missing_fstring_syntax;
mod mutable_class_default;
Expand Down
Loading
Loading