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

[pycodestyle]: Make blank lines in typing stub files optional (E3*) #10098

Merged
merged 1 commit into from
Mar 5, 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
50 changes: 50 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import json

from typing import Any, Sequence

class MissingCommand(TypeError): ...
class AnoherClass: ...

def a(): ...

@overload
def a(arg: int): ...

@overload
def a(arg: int, name: str): ...


def grouped1(): ...
def grouped2(): ...
def grouped3( ): ...


class BackendProxy:
backend_module: str
backend_object: str | None
backend: Any

def grouped1(): ...
def grouped2(): ...
def grouped3( ): ...
@decorated

def with_blank_line(): ...


def ungrouped(): ...
a = "test"

def function_def():
pass
b = "test"


def outer():
def inner():
pass
def inner2():
pass

class Foo: ...
class Bar: ...
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
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: ...

if TYPE_CHECKING:
import os



from typing_extensions import TypeAlias


abcd.foo()

def __call__(self, name: str, *args: Any, **kwargs: Any) -> Any:
...

if TYPE_CHECKING:
from typing_extensions import TypeAlias

def __call__2(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: ...


from typing import Any, Sequence

class MissingCommand(TypeError): ... # noqa: N818
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ pub(crate) fn check_tokens(
Rule::BlankLinesAfterFunctionOrClass,
Rule::BlankLinesBeforeNestedDefinition,
]) {
BlankLinesChecker::new(locator, stylist, settings).check_lines(tokens, &mut diagnostics);
BlankLinesChecker::new(locator, stylist, settings, source_type)
.check_lines(tokens, &mut diagnostics);
}

if settings.rules.enabled(Rule::BlanketNOQA) {
Expand Down
32 changes: 32 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,38 @@ mod tests {
Ok(())
}

#[test_case(Rule::BlankLineBetweenMethods)]
#[test_case(Rule::BlankLinesTopLevel)]
#[test_case(Rule::TooManyBlankLines)]
#[test_case(Rule::BlankLineAfterDecorator)]
#[test_case(Rule::BlankLinesAfterFunctionOrClass)]
#[test_case(Rule::BlankLinesBeforeNestedDefinition)]
fn blank_lines_typing_stub(rule_code: Rule) -> Result<()> {
let snapshot = format!("blank_lines_{}_typing_stub", rule_code.noqa_code());
let diagnostics = test_path(
Path::new("pycodestyle").join("E30.pyi"),
&settings::LinterSettings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test]
fn blank_lines_typing_stub_isort() -> Result<()> {
let diagnostics = test_path(
Path::new("pycodestyle").join("E30_isort.pyi"),
&settings::LinterSettings {
..settings::LinterSettings::for_rules([
Rule::TooManyBlankLines,
Rule::BlankLinesTopLevel,
Rule::UnsortedImports,
])
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test]
fn constant_literals() -> Result<()> {
let diagnostics = test_path(
Expand Down
63 changes: 52 additions & 11 deletions crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Edit;
use ruff_diagnostics::Fix;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist;
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::lexer::LexicalError;
Expand Down Expand Up @@ -51,9 +52,14 @@ const BLANK_LINES_NESTED_LEVEL: u32 = 1;
/// pass
/// ```
///
/// ## Typing stub files (`.pyi`)
/// The typing style guide recommends to not use blank lines between methods except to group
/// them. That's why this rule is not enabled in typing stub files.
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
/// - [Flake 8 rule](https://www.flake8rules.com/rules/E301.html)
/// - [Typing Style Guide](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
#[violation]
pub struct BlankLineBetweenMethods;

Expand Down Expand Up @@ -96,9 +102,14 @@ impl AlwaysFixableViolation for BlankLineBetweenMethods {
/// pass
/// ```
///
/// ## Typing stub files (`.pyi`)
/// The typing style guide recommends to not use blank lines between classes and functions except to group
/// them. That's why this rule is not enabled in typing stub files.
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
/// - [Flake 8 rule](https://www.flake8rules.com/rules/E302.html)
/// - [Typing Style Guide](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
#[violation]
pub struct BlankLinesTopLevel {
actual_blank_lines: u32,
Expand Down Expand Up @@ -150,13 +161,17 @@ impl AlwaysFixableViolation for BlankLinesTopLevel {
/// pass
/// ```
///
/// ## Typing stub files (`.pyi`)
/// The rule allows at most one blank line in typing stub files in accordance to the typing style guide recommendation.
///
/// 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 a `from ... import ...` statement or vice versa.
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
/// - [Flake 8 rule](https://www.flake8rules.com/rules/E303.html)
/// - [Typing Style Guide](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
#[violation]
pub struct TooManyBlankLines {
actual_blank_lines: u32,
Expand Down Expand Up @@ -246,9 +261,14 @@ impl AlwaysFixableViolation for BlankLineAfterDecorator {
/// user = User()
/// ```
///
/// ## Typing stub files (`.pyi`)
/// The typing style guide recommends to not use blank lines between statements except to group
/// them. That's why this rule is not enabled in typing stub files.
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
/// - [Flake 8 rule](https://www.flake8rules.com/rules/E305.html)
/// - [Typing Style Guide](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
#[violation]
pub struct BlankLinesAfterFunctionOrClass {
actual_blank_lines: u32,
Expand Down Expand Up @@ -295,9 +315,14 @@ impl AlwaysFixableViolation for BlankLinesAfterFunctionOrClass {
/// pass
/// ```
///
/// ## Typing stub files (`.pyi`)
/// The typing style guide recommends to not use blank lines between classes and functions except to group
/// them. That's why this rule is not enabled in typing stub files.
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
/// - [Flake 8 rule](https://www.flake8rules.com/rules/E306.html)
/// - [Typing Style Guide](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
#[violation]
pub struct BlankLinesBeforeNestedDefinition;

Expand Down Expand Up @@ -628,20 +653,23 @@ pub(crate) struct BlankLinesChecker<'a> {
indent_width: IndentWidth,
lines_after_imports: isize,
lines_between_types: usize,
source_type: PySourceType,
}

impl<'a> BlankLinesChecker<'a> {
pub(crate) fn new(
locator: &'a Locator<'a>,
stylist: &'a Stylist<'a>,
settings: &crate::settings::LinterSettings,
source_type: PySourceType,
) -> BlankLinesChecker<'a> {
BlankLinesChecker {
stylist,
locator,
indent_width: settings.tab_size,
lines_after_imports: settings.isort.lines_after_imports,
lines_between_types: settings.isort.lines_between_types,
source_type,
}
}

Expand Down Expand Up @@ -739,6 +767,8 @@ impl<'a> BlankLinesChecker<'a> {
&& !matches!(state.follows, Follows::Docstring | Follows::Decorator)
// Do not trigger when the def follows an if/while/etc...
&& prev_indent_length.is_some_and(|prev_indent_length| prev_indent_length >= line.indent_length)
// Blank lines in stub files are only used for grouping. Don't enforce blank lines.
&& !self.source_type.is_stub()
{
// E301
let mut diagnostic = Diagnostic::new(BlankLineBetweenMethods, line.first_token_range);
Expand All @@ -750,20 +780,31 @@ impl<'a> BlankLinesChecker<'a> {
diagnostics.push(diagnostic);
}

// Blank lines in stub files are used to group definitions. Don't enforce blank lines.
let max_lines_level = if self.source_type.is_stub() {
1
} else {
if line.indent_length == 0 {
BLANK_LINES_TOP_LEVEL
} else {
BLANK_LINES_NESTED_LEVEL
}
};

let expected_blank_lines_before_definition = if line.indent_length == 0 {
// 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.
// `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
// `isort` defaults to 2 if before a class or function definition (except in stubs where it is one) and 1 otherwise.
// Defaulting to 2 (or 1 in stubs) 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)
u32::try_from(self.lines_after_imports).unwrap_or(max_lines_level)
} else {
BLANK_LINES_TOP_LEVEL
max_lines_level
}
} else {
BLANK_LINES_NESTED_LEVEL
max_lines_level
};

if line.preceding_blank_lines < expected_blank_lines_before_definition
Expand All @@ -775,6 +816,8 @@ impl<'a> BlankLinesChecker<'a> {
&& line.indent_length == 0
// Only apply to functions or classes.
&& 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()
{
// E302
let mut diagnostic = Diagnostic::new(
Expand Down Expand Up @@ -804,12 +847,6 @@ impl<'a> BlankLinesChecker<'a> {
diagnostics.push(diagnostic);
}

let max_lines_level = if line.indent_length == 0 {
BLANK_LINES_TOP_LEVEL
} else {
BLANK_LINES_NESTED_LEVEL
};

// 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
Expand Down Expand Up @@ -893,6 +930,8 @@ impl<'a> BlankLinesChecker<'a> {
&& line.indent_length == 0
&& !line.is_comment_only
&& !line.kind.is_class_function_or_decorator()
// Blank lines in stub files are used for grouping, don't enforce blank lines.
&& !self.source_type.is_stub()
{
// E305
let mut diagnostic = Diagnostic::new(
Expand Down Expand Up @@ -933,6 +972,8 @@ impl<'a> BlankLinesChecker<'a> {
&& prev_indent_length.is_some_and(|prev_indent_length| prev_indent_length >= line.indent_length)
// Allow groups of one-liners.
&& !(matches!(state.follows, Follows::Def) && line.last_token != TokenKind::Colon)
// Blank lines in stub files are only used for grouping. Don't enforce blank lines.
&& !self.source_type.is_stub()
{
// E306
let mut diagnostic =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---

Loading
Loading