From 0d05d4822694e5c25d41bbe42dde0f0bfc32ac52 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 23 Feb 2024 14:15:44 +0100 Subject: [PATCH] Disable blank line rules after top level import statements --- .../test/fixtures/pycodestyle/E30_isort.py | 43 +++++++ crates/ruff_linter/src/checkers/tokens.rs | 3 +- .../ruff_linter/src/rules/pycodestyle/mod.rs | 43 +++++-- .../rules/pycodestyle/rules/blank_lines.rs | 112 +++++++++++------- ...estyle__tests__blank_lines_E302_isort.snap | 27 +++++ ...estyle__tests__blank_lines_E303_isort.snap | 44 +++++++ 6 files changed, 217 insertions(+), 55 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pycodestyle/E30_isort.py create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E302_isort.snap create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E303_isort.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30_isort.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30_isort.py new file mode 100644 index 00000000000000..52aa4a6e36c550 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E30_isort.py @@ -0,0 +1,43 @@ +import json + + + +from typing import Any, Sequence + + +class MissingCommand(TypeError): ... # noqa: N818 + + +class BackendProxy: + backend_module: str + backend_object: str | None + backend: Any + + +if __name__ == "__main__": + import abcd + + + + abcd.foo() + + +def __init__(self, backend_module: str, backend_obj: str | None) -> None: ... + + +def __call__(self, name: str, *args: Any, **kwargs: Any) -> Any: ... + + +def _exit(self) -> None: ... + + +def _optional_commands(self) -> dict[str, bool]: ... + + +def run(argv: Sequence[str]) -> int: ... + + +def read_line(fd: int = 0) -> bytearray: ... + + +def flush() -> None: ... diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index d2045a7a9e0c4e..dea5c8d8a9e4d4 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -41,8 +41,7 @@ pub(crate) fn check_tokens( Rule::BlankLinesAfterFunctionOrClass, Rule::BlankLinesBeforeNestedDefinition, ]) { - BlankLinesChecker::new(locator, stylist, settings.tab_size) - .check_lines(tokens, &mut diagnostics); + BlankLinesChecker::new(locator, stylist, settings).check_lines(tokens, &mut diagnostics); } if settings.rules.enabled(Rule::BlanketNOQA) { diff --git a/crates/ruff_linter/src/rules/pycodestyle/mod.rs b/crates/ruff_linter/src/rules/pycodestyle/mod.rs index 34b63a26ffbe9a..6e9685518b8933 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/mod.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/mod.rs @@ -16,7 +16,7 @@ mod tests { use crate::line_width::LineLength; use crate::registry::Rule; - use crate::rules::pycodestyle; + use crate::rules::{isort, pycodestyle}; use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -121,8 +121,8 @@ mod tests { #[test_case(Rule::MissingWhitespaceAroundOperator, Path::new("E22.py"))] #[test_case(Rule::MissingWhitespaceAroundArithmeticOperator, Path::new("E22.py"))] #[test_case( - Rule::MissingWhitespaceAroundBitwiseOrShiftOperator, - Path::new("E22.py") + Rule::MissingWhitespaceAroundBitwiseOrShiftOperator, + Path::new("E22.py") )] #[test_case(Rule::MissingWhitespaceAroundModuloOperator, Path::new("E22.py"))] #[test_case(Rule::MissingWhitespace, Path::new("E23.py"))] @@ -134,18 +134,27 @@ mod tests { #[test_case(Rule::WhitespaceBeforePunctuation, Path::new("E20.py"))] #[test_case(Rule::WhitespaceBeforeParameters, Path::new("E21.py"))] #[test_case( - Rule::UnexpectedSpacesAroundKeywordParameterEquals, - Path::new("E25.py") + Rule::UnexpectedSpacesAroundKeywordParameterEquals, + Path::new("E25.py") )] #[test_case(Rule::MissingWhitespaceAroundParameterEquals, Path::new("E25.py"))] + fn logical(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"))] #[test_case(Rule::BlankLineAfterDecorator, Path::new("E30.py"))] #[test_case(Rule::BlankLinesAfterFunctionOrClass, Path::new("E30.py"))] #[test_case(Rule::BlankLinesBeforeNestedDefinition, Path::new("E30.py"))] - - fn logical(rule_code: Rule, path: &Path) -> Result<()> { + fn blank_lines(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(), @@ -155,6 +164,26 @@ mod tests { Ok(()) } + /// Tests the compatibility of the blank line rules and isort. + #[test_case(Rule::BlankLinesTopLevel)] + #[test_case(Rule::TooManyBlankLines)] + fn blank_lines_isort(rule_code: Rule) -> Result<()> { + let snapshot = format!("blank_lines_{}_isort", rule_code.noqa_code()); + let diagnostics = test_path( + Path::new("pycodestyle").join("E30_isort.py"), + &settings::LinterSettings { + isort: isort::settings::Settings { + lines_after_imports: 1, + lines_between_types: 1, + ..isort::settings::Settings::default() + }, + ..settings::LinterSettings::for_rules([rule_code, Rule::UnsortedImports]) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test] fn constant_literals() -> Result<()> { let diagnostics = test_path( 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 cae2cd9288be02..16e98f0de7fe57 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs @@ -18,6 +18,7 @@ use ruff_text_size::TextRange; use ruff_text_size::TextSize; use crate::checkers::logical_lines::expand_indent; +use crate::codes::Rule; use crate::line_width::IndentWidth; use ruff_python_trivia::PythonWhitespace; @@ -415,6 +416,7 @@ impl<'a> Iterator for LinePreprocessor<'a> { { LogicalLineKind::Function } + TokenKind::Import | TokenKind::From => LogicalLineKind::Import, _ => LogicalLineKind::Other, }; @@ -560,9 +562,16 @@ enum Follows { Other, Decorator, Def, + Import, Docstring, } +impl Follows { + const fn is_import(self) -> bool { + matches!(self, Follows::Import) + } +} + #[derive(Copy, Clone, Debug, Default)] enum Status { /// Stores the indent level where the nesting started. @@ -607,18 +616,20 @@ pub(crate) struct BlankLinesChecker<'a> { stylist: &'a Stylist<'a>, locator: &'a Locator<'a>, indent_width: IndentWidth, + isort_enabled: bool, } impl<'a> BlankLinesChecker<'a> { pub(crate) fn new( locator: &'a Locator<'a>, stylist: &'a Stylist<'a>, - indent_width: IndentWidth, + settings: &crate::settings::LinterSettings, ) -> BlankLinesChecker<'a> { BlankLinesChecker { stylist, locator, - indent_width, + indent_width: settings.tab_size, + isort_enabled: settings.rules.enabled(Rule::UnsortedImports), } } @@ -629,7 +640,55 @@ impl<'a> BlankLinesChecker<'a> { let line_preprocessor = LinePreprocessor::new(tokens, self.locator, self.indent_width); for logical_line in line_preprocessor { - state = self.check_line(&logical_line, state, prev_indent_length, diagnostics); + state.class_status.update(&logical_line); + state.fn_status.update(&logical_line); + + // Leave checking of empty lines after imports to isort if it is enabled. + // This is to avoid conflicts where isort e.g. requires one empty line but blank lines + // requires two empty lines. + if !state.follows.is_import() || !self.isort_enabled || logical_line.indent_length != 0 + { + self.check_line(&logical_line, &state, prev_indent_length, diagnostics); + } + + match logical_line.kind { + LogicalLineKind::Class => { + if matches!(state.class_status, Status::Outside) { + state.class_status = Status::Inside(logical_line.indent_length); + } + state.follows = Follows::Other; + } + LogicalLineKind::Decorator => { + state.follows = Follows::Decorator; + } + LogicalLineKind::Function => { + if matches!(state.fn_status, Status::Outside) { + state.fn_status = Status::Inside(logical_line.indent_length); + } + state.follows = Follows::Def; + } + LogicalLineKind::Comment => {} + LogicalLineKind::Import => { + state.follows = Follows::Import; + } + LogicalLineKind::Other => { + state.follows = Follows::Other; + } + } + + if logical_line.is_docstring { + state.follows = Follows::Docstring; + } + + if !logical_line.is_comment_only { + state.is_not_first_logical_line = true; + + state.last_non_comment_line_end = logical_line.logical_line_end; + + if logical_line.indent_length == 0 { + state.previous_unindented_line_kind = Some(logical_line.kind); + } + } if !logical_line.is_comment_only { prev_indent_length = Some(logical_line.indent_length); @@ -641,13 +700,10 @@ impl<'a> BlankLinesChecker<'a> { fn check_line( &self, line: &LogicalLineInfo, - mut state: BlankLinesState, + state: &BlankLinesState, prev_indent_length: Option, diagnostics: &mut Vec, - ) -> BlankLinesState { - state.class_status.update(line); - state.fn_status.update(line); - + ) { // Don't expect blank lines before the first non comment line. if state.is_not_first_logical_line { if line.preceding_blank_lines == 0 @@ -836,44 +892,6 @@ impl<'a> BlankLinesChecker<'a> { diagnostics.push(diagnostic); } } - - match line.kind { - LogicalLineKind::Class => { - if matches!(state.class_status, Status::Outside) { - state.class_status = Status::Inside(line.indent_length); - } - state.follows = Follows::Other; - } - LogicalLineKind::Decorator => { - state.follows = Follows::Decorator; - } - LogicalLineKind::Function => { - if matches!(state.fn_status, Status::Outside) { - state.fn_status = Status::Inside(line.indent_length); - } - state.follows = Follows::Def; - } - LogicalLineKind::Comment => {} - LogicalLineKind::Other => { - state.follows = Follows::Other; - } - } - - if line.is_docstring { - state.follows = Follows::Docstring; - } - - if !line.is_comment_only { - state.is_not_first_logical_line = true; - - state.last_non_comment_line_end = line.logical_line_end; - - if line.indent_length == 0 { - state.previous_unindented_line_kind = Some(line.kind); - } - } - - state } } @@ -900,6 +918,8 @@ enum LogicalLineKind { Function, /// A comment only line Comment, + /// An import statement + Import, /// Any other statement or clause header Other, } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E302_isort.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E302_isort.snap new file mode 100644 index 00000000000000..0346d7b0c84280 --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E302_isort.snap @@ -0,0 +1,27 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +E30_isort.py:1:1: I001 [*] Import block is un-sorted or un-formatted + | +1 | / import json +2 | | +3 | | +4 | | +5 | | from typing import Any, Sequence +6 | | +7 | | +8 | | class MissingCommand(TypeError): ... # noqa: N818 + | |_^ I001 + | + = help: Organize imports + +ℹ Safe fix +1 1 | import json +2 2 | +3 |- +4 |- +5 3 | from typing import Any, Sequence +6 |- +7 4 | +8 5 | class MissingCommand(TypeError): ... # noqa: N818 +9 6 | diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E303_isort.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E303_isort.snap new file mode 100644 index 00000000000000..7c7a82b08f652f --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E303_isort.snap @@ -0,0 +1,44 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +E30_isort.py:1:1: I001 [*] Import block is un-sorted or un-formatted + | +1 | / import json +2 | | +3 | | +4 | | +5 | | from typing import Any, Sequence +6 | | +7 | | +8 | | class MissingCommand(TypeError): ... # noqa: N818 + | |_^ I001 + | + = help: Organize imports + +ℹ Safe fix +1 1 | import json +2 2 | +3 |- +4 |- +5 3 | from typing import Any, Sequence +6 |- +7 4 | +8 5 | class MissingCommand(TypeError): ... # noqa: N818 +9 6 | + +E30_isort.py:22:5: E303 [*] Too many blank lines (3) + | +22 | abcd.foo() + | ^^^^ E303 + | + = help: Remove extraneous blank line(s) + +ℹ Safe fix +17 17 | if __name__ == "__main__": +18 18 | import abcd +19 19 | +20 |- +21 |- +22 20 | abcd.foo() +23 21 | +24 22 |