Skip to content

Commit

Permalink
Improve handling of metaclasses in various linter rules (#12579)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood authored Jul 30, 2024
1 parent ac1666d commit 7a4419a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 29 deletions.
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

0 comments on commit 7a4419a

Please sign in to comment.