Skip to content

Improve handling of metaclasses in various linter rules #12579

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

Merged
merged 1 commit into from
Jul 30, 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
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,9 @@ class Foo(enum.Enum):
@functools.cache
def bar(self, arg: str) -> str:
return f"{self} - {arg}"


class Metaclass(type):
@functools.lru_cache
def lru_cached_instance_method_on_metaclass(cls, x: int):
...
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,11 @@ B019.py:106:5: B019 Use of `functools.lru_cache` or `functools.cache` on methods
108 | ...
|


B019.py:124:5: B019 Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks
|
123 | class Metaclass(type):
124 | @functools.lru_cache
| ^^^^^^^^^^^^^^^^^^^^ B019
125 | def lru_cached_instance_method_on_metaclass(cls, x: int):
126 | ...
|
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ pub(crate) fn non_self_return_type(
};

// PEP 673 forbids the use of `typing(_extensions).Self` in metaclasses.
if is_metaclass(class_def, semantic) {
if analyze::class::is_metaclass(class_def, semantic) {
return;
}

Expand Down Expand Up @@ -219,16 +219,6 @@ pub(crate) fn non_self_return_type(
}
}

/// Returns `true` if the given class is a metaclass.
fn is_metaclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
analyze::class::any_qualified_name(class_def, semantic, &|qualified_name| {
matches!(
qualified_name.segments(),
["" | "builtins", "type"] | ["abc", "ABCMeta"] | ["enum", "EnumMeta" | "EnumType"]
)
})
}

/// Returns `true` if the method is an in-place binary operator.
fn is_inplace_bin_op(name: &str) -> bool {
matches!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::ParameterWithDefault;
use ruff_python_codegen::Stylist;
use ruff_python_semantic::analyze::class::is_metaclass;
use ruff_python_semantic::analyze::function_type;
use ruff_python_semantic::{Scope, ScopeKind, SemanticModel};
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -190,22 +191,34 @@ pub(crate) fn invalid_first_argument_name(
panic!("Expected ScopeKind::Function")
};

let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else {
let semantic = checker.semantic();

let Some(parent_scope) = semantic.first_non_type_parent_scope(scope) else {
return;
};

let ScopeKind::Class(parent) = parent_scope.kind else {
return;
};

let function_type = match function_type::classify(
name,
decorator_list,
parent,
checker.semantic(),
parent_scope,
semantic,
&checker.settings.pep8_naming.classmethod_decorators,
&checker.settings.pep8_naming.staticmethod_decorators,
) {
function_type::FunctionType::Function | function_type::FunctionType::StaticMethod => {
return;
}
function_type::FunctionType::Method => FunctionType::Method,
function_type::FunctionType::Method => {
if is_metaclass(parent, semantic) {
FunctionType::ClassMethod
} else {
FunctionType::Method
}
}
function_type::FunctionType::ClassMethod => FunctionType::ClassMethod,
};
if !checker.enabled(function_type.rule()) {
Expand Down
10 changes: 10 additions & 0 deletions crates/ruff_python_semantic/src/analyze/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,13 @@ pub fn is_enumeration(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -
)
})
}

/// Returns `true` if the given class is a metaclass.
pub fn is_metaclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
any_qualified_name(class_def, semantic, &|qualified_name| {
matches!(
qualified_name.segments(),
["" | "builtins", "type"] | ["abc", "ABCMeta"] | ["enum", "EnumMeta" | "EnumType"]
)
})
}
17 changes: 4 additions & 13 deletions crates/ruff_python_semantic/src/analyze/function_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
use ruff_python_ast::{Decorator, Expr, Stmt, StmtExpr, StmtFunctionDef, StmtRaise};

use crate::model::SemanticModel;
use crate::scope::{Scope, ScopeKind};
use crate::scope::Scope;

#[derive(Debug, Copy, Clone)]
pub enum FunctionType {
Expand All @@ -17,29 +17,20 @@ pub enum FunctionType {
pub fn classify(
name: &str,
decorator_list: &[Decorator],
scope: &Scope,
parent_scope: &Scope,
semantic: &SemanticModel,
classmethod_decorators: &[String],
staticmethod_decorators: &[String],
) -> FunctionType {
let ScopeKind::Class(class_def) = &scope.kind else {
if !parent_scope.kind.is_class() {
return FunctionType::Function;
};
if decorator_list
.iter()
.any(|decorator| is_static_method(decorator, semantic, staticmethod_decorators))
{
FunctionType::StaticMethod
} else if matches!(name, "__new__" | "__init_subclass__" | "__class_getitem__")
// Special-case class method, like `__new__`.
|| class_def.bases().iter().any(|expr| {
// The class itself extends a known metaclass, so all methods are class methods.
semantic
.resolve_qualified_name(map_callable(expr))
.is_some_and( |qualified_name| {
matches!(qualified_name.segments(), ["" | "builtins", "type"] | ["abc", "ABCMeta"])
})
})
} else if matches!(name, "__new__" | "__init_subclass__" | "__class_getitem__") // Special-case class method, like `__new__`.
|| decorator_list.iter().any(|decorator| is_class_method(decorator, semantic, classmethod_decorators))
{
FunctionType::ClassMethod
Expand Down
Loading