Don't flag B009/B010 if identifiers would be mangled (#2204)

This commit is contained in:
Samuel Cormier-Iijima 2023-01-26 12:56:18 -05:00 committed by GitHub
parent 0ad6b8224d
commit 7370a27c09
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 101 additions and 50 deletions

View File

@ -1,7 +1,7 @@
"""
Should emit:
B009 - Line 18, 19, 20, 21, 22
B010 - Line 33, 34, 35, 36
B009 - Line 19, 20, 21, 22, 23, 24
B010 - Line 40, 41, 42, 43, 44, 45
"""
# Valid getattr usage
@ -13,10 +13,12 @@ getattr(foo, bar, None)
getattr(foo, "123abc")
getattr(foo, r"123\abc")
getattr(foo, "except")
getattr(foo, "__123abc")
# Invalid usage
getattr(foo, "bar")
getattr(foo, "_123abc")
getattr(foo, "__123abc__")
getattr(foo, "abc123")
getattr(foo, r"abc123")
_ = lambda x: getattr(x, "bar")
@ -27,6 +29,7 @@ if getattr(x, "bar"):
setattr(foo, bar, None)
setattr(foo, "bar{foo}".format(foo="a"), None)
setattr(foo, "123abc", None)
setattr(foo, "__123abc", None)
setattr(foo, r"123\abc", None)
setattr(foo, "except", None)
_ = lambda x: setattr(x, "bar", 1)
@ -36,6 +39,7 @@ if setattr(x, "bar", 1):
# Invalid usage
setattr(foo, "bar", None)
setattr(foo, "_123abc", None)
setattr(foo, "__123abc__", None)
setattr(foo, "abc123", None)
setattr(foo, r"abc123", None)
setattr(foo.bar, r"baz", None)

View File

@ -13,3 +13,12 @@ pub fn is_identifier(s: &str) -> bool {
// Are the rest of the characters letters, digits, or underscores?
s.chars().skip(1).all(|c| c.is_alphanumeric() || c == '_')
}
/// Returns `true` if a string is a private identifier, such that, when the
/// identifier is defined in a class definition, it will be mangled prior to
/// code generation.
///
/// See: <https://docs.python.org/3.5/reference/expressions.html?highlight=mangling#index-5>.
pub fn is_mangled_private(id: &str) -> bool {
id.starts_with("__") && !id.ends_with("__")
}

View File

@ -4,7 +4,7 @@ use crate::ast::helpers::unparse_expr;
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::python::identifiers::is_identifier;
use crate::python::identifiers::{is_identifier, is_mangled_private};
use crate::python::keyword::KWLIST;
use crate::registry::Diagnostic;
use crate::violations;
@ -41,12 +41,13 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
if !is_identifier(value) {
return;
}
if KWLIST.contains(&value.as_str()) {
if KWLIST.contains(&value.as_str()) || is_mangled_private(value.as_str()) {
return;
}
let mut diagnostic =
Diagnostic::new(violations::GetAttrWithConstant, Range::from_located(expr));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
unparse_expr(&attribute(obj, value), checker.stylist),

View File

@ -4,7 +4,7 @@ use crate::ast::helpers::unparse_stmt;
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::python::identifiers::is_identifier;
use crate::python::identifiers::{is_identifier, is_mangled_private};
use crate::python::keyword::KWLIST;
use crate::registry::Diagnostic;
use crate::source_code::Stylist;
@ -51,7 +51,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
if !is_identifier(name) {
return;
}
if KWLIST.contains(&name.as_str()) {
if KWLIST.contains(&name.as_str()) || is_mangled_private(name.as_str()) {
return;
}
// We can only replace a `setattr` call (which is an `Expr`) with an assignment
@ -61,6 +61,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
if expr == child.as_ref() {
let mut diagnostic =
Diagnostic::new(violations::SetAttrWithConstant, Range::from_located(expr));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
assignment(obj, name, value, checker.stylist),

View File

@ -5,109 +5,127 @@ expression: diagnostics
- kind:
GetAttrWithConstant: ~
location:
row: 18
row: 19
column: 0
end_location:
row: 18
row: 19
column: 19
fix:
content:
- foo.bar
location:
row: 18
row: 19
column: 0
end_location:
row: 18
row: 19
column: 19
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 19
row: 20
column: 0
end_location:
row: 19
row: 20
column: 23
fix:
content:
- foo._123abc
location:
row: 19
row: 20
column: 0
end_location:
row: 19
row: 20
column: 23
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 20
row: 21
column: 0
end_location:
row: 20
row: 21
column: 26
fix:
content:
- foo.__123abc__
location:
row: 21
column: 0
end_location:
row: 21
column: 26
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 22
column: 0
end_location:
row: 22
column: 22
fix:
content:
- foo.abc123
location:
row: 20
row: 22
column: 0
end_location:
row: 20
row: 22
column: 22
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 21
row: 23
column: 0
end_location:
row: 21
row: 23
column: 23
fix:
content:
- foo.abc123
location:
row: 21
row: 23
column: 0
end_location:
row: 21
row: 23
column: 23
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 22
row: 24
column: 14
end_location:
row: 22
row: 24
column: 31
fix:
content:
- x.bar
location:
row: 22
row: 24
column: 14
end_location:
row: 22
row: 24
column: 31
parent: ~
- kind:
GetAttrWithConstant: ~
location:
row: 23
row: 25
column: 3
end_location:
row: 23
row: 25
column: 20
fix:
content:
- x.bar
location:
row: 23
row: 25
column: 3
end_location:
row: 23
row: 25
column: 20
parent: ~

View File

@ -5,91 +5,109 @@ expression: diagnostics
- kind:
SetAttrWithConstant: ~
location:
row: 37
row: 40
column: 0
end_location:
row: 37
row: 40
column: 25
fix:
content:
- foo.bar = None
location:
row: 37
row: 40
column: 0
end_location:
row: 37
row: 40
column: 25
parent: ~
- kind:
SetAttrWithConstant: ~
location:
row: 38
row: 41
column: 0
end_location:
row: 38
row: 41
column: 29
fix:
content:
- foo._123abc = None
location:
row: 38
row: 41
column: 0
end_location:
row: 38
row: 41
column: 29
parent: ~
- kind:
SetAttrWithConstant: ~
location:
row: 39
row: 42
column: 0
end_location:
row: 39
row: 42
column: 32
fix:
content:
- foo.__123abc__ = None
location:
row: 42
column: 0
end_location:
row: 42
column: 32
parent: ~
- kind:
SetAttrWithConstant: ~
location:
row: 43
column: 0
end_location:
row: 43
column: 28
fix:
content:
- foo.abc123 = None
location:
row: 39
row: 43
column: 0
end_location:
row: 39
row: 43
column: 28
parent: ~
- kind:
SetAttrWithConstant: ~
location:
row: 40
row: 44
column: 0
end_location:
row: 40
row: 44
column: 29
fix:
content:
- foo.abc123 = None
location:
row: 40
row: 44
column: 0
end_location:
row: 40
row: 44
column: 29
parent: ~
- kind:
SetAttrWithConstant: ~
location:
row: 41
row: 45
column: 0
end_location:
row: 41
row: 45
column: 30
fix:
content:
- foo.bar.baz = None
location:
row: 41
row: 45
column: 0
end_location:
row: 41
row: 45
column: 30
parent: ~