Skip to content

Commit 46ab9de

Browse files
[pycodestyle] Respect isort settings in blank line rules (E3*) (#10096)
## Summary This PR changes the `E3*` rules to respect the `isort` `lines-after-imports` and `lines-between-types` settings. Specifically, the following rules required changing * `TooManyBlannkLines` : Respects both settings. * `BlankLinesTopLevel`: Respects `lines-after-imports`. Doesn't need to respect `lines-between-types` because it only applies to classes and functions The downside of this approach is that `isort` and the blank line rules emit a diagnostic when there are too many blank lines. The fixes aren't identical, the blank line is less opinionated, but blank lines accepts the fix of `isort`. <details> <summary>Outdated approach</summary> Fixes #10077 (comment) This PR changes the blank line rules to not enforce the number of blank lines after imports (top-level) if isort is enabled and leave it to isort to enforce the right number of lines (depends on the `isort.lines-after-imports` and `isort.lines-between-types` settings). The reason to give `isort` precedence over the blank line rules is that they are configurable. Users that always want to blank lines after imports can use `isort.lines-after-imports=2` to enforce that (specifically for imports). This PR does not fix the incompatibility with the formatter in pyi files that only uses 0 to 1 blank lines. I'll address this separately. </details> ## Review The first commit is a small refactor that simplified implementing the fix (and makes it easier to reason about what's mutable and what's not). ## Test Plan I added a new test and verified that it fails with an error that the fix never converges. I verified the snapshot output after implementing the fix. --------- Co-authored-by: Hoël Bagard <[email protected]>
1 parent d441338 commit 46ab9de

16 files changed

+1565
-248
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# These rules test for intentional "odd" formatting. Using a formatter fixes that
2+
[E{1,2,3}*.py]
3+
generated_code = true
4+
ij_formatter_enabled = false
5+
6+
[W*.py]
7+
generated_code = true
8+
ij_formatter_enabled = false
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import json
2+
3+
4+
5+
from typing import Any, Sequence
6+
7+
8+
class MissingCommand(TypeError): ... # noqa: N818
9+
10+
11+
class BackendProxy:
12+
backend_module: str
13+
backend_object: str | None
14+
backend: Any
15+
16+
17+
if __name__ == "__main__":
18+
import abcd
19+
20+
21+
abcd.foo()
22+
23+
def __init__(self, backend_module: str, backend_obj: str | None) -> None: ...
24+
25+
if TYPE_CHECKING:
26+
import os
27+
28+
29+
30+
from typing_extensions import TypeAlias
31+
32+
33+
abcd.foo()
34+
35+
def __call__(self, name: str, *args: Any, **kwargs: Any) -> Any:
36+
...
37+
38+
if TYPE_CHECKING:
39+
from typing_extensions import TypeAlias
40+
41+
def __call__2(self, name: str, *args: Any, **kwargs: Any) -> Any:
42+
...
43+
44+
45+
def _exit(self) -> None: ...
46+
47+
48+
def _optional_commands(self) -> dict[str, bool]: ...
49+
50+
51+
def run(argv: Sequence[str]) -> int: ...
52+
53+
54+
def read_line(fd: int = 0) -> bytearray: ...
55+
56+
57+
def flush() -> None: ...
58+
59+
60+
from typing import Any, Sequence
61+
62+
class MissingCommand(TypeError): ... # noqa: N818

crates/ruff_linter/src/checkers/tokens.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,7 @@ pub(crate) fn check_tokens(
4141
Rule::BlankLinesAfterFunctionOrClass,
4242
Rule::BlankLinesBeforeNestedDefinition,
4343
]) {
44-
let mut blank_lines_checker = BlankLinesChecker::default();
45-
blank_lines_checker.check_lines(
46-
tokens,
47-
locator,
48-
stylist,
49-
settings.tab_size,
50-
&mut diagnostics,
51-
);
44+
BlankLinesChecker::new(locator, stylist, settings).check_lines(tokens, &mut diagnostics);
5245
}
5346

5447
if settings.rules.enabled(Rule::BlanketNOQA) {

crates/ruff_linter/src/rules/pycodestyle/mod.rs

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ mod tests {
1616

1717
use crate::line_width::LineLength;
1818
use crate::registry::Rule;
19-
use crate::rules::pycodestyle;
19+
use crate::rules::{isort, pycodestyle};
2020
use crate::settings::types::PreviewMode;
2121
use crate::test::test_path;
2222
use crate::{assert_messages, settings};
@@ -138,14 +138,23 @@ mod tests {
138138
Path::new("E25.py")
139139
)]
140140
#[test_case(Rule::MissingWhitespaceAroundParameterEquals, Path::new("E25.py"))]
141+
fn logical(rule_code: Rule, path: &Path) -> Result<()> {
142+
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
143+
let diagnostics = test_path(
144+
Path::new("pycodestyle").join(path).as_path(),
145+
&settings::LinterSettings::for_rule(rule_code),
146+
)?;
147+
assert_messages!(snapshot, diagnostics);
148+
Ok(())
149+
}
150+
141151
#[test_case(Rule::BlankLineBetweenMethods, Path::new("E30.py"))]
142152
#[test_case(Rule::BlankLinesTopLevel, Path::new("E30.py"))]
143153
#[test_case(Rule::TooManyBlankLines, Path::new("E30.py"))]
144154
#[test_case(Rule::BlankLineAfterDecorator, Path::new("E30.py"))]
145155
#[test_case(Rule::BlankLinesAfterFunctionOrClass, Path::new("E30.py"))]
146156
#[test_case(Rule::BlankLinesBeforeNestedDefinition, Path::new("E30.py"))]
147-
148-
fn logical(rule_code: Rule, path: &Path) -> Result<()> {
157+
fn blank_lines(rule_code: Rule, path: &Path) -> Result<()> {
149158
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
150159
let diagnostics = test_path(
151160
Path::new("pycodestyle").join(path).as_path(),
@@ -155,6 +164,64 @@ mod tests {
155164
Ok(())
156165
}
157166

167+
/// Tests the compatibility of the blank line top level rule and isort.
168+
#[test_case(-1, 0)]
169+
#[test_case(1, 1)]
170+
#[test_case(0, 0)]
171+
#[test_case(4, 4)]
172+
fn blank_lines_top_level_isort_compatibility(
173+
lines_after_imports: isize,
174+
lines_between_types: usize,
175+
) -> Result<()> {
176+
let snapshot = format!(
177+
"blank_lines_top_level_isort_compatibility-lines-after({lines_after_imports})-between({lines_between_types})"
178+
);
179+
let diagnostics = test_path(
180+
Path::new("pycodestyle").join("E30_isort.py"),
181+
&settings::LinterSettings {
182+
isort: isort::settings::Settings {
183+
lines_after_imports,
184+
lines_between_types,
185+
..isort::settings::Settings::default()
186+
},
187+
..settings::LinterSettings::for_rules([
188+
Rule::BlankLinesTopLevel,
189+
Rule::UnsortedImports,
190+
])
191+
},
192+
)?;
193+
assert_messages!(snapshot, diagnostics);
194+
Ok(())
195+
}
196+
197+
/// Tests the compatibility of the blank line too many lines and isort.
198+
#[test_case(-1, 0)]
199+
#[test_case(1, 1)]
200+
#[test_case(0, 0)]
201+
#[test_case(4, 4)]
202+
fn too_many_blank_lines_isort_compatibility(
203+
lines_after_imports: isize,
204+
lines_between_types: usize,
205+
) -> Result<()> {
206+
let snapshot = format!("too_many_blank_lines_isort_compatibility-lines-after({lines_after_imports})-between({lines_between_types})");
207+
let diagnostics = test_path(
208+
Path::new("pycodestyle").join("E30_isort.py"),
209+
&settings::LinterSettings {
210+
isort: isort::settings::Settings {
211+
lines_after_imports,
212+
lines_between_types,
213+
..isort::settings::Settings::default()
214+
},
215+
..settings::LinterSettings::for_rules([
216+
Rule::TooManyBlankLines,
217+
Rule::UnsortedImports,
218+
])
219+
},
220+
)?;
221+
assert_messages!(snapshot, diagnostics);
222+
Ok(())
223+
}
224+
158225
#[test]
159226
fn constant_literals() -> Result<()> {
160227
let diagnostics = test_path(

0 commit comments

Comments
 (0)