From 7bd7e710ba59fd9137062db9c8b5a59259406809 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 5 Mar 2024 11:02:39 +0100 Subject: [PATCH] Code review feedback --- .../src/rules/pycodestyle/rules/blank_lines.rs | 15 ++++++--------- ...t_compatibility-lines-after(1)-between(1).snap | 8 ++++---- 2 files changed, 10 insertions(+), 13 deletions(-) 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 a7d151148f1fe..9120c4d3ffff1 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs @@ -152,7 +152,7 @@ impl AlwaysFixableViolation for BlankLinesTopLevel { /// /// Note: The rule respects the following `isort` settings when determining the maximum number of blank lines allowed between two statements: /// * [`lint.isort.lines-after-imports`]: For top-level statements directly following an import statement. -/// * [`lint.isort.lines-between-types`]: For `import` statements directly following an `from..import` statement or vice versa. +/// * [`lint.isort.lines-between-types`]: For `import` statements directly following a `from ... import ...` statement or vice versa. /// /// ## References /// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines) @@ -754,8 +754,9 @@ impl<'a> BlankLinesChecker<'a> { // Mimic the isort rules for the number of blank lines before classes and functions if state.follows.is_any_import() { // Fallback to the default if the value is too large for an u32 or if it is negative. - // A negative value means that isort should determine the blank lines automatically - // Defaulting to 2 is correct because the variable is only used when testing the + // A negative value means that isort should determine the blank lines automatically. + // `isort` defaults to 2 if before a class or function definition and 1 otherwise. + // Defaulting to 2 here is correct because the variable is only used when testing the // blank lines before a class or function definition. u32::try_from(self.lines_after_imports).unwrap_or(BLANK_LINES_TOP_LEVEL) } else { @@ -809,7 +810,7 @@ impl<'a> BlankLinesChecker<'a> { BLANK_LINES_NESTED_LEVEL }; - // If between `import` and `from..import` or the other way round, + // If between `import` and `from .. import ..` or the other way round, // allow up to `lines_between_types` newlines for isort compatibility. // We let `isort` remove extra blank lines when the imports belong // to different sections. @@ -818,11 +819,7 @@ impl<'a> BlankLinesChecker<'a> { (LogicalLineKind::Import, Follows::FromImport) | (LogicalLineKind::FromImport, Follows::Import) ) { - if self.lines_between_types == 0 { - max_lines_level - } else { - u32::try_from(self.lines_between_types).unwrap_or(u32::MAX) - } + max_lines_level.max(u32::try_from(self.lines_between_types).unwrap_or(u32::MAX)) } else { expected_blank_lines_before_definition }; diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__too_many_blank_lines_isort_compatibility-lines-after(1)-between(1).snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__too_many_blank_lines_isort_compatibility-lines-after(1)-between(1).snap index deadf5b3c0c09..3ba9a6f5e8c21 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__too_many_blank_lines_isort_compatibility-lines-after(1)-between(1).snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__too_many_blank_lines_isort_compatibility-lines-after(1)-between(1).snap @@ -36,11 +36,11 @@ E30_isort.py:5:1: E303 [*] Too many blank lines (3) ℹ Safe fix 1 1 | import json 2 2 | -3 |- +3 3 | 4 |- -5 3 | from typing import Any, Sequence -6 4 | -7 5 | +5 4 | from typing import Any, Sequence +6 5 | +7 6 | E30_isort.py:8:1: E303 [*] Too many blank lines (2) |