From 7e6b19048ea8615f9e18b6060d2994937ffbdd83 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 31 Jul 2024 22:09:05 -0400 Subject: [PATCH] Don't attach comments with mismatched indents (#12604) ## Summary Given: ```python def test_update(): pass # comment def test_clientmodel(): pass ``` We don't want `# comment` to be attached to `def test_clientmodel()`. Closes https://github.com/astral-sh/ruff/issues/12589. --- .../test/fixtures/pycodestyle/E30.py | 27 +++++++ .../rules/pycodestyle/rules/blank_lines.rs | 35 ++++++++-- ...ules__pycodestyle__tests__E302_E30.py.snap | 70 +++++++++++++++++-- 3 files changed, 122 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py index 2fdc72c2e3835..2ee487144d7fc 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py @@ -935,3 +935,30 @@ def arrow_strip_whitespace(obj: Array, /, *cols: str) -> Array: ... # type: ign def arrow_strip_whitespace(obj, /, *cols): ... # end + + +# E302 +def test_update(): + pass + # comment +def test_clientmodel(): + pass +# end + + +# E302 +def test_update(): + pass + # comment +def test_clientmodel(): + pass +# end + + +# E302 +def test_update(): + pass +# comment +def test_clientmodel(): + pass +# end diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs index 98bcbbb36ef75..09467e213e91d 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs @@ -352,13 +352,13 @@ struct LogicalLineInfo { kind: LogicalLineKind, first_token_range: TextRange, - // The kind of the last non-trivia token before the newline ending the logical line. + /// The kind of the last non-trivia token before the newline ending the logical line. last_token: TokenKind, - // The end of the logical line including the newline. + /// The end of the logical line including the newline. logical_line_end: TextSize, - // `true` if this is not a blank but only consists of a comment. + /// `true` if this is not a blank but only consists of a comment. is_comment_only: bool, /// If running on a notebook, whether the line is the first logical line (or a comment preceding it) of its cell. @@ -721,6 +721,7 @@ impl<'a> BlankLinesChecker<'a> { /// E301, E302, E303, E304, E305, E306 pub(crate) fn check_lines(&self, tokens: &Tokens, diagnostics: &mut Vec) { let mut prev_indent_length: Option = None; + let mut prev_logical_line: Option = None; let mut state = BlankLinesState::default(); let line_preprocessor = LinePreprocessor::new(tokens, self.locator, self.indent_width, self.cell_offsets); @@ -739,6 +740,23 @@ impl<'a> BlankLinesChecker<'a> { } } + // Reset the previous line end after an indent or dedent: + // ```python + // if True: + // import test + // # comment + // a = 10 + // ``` + // The `# comment` should be attached to the `import` statement, rather than the + // assignment. + if let Some(prev_logical_line) = prev_logical_line { + if prev_logical_line.is_comment_only { + if prev_logical_line.indent_length != logical_line.indent_length { + state.last_non_comment_line_end = prev_logical_line.logical_line_end; + } + } + } + state.class_status.update(&logical_line); state.fn_status.update(&logical_line); @@ -793,6 +811,8 @@ impl<'a> BlankLinesChecker<'a> { if !logical_line.is_comment_only { prev_indent_length = Some(logical_line.indent_length); } + + prev_logical_line = Some(logical_line); } } @@ -882,6 +902,8 @@ impl<'a> BlankLinesChecker<'a> { line.first_token_range, ); + // Check if the preceding comment + if let Some(blank_lines_range) = line.blank_lines.range() { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( self.stylist @@ -891,9 +913,10 @@ impl<'a> BlankLinesChecker<'a> { ))); } else { diagnostic.set_fix(Fix::safe_edit(Edit::insertion( - self.stylist - .line_ending() - .repeat(expected_blank_lines_before_definition as usize), + self.stylist.line_ending().repeat( + (expected_blank_lines_before_definition + - line.preceding_blank_lines.count()) as usize, + ), self.locator.line_start(state.last_non_comment_line_end), ))); } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E302_E30.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E302_E30.py.snap index 3ae60bb0da739..e3fe5c1f337a7 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E302_E30.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E302_E30.py.snap @@ -179,10 +179,9 @@ E30.py:602:1: E302 [*] Expected 2 blank lines, found 1 599 599 | pass 600 600 | 601 |+ - 602 |+ -601 603 | # comment -602 604 | @decorator -603 605 | def g(): +601 602 | # comment +602 603 | @decorator +603 604 | def g(): E30.py:624:1: E302 [*] Expected 2 blank lines, found 0 | @@ -223,3 +222,66 @@ E30.py:634:1: E302 [*] Expected 2 blank lines, found 1 634 635 | def fn(a: int | str) -> int | str: 635 636 | ... 636 637 | # end + +E30.py:944:1: E302 [*] Expected 2 blank lines, found 0 + | +942 | pass +943 | # comment +944 | def test_clientmodel(): + | ^^^ E302 +945 | pass +946 | # end + | + = help: Add missing blank line(s) + +ℹ Safe fix +941 941 | def test_update(): +942 942 | pass +943 943 | # comment + 944 |+ + 945 |+ +944 946 | def test_clientmodel(): +945 947 | pass +946 948 | # end + +E30.py:953:1: E302 [*] Expected 2 blank lines, found 0 + | +951 | pass +952 | # comment +953 | def test_clientmodel(): + | ^^^ E302 +954 | pass +955 | # end + | + = help: Add missing blank line(s) + +ℹ Safe fix +950 950 | def test_update(): +951 951 | pass +952 952 | # comment + 953 |+ + 954 |+ +953 955 | def test_clientmodel(): +954 956 | pass +955 957 | # end + +E30.py:962:1: E302 [*] Expected 2 blank lines, found 0 + | +960 | pass +961 | # comment +962 | def test_clientmodel(): + | ^^^ E302 +963 | pass +964 | # end + | + = help: Add missing blank line(s) + +ℹ Safe fix +958 958 | # E302 +959 959 | def test_update(): +960 960 | pass + 961 |+ + 962 |+ +961 963 | # comment +962 964 | def test_clientmodel(): +963 965 | pass