ref: simplify and type flake8 plugin (#35645)

mypy.ini

@@ -104,7 +104,8 @@ 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/
+        tests/sentry/utils/appleconnect/,
+        tools/
 
 ; Enable all options used with --strict
 warn_unused_configs=True

tests/tools/test_flake8_plugin.py

@@ -1,15 +1,11 @@
 import ast
 
-from tools.flake8_plugin import S001, S002, SentryCheck
+from tools.flake8_plugin import SentryCheck
 
 
 def _run(src):
     tree = ast.parse(src)
-    return list(SentryCheck(tree=tree).run())
-
-
-def _errors(*errors):
-    return [SentryCheck.adapt_error(e) for e in errors]
+    return sorted("t.py:{}:{}: {}".format(*error) for error in SentryCheck(tree=tree).run())
 
 
 def test_S001():
@@ -23,7 +19,10 @@ A().called_once()
 """
 
     errors = _run(S001_py)
-    assert errors == _errors(S001(6, 0, vars=("called_once",)))
+    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.",
+    ]
 
 
 def test_S002():
@@ -32,7 +31,7 @@ print("print statements are not allowed")
 """
 
     errors = _run(S002_py)
-    assert errors == _errors(S002(1, 0))
+    assert errors == ["t.py:1:0: S002 print functions or statements are not allowed."]
 
 
 def test_S003():
@@ -54,28 +53,8 @@ def bad_code():
 
     errors = _run(S003_py)
     assert errors == [
-        (
-            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,
-        ),
+        "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.",
     ]

tools/flake8_plugin.py

@@ -1,31 +1,49 @@
+from __future__ import annotations
+
 import ast
-from collections import namedtuple
-from functools import partial
+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"}
 
 
 class SentryVisitor(ast.NodeVisitor):
-    def __init__(self):
-        self.errors = []
+    def __init__(self) -> None:
+        self.errors: list[tuple[int, int, str]] = []
 
-    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
+    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_Import(self, node):
+        self.generic_visit(node)
+
+    def visit_Import(self, node: ast.Import) -> None:
         for alias in node.names:
-            if alias.name.split(".", 1)[0] in S003.modules:
-                self.errors.append(S003(node.lineno, node.col_offset))
+            if alias.name.split(".")[0] in S003_modules:
+                self.errors.append((node.lineno, node.col_offset, S003_msg))
+
+        self.generic_visit(node)
 
-    def visit_Attribute(self, node):
-        if node.attr in S001.methods:
-            self.errors.append(S001(node.lineno, node.col_offset, vars=(node.attr,)))
+    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_Name(self, node):
+        self.generic_visit(node)
+
+    def visit_Name(self, node: ast.Name) -> None:
         if node.id == "print":
-            self.errors.append(S002(lineno=node.lineno, col=node.col_offset))
+            self.errors.append((node.lineno, node.col_offset, S002_msg))
+
+        self.generic_visit(node)
 
 
 class SentryCheck:
@@ -35,43 +53,9 @@ class SentryCheck:
     def __init__(self, tree: ast.AST) -> None:
         self.tree = tree
 
-    def run(self):
+    def run(self) -> Generator[tuple[int, int, str, type[Any]], None, None]:
         visitor = SentryVisitor()
         visitor.visit(self.tree)
 
         for e in visitor.errors:
-            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",
-}
+            yield (*e, type(self))