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

[ruff] Implemented wrong-class-body-content (RUF050) #14607

Closed
Closed
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
133 changes: 133 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import math # Import statements can appear at the top of a file or within a class

class ExampleClass:
"""A class demonstrating all kinds of statements."""

# Class attributes (class variables)
class_var1 = "class-level attribute"
class_var2 = 42

# Instance attributes will be defined in the constructor
def __init__(self, instance_var):
"""Constructor to initialize instance attributes."""
self.instance_var = instance_var # Instance variable
self.computed_var = self._helper_method() # Using a helper method in initialization

# Instance method
def instance_method(self):
"""Regular instance method."""
print("This is an instance method.")
self._private_method()

# Private method (conventionally prefixed with an underscore)
def _private_method(self):
"""A private helper method."""
print("This is a private method.")

# Protected method (by convention, a single leading underscore indicates protected)
def _protected_method(self):
print("This is a protected method.")

# Static method
@staticmethod
def static_method():
"""A static method."""
print("Static method called. No access to instance or class data.")

# Class method
@classmethod
def class_method(cls):
"""A class method."""
print(f"Class method called. class_var1 = {cls.class_var1}")

# Special method (dunder methods)
def __str__(self):
"""Special method for string representation."""
return f"ExampleClass(instance_var={self.instance_var}, computed_var={self.computed_var})"

def __len__(self):
"""Special method to define the behavior of len()."""
return len(self.instance_var)

# Nested class
class NestedClass:
"""A class defined within another class."""
def nested_method(self):
print("Method of a nested class.")

# Pass statement (used as a placeholder)
def placeholder_method(self):
"""A method with no implementation yet."""
pass

# Try/Except block inside a method
def error_handling_method(self):
"""A method with error handling."""
try:
result = 10 / 0 # Intentional error
except ZeroDivisionError as e:
print(f"Caught an exception: {e}")
finally:
print("Cleanup actions can be placed here.")

# Using a decorator
@property
def readonly_property(self):
"""A read-only property."""
return self.computed_var

@readonly_property.setter
def readonly_property(self, value):
"""Attempt to set this property raises an error."""
raise AttributeError("This property is read-only.")

# Conditional logic inside the class (not recommended but valid)
if math.pi > 3:
def conditionally_defined_method(self):
print("This method exists because math.pi > 3.")

# For loop inside the class (unusual but valid)
for i in range(3):
exec(f"def loop_method_{i}(self): print('Loop-defined method {i}')")

# Docstrings for the class
"""
Additional class-level comments can be included in the docstring.
"""

# List comprehensions inside a class (valid but rarely used)
squares = [x ** 2 for x in range(5)]

# Lambda functions inside a class (unusual but valid)
double = lambda x: x * 2


g = 'module attribute (module-global variable)'
"""This is g's docstring."""

class AClass:

c = 'class attribute'
"""This is AClass.c's docstring."""

def __init__(self):
"""Method __init__'s docstring."""

self.i = 'instance attribute'
"""This is self.i's docstring."""

class BClass:
...

class CClass:
pass

class DClass:
"""Empty class."""

def f(x):
"""Function f's docstring."""
return x**2

f.a = 1
"""Function attribute f.a's docstring."""
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::FunctionCallInDataclassDefaultArgument) {
ruff::rules::function_call_in_dataclass_default(checker, class_def);
}
if checker.enabled(Rule::WrongClassBodyContent) {
ruff::rules::wrong_class_body_content(checker, class_def);
}
if checker.enabled(Rule::FStringDocstring) {
flake8_bugbear::rules::f_string_docstring(checker, body);
}
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -976,9 +976,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
(Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
(Ruff, "038") => (RuleGroup::Preview, rules::ruff::rules::RedundantBoolLiteral),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "039") => (RuleGroup::Preview, rules::ruff::rules::UnrawRePattern),
(Ruff, "040") => (RuleGroup::Preview, rules::ruff::rules::InvalidAssertMessageLiteralArgument),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "050") => (RuleGroup::Preview, rules::ruff::rules::WrongClassBodyContent),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ mod tests {
#[test_case(Rule::RedundantBoolLiteral, Path::new("RUF038.py"))]
#[test_case(Rule::RedundantBoolLiteral, Path::new("RUF038.pyi"))]
#[test_case(Rule::InvalidAssertMessageLiteralArgument, Path::new("RUF040.py"))]
#[test_case(Rule::WrongClassBodyContent, Path::new("RUF050.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) use unsafe_markup_use::*;
pub(crate) use unused_async::*;
pub(crate) use unused_noqa::*;
pub(crate) use useless_if_else::*;
pub(crate) use wrong_class_body_content::*;
pub(crate) use zip_instead_of_pairwise::*;

mod ambiguous_unicode_character;
Expand Down Expand Up @@ -82,6 +83,7 @@ mod unsafe_markup_use;
mod unused_async;
mod unused_noqa;
mod useless_if_else;
mod wrong_class_body_content;
mod zip_instead_of_pairwise;

#[derive(Clone, Copy)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{helpers::is_docstring_stmt, Stmt, StmtClassDef, StmtExpr};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for disallowed statements in the body of a class.
///
/// ## Why is this bad?
/// Python allows us to have conditions, context managers,
/// and even infinite loops inside class definitions.
/// On the other hand, only methods, attributes, and docstrings make sense.
/// So, we discourage using anything except these nodes in class bodies.
///
/// ## Example
/// ```python
/// class Test:
/// for _ in range(10):
/// print("What?!")
/// ```
///
/// ## References
/// - [WPS: wrong class body content](https://wemake-python-styleguide.readthedocs.io/en/0.19.2/pages/usage/violations/oop.html#wemake_python_styleguide.violations.oop.WrongClassBodyContentViolation)
#[violation]
pub struct WrongClassBodyContent;

impl Violation for WrongClassBodyContent {
#[derive_message_formats]
fn message(&self) -> String {
"Wrong statement inside class definition".to_string()
}
}

/// RUF050
pub(crate) fn wrong_class_body_content(checker: &mut Checker, class: &StmtClassDef) {
let StmtClassDef { body, .. } = class;
let is_stub = checker.source_type.is_stub();
for stmt in body {
if !is_docstring_stmt(stmt) && !is_allowed_statement(stmt, is_stub) {
checker
.diagnostics
.push(Diagnostic::new(WrongClassBodyContent, stmt.range()));
}
}
}

fn is_allowed_statement(stmt: &Stmt, is_stub: bool) -> bool {
match stmt {
Stmt::FunctionDef(_)
| Stmt::ClassDef(_)
| Stmt::Assign(_)
| Stmt::AnnAssign(_)
| Stmt::Pass(_) => true,
Stmt::Expr(StmtExpr { value, .. }) => value.is_ellipsis_literal_expr(),
Stmt::If(_) => {
// If statement are allowed in stubs
is_stub
// TODO: allow also use of if for type checking
}
_ => false,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF050.py:85:5: RUF050 Wrong statement inside class definition
|
84 | # Conditional logic inside the class (not recommended but valid)
85 | if math.pi > 3:
| _____^
86 | | def conditionally_defined_method(self):
87 | | print("This method exists because math.pi > 3.")
| |____________________________________________________________^ RUF050
88 |
89 | # For loop inside the class (unusual but valid)
|

RUF050.py:90:5: RUF050 Wrong statement inside class definition
|
89 | # For loop inside the class (unusual but valid)
90 | for i in range(3):
| _____^
91 | | exec(f"def loop_method_{i}(self): print('Loop-defined method {i}')")
| |____________________________________________________________________________^ RUF050
92 |
93 | # Docstrings for the class
|
2 changes: 2 additions & 0 deletions ruff.schema.json

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