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

Don't attach comments with mismatched indents #12604

Merged
merged 1 commit into from
Aug 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading