Browse Source

Revert "ref: simplify and type flake8 plugin (#35645)" (#35651)

Evan Purkhiser 2 years ago
parent
commit
8b9bcdc92d
3 changed files with 87 additions and 51 deletions
  1. 1 2
      mypy.ini
  2. 32 11
      tests/tools/test_flake8_plugin.py
  3. 54 38
      tools/flake8_plugin.py

+ 1 - 2
mypy.ini

@@ -104,8 +104,7 @@ files = src/sentry/analytics/,
         tests/sentry/lang/native/test_appconnect.py,
         tests/sentry/processing/realtime_metrics/,
         tests/sentry/tasks/test_low_priority_symbolication.py,
-        tests/sentry/utils/appleconnect/,
-        tools/
+        tests/sentry/utils/appleconnect/
 
 ; Enable all options used with --strict
 warn_unused_configs=True

+ 32 - 11
tests/tools/test_flake8_plugin.py

@@ -1,11 +1,15 @@
 import ast
 
-from tools.flake8_plugin import SentryCheck
+from tools.flake8_plugin import S001, S002, SentryCheck
 
 
 def _run(src):
     tree = ast.parse(src)
-    return sorted("t.py:{}:{}: {}".format(*error) for error in SentryCheck(tree=tree).run())
+    return list(SentryCheck(tree=tree).run())
+
+
+def _errors(*errors):
+    return [SentryCheck.adapt_error(e) for e in errors]
 
 
 def test_S001():
@@ -19,10 +23,7 @@ A().called_once()
 """
 
     errors = _run(S001_py)
-    assert errors == [
-        "t.py:6:0: S001 Avoid using the called_once mock call as it is confusing and "
-        "prone to causing invalid test behavior.",
-    ]
+    assert errors == _errors(S001(6, 0, vars=("called_once",)))
 
 
 def test_S002():
@@ -31,7 +32,7 @@ print("print statements are not allowed")
 """
 
     errors = _run(S002_py)
-    assert errors == ["t.py:1:0: S002 print functions or statements are not allowed."]
+    assert errors == _errors(S002(1, 0))
 
 
 def test_S003():
@@ -53,8 +54,28 @@ def bad_code():
 
     errors = _run(S003_py)
     assert errors == [
-        "t.py:1:0: S003 Use ``from sentry.utils import json`` instead.",
-        "t.py:2:0: S003 Use ``from sentry.utils import json`` instead.",
-        "t.py:3:0: S003 Use ``from sentry.utils import json`` instead.",
-        "t.py:4:0: S003 Use ``from sentry.utils import json`` instead.",
+        (
+            1,
+            0,
+            "S003: Use ``from sentry.utils import json`` instead.",
+            SentryCheck,
+        ),
+        (
+            2,
+            0,
+            "S003: Use ``from sentry.utils import json`` instead.",
+            SentryCheck,
+        ),
+        (
+            3,
+            0,
+            "S003: Use ``from sentry.utils import json`` instead.",
+            SentryCheck,
+        ),
+        (
+            4,
+            0,
+            "S003: Use ``from sentry.utils import json`` instead.",
+            SentryCheck,
+        ),
     ]

+ 54 - 38
tools/flake8_plugin.py

@@ -1,49 +1,31 @@
-from __future__ import annotations
-
 import ast
-from typing import Any, Generator
-
-S001_fmt = (
-    "S001 Avoid using the {} mock call as it is "
-    "confusing and prone to causing invalid test "
-    "behavior."
-)
-S001_methods = frozenset(("not_called", "called_once", "called_once_with"))
-
-S002_msg = "S002 print functions or statements are not allowed."
-
-S003_msg = "S003 Use ``from sentry.utils import json`` instead."
-S003_modules = {"json", "simplejson"}
+from collections import namedtuple
+from functools import partial
 
 
 class SentryVisitor(ast.NodeVisitor):
-    def __init__(self) -> None:
-        self.errors: list[tuple[int, int, str]] = []
+    def __init__(self):
+        self.errors = []
 
-    def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
-        if node.module and node.module.split(".")[0] in S003_modules:
-            self.errors.append((node.lineno, node.col_offset, S003_msg))
+    def visit_ImportFrom(self, node):
+        if node.module in S003.modules:
+            for nameproxy in node.names:
+                if nameproxy.name in S003.names:
+                    self.errors.append(S003(node.lineno, node.col_offset))
+                    break
 
-        self.generic_visit(node)
-
-    def visit_Import(self, node: ast.Import) -> None:
+    def visit_Import(self, node):
         for alias in node.names:
-            if alias.name.split(".")[0] in S003_modules:
-                self.errors.append((node.lineno, node.col_offset, S003_msg))
-
-        self.generic_visit(node)
+            if alias.name.split(".", 1)[0] in S003.modules:
+                self.errors.append(S003(node.lineno, node.col_offset))
 
-    def visit_Attribute(self, node: ast.Attribute) -> None:
-        if node.attr in S001_methods:
-            self.errors.append((node.lineno, node.col_offset, S001_fmt.format(node.attr)))
+    def visit_Attribute(self, node):
+        if node.attr in S001.methods:
+            self.errors.append(S001(node.lineno, node.col_offset, vars=(node.attr,)))
 
-        self.generic_visit(node)
-
-    def visit_Name(self, node: ast.Name) -> None:
+    def visit_Name(self, node):
         if node.id == "print":
-            self.errors.append((node.lineno, node.col_offset, S002_msg))
-
-        self.generic_visit(node)
+            self.errors.append(S002(lineno=node.lineno, col=node.col_offset))
 
 
 class SentryCheck:
@@ -53,9 +35,43 @@ class SentryCheck:
     def __init__(self, tree: ast.AST) -> None:
         self.tree = tree
 
-    def run(self) -> Generator[tuple[int, int, str, type[Any]], None, None]:
+    def run(self):
         visitor = SentryVisitor()
         visitor.visit(self.tree)
 
         for e in visitor.errors:
-            yield (*e, type(self))
+            yield self.adapt_error(e)
+
+    @classmethod
+    def adapt_error(cls, e):
+        """Adapts the extended error namedtuple to be compatible with Flake8."""
+        return e._replace(message=e.message.format(*e.vars))[:4]
+
+
+error = namedtuple("error", "lineno col message type vars")
+Error = partial(partial, error, message="", type=SentryCheck, vars=())
+
+S001 = Error(
+    message="S001: Avoid using the {} mock call as it is "
+    "confusing and prone to causing invalid test "
+    "behavior."
+)
+S001.methods = {
+    "not_called",
+    "called_once",
+    "called_once_with",
+}
+
+S002 = Error(message="S002: print functions or statements are not allowed.")
+
+S003 = Error(message="S003: Use ``from sentry.utils import json`` instead.")
+S003.modules = {"json", "simplejson"}
+S003.names = {
+    "load",
+    "loads",
+    "dump",
+    "dumps",
+    "JSONEncoder",
+    "JSONDecodeError",
+    "_default_encoder",
+}