Skip to content

Commit

Permalink
Don't attach comments with mismatched indents (#12604)
Browse files Browse the repository at this point in the history
## 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 #12589.
  • Loading branch information
charliermarsh authored Aug 1, 2024
1 parent 8e383b9 commit 7e6b190
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 10 deletions.
27 changes: 27 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
35 changes: 29 additions & 6 deletions crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<Diagnostic>) {
let mut prev_indent_length: Option<usize> = None;
let mut prev_logical_line: Option<LogicalLineInfo> = None;
let mut state = BlankLinesState::default();
let line_preprocessor =
LinePreprocessor::new(tokens, self.locator, self.indent_width, self.cell_offsets);
Expand All @@ -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);

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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),
)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down Expand Up @@ -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

0 comments on commit 7e6b190

Please sign in to comment.