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

[flake8-class-newline] Add new flake8 plugin. #10033

Closed
wants to merge 11 commits into from
Closed
21 changes: 21 additions & 0 deletions LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -1371,3 +1371,24 @@ are:
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
"""

- flake8-class-newline, licensed as follows:
"""
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
"""
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
class A: # CNL100
def __init__(self):
pass


class B: # correct

def __init__(self):
pass


class C: # CNL100
async def foo(self):
pass


class D: # correct

async def foo(self):
pass
16 changes: 14 additions & 2 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use crate::directives::TodoComment;
use crate::registry::{AsRule, Rule};
use crate::rules::pycodestyle::rules::BlankLinesChecker;
use crate::rules::{
eradicate, flake8_commas, flake8_executable, flake8_fixme, flake8_implicit_str_concat,
flake8_pyi, flake8_quotes, flake8_todos, pycodestyle, pygrep_hooks, pylint, pyupgrade, ruff,
eradicate, flake8_class_newline, flake8_commas, flake8_executable, flake8_fixme,
flake8_implicit_str_concat, flake8_pyi, flake8_quotes, flake8_todos, pycodestyle, pygrep_hooks,
pylint, pyupgrade, ruff,
};
use crate::settings::LinterSettings;

Expand Down Expand Up @@ -209,6 +210,17 @@ pub(crate) fn check_tokens(
flake8_fixme::rules::todos(&mut diagnostics, &todo_comments);
}

if !source_type.is_stub() && settings.rules.enabled(Rule::MissingClassNewLine) {
let mut blank_lines_checker = flake8_class_newline::rules::BlankLinesChecker::default();
blank_lines_checker.check_lines(
tokens,
locator,
stylist,
settings.tab_size,
&mut diagnostics,
);
}

diagnostics.retain(|diagnostic| settings.rules.enabled(diagnostic.kind.rule()));

diagnostics
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Logging, "007") => (RuleGroup::Stable, rules::flake8_logging::rules::ExceptionWithoutExcInfo),
(Flake8Logging, "009") => (RuleGroup::Stable, rules::flake8_logging::rules::UndocumentedWarn),

// flake8-class-newline
(Flake8ClassNewLine, "100") => (RuleGroup::Preview, rules::flake8_class_newline::rules::MissingClassNewLine),

_ => return None,
})
}
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ pub enum Linter {
/// [flake8-builtins](https://pypi.org/project/flake8-builtins/)
#[prefix = "A"]
Flake8Builtins,
/// [flake8-class-newline](https://pypi.org/project/flake8-class-newline/)
#[prefix = "CNL"]
Flake8ClassNewLine,
/// [flake8-commas](https://pypi.org/project/flake8-commas/)
#[prefix = "COM"]
Flake8Commas,
Expand Down Expand Up @@ -282,6 +285,7 @@ impl Rule {
| Rule::LineContainsHack
| Rule::LineContainsTodo
| Rule::LineContainsXxx
| Rule::MissingClassNewLine
| Rule::MissingSpaceAfterTodoColon
| Rule::MissingTodoAuthor
| Rule::MissingTodoColon
Expand Down
25 changes: 25 additions & 0 deletions crates/ruff_linter/src/rules/flake8_class_newline/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! Rules from [flake8-class-newline](https://pypi.org/project/flake8-class-newline/).
pub(crate) mod rules;

#[cfg(test)]
mod tests {
use std::path::Path;

use anyhow::Result;
use test_case::test_case;

use crate::registry::Rule;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Path::new("CNL100.py"))]
fn rules(path: &Path) -> Result<()> {
let snapshot = path.to_string_lossy().into_owned();
let diagnostics = test_path(
Path::new("flake8_class_newline").join(path).as_path(),
&settings::LinterSettings::for_rule(Rule::MissingClassNewLine),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
use ruff_diagnostics::AlwaysFixableViolation;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Edit;
use ruff_diagnostics::Fix;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_codegen::Stylist;
use ruff_python_parser::lexer::LexResult;
use ruff_source_file::Locator;

use crate::line_width::IndentWidth;
use crate::rules::pycodestyle::rules::{LinePreprocessor, LogicalLineInfo, LogicalLineKind};

/// ## What it does
/// Checks for a missing blank line between a class definition and its first method.
///
/// ## Why is this bad?
/// PEP8 says we should surround every class method with a single blank line.
/// However it is ambiguous about the first method having a blank line above it.
/// This rule enforces a single blank line, for consistency across classes.
Comment on lines +17 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should exclude pyi files from this rule because empty lines in stub files should only be used for grouping methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

///
/// ## Example
/// ```python
/// class MyClass(object):
/// def func1():
/// pass
/// ```
///
/// Use instead:
/// ```python
/// class MyClass(object):
///
/// def func1():
/// pass
/// ```
///
/// ## References
/// - [PEP 8](https://peps.python.org/pep-0008/#blank-lines)
#[violation]
pub struct MissingClassNewLine;

impl AlwaysFixableViolation for MissingClassNewLine {
#[derive_message_formats]
fn message(&self) -> String {
format!("Expected 1 blank line after class declaration, found 0")
}

fn fix_title(&self) -> String {
"Add missing blank line".to_string()
}
}

#[derive(Copy, Clone, Debug, Default)]
enum Follows {
#[default]
Class,
Other,
}

/// Contains variables used for the linting of blank lines.
#[derive(Debug, Default)]
pub(crate) struct BlankLinesChecker {
follows: Follows,
}

impl BlankLinesChecker {
pub(crate) fn check_lines(
&mut self,
tokens: &[LexResult],
locator: &Locator,
stylist: &Stylist,
indent_width: IndentWidth,
diagnostics: &mut Vec<Diagnostic>,
) {
let line_preprocessor = LinePreprocessor::new(tokens, locator, indent_width);

for logical_line in line_preprocessor {
self.check_new_line_after_class_declaration(
&logical_line,
locator,
stylist,
diagnostics,
);
}
}

fn check_new_line_after_class_declaration(
&mut self,
line: &LogicalLineInfo,
locator: &Locator,
stylist: &Stylist,
diagnostics: &mut Vec<Diagnostic>,
) {
if (matches!(self.follows, Follows::Class)
&& matches!(
line.kind,
LogicalLineKind::Function | LogicalLineKind::Decorator
)
&& line.preceding_blank_lines == 0)
{
let mut diagnostic = Diagnostic::new(MissingClassNewLine, line.first_token_range);
diagnostic.set_fix(Fix::safe_edit(Edit::insertion(
stylist.line_ending().to_string(),
locator.line_start(line.first_token_range.start()),
)));

diagnostics.push(diagnostic);
}

// Update the `self.follows` state based on the current line
match line.kind {
LogicalLineKind::Class => self.follows = Follows::Class,
_ => self.follows = Follows::Other,
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub(crate) use missing_class_newline::*;

mod missing_class_newline;
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/ruff_linter/src/rules/flake8_class_newline/mod.rs
---
CNL100.py:2:5: CNL100 [*] Expected 1 blank line after class declaration, found 0
|
1 | class A: # CNL100
2 | def __init__(self):
| ^^^ CNL100
3 | pass
|
= help: Add missing blank line

ℹ Safe fix
1 1 | class A: # CNL100
2 |+
2 3 | def __init__(self):
3 4 | pass
4 5 |

CNL100.py:13:5: CNL100 [*] Expected 1 blank line after class declaration, found 0
|
12 | class C: # CNL100
13 | async def foo(self):
| ^^^^^ CNL100
14 | pass
|
= help: Add missing blank line

ℹ Safe fix
10 10 |
11 11 |
12 12 | class C: # CNL100
13 |+
13 14 | async def foo(self):
14 15 | pass
15 16 |


1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod flake8_blind_except;
pub mod flake8_boolean_trap;
pub mod flake8_bugbear;
pub mod flake8_builtins;
pub mod flake8_class_newline;
pub mod flake8_commas;
pub mod flake8_comprehensions;
pub mod flake8_copyright;
Expand Down
30 changes: 15 additions & 15 deletions crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,39 +304,39 @@ impl AlwaysFixableViolation for BlankLinesBeforeNestedDefinition {
}

#[derive(Debug)]
struct LogicalLineInfo {
kind: LogicalLineKind,
first_token_range: TextRange,
pub(crate) struct LogicalLineInfo {
pub(crate) kind: LogicalLineKind,
pub(crate) first_token_range: TextRange,

// The token's kind right before the newline ending the logical line.
last_token: TokenKind,
pub(crate) last_token: TokenKind,

// The end of the logical line including the newline.
logical_line_end: TextSize,
pub(crate) logical_line_end: TextSize,

// `true` if this is not a blank but only consists of a comment.
is_comment_only: bool,
pub(crate) is_comment_only: bool,

/// `true` if the line is a string only (including trivia tokens) line, which is a docstring if coming right after a class/function definition.
is_docstring: bool,
pub(crate) is_docstring: bool,

/// The indentation length in columns. See [`expand_indent`] for the computation of the indent.
indent_length: usize,
pub(crate) indent_length: usize,

/// The number of blank lines preceding the current line.
blank_lines: BlankLines,
pub(crate) blank_lines: BlankLines,

/// The maximum number of consecutive blank lines between the current line
/// and the previous non-comment logical line.
/// One of its main uses is to allow a comments to directly precede or follow a class/function definition.
/// As such, `preceding_blank_lines` is used for rules that cannot trigger on comments (all rules except E303),
/// and `blank_lines` is used for the rule that can trigger on comments (E303).
preceding_blank_lines: BlankLines,
pub(crate) preceding_blank_lines: BlankLines,
}

/// Iterator that processes tokens until a full logical line (or comment line) is "built".
/// It then returns characteristics of that logical line (see `LogicalLineInfo`).
struct LinePreprocessor<'a> {
pub(crate) struct LinePreprocessor<'a> {
tokens: Iter<'a, Result<(Tok, TextRange), LexicalError>>,
locator: &'a Locator<'a>,
indent_width: IndentWidth,
Expand All @@ -348,7 +348,7 @@ struct LinePreprocessor<'a> {
}

impl<'a> LinePreprocessor<'a> {
fn new(
pub(crate) fn new(
tokens: &'a [LexResult],
locator: &'a Locator,
indent_width: IndentWidth,
Expand Down Expand Up @@ -482,7 +482,7 @@ impl<'a> Iterator for LinePreprocessor<'a> {
}

#[derive(Clone, Copy, Debug, Default)]
enum BlankLines {
pub(crate) enum BlankLines {
/// No blank lines
#[default]
Zero,
Expand Down Expand Up @@ -564,7 +564,7 @@ enum Follows {
}

#[derive(Copy, Clone, Debug, Default)]
enum Status {
pub(crate) enum Status {
/// Stores the indent level where the nesting started.
Inside(usize),
/// This is used to rectify a Inside switched to a Outside because of a dedented comment.
Expand Down Expand Up @@ -872,7 +872,7 @@ impl BlankLinesChecker {
}

#[derive(Copy, Clone, Debug)]
enum LogicalLineKind {
pub(crate) enum LogicalLineKind {
/// The clause header of a class definition
Class,
/// A decorator
Expand Down
4 changes: 4 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading