Skip to content

Commit

Permalink
[pycodestyle] Do not ignore lines before the first logical line in …
Browse files Browse the repository at this point in the history
…blank lines rules (#10382)

## Summary

Ignoring all lines until the first logical line does not match the
behavior from pycodestyle. This PR therefore removes the `if
state.is_not_first_logical_line` skipping the line check before the
first logical line, and applies it only to `E302`.

For example, in the snippet below a rule violation should be detected on
the second comment and on the import.

```python
# first comment




# second comment




import foo
```

Fixes #10374

## Test Plan

Add test cases, update the snapshots and verify the ecosystem check output
  • Loading branch information
hoel-bagard authored Mar 14, 2024
1 parent 5f40371 commit e944c16
Show file tree
Hide file tree
Showing 18 changed files with 211 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"""Test where the error is after the module's docstring."""

def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"Test where the first line is a comment, " + "and the rule violation follows it."

def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def fn1():
pass

def fn2():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
print("Test where the first line is a statement, and the rule violation follows it.")

def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Test where the first line is a comment, and the rule violation follows it.



def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""Test where the error is after the module's docstring."""



def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"Test where the first line is a comment, " + "and the rule violation follows it."



def fn():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
print("Test where the first line is a statement, and the rule violation follows it.")



def fn():
pass
18 changes: 18 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,24 @@ mod tests {
Ok(())
}

#[test_case(Rule::BlankLinesTopLevel, Path::new("E302_first_line_docstring.py"))]
#[test_case(Rule::BlankLinesTopLevel, Path::new("E302_first_line_expression.py"))]
#[test_case(Rule::BlankLinesTopLevel, Path::new("E302_first_line_function.py"))]
#[test_case(Rule::BlankLinesTopLevel, Path::new("E302_first_line_statement.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E303_first_line_comment.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E303_first_line_docstring.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E303_first_line_expression.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E303_first_line_statement.py"))]
fn blank_lines_first_line(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("pycodestyle").join(path).as_path(),
&settings::LinterSettings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::BlankLineBetweenMethods, Path::new("E30.py"))]
#[test_case(Rule::BlankLinesTopLevel, Path::new("E30.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E30.py"))]
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,7 @@ impl<'a> BlankLinesChecker<'a> {
state.class_status.update(&logical_line);
state.fn_status.update(&logical_line);

if state.is_not_first_logical_line {
self.check_line(&logical_line, &state, prev_indent_length, diagnostics);
}
self.check_line(&logical_line, &state, prev_indent_length, diagnostics);

match logical_line.kind {
LogicalLineKind::Class => {
Expand Down Expand Up @@ -818,6 +816,8 @@ impl<'a> BlankLinesChecker<'a> {
&& line.kind.is_class_function_or_decorator()
// Blank lines in stub files are used to group definitions. Don't enforce blank lines.
&& !self.source_type.is_stub()
// Do not expect blank lines before the first logical line.
&& state.is_not_first_logical_line
{
// E302
let mut diagnostic = Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E302_first_line_docstring.py:3:1: E302 [*] Expected 2 blank lines, found 1
|
1 | """Test where the error is after the module's docstring."""
2 |
3 | def fn():
| ^^^ E302
4 | pass
|
= help: Add missing blank line(s)

Safe fix
1 1 | """Test where the error is after the module's docstring."""
2 2 |
3 |+
3 4 | def fn():
4 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E302_first_line_expression.py:3:1: E302 [*] Expected 2 blank lines, found 1
|
1 | "Test where the first line is a comment, " + "and the rule violation follows it."
2 |
3 | def fn():
| ^^^ E302
4 | pass
|
= help: Add missing blank line(s)

Safe fix
1 1 | "Test where the first line is a comment, " + "and the rule violation follows it."
2 2 |
3 |+
3 4 | def fn():
4 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E302_first_line_function.py:4:1: E302 [*] Expected 2 blank lines, found 1
|
2 | pass
3 |
4 | def fn2():
| ^^^ E302
5 | pass
|
= help: Add missing blank line(s)

Safe fix
1 1 | def fn1():
2 2 | pass
3 3 |
4 |+
4 5 | def fn2():
5 6 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E302_first_line_statement.py:3:1: E302 [*] Expected 2 blank lines, found 1
|
1 | print("Test where the first line is a statement, and the rule violation follows it.")
2 |
3 | def fn():
| ^^^ E302
4 | pass
|
= help: Add missing blank line(s)

Safe fix
1 1 | print("Test where the first line is a statement, and the rule violation follows it.")
2 2 |
3 |+
3 4 | def fn():
4 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E303_first_line_comment.py:5:1: E303 [*] Too many blank lines (3)
|
5 | def fn():
| ^^^ E303
6 | pass
|
= help: Remove extraneous blank line(s)

Safe fix
1 1 | # Test where the first line is a comment, and the rule violation follows it.
2 2 |
3 3 |
4 |-
5 4 | def fn():
6 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E303_first_line_docstring.py:5:1: E303 [*] Too many blank lines (3)
|
5 | def fn():
| ^^^ E303
6 | pass
|
= help: Remove extraneous blank line(s)

Safe fix
1 1 | """Test where the error is after the module's docstring."""
2 2 |
3 3 |
4 |-
5 4 | def fn():
6 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E303_first_line_expression.py:5:1: E303 [*] Too many blank lines (3)
|
5 | def fn():
| ^^^ E303
6 | pass
|
= help: Remove extraneous blank line(s)

Safe fix
1 1 | "Test where the first line is a comment, " + "and the rule violation follows it."
2 2 |
3 3 |
4 |-
5 4 | def fn():
6 5 | pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E303_first_line_statement.py:5:1: E303 [*] Too many blank lines (3)
|
5 | def fn():
| ^^^ E303
6 | pass
|
= help: Remove extraneous blank line(s)

Safe fix
1 1 | print("Test where the first line is a statement, and the rule violation follows it.")
2 2 |
3 3 |
4 |-
5 4 | def fn():
6 5 | pass

0 comments on commit e944c16

Please sign in to comment.