Skip to content

Commit

Permalink
Don't flag B009/B010 if identifiers would be mangled (#2204)
Browse files Browse the repository at this point in the history
  • Loading branch information
sciyoshi committed Jan 26, 2023
1 parent 0ad6b82 commit 7370a27
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 50 deletions.
8 changes: 6 additions & 2 deletions resources/test/fixtures/flake8_bugbear/B009_B010.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -13,10 +13,12 @@
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")
Expand All @@ -27,6 +29,7 @@
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)
Expand All @@ -36,6 +39,7 @@
# 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)
9 changes: 9 additions & 0 deletions src/python/identifiers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("__")
}
5 changes: 3 additions & 2 deletions src/rules/flake8_bugbear/rules/getattr_with_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 3 additions & 2 deletions src/rules/flake8_bugbear/rules/setattr_with_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

0 comments on commit 7370a27

Please sign in to comment.