mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-24 13:33:50 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Improve TypedDict conversion logic for shadowed builtins and dunder methods (#5038)

Browse source 
## Summary

This PR (1) avoids flagging `TypedDict` and `NamedTuple` conversions
when attributes are dunder methods, like `__dict__`, and (2) avoids
flagging the `A003` shadowed-attribute rule for `TypedDict` classes at
all, where it doesn't really apply (since those attributes are only
accessed via subscripting anyway).

Closes #5027.
This commit is contained in:
Charlie Marsh 2023-06-12 17:23:39 -04:00 • committed by GitHub
parent 4080f36850
commit ab11dd08df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 78 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

9
crates/ruff/resources/test/fixtures/flake8_builtins/A003.py vendored
View file

@ -1,6 +1,6 @@
class MyClass:
ImportError = 4
id = 5
id: int
dir = "/"
def __init__(self):
@ -10,3 +10,10 @@ class MyClass:
def str(self):
pass
from typing import TypedDict
class MyClass(TypedDict):
id: int

6
crates/ruff/src/checkers/ast/mod.rs
View file

@ -656,10 +656,11 @@ where
pyupgrade::rules::yield_in_for_loop(self, stmt);
}
if self.semantic_model.scope().kind.is_class() {
if let ScopeKind::Class(class_def) = self.semantic_model.scope().kind {
if self.enabled(Rule::BuiltinAttributeShadowing) {
flake8_builtins::rules::builtin_attribute_shadowing(
self,
class_def,
name,
AnyShadowing::from(stmt),
);
@ -2369,10 +2370,11 @@ where
}
}
if self.semantic_model.scope().kind.is_class() {
if let ScopeKind::Class(class_def) = self.semantic_model.scope().kind {
if self.enabled(Rule::BuiltinAttributeShadowing) {
flake8_builtins::rules::builtin_attribute_shadowing(
self,
class_def,
id,
AnyShadowing::from(expr),
);

2
crates/ruff/src/rules/flake8_builtins/helpers.rs
View file

@ -1,9 +1,9 @@
use ruff_text_size::TextRange;
use rustpython_parser::ast::{Excepthandler, Expr, Ranged, Stmt};
use ruff_python_ast::helpers::identifier_range;
use ruff_python_ast::source_code::Locator;
use ruff_python_stdlib::builtins::BUILTINS;
use ruff_text_size::TextRange;
pub(super) fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool {
BUILTINS.contains(&name) && ignorelist.iter().all(|ignore| ignore != name)

12
crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs
View file

@ -1,6 +1,7 @@
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use rustpython_parser::ast;
use crate::checkers::ast::Checker;
@ -64,10 +65,21 @@ impl Violation for BuiltinAttributeShadowing {
/// A003
pub(crate) fn builtin_attribute_shadowing(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
name: &str,
shadowing: AnyShadowing,
) {
if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) {
// Ignore shadowing within `TypedDict` definitions, since these are only accessible through
// subscripting and not through attribute access.
if class_def.bases.iter().any(|base| {
checker
.semantic_model()
.match_typing_expr(base, "TypedDict")
}) {
return;
}
checker.diagnostics.push(Diagnostic::new(
BuiltinAttributeShadowing {
name: name.to_string(),

6
crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap
View file

@ -6,7 +6,7 @@ A003.py:2:5: A003 Class attribute `ImportError` is shadowing a Python builtin
1 | class MyClass:
2 | ImportError = 4
| ^^^^^^^^^^^ A003
3 | id = 5
3 | id: int
4 | dir = "/"
|
@ -14,7 +14,7 @@ A003.py:3:5: A003 Class attribute `id` is shadowing a Python builtin
|
1 | class MyClass:
2 | ImportError = 4
3 | id = 5
3 | id: int
| ^^ A003
4 | dir = "/"
|
@ -22,7 +22,7 @@ A003.py:3:5: A003 Class attribute `id` is shadowing a Python builtin
A003.py:4:5: A003 Class attribute `dir` is shadowing a Python builtin
|
2 | ImportError = 4
3 | id = 5
3 | id: int
4 | dir = "/"
| ^^^ A003
5 |

2
crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap
View file

@ -6,7 +6,7 @@ A003.py:2:5: A003 Class attribute `ImportError` is shadowing a Python builtin
1 | class MyClass:
2 | ImportError = 4
| ^^^^^^^^^^^ A003
3 | id = 5
3 | id: int
4 | dir = "/"
|

27
crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs
View file

@ -5,6 +5,7 @@ use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Keyword, Ranged,
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_dunder;
use ruff_python_ast::source_code::Generator;
use ruff_python_semantic::model::SemanticModel;
use ruff_python_stdlib::identifiers::is_identifier;
@ -66,19 +67,21 @@ fn create_property_assignment_stmt(
annotation: &Expr,
value: Option<&Expr>,
) -> Stmt {
let node = ast::ExprName {
ast::StmtAnnAssign {
target: Box::new(
ast::ExprName {
id: property.into(),
ctx: ExprContext::Load,
range: TextRange::default(),
};
let node1 = ast::StmtAnnAssign {
target: Box::new(node.into()),
}
.into(),
),
annotation: Box::new(annotation.clone()),
value: value.map(|value| Box::new(value.clone())),
simple: true,
range: TextRange::default(),
};
node1.into()
}
.into()
}
/// Match the `defaults` keyword in a `NamedTuple(...)` call.
@ -140,6 +143,9 @@ fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result<Vec<S
if !is_identifier(property) {
bail!("Invalid property name: {}", property)
}
if is_dunder(property) {
bail!("Cannot use dunder property name: {}", property)
}
Ok(create_property_assignment_stmt(
property, annotation, default,
))
@ -150,15 +156,15 @@ fn create_properties_from_args(args: &[Expr], defaults: &[Expr]) -> Result<Vec<S
/// Generate a `StmtKind:ClassDef` statement based on the provided body and
/// keywords.
fn create_class_def_stmt(typename: &str, body: Vec<Stmt>, base_class: &Expr) -> Stmt {
let node = ast::StmtClassDef {
ast::StmtClassDef {
name: typename.into(),
bases: vec![base_class.clone()],
keywords: vec![],
body,
decorator_list: vec![],
range: TextRange::default(),
};
node.into()
}
.into()
}
/// Generate a `Fix` to convert a `NamedTuple` assignment to a class definition.
@ -169,8 +175,7 @@ fn convert_to_class(
base_class: &Expr,
generator: Generator,
) -> Fix {
#[allow(deprecated)]
Fix::unspecified(Edit::range_replacement(
Fix::suggested(Edit::range_replacement(
generator.stmt(&create_class_def_stmt(typename, body, base_class)),
stmt.range(),
))

49
crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs
View file

@ -5,6 +5,7 @@ use rustpython_parser::ast::{self, Constant, Expr, ExprContext, Keyword, Ranged,
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_dunder;
use ruff_python_ast::source_code::Generator;
use ruff_python_semantic::model::SemanticModel;
use ruff_python_stdlib::identifiers::is_identifier;
@ -62,20 +63,21 @@ fn match_typed_dict_assign<'a>(
/// Generate a `Stmt::AnnAssign` representing the provided property
/// definition.
fn create_property_assignment_stmt(property: &str, annotation: &Expr) -> Stmt {
let node = annotation.clone();
let node1 = ast::ExprName {
ast::StmtAnnAssign {
target: Box::new(
ast::ExprName {
id: property.into(),
ctx: ExprContext::Load,
range: TextRange::default(),
};
let node2 = ast::StmtAnnAssign {
target: Box::new(node1.into()),
annotation: Box::new(node),
}
.into(),
),
annotation: Box::new(annotation.clone()),
value: None,
simple: true,
range: TextRange::default(),
};
node2.into()
}
.into()
}
/// Generate a `StmtKind:ClassDef` statement based on the provided body,
@ -90,15 +92,15 @@ fn create_class_def_stmt(
Some(keyword) => vec![keyword.clone()],
None => vec![],
};
let node = ast::StmtClassDef {
ast::StmtClassDef {
name: class_name.into(),
bases: vec![base_class.clone()],
keywords,
body,
decorator_list: vec![],
range: TextRange::default(),
};
node.into()
}
.into()
}
fn properties_from_dict_literal(keys: &[Option<Expr>], values: &[Expr]) -> Result<Vec<Stmt>> {
@ -116,11 +118,13 @@ fn properties_from_dict_literal(keys: &[Option<Expr>], values: &[Expr]) -> Resul
value: Constant::Str(property),
..
})) => {
if is_identifier(property) {
Ok(create_property_assignment_stmt(property, value))
} else {
bail!("Property name is not valid identifier: {}", property)
if !is_identifier(property) {
bail!("Invalid property name: {}", property)
}
if is_dunder(property) {
bail!("Cannot use dunder property name: {}", property)
}
Ok(create_property_assignment_stmt(property, value))
}
_ => bail!("Expected `key` to be `Constant::Str`"),
})
@ -170,12 +174,12 @@ fn properties_from_keywords(keywords: &[Keyword]) -> Result<Vec<Stmt>> {
// TypedDict('name', {'a': int}, total=True)
// ```
fn match_total_from_only_keyword(keywords: &[Keyword]) -> Option<&Keyword> {
let keyword = keywords.get(0)?;
let arg = &keyword.arg.as_ref()?;
match arg.as_str() {
"total" => Some(keyword),
_ => None,
}
keywords.iter().find(|keyword| {
let Some(arg) = &keyword.arg else {
return false
};
arg.as_str() == "total"
})
}
fn match_properties_and_total<'a>(
@ -219,8 +223,7 @@ fn convert_to_class(
base_class: &Expr,
generator: Generator,
) -> Fix {
#[allow(deprecated)]
Fix::unspecified(Edit::range_replacement(
Fix::suggested(Edit::range_replacement(
generator.stmt(&create_class_def_stmt(
class_name,
body,

2
crates/ruff_python_ast/src/helpers.rs
View file

@ -546,7 +546,7 @@ where
body.iter().any(|stmt| any_over_stmt(stmt, func))
}
fn is_dunder(id: &str) -> bool {
pub fn is_dunder(id: &str) -> bool {
id.starts_with("__") && id.ends_with("__")
}