diff --git a/crates/ruff/resources/test/fixtures/ruff/ruff_noqa.py b/crates/ruff/resources/test/fixtures/ruff/ruff_noqa_all.py similarity index 100% rename from crates/ruff/resources/test/fixtures/ruff/ruff_noqa.py rename to crates/ruff/resources/test/fixtures/ruff/ruff_noqa_all.py diff --git a/crates/ruff/resources/test/fixtures/ruff/ruff_targeted_noqa.py b/crates/ruff/resources/test/fixtures/ruff/ruff_noqa_codes.py similarity index 100% rename from crates/ruff/resources/test/fixtures/ruff/ruff_targeted_noqa.py rename to crates/ruff/resources/test/fixtures/ruff/ruff_noqa_codes.py diff --git a/crates/ruff/resources/test/fixtures/ruff/ruff_noqa_invalid.py b/crates/ruff/resources/test/fixtures/ruff/ruff_noqa_invalid.py new file mode 100644 index 00000000000000..cff06b194ff3b8 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/ruff_noqa_invalid.py @@ -0,0 +1,5 @@ +import os # ruff: noqa: F401 + + +def f(): + x = 1 diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index ca0ffa959fa85a..c2517699b06ac5 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -12,6 +12,7 @@ use ruff_python_ast::Ranged; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_diagnostics::Diagnostic; +use ruff_python_trivia::indentation_at_offset; use ruff_source_file::{LineEnding, Locator}; use crate::codes::NoqaCode; @@ -249,18 +250,11 @@ impl FileExemption { warn!("Invalid `# ruff: noqa` directive at {path_display}:{line}: {err}"); } Ok(Some(exemption)) => { - if !locator - .slice(TextRange::new( - locator.line_start(range.start()), - range.start(), - )) - .chars() - .all(char::is_whitespace) - { + if indentation_at_offset(range.start(), locator).is_none() { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); let path_display = relativize_path(path); - warn!("Unexpected end-of-line `# ruff: noqa` directive at {path_display}:{line}"); + warn!("Unexpected `# ruff: noqa` directive at {path_display}:{line}. File-level suppression comments must appear on their own line."); continue; } @@ -277,7 +271,7 @@ impl FileExemption { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); let path_display = relativize_path(path); - warn!("Invalid code provided to `# ruff: noqa` at {path_display}:{line}: {code}"); + warn!("Invalid rule code provided to `# ruff: noqa` at {path_display}:{line}: {code}"); None } })); diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index f85b8045ce431b..42350547abd151 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -167,9 +167,9 @@ mod tests { } #[test] - fn ruff_noqa() -> Result<()> { + fn ruff_noqa_all() -> Result<()> { let diagnostics = test_path( - Path::new("ruff/ruff_noqa.py"), + Path::new("ruff/ruff_noqa_all.py"), &settings::Settings::for_rules(vec![Rule::UnusedImport, Rule::UnusedVariable]), )?; assert_messages!(diagnostics); @@ -177,9 +177,19 @@ mod tests { } #[test] - fn ruff_targeted_noqa() -> Result<()> { + fn ruff_noqa_codes() -> Result<()> { let diagnostics = test_path( - Path::new("ruff/ruff_targeted_noqa.py"), + Path::new("ruff/ruff_noqa_codes.py"), + &settings::Settings::for_rules(vec![Rule::UnusedImport, Rule::UnusedVariable]), + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn ruff_noqa_invalid() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/ruff_noqa_invalid.py"), &settings::Settings::for_rules(vec![Rule::UnusedImport, Rule::UnusedVariable]), )?; assert_messages!(diagnostics); diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF014_RUF014.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF014_RUF014.py.snap deleted file mode 100644 index f2457017e323f3..00000000000000 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF014_RUF014.py.snap +++ /dev/null @@ -1,249 +0,0 @@ ---- -source: crates/ruff/src/rules/ruff/mod.rs ---- -RUF014.py:3:5: RUF014 Unreachable code in after_return - | -1 | def after_return(): -2 | return "reachable" -3 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -4 | -5 | async def also_works_on_async_functions(): - | - -RUF014.py:7:5: RUF014 Unreachable code in also_works_on_async_functions - | -5 | async def also_works_on_async_functions(): -6 | return "reachable" -7 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -8 | -9 | def if_always_true(): - | - -RUF014.py:12:5: RUF014 Unreachable code in if_always_true - | -10 | if True: -11 | return "reachable" -12 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -13 | -14 | def if_always_false(): - | - -RUF014.py:16:9: RUF014 Unreachable code in if_always_false - | -14 | def if_always_false(): -15 | if False: -16 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -17 | return "reachable" - | - -RUF014.py:21:9: RUF014 Unreachable code in if_elif_always_false - | -19 | def if_elif_always_false(): -20 | if False: -21 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -22 | elif False: -23 | return "also unreachable" - | - -RUF014.py:23:9: RUF014 Unreachable code in if_elif_always_false - | -21 | return "unreachable" -22 | elif False: -23 | return "also unreachable" - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF014 -24 | return "reachable" - | - -RUF014.py:28:9: RUF014 Unreachable code in if_elif_always_true - | -26 | def if_elif_always_true(): -27 | if False: -28 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -29 | elif True: -30 | return "reachable" - | - -RUF014.py:31:5: RUF014 Unreachable code in if_elif_always_true - | -29 | elif True: -30 | return "reachable" -31 | return "also unreachable" - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF014 -32 | -33 | def ends_with_if(): - | - -RUF014.py:35:9: RUF014 Unreachable code in ends_with_if - | -33 | def ends_with_if(): -34 | if False: -35 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -36 | else: -37 | return "reachable" - | - -RUF014.py:42:5: RUF014 Unreachable code in infinite_loop - | -40 | while True: -41 | continue -42 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -43 | -44 | ''' TODO: we could determine these, but we don't yet. - | - -RUF014.py:75:5: RUF014 Unreachable code in match_wildcard - | -73 | case _: -74 | return "reachable" -75 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -76 | -77 | def match_case_and_wildcard(status): - | - -RUF014.py:83:5: RUF014 Unreachable code in match_case_and_wildcard - | -81 | case _: -82 | return "reachable" -83 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -84 | -85 | def raise_exception(): - | - -RUF014.py:87:5: RUF014 Unreachable code in raise_exception - | -85 | def raise_exception(): -86 | raise Exception -87 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -88 | -89 | def while_false(): - | - -RUF014.py:91:9: RUF014 Unreachable code in while_false - | -89 | def while_false(): -90 | while False: -91 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -92 | return "reachable" - | - -RUF014.py:96:9: RUF014 Unreachable code in while_false_else - | -94 | def while_false_else(): -95 | while False: -96 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -97 | else: -98 | return "reachable" - | - -RUF014.py:102:9: RUF014 Unreachable code in while_false_else_return - | -100 | def while_false_else_return(): -101 | while False: -102 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -103 | else: -104 | return "reachable" - | - -RUF014.py:105:5: RUF014 Unreachable code in while_false_else_return - | -103 | else: -104 | return "reachable" -105 | return "also unreachable" - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF014 -106 | -107 | def while_true(): - | - -RUF014.py:110:5: RUF014 Unreachable code in while_true - | -108 | while True: -109 | return "reachable" -110 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -111 | -112 | def while_true_else(): - | - -RUF014.py:116:9: RUF014 Unreachable code in while_true_else - | -114 | return "reachable" -115 | else: -116 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -117 | -118 | def while_true_else_return(): - | - -RUF014.py:122:9: RUF014 Unreachable code in while_true_else_return - | -120 | return "reachable" -121 | else: -122 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -123 | return "also unreachable" - | - -RUF014.py:123:5: RUF014 Unreachable code in while_true_else_return - | -121 | else: -122 | return "unreachable" -123 | return "also unreachable" - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF014 -124 | -125 | def while_false_var_i(): - | - -RUF014.py:128:9: RUF014 Unreachable code in while_false_var_i - | -126 | i = 0 -127 | while False: -128 | i += 1 - | ^^^^^^ RUF014 -129 | return i - | - -RUF014.py:135:5: RUF014 Unreachable code in while_true_var_i - | -133 | while True: -134 | i += 1 -135 | return i - | ^^^^^^^^ RUF014 -136 | -137 | def while_infinite(): - | - -RUF014.py:140:5: RUF014 Unreachable code in while_infinite - | -138 | while True: -139 | pass -140 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -141 | -142 | def while_if_true(): - | - -RUF014.py:146:5: RUF014 Unreachable code in while_if_true - | -144 | if True: -145 | return "reachable" -146 | return "unreachable" - | ^^^^^^^^^^^^^^^^^^^^ RUF014 -147 | -148 | # Test case found in the Bokeh repository that trigger a false positive. - | - - diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_all.snap similarity index 100% rename from crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa.snap rename to crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_all.snap diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_targeted_noqa.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_codes.snap similarity index 73% rename from crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_targeted_noqa.snap rename to crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_codes.snap index 52cb5f53af6406..582168670881b9 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_targeted_noqa.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_codes.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/ruff/mod.rs --- -ruff_targeted_noqa.py:8:5: F841 [*] Local variable `x` is assigned to but never used +ruff_noqa_codes.py:8:5: F841 [*] Local variable `x` is assigned to but never used | 7 | def f(): 8 | x = 1 diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_invalid.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_invalid.snap new file mode 100644 index 00000000000000..73a4e5554512f1 --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_noqa_invalid.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +--- +ruff_noqa_invalid.py:1:8: F401 [*] `os` imported but unused + | +1 | import os # ruff: noqa: F401 + | ^^ F401 + | + = help: Remove unused import: `os` + +ℹ Fix +1 |-import os # ruff: noqa: F401 +2 1 | +3 2 | +4 3 | def f(): + +ruff_noqa_invalid.py:5:5: F841 [*] Local variable `x` is assigned to but never used + | +4 | def f(): +5 | x = 1 + | ^ F841 + | + = help: Remove assignment to unused variable `x` + +ℹ Suggested fix +2 2 | +3 3 | +4 4 | def f(): +5 |- x = 1 + 5 |+ pass + + diff --git a/crates/ruff_python_ast/src/whitespace.rs b/crates/ruff_python_ast/src/whitespace.rs index aceec2ec9988e4..282076ee297813 100644 --- a/crates/ruff_python_ast/src/whitespace.rs +++ b/crates/ruff_python_ast/src/whitespace.rs @@ -12,7 +12,7 @@ pub fn indentation<'a, T>(locator: &'a Locator, located: &T) -> Option<&'a str> where T: Ranged, { - indentation_at_offset(locator, located.start()) + indentation_at_offset(located.start(), locator) } /// Return the end offset at which the empty lines following a statement. diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 4d8488360c72b3..a09a5b1e6d4a15 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -297,7 +297,7 @@ fn handle_own_line_comment_between_branches<'a>( // It depends on the indentation level of the comment if it is a leading comment for the // following branch or if it a trailing comment of the previous body's last statement. - let comment_indentation = indentation_at_offset(locator, comment.slice().range().start()) + let comment_indentation = indentation_at_offset(comment.slice().range().start(), locator) .unwrap_or_default() .len(); @@ -402,7 +402,7 @@ fn handle_match_comment<'a>( let next_case = match_stmt.cases.get(current_case_index + 1); - let comment_indentation = indentation_at_offset(locator, comment.slice().range().start()) + let comment_indentation = indentation_at_offset(comment.slice().range().start(), locator) .unwrap_or_default() .len(); let match_case_indentation = indentation(locator, match_case).unwrap().len(); @@ -480,7 +480,7 @@ fn handle_own_line_comment_after_branch<'a>( // We only care about the length because indentations with mixed spaces and tabs are only valid if // the indent-level doesn't depend on the tab width (the indent level must be the same if the tab width is 1 or 8). - let comment_indentation = indentation_at_offset(locator, comment.slice().range().start()) + let comment_indentation = indentation_at_offset(comment.slice().range().start(), locator) .unwrap_or_default() .len(); @@ -493,7 +493,7 @@ fn handle_own_line_comment_after_branch<'a>( // # Trailing if comment // ``` // Here we keep the comment a trailing comment of the `if` - let preceding_indentation = indentation_at_offset(locator, preceding_node.start()) + let preceding_indentation = indentation_at_offset(preceding_node.start(), locator) .unwrap_or_default() .len(); if comment_indentation == preceding_indentation { diff --git a/crates/ruff_python_trivia/src/whitespace.rs b/crates/ruff_python_trivia/src/whitespace.rs index 05c40e90b386bd..cc8f5563b7b411 100644 --- a/crates/ruff_python_trivia/src/whitespace.rs +++ b/crates/ruff_python_trivia/src/whitespace.rs @@ -2,7 +2,7 @@ use ruff_source_file::Locator; use ruff_text_size::{TextRange, TextSize}; /// Extract the leading indentation from a line. -pub fn indentation_at_offset<'a>(locator: &'a Locator, offset: TextSize) -> Option<&'a str> { +pub fn indentation_at_offset<'a>(offset: TextSize, locator: &'a Locator) -> Option<&'a str> { let line_start = locator.line_start(offset); let indentation = &locator.contents()[TextRange::new(line_start, offset)];