Silence B009/B010 for non-identifiers (#116)

If an object implements __getattr__ and/or __setattr__, the second
parameter to getattr() or setattr() may not be a valid Python
identifier (e.g. getattr(foo, "123abc"). In this case bugbear should
not emit B009/B010, since changing to "foo.123abc" would be a
SyntaxError. Update the B009/B010 detection to check using a regex.

Author: pkolbus

bugbear.py

@@ -5,6 +5,7 @@
 from functools import lru_cache, partial
 import itertools
 import logging
+import re
 
 import attr
 import pycodestyle
@@ -110,6 +111,16 @@ def should_warn(self, code):
         return False
 
 
+def _is_identifier(arg):
+    # Return True if arg is a valid identifier, per
+    # https://docs.python.org/2/reference/lexical_analysis.html#identifiers
+
+    if not isinstance(arg, ast.Str):
+        return False
+
+    return re.match(r"^[A-Za-z_][A-Za-z0-9_]*$", arg.s) is not None
+
+
 def _to_name_str(node):
     # Turn Name and Attribute nodes to strings, e.g "ValueError" or
     # "pkg.mod.error", handling any depth of attribute accesses.
@@ -213,13 +224,13 @@ def visit_Call(self, node):
                 if (
                     node.func.id == "getattr"
                     and len(node.args) == 2  # noqa: W503
-                    and isinstance(node.args[1], ast.Str)  # noqa: W503
+                    and _is_identifier(node.args[1])  # noqa: W503
                 ):
                     self.errors.append(B009(node.lineno, node.col_offset))
                 elif (
                     node.func.id == "setattr"
                     and len(node.args) == 3  # noqa: W503
-                    and isinstance(node.args[1], ast.Str)  # noqa: W503
+                    and _is_identifier(node.args[1])  # noqa: W503
                 ):
                     self.errors.append(B010(node.lineno, node.col_offset))
 

tests/b009_b010.py

@@ -1,7 +1,7 @@
 """
 Should emit:
-B009 - Line 15
-B010 - Line 22
+B009 - Line 16, 17, 18
+B010 - Line 26, 27, 28
 """
 
 # Valid getattr usage
@@ -10,13 +10,19 @@
 getattr(foo, "bar{foo}".format(foo="a"), None)
 getattr(foo, "bar{foo}".format(foo="a"))
 getattr(foo, bar, None)
+getattr(foo, "123abc")
 
 # Invalid usage
 getattr(foo, "bar")
+getattr(foo, "_123abc")
+getattr(foo, "abc123")
 
 # Valid setattr usage
 setattr(foo, bar, None)
 setattr(foo, "bar{foo}".format(foo="a"), None)
+setattr(foo, "123abc", None)
 
 # Invalid usage
 setattr(foo, "bar", None)
+setattr(foo, "_123abc", None)
+setattr(foo, "abc123", None)

tests/test_bugbear.py

@@ -123,7 +123,15 @@ def test_b009_b010(self):
         filename = Path(__file__).absolute().parent / "b009_b010.py"
         bbc = BugBearChecker(filename=str(filename))
         errors = list(bbc.run())
-        self.assertEqual(errors, self.errors(B009(15, 0), B010(22, 0)))
+        all_errors = [
+            B009(16, 0),
+            B009(17, 0),
+            B009(18, 0),
+            B010(26, 0),
+            B010(27, 0),
+            B010(28, 0),
+        ]
+        self.assertEqual(errors, self.errors(*all_errors))
 
     def test_b011(self):
         filename = Path(__file__).absolute().parent / "b011.py"