Port verification rule updates from smithy-python

Author: kuhe

scripts/verifications/dunder_all.py

@@ -10,7 +10,9 @@
 never declared public. Unresolvable names are already caught by ruff F822.
 
 Only modules that declare ``__all__`` are checked. A name counts as public when it
-is defined in the module and has no leading underscore.
+is defined in the module and has no leading underscore. In a package
+``__init__``, an import the module never uses exists only to re-export, so it must
+be listed in ``__all__``. A used import may be left out as a private re-export.
 """
 
 import ast
@@ -47,7 +49,7 @@ def check() -> list[Finding]:
         for name, imp in sorted(module.imported.items()):
             if name.startswith("_") or imp.top not in owned_roots() or imp.name is None:
                 continue
-            # An import the module itself uses is a dependency, not a re-export.
+            # A used import is a private re-export, and a TYPE_CHECKING one is unbound.
             if name not in exported_set and (
                 imp.context == "type_checking" or name in used
             ):
@@ -58,10 +60,10 @@ def check() -> list[Finding]:
             if name not in exported_set:
                 findings.append(
                     Finding(
-                        f"re-exported {name} (from {target}) is missing from __all__",
+                        f"{name} (from {target}) is imported only to re-export but "
+                        "is missing from __all__",
                         module.path,
                         imp.line,
-                        "warning",
                     )
                 )
                 continue
@@ -84,15 +86,34 @@ def check() -> list[Finding]:
 
 def _used_names(tree: ast.Module) -> set[str]:
     used = {n.id for n in ast.walk(tree) if isinstance(n, ast.Name)}
-    for node in ast.walk(tree):
-        if isinstance(node, ast.Constant) and isinstance(node.value, str):
-            try:
-                annotation = ast.parse(node.value, mode="eval")
-            except SyntaxError:
-                continue
-            used.update(n.id for n in ast.walk(annotation) if isinstance(n, ast.Name))
+    for annotation in _annotations(tree):
+        for node in ast.walk(annotation):
+            if isinstance(node, ast.Constant) and isinstance(node.value, str):
+                try:
+                    parsed = ast.parse(node.value, mode="eval")
+                except SyntaxError:
+                    continue
+                used.update(n.id for n in ast.walk(parsed) if isinstance(n, ast.Name))
     return used
 
 
+def _annotations(tree: ast.Module) -> list[ast.expr]:
+    """Annotation expressions, where a string literal is a forward reference."""
+    found: list[ast.expr] = []
+    for node in ast.walk(tree):
+        match node:
+            case ast.arg(annotation=ann) | ast.AnnAssign(annotation=ann) if ann:
+                found.append(ann)
+            case ast.FunctionDef(returns=ret) | ast.AsyncFunctionDef(returns=ret) if (
+                ret
+            ):
+                found.append(ret)
+            case ast.TypeAlias(value=value):
+                found.append(value)
+            case _:
+                pass
+    return found
+
+
 if __name__ == "__main__":
     main("__all__ consistency", check)

scripts/verifications/export_names.py

@@ -42,10 +42,8 @@ def check() -> list[Finding]:
                 )
             )
             continue
-        # Without __all__, an imported name is the module's dependency, not its export.
-        imported = by_name[modname].imported
         names = getattr(module, "__all__", None) or [
-            n for n in vars(module) if not n.startswith("_") and n not in imported
+            n for n in vars(module) if not n.startswith("_")
         ]
         stub_only = [
             n

scripts/verifications/version_consistency.py

@@ -7,12 +7,15 @@
 gates read; the distribution version is what resolvers and PyPI read. If they
 disagree, telemetry and support misattribute behavior to the wrong release. The
 version should also come from one place, sourced the same way in every package.
+
+Everything is read from the source tree, because installed metadata lags the
+source whenever an editable install isn't rebuilt after a version bump.
 """
 
-import importlib
-import importlib.metadata
+import ast
+from pathlib import Path
 
-from _common import Finding, main, packages
+from _common import Finding, Package, main, packages
 
 
 def check() -> list[Finding]:
@@ -29,41 +32,92 @@ def check() -> list[Finding]:
                     pyproject,
                 )
             )
-        elif package.version_file is None:
+            findings.extend(_check_static(package, str(project.get("version"))))
+        else:
+            findings.extend(_check_dynamic(package))
+    return findings
+
+
+def _check_dynamic(package: Package) -> list[Finding]:
+    """Hatch reads the version file, so every root must agree with its literal."""
+    pyproject = package.dir / "pyproject.toml"
+    version_file = package.version_file
+    if version_file is None:
+        return [
+            Finding(
+                f"{package.dist}: dynamic version without a hatch version path",
+                pyproject,
+            )
+        ]
+    if not version_file.exists():
+        return [
+            Finding(f"{package.dist}: hatch version path does not exist", pyproject)
+        ]
+    expected = _literal_version(version_file)
+    if expected is None:
+        return [
+            Finding(
+                f"{package.dist}: hatch version path has no literal __version__",
+                version_file,
+            )
+        ]
+    findings: list[Finding] = []
+    for root in package.roots:
+        init = package.src / root / "__init__.py"
+        found = _literal_version(init)
+        if found is None:
+            findings.append(Finding(f"{root} defines no literal __version__", init))
+        elif found != expected:
             findings.append(
                 Finding(
-                    f"{package.dist}: dynamic version without a hatch version path",
-                    pyproject,
+                    f"{root}.__version__ is {found!r}, distribution version is {expected!r}",
+                    init,
                 )
             )
-        if package.source_version is None:
+    return findings
+
+
+def _check_static(package: Package, expected: str) -> list[Finding]:
+    """A static version is a second source, so every root must still agree with it."""
+    findings: list[Finding] = []
+    for root in package.roots:
+        init = package.src / root / "__init__.py"
+        found = _literal_version(init)
+        if found is None:
+            findings.append(Finding(f"{root} defines no literal __version__", init))
+        elif found != expected:
             findings.append(
                 Finding(
-                    f"{package.dist}: __version__ is not a literal in the package source",
-                    pyproject,
-                    severity="warning",
+                    f"{root}.__version__ is {found!r}, static version is {expected!r}",
+                    init,
                 )
             )
-        try:
-            installed = importlib.metadata.version(package.dist)
-        except importlib.metadata.PackageNotFoundError:
-            findings.append(Finding(f"{package.dist}: not installed", pyproject))
-            continue
-        for root in package.roots:
-            runtime = getattr(importlib.import_module(root), "__version__", None)
-            if runtime is None:
-                findings.append(
-                    Finding(f"{root} defines no __version__", package.src / root)
-                )
-            elif runtime != installed:
-                findings.append(
-                    Finding(
-                        f"{root}.__version__ is {runtime!r}, distribution version is {installed!r}",
-                        package.src / root / "__init__.py",
-                    )
-                )
     return findings
 
 
+def _version_value(path: Path) -> ast.expr | None:
+    if not path.exists():
+        return None
+    for node in ast.parse(path.read_text()).body:
+        match node:
+            case ast.Assign(targets=targets, value=value) if any(
+                isinstance(t, ast.Name) and t.id == "__version__" for t in targets
+            ):
+                return value
+            # Generated clients annotate it: ``__version__: str = "..."``.
+            case ast.AnnAssign(target=ast.Name(id="__version__"), value=value):
+                return value
+            case _:
+                pass
+    return None
+
+
+def _literal_version(path: Path) -> str | None:
+    value = _version_value(path)
+    if isinstance(value, ast.Constant) and isinstance(value.value, str):
+        return value.value
+    return None
+
+
 if __name__ == "__main__":
     main("version consistency", check)