Skip to content

Commit

Permalink
Avoid no-self-use for attrs-style validators (#13166)
Browse files Browse the repository at this point in the history
## Summary

Closes #12568.
  • Loading branch information
charliermarsh committed Aug 30, 2024
1 parent 34dafb6 commit a73bebc
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
20 changes: 20 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,23 @@ def bar(self):
def baz(self):
if super().foo():
...


# See: https://github.com/astral-sh/ruff/issues/12568
from attrs import define, field


@define
class Foo:
x: int = field()
y: int

@x.validator
def validate_x(self, attribute, value):
if value <= 0:
raise ValueError("x must be a positive integer")

@y.validator
def validate_y(self, attribute, value):
if value <= 0:
raise ValueError("y must be a positive integer")
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub(crate) fn no_self_use(
|| visibility::is_override(decorator_list, semantic)
|| visibility::is_overload(decorator_list, semantic)
|| visibility::is_property(decorator_list, extra_property_decorators, semantic)
|| visibility::is_validator(decorator_list, semantic)
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,11 @@ no_self_use.py:13:9: PLR6301 Method `greeting_2` could be a function, class meth
14 | print("Hi!")
|


no_self_use.py:103:9: PLR6301 Method `validate_y` could be a function, class method, or static method
|
102 | @y.validator
103 | def validate_y(self, attribute, value):
| ^^^^^^^^^^ PLR6301
104 | if value <= 0:
105 | raise ValueError("y must be a positive integer")
|
23 changes: 22 additions & 1 deletion crates/ruff_python_semantic/src/analyze/visibility.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ruff_python_ast::{self as ast, Decorator};
use ruff_python_ast::{self as ast, Decorator, Expr};

use ruff_python_ast::helpers::map_callable;
use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
Expand Down Expand Up @@ -90,6 +90,27 @@ where
})
}

/// Returns `true` if a function definition is an `attrs`-like validator based on its decorators.
pub fn is_validator(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
decorator_list.iter().any(|decorator| {
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = &decorator.expression else {
return false;
};

if attr.as_str() != "validator" {
return false;
}

let Expr::Name(value) = value.as_ref() else {
return false;
};

semantic
.resolve_name(value)
.is_some_and(|id| semantic.binding(id).kind.is_assignment())
})
}

/// Returns `true` if a class is an `final`.
pub fn is_final(decorator_list: &[Decorator], semantic: &SemanticModel) -> bool {
decorator_list
Expand Down

0 comments on commit a73bebc

Please sign in to comment.