From 549af935642c2f12cd3a1ad488df925329e84109 Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 10 Nov 2026 16:33:28 +0000 Subject: [PATCH 1/4] Add verification suite --- .gitignore | 1 + Makefile | 127 +++- scripts/verifications/_common.py | 706 +++++++++++++++++++ scripts/verifications/_probe.py | 219 ++++++ scripts/verifications/banned_imports.py | 63 ++ scripts/verifications/canonical_imports.py | 96 +++ scripts/verifications/declared_deps.py | 81 +++ scripts/verifications/dep_specifiers.py | 106 +++ scripts/verifications/docstrings.py | 152 ++++ scripts/verifications/dunder_all.py | 98 +++ scripts/verifications/dynamic_imports.py | 44 ++ scripts/verifications/exception_hierarchy.py | 100 +++ scripts/verifications/export_names.py | 88 +++ scripts/verifications/import-cost.json | 66 ++ scripts/verifications/import_cost.py | 122 ++++ scripts/verifications/import_cycles.py | 106 +++ scripts/verifications/import_deprecations.py | 44 ++ scripts/verifications/import_hygiene.py | 77 ++ scripts/verifications/import_side_effects.py | 50 ++ scripts/verifications/lowest_bounds.py | 143 ++++ scripts/verifications/optional_deps.py | 76 ++ scripts/verifications/package_metadata.py | 95 +++ scripts/verifications/secret_repr.py | 97 +++ scripts/verifications/source_order.py | 165 +++++ scripts/verifications/type_completeness.py | 127 ++++ scripts/verifications/version_consistency.py | 69 ++ scripts/verifications/wheel_contents.py | 117 +++ 27 files changed, 3234 insertions(+), 1 deletion(-) create mode 100644 scripts/verifications/_common.py create mode 100644 scripts/verifications/_probe.py create mode 100644 scripts/verifications/banned_imports.py create mode 100644 scripts/verifications/canonical_imports.py create mode 100644 scripts/verifications/declared_deps.py create mode 100644 scripts/verifications/dep_specifiers.py create mode 100644 scripts/verifications/docstrings.py create mode 100644 scripts/verifications/dunder_all.py create mode 100644 scripts/verifications/dynamic_imports.py create mode 100644 scripts/verifications/exception_hierarchy.py create mode 100644 scripts/verifications/export_names.py create mode 100644 scripts/verifications/import-cost.json create mode 100644 scripts/verifications/import_cost.py create mode 100644 scripts/verifications/import_cycles.py create mode 100644 scripts/verifications/import_deprecations.py create mode 100644 scripts/verifications/import_hygiene.py create mode 100644 scripts/verifications/import_side_effects.py create mode 100644 scripts/verifications/lowest_bounds.py create mode 100644 scripts/verifications/optional_deps.py create mode 100644 scripts/verifications/package_metadata.py create mode 100644 scripts/verifications/secret_repr.py create mode 100644 scripts/verifications/source_order.py create mode 100644 scripts/verifications/type_completeness.py create mode 100644 scripts/verifications/version_consistency.py create mode 100644 scripts/verifications/wheel_contents.py diff --git a/.gitignore b/.gitignore index 5c16ff0b..7a1e72c6 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ docs/clients/ # Virtual environments .venv +.venv-verify venv # Caches diff --git a/Makefile b/Makefile index f0316ad5..d2314ddf 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,11 @@ SMITHY_PYTHON ?= ../smithy-python # Clients per gradle build in `make clients`; lower it if the build runs out of heap. BATCH_SIZE ?= 25 -.PHONY: docs docs-serve docs-clean docs-install docs-lock venv gen clients fragments publish-codegen aio-check meta-package-check static-analysis test-unit test-functional test-integration +.PHONY: docs docs-serve docs-clean docs-install docs-lock venv gen clients fragments publish-codegen aio-check meta-package-check static-analysis test-unit test-functional test-integration \ + check-imports check-conventions check-api check-dependencies check-packaging check-import-stability verify-venv \ + canonical-imports dunder-all source-order dynamic-imports banned-imports docstrings declared-deps import-cycles \ + dep-specifiers package-metadata version-consistency export-names exception-hierarchy secret-repr import-hygiene \ + import-deprecations import-side-effects import-cost wheel-contents type-completeness optional-deps lowest-bounds # Each client and package runs in its own .venv. Dependencies outside this repo, # including smithy-python, resolve from PyPI at the versions each one declares. @@ -31,6 +35,12 @@ static-analysis: done # TODO: enable once aws-credentials-sts passes the aio naming rules. # $(MAKE) aio-check + $(MAKE) check-imports + $(MAKE) check-conventions + $(MAKE) check-api + $(MAKE) check-dependencies + $(MAKE) check-packaging + $(MAKE) check-import-stability test-unit: @set -e; for p in $(PROJECTS); do \ @@ -93,6 +103,121 @@ aio-check: meta-package-check: uv run --no-project --python $(PYTHON_VERSION) python scripts/meta-package/meta_package.py +check-imports: ## Keeps the import graph explicit, acyclic, and free of test-only modules. + $(MAKE) canonical-imports +# $(MAKE) import-cycles # TODO: enable once it passes +# $(MAKE) dynamic-imports # TODO: enable once it passes + $(MAKE) banned-imports + +check-conventions: ## Holds shipped code to our ordering, docstring, error, and secret-handling conventions. +# $(MAKE) source-order # TODO: enable once it passes +# $(MAKE) docstrings # TODO: enable once it passes + $(MAKE) exception-hierarchy +# $(MAKE) secret-repr # TODO: enable once it passes + +check-api: ## Keeps the public API surface typed and exported under its own names. + $(MAKE) export-names + $(MAKE) dunder-all +# $(MAKE) type-completeness # TODO: enable once it passes + +check-dependencies: ## Keeps declared dependencies honest for customers who install only some packages. +# $(MAKE) declared-deps # TODO: enable once it passes + $(MAKE) dep-specifiers + $(MAKE) optional-deps + $(MAKE) lowest-bounds + +check-packaging: ## Keeps what we publish to PyPI consistent across packages. +# $(MAKE) package-metadata # TODO: enable once it passes +# $(MAKE) version-consistency # TODO: enable once it passes +# $(MAKE) wheel-contents # TODO: enable once it passes + +check-import-stability: ## Keeps importing our packages cheap and free of side effects. + $(MAKE) import-hygiene + $(MAKE) import-deprecations + $(MAKE) import-side-effects + $(MAKE) import-cost + +# The verifications import, build and inspect every client and package together, +# so they share one venv holding all of them editable, plus the pinned tools. +VERIFY_VENV := .venv-verify +VERIFY_STAMP := $(VERIFY_VENV)/.installed +VERIFY = $(VERIFY_VENV)/bin/python scripts/verifications + +$(VERIFY_STAMP): $(addsuffix /pyproject.toml,$(PROJECTS)) + uv venv --allow-existing --quiet --python $(PYTHON_VERSION) $(VERIFY_VENV) + uv pip install --quiet --python $(VERIFY_VENV)/bin/python --all-extras \ + packaging==25.0 ruff==$(RUFF_VERSION) pyright==$(PYRIGHT_VERSION) \ + $(foreach p,$(PROJECTS),-e $(p) -r $(p)/pyproject.toml) + touch $@ + +verify-venv: $(VERIFY_STAMP) ## Installs every client and package into the shared verification venv. + +canonical-imports: $(VERIFY_STAMP) ## Prevents re-export imports, which hide where symbols live and invite cycles. + $(VERIFY)/canonical_imports.py + +dunder-all: $(VERIFY_STAMP) ## Keeps __all__ in sync with the public surface that type checkers and docs read. + $(VERIFY)/dunder_all.py + +source-order: $(VERIFY_STAMP) ## Keeps the public API at the top of every file and class. + $(VERIFY)/source_order.py + +dynamic-imports: $(VERIFY_STAMP) ## Keeps every import visible to static tools. + $(VERIFY)/dynamic_imports.py + +banned-imports: $(VERIFY_STAMP) ## Keeps test-only modules out of shipped code. + $(VERIFY)/banned_imports.py + +docstrings: $(VERIFY_STAMP) ## Keeps the public API documented in editors and the API reference. + $(VERIFY)/docstrings.py + +declared-deps: $(VERIFY_STAMP) ## Catches imports that only work because something else installed the dependency. + $(VERIFY)/declared_deps.py + +import-cycles: $(VERIFY_STAMP) ## Catches cycles that make import success depend on import order. + $(VERIFY)/import_cycles.py + +dep-specifiers: $(VERIFY_STAMP) ## Keeps packages co-installable and in-repo version bounds current. + $(VERIFY)/dep_specifiers.py + +package-metadata: $(VERIFY_STAMP) ## Catches pyproject.toml drift between packages. + $(VERIFY)/package_metadata.py + +version-consistency: $(VERIFY_STAMP) ## Keeps __version__ truthful for user agents and bug reports. + $(VERIFY)/version_consistency.py + +export-names: $(VERIFY_STAMP) ## Catches objects exported under a name other than their own. + $(VERIFY)/export_names.py + +exception-hierarchy: $(VERIFY_STAMP) ## Lets customers catch every SDK error with one except clause. + $(VERIFY)/exception_hierarchy.py + +secret-repr: $(VERIFY_STAMP) ## Keeps credentials out of logs and tracebacks. + $(VERIFY)/secret_repr.py + +import-hygiene: $(VERIFY_STAMP) ## Catches modules that only import after a sibling has warmed sys.modules. + $(VERIFY)/import_hygiene.py + +import-deprecations: $(VERIFY_STAMP) ## Catches removals that Python or a dependency has already scheduled. + $(VERIFY)/import_deprecations.py + +import-side-effects: $(VERIFY_STAMP) ## Keeps imports from overriding the host application's global state. + $(VERIFY)/import_side_effects.py + +import-cost: $(VERIFY_STAMP) ## Catches growth in how many of our modules an import loads. + $(VERIFY)/import_cost.py + +wheel-contents: $(VERIFY_STAMP) ## Catches wheels that miss source or py.typed, or that ship tests. + $(VERIFY)/wheel_contents.py + +type-completeness: $(VERIFY_STAMP) ## Verifies the py.typed promise as a customer's type checker sees it. + $(VERIFY)/type_completeness.py + +optional-deps: $(VERIFY_STAMP) ## Checks that a minimal install without extras works or fails clearly. + $(VERIFY)/optional_deps.py + +lowest-bounds: $(VERIFY_STAMP) ## Proves the declared lower bounds actually work. + $(VERIFY)/lowest_bounds.py + venv: uv venv --python $(PYTHON_VERSION) diff --git a/scripts/verifications/_common.py b/scripts/verifications/_common.py new file mode 100644 index 00000000..8213a71d --- /dev/null +++ b/scripts/verifications/_common.py @@ -0,0 +1,706 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Shared package, module and import-graph index for the verification scripts.""" + +import ast +import functools +import importlib.metadata +import json +import os +import subprocess +import sys +import tomllib +from collections.abc import Callable, Iterable, Iterator +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass +from pathlib import Path +from types import ModuleType +from typing import Any, Literal + +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name +from packaging.version import Version + +HERE = Path(__file__).resolve().parent +ROOT = HERE.parent.parent +PROJECT_GLOBS = ("clients/*/pyproject.toml", "packages/*/pyproject.toml") +PROBE = HERE / "_probe.py" +# Matched with and without suffix, so a generated models.pyi stub counts too. +MODEL_DATA = ("models", "_private/schemas") +AIO = "aio" + +type Severity = Literal["error", "warning", "info"] +type Context = Literal["runtime", "type_checking", "deferred"] + + +@dataclass(frozen=True) +class Finding: + message: str + path: Path | None = None + line: int = 0 + severity: Severity = "error" + + def __str__(self) -> str: + where = "" + if self.path is not None: + where = ( + f"{relative(self.path)}:{self.line}: " + if self.line + else f"{relative(self.path)}: " + ) + return f"{where}{self.severity}: {self.message}" + + +@dataclass +class Package: + dir: Path + pyproject: dict[str, Any] + + @property + def dist(self) -> str: + return canonicalize_name(self.pyproject["project"]["name"]) + + @property + def src(self) -> Path: + return self.dir / "src" + + @property + def roots(self) -> list[str]: + return sorted( + p.name for p in self.src.iterdir() if (p / "__init__.py").exists() + ) + + @property + def dependencies(self) -> list[Requirement]: + return [ + Requirement(r) for r in self.pyproject["project"].get("dependencies", []) + ] + + @property + def extras(self) -> dict[str, list[Requirement]]: + optional = self.pyproject["project"].get("optional-dependencies", {}) + return {k: [Requirement(r) for r in v] for k, v in optional.items()} + + @property + def version_file(self) -> Path | None: + """The file hatch reads a dynamic version from.""" + path = ( + self.pyproject.get("tool", {}) + .get("hatch", {}) + .get("version", {}) + .get("path") + ) + return self.dir / path if path else None + + @property + def source_version(self) -> str | None: + """The literal ``__version__`` in the version file or package ``__init__``.""" + candidates = [self.version_file] + [ + self.src / r / "__init__.py" for r in self.roots + ] + for path in candidates: + if path is None or not path.exists(): + continue + for node in ast.parse(path.read_text()).body: + if ( + isinstance(node, ast.Assign) + and any( + isinstance(t, ast.Name) and t.id == "__version__" + for t in node.targets + ) + and isinstance(node.value, ast.Constant) + ): + return str(node.value.value) + return None + + @property + def current_version(self) -> Version | None: + project = self.pyproject["project"] + if "version" in project: + return Version(project["version"]) + found = self.source_version + return Version(found) if found else None + + @property + def extras_only(self) -> set[str]: + """Distributions declared only under extras.""" + required = {canonicalize_name(r.name) for r in self.dependencies} + return { + canonicalize_name(r.name) for reqs in self.extras.values() for r in reqs + } - required + + +@dataclass +class Import: + module: "Module" + target: str + name: str | None + asname: str | None + line: int + context: Context + guarded: bool + + @property + def top(self) -> str: + return self.target.split(".")[0] + + +@dataclass +class Module: + name: str + path: Path + src: Path + package: Package | None + + @functools.cached_property + def tree(self) -> ast.Module: + return ast.parse(self.path.read_bytes(), filename=str(self.path)) + + @property + def is_package(self) -> bool: + return self.path.stem == "__init__" + + @property + def public(self) -> bool: + return not any(p.startswith("_") for p in self.name.split(".")) + + @property + def in_aio(self) -> bool: + return AIO in self.name.split(".") + + @property + def model_data(self) -> bool: + """Whether this is a generated client's per-shape module. + + These hold most of the repo's source and repeat one codegen template per + shape, so AST analyses skip them; checks that import modules still cover them. + """ + return self.package is not None and is_model_data(self.package, self.path) + + @functools.cached_property + def imports(self) -> list[Import]: + if self.model_data: + return [] + return list(_ImportCollector(self).collect()) + + @functools.cached_property + def defined(self) -> set[str]: + """Names bound at module level by def, class, assignment or type alias. + + Includes a sibling stub's bindings: lazily built models exist only there. + """ + return set(_top_level_bindings(self.tree.body)) | set(self.stub_names) + + @functools.cached_property + def stub_names(self) -> list[str]: + """Names bound at module level in the sibling ``.pyi`` stub, in order. + + Lazily built members are in neither ``vars()`` nor ``dir()`` of the + module, so the stub is the only place that lists them. + """ + stub = self.path.with_suffix(".pyi") + if not stub.exists(): + return [] + body = ast.parse(stub.read_bytes(), filename=str(stub)).body + return list(dict.fromkeys(_top_level_bindings(body))) + + @functools.cached_property + def imported(self) -> dict[str, Import]: + """Names bound at module level by an import, keyed by bound name.""" + bound: dict[str, Import] = {} + for imp in self.imports: + if imp.context == "deferred": + continue + if imp.name is None: + key = imp.asname or imp.target.split(".")[0] + else: + key = imp.asname or imp.name + bound[key] = imp + return bound + + @functools.cached_property + def dunder_all(self) -> list[str] | None: + for node in self.tree.body: + targets: list[ast.expr] = [] + value: ast.expr | None = None + if isinstance(node, ast.Assign): + targets, value = node.targets, node.value + elif isinstance(node, ast.AnnAssign) and node.value is not None: + targets, value = [node.target], node.value + if any(isinstance(t, ast.Name) and t.id == "__all__" for t in targets): + if isinstance(value, ast.List | ast.Tuple): + return [ + e.value + for e in value.elts + if isinstance(e, ast.Constant) and isinstance(e.value, str) + ] + return None + + +def relative(path: Path) -> str: + try: + return str(path.relative_to(ROOT)) + except ValueError: + return str(path) + + +@functools.cache +def packages() -> list[Package]: + """Every generated client and hand-written package.""" + return [ + Package(p.parent, tomllib.loads(p.read_text())) + for g in PROJECT_GLOBS + for p in sorted(ROOT.glob(g)) + ] + + +@functools.cache +def package_by_dist() -> dict[str, Package]: + return {p.dist: p for p in packages()} + + +@functools.cache +def runtime_modules() -> list[Module]: + found: list[Module] = [] + for package in packages(): + found.extend(_load_src(package.src, package)) + return found + + +def is_model_data(package: Package, path: Path) -> bool: + """Whether ``path`` is one of a generated client's per-shape modules or their stubs.""" + if package.dir.parent.name != "clients": + return False + rel = path.relative_to(package.src).with_suffix("").as_posix().split("/", 1)[-1] + return rel in MODEL_DATA + + +def model_data_globs(package: Package) -> list[str]: + """Paths of ``package``'s model data for a tool's exclude option.""" + if package.dir.parent.name != "clients": + return [] + return [ + str(package.src / root / f"{m}.{ext}") + for root in package.roots + for m in MODEL_DATA + for ext in ("py", "pyi") + ] + + +def analyzed_modules() -> list[Module]: + """The modules AST analyses read: every runtime module except model data.""" + return [m for m in runtime_modules() if not m.model_data] + + +@functools.cache +def module_index() -> dict[str, Module]: + return {m.name: m for m in runtime_modules()} + + +@functools.cache +def owned_roots() -> frozenset[str]: + return frozenset(r for p in packages() for r in p.roots) + + +@functools.cache +def import_to_dists() -> dict[str, set[str]]: + """Maps a top-level import name to the distributions that provide it.""" + mapping: dict[str, set[str]] = { + top: {canonicalize_name(d) for d in dists} + for top, dists in importlib.metadata.packages_distributions().items() + } + # Editable installs don't always report their top-level names. + for package in packages(): + for root in package.roots: + mapping.setdefault(root, set()).add(package.dist) + return mapping + + +def is_stdlib(top: str) -> bool: + return top in sys.stdlib_module_names or top == "__future__" + + +def resolve_from(imp: Import) -> tuple[str, str | None]: + """The module an import binds and the attribute read from it, if any. + + ``from pkg import sub`` resolves to the submodule when one exists. + """ + if imp.name is not None: + sub = f"{imp.target}.{imp.name}" + if sub in module_index() or sub in native_modules(): + return sub, None + return imp.target, imp.name + + +@functools.cache +def native_modules() -> frozenset[str]: + """Extension modules, which have a stub or shared object but no ``.py`` source.""" + names: set[str] = set() + for package in packages(): + for path in [*package.src.rglob("*.pyi"), *package.src.rglob("*.so")]: + if path.with_name(path.name.split(".")[0] + ".py").exists(): + continue + rel = path.relative_to(package.src) + names.add(".".join([*rel.parts[:-1], rel.name.split(".")[0]])) + return frozenset(names) + + +def canonical_definition(module: str, name: str, depth: int = 0) -> str | None: + """Follows re-export chains to the module that defines ``name``.""" + found = module_index().get(module) + if found is None or depth > 16: + return None + if name in found.defined: + return module + imp = found.imported.get(name) + if imp is None or imp.name is None: + return None + target, attr = resolve_from(imp) + if attr is None: + return target + return canonical_definition(target, attr, depth + 1) + + +def tarjan(graph: dict[str, set[str]]) -> list[list[str]]: + """Strongly connected components with a cycle: more than one node, or a self-loop.""" + index: dict[str, int] = {} + low: dict[str, int] = {} + stack: list[str] = [] + on_stack: set[str] = set() + result: list[list[str]] = [] + counter = 0 + + def strongconnect(v: str) -> None: + nonlocal counter + index[v] = low[v] = counter + counter += 1 + stack.append(v) + on_stack.add(v) + for w in graph.get(v, ()): + if w not in index: + strongconnect(w) + low[v] = min(low[v], low[w]) + elif w in on_stack: + low[v] = min(low[v], index[w]) + if low[v] == index[v]: + component: list[str] = [] + while True: + w = stack.pop() + on_stack.discard(w) + component.append(w) + if w == v: + break + if len(component) > 1 or v in graph.get(v, ()): + result.append(sorted(component)) + + sys.setrecursionlimit(max(sys.getrecursionlimit(), 10_000)) + for v in sorted(graph): + if v not in index: + strongconnect(v) + return result + + +def main(title: str, check: Callable[[], list[Finding]]) -> None: + """Runs one verification, prints its findings, and exits nonzero on any error.""" + findings = check() + for finding in findings: + sys.stdout.write(f"{finding}\n") + sys.stdout.flush() + errors = sum(f.severity == "error" for f in findings) + warnings = sum(f.severity == "warning" for f in findings) + sys.stderr.write(f"{title}: {errors} error(s), {warnings} warning(s).\n") + sys.exit(1 if errors else 0) + + +def run( + args: list[str], + *, + cwd: Path = ROOT, + env: dict[str, str] | None = None, + timeout: int = 1800, +) -> subprocess.CompletedProcess[str]: + return subprocess.run( # noqa: S603 + args, + cwd=cwd, + env={**os.environ, **(env or {})}, + capture_output=True, + text=True, + timeout=timeout, + check=False, + ) + + +def build_wheels(dest: Path) -> list[Path]: + """Builds a wheel of every package into ``dest``.""" + + def one(package: Package) -> None: + proc = run(["uv", "build", "--wheel", "--out-dir", str(dest), str(package.dir)]) + if proc.returncode != 0: + raise RuntimeError( + f"uv build of {package.dist} failed:\n{proc.stderr[-3000:]}" + ) + + with ThreadPoolExecutor(max_workers=os.cpu_count() or 4) as pool: + list(pool.map(one, packages())) + return sorted(dest.glob("*.whl")) + + +def make_venv(path: Path, *install: str) -> str: + """Creates a Python 3.12 venv at ``path``, installs ``install`` into it, and returns its python.""" + proc = run(["uv", "venv", "--quiet", "--python", "3.12", str(path)]) + if proc.returncode != 0: + raise RuntimeError(f"uv venv failed:\n{proc.stderr[-3000:]}") + python = str(path / "bin" / "python") + if install: + proc = run(["uv", "pip", "install", "--python", python, *install]) + if proc.returncode != 0: + raise RuntimeError(f"uv pip install failed:\n{proc.stderr[-3000:]}") + return python + + +def probe( + modules: Iterable[str], python: str = sys.executable +) -> dict[str, dict[str, Any]]: + """Imports each module in a fresh interpreter and returns the probe reports.""" + names = list(modules) + return _probe_cached(tuple(names), python) + + +def members(module: Module, loaded: ModuleType) -> Iterator[tuple[str, object]]: + """The loaded module's globals, then the members only its stub names. + + Reading a stub-only name with ``getattr`` builds it through the module's + ``__getattr__``. + """ + namespace = vars(loaded) + yield from list(namespace.items()) + for name in module.stub_names: + if name not in namespace and hasattr(loaded, name): + yield name, getattr(loaded, name) + + +def public_runtime_module_names() -> list[str]: + return [m.name for m in runtime_modules() if m.public] + + +def all_runtime_module_names() -> list[str]: + return [m.name for m in runtime_modules()] + + +def import_call_aliases(tree: ast.Module) -> set[str]: + """Local names bound to ``importlib.import_module`` or ``__import__``.""" + names = {"__import__"} + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module == "importlib": + for alias in node.names: + if alias.name in ("import_module", "__import__"): + names.add(alias.asname or alias.name) + return names + + +def is_import_call(func: ast.expr, aliases: set[str]) -> bool: + """Whether ``func`` is ``importlib.import_module``, ``__import__`` or a local alias of them.""" + if isinstance(func, ast.Name): + return func.id in aliases + return ( + isinstance(func, ast.Attribute) + and func.attr in ("import_module", "__import__") + and isinstance(func.value, ast.Name) + and func.value.id == "importlib" + ) + + +@functools.cache +def _probe_cached(names: tuple[str, ...], python: str) -> dict[str, dict[str, Any]]: + roots = [str(p.src) for p in packages()] + + def one(name: str) -> tuple[str, dict[str, Any]]: + proc = run([python, str(PROBE), name, *roots], timeout=300) + try: + return name, json.loads(proc.stdout.strip().splitlines()[-1]) + except (IndexError, json.JSONDecodeError): + return name, { + "ok": False, + "error": { + "type": "ProbeCrash", + "message": proc.stderr[-2000:], + "missing": None, + }, + "warnings": [], + "side_effects": [], + } + + with ThreadPoolExecutor(max_workers=os.cpu_count() or 4) as pool: + return dict(pool.map(one, names)) + + +def _load_src(src: Path, package: Package | None) -> Iterator[Module]: + for path in sorted(src.rglob("*.py")): + rel = path.relative_to(src) + if "__pycache__" in rel.parts: + continue + parts = list(rel.parts[:-1]) + if path.stem != "__init__": + parts.append(path.stem) + yield Module(name=".".join(parts), path=path, src=src, package=package) + + +def _top_level_bindings(body: list[ast.stmt]) -> Iterator[str]: + for node in body: + match node: + case ast.FunctionDef() | ast.AsyncFunctionDef() | ast.ClassDef(): + yield node.name + case ast.Assign(): + for target in node.targets: + yield from _target_names(target) + case ast.AnnAssign(): + yield from _target_names(node.target) + case ast.TypeAlias(): + if isinstance(node.name, ast.Name): + yield node.name.id + case ast.If() | ast.Try(): + yield from _top_level_bindings(node.body) + yield from _top_level_bindings(node.orelse) + if isinstance(node, ast.Try): + for handler in node.handlers: + yield from _top_level_bindings(handler.body) + yield from _top_level_bindings(node.finalbody) + case _: + pass + + +def _target_names(target: ast.expr) -> Iterator[str]: + if isinstance(target, ast.Name): + yield target.id + elif isinstance(target, ast.Tuple | ast.List): + for elt in target.elts: + yield from _target_names(elt) + + +def _is_type_checking(test: ast.expr) -> bool: + return (isinstance(test, ast.Name) and test.id == "TYPE_CHECKING") or ( + isinstance(test, ast.Attribute) and test.attr == "TYPE_CHECKING" + ) + + +_IMPORT_ERRORS = frozenset( + {"ImportError", "ModuleNotFoundError", "Exception", "BaseException"} +) + + +def _catches_import_error(node: ast.Try) -> bool: + for handler in node.handlers: + if handler.type is None: + return True + types = ( + handler.type.elts if isinstance(handler.type, ast.Tuple) else [handler.type] + ) + for t in types: + if isinstance(t, ast.Name) and t.id in _IMPORT_ERRORS: + return True + return False + + +class _ImportCollector: + def __init__(self, module: Module) -> None: + self.module = module + self.call_aliases = import_call_aliases(module.tree) + + def collect(self) -> Iterator[Import]: + yield from self._walk(self.module.tree.body, "runtime", guarded=False) + + def _walk( + self, body: list[ast.stmt], context: Context, *, guarded: bool + ) -> Iterator[Import]: + for node in body: + yield from self._visit(node, context, guarded=guarded) + + def _visit( + self, node: ast.AST, context: Context, *, guarded: bool + ) -> Iterator[Import]: + match node: + case ast.Import(): + for alias in node.names: + yield Import( + self.module, + alias.name, + None, + alias.asname, + node.lineno, + context, + guarded, + ) + case ast.Call() if is_import_call(node.func, self.call_aliases): + if (target := self._call_target(node)) is not None: + yield Import( + self.module, target, None, None, node.lineno, context, guarded + ) + for child in ast.iter_child_nodes(node): + yield from self._visit(child, context, guarded=guarded) + case ast.ImportFrom(): + target = self._absolute(node.level, node.module) + for alias in node.names: + yield Import( + self.module, + target, + alias.name, + alias.asname, + node.lineno, + context, + guarded, + ) + case ast.FunctionDef() | ast.AsyncFunctionDef() | ast.Lambda(): + for child in ast.iter_child_nodes(node): + yield from self._visit(child, "deferred", guarded=guarded) + case ast.If() if _is_type_checking(node.test): + inner: Context = ( + "deferred" if context == "deferred" else "type_checking" + ) + yield from self._walk(node.body, inner, guarded=guarded) + yield from self._walk(node.orelse, context, guarded=guarded) + case ast.Try(): + catches = _catches_import_error(node) + yield from self._walk(node.body, context, guarded=guarded or catches) + for handler in node.handlers: + yield from self._walk(handler.body, context, guarded=guarded) + yield from self._walk(node.orelse, context, guarded=guarded) + yield from self._walk(node.finalbody, context, guarded=guarded) + case _: + for child in ast.iter_child_nodes(node): + yield from self._visit(child, context, guarded=guarded) + + def _call_target(self, node: ast.Call) -> str | None: + """The absolute module a literal ``import_module`` call loads, if statically known.""" + first = node.args[0] if node.args else None + if not (isinstance(first, ast.Constant) and isinstance(first.value, str)): + return None + name = first.value + stripped = name.lstrip(".") + level = len(name) - len(stripped) + if not level: + return name + package = next((k.value for k in node.keywords if k.arg == "package"), None) + if len(node.args) > 1: + package = node.args[1] + # Relative calls resolve against the caller only when anchored to it. + if not isinstance(package, ast.Name): + return None + if package.id == "__package__": + return self._absolute(level, stripped or None) + if package.id == "__name__": + return self._absolute(level, stripped or None, base=self.module.name) + return None + + def _absolute(self, level: int, module: str | None, base: str | None = None) -> str: + if not level: + return module or "" + if base is not None: + parts = base.split(".") + else: + parts = self.module.name.split(".") + if not self.module.is_package: + parts = parts[:-1] + parts = parts[: len(parts) - (level - 1)] + if module: + parts.append(module) + return ".".join(parts) diff --git a/scripts/verifications/_probe.py b/scripts/verifications/_probe.py new file mode 100644 index 00000000..98d1e570 --- /dev/null +++ b/scripts/verifications/_probe.py @@ -0,0 +1,219 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Imports one module in this fresh interpreter and prints a JSON report of what happened.""" + +import atexit +import importlib +import json +import logging +import os +import signal +import socket +import sys +import sysconfig +import threading +import time +import traceback +import warnings +from typing import Any + +_WATCHED_SIGNALS = [s for s in (signal.SIGINT, signal.SIGTERM, signal.SIGHUP) if s] + + +def main() -> None: + module, roots = sys.argv[1], tuple(sys.argv[2:]) + effects: list[dict[str, Any]] = [] + + stdlib = sysconfig.get_paths()["stdlib"] + + def blame() -> str | None: + """The first non-stdlib caller, when it is our code.""" + frame = sys._getframe(2) + while frame is not None: + filename = frame.f_code.co_filename + if not (filename.startswith((stdlib, " Any: + if (where := blame()) is not None: + effects.append({"kind": "env-read", "detail": key, "where": where}) + return original_getitem(self, key) + + def setitem(self: Any, key: str, value: str) -> None: + if (where := blame()) is not None: + effects.append({"kind": "env-write", "detail": key, "where": where}) + original_setitem(self, key, value) + + def contains(self: Any, key: object) -> bool: + if (where := blame()) is not None: + effects.append({"kind": "env-read", "detail": str(key), "where": where}) + return original_contains(self, key) + + original_atexit = atexit.register + + def register(func: Any, *args: Any, **kwargs: Any) -> Any: + effects.append( + { + "kind": "atexit", + "detail": getattr(func, "__qualname__", repr(func)), + "where": blame(), + } + ) + return original_atexit(func, *args, **kwargs) + + environ_type.__getitem__ = getitem # type: ignore[method-assign] + environ_type.__setitem__ = setitem # type: ignore[method-assign] + environ_type.__contains__ = contains # type: ignore[method-assign] + atexit.register = register # type: ignore[assignment] + + loggers_before = _logger_state() + threads_before = {t.ident for t in threading.enumerate()} + path_before = list(sys.path) + meta_path_before = list(sys.meta_path) + excepthook_before = sys.excepthook + timeout_before = socket.getdefaulttimeout() + signals_before = {s: signal.getsignal(s) for s in _WATCHED_SIGNALS} + modules_before = set(sys.modules) + + original_filterwarnings = warnings.filterwarnings + original_simplefilter = warnings.simplefilter + # A dependency's own filters are its business, not an effect of our import. + third_party_filters: list[Any] = [] + + def attributed(original: Any) -> Any: + def install(*args: Any, **kwargs: Any) -> None: + before = list(warnings.filters) + where = blame() + original(*args, **kwargs) + if where is None: + third_party_filters.extend( + f for f in warnings.filters if f not in before + ) + + return install + + error: dict[str, Any] | None = None + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + filters_before = list(warnings.filters) + warnings.filterwarnings = attributed(original_filterwarnings) + warnings.simplefilter = attributed(original_simplefilter) + start = time.perf_counter() + try: + importlib.import_module(module) + except BaseException as e: + error = _describe(e) + elapsed = time.perf_counter() - start + warnings.filterwarnings = original_filterwarnings + warnings.simplefilter = original_simplefilter + filters_after = [f for f in warnings.filters if f not in third_party_filters] + + environ_type.__getitem__ = original_getitem # type: ignore[method-assign] + environ_type.__setitem__ = original_setitem # type: ignore[method-assign] + environ_type.__contains__ = original_contains # type: ignore[method-assign] + atexit.register = original_atexit # type: ignore[assignment] + + for name, state in _logger_state().items(): + if loggers_before.get(name) != state and state["handlers"]: + effects.append( + { + "kind": "logging", + "detail": f"{name or 'root'}: {state}", + "where": None, + } + ) + if logging.root.level != loggers_before.get("", {}).get( + "level", logging.root.level + ): + effects.append( + {"kind": "logging", "detail": "root level changed", "where": None} + ) + for thread in threading.enumerate(): + if thread.ident not in threads_before: + effects.append({"kind": "thread", "detail": thread.name, "where": None}) + if filters_after != filters_before: + added = [str(f) for f in filters_after if f not in filters_before] + effects.append( + {"kind": "warnings-filter", "detail": "; ".join(added), "where": None} + ) + if sys.path != path_before: + effects.append({"kind": "sys.path", "detail": "modified", "where": None}) + if sys.meta_path != meta_path_before: + effects.append({"kind": "sys.meta_path", "detail": "modified", "where": None}) + if sys.excepthook is not excepthook_before: + effects.append({"kind": "excepthook", "detail": "replaced", "where": None}) + if socket.getdefaulttimeout() != timeout_before: + effects.append({"kind": "socket-timeout", "detail": "changed", "where": None}) + for sig, handler in signals_before.items(): + if signal.getsignal(sig) != handler: + effects.append( + {"kind": "signal", "detail": signal.Signals(sig).name, "where": None} + ) + + report = { + "ok": error is None, + "error": error, + "seconds": elapsed, + "new_modules": len(set(sys.modules) - modules_before), + "warnings": [ + { + "category": w.category.__name__, + "message": str(w.message), + "where": f"{w.filename}:{w.lineno}", + } + for w in caught + ], + "side_effects": effects, + } + sys.stdout.write("\n" + json.dumps(report) + "\n") + + +def _logger_state() -> dict[str, dict[str, Any]]: + state: dict[str, dict[str, Any]] = { + "": { + "level": logging.root.level, + "handlers": [type(h).__name__ for h in logging.root.handlers], + } + } + for name, logger in logging.Logger.manager.loggerDict.items(): + if isinstance(logger, logging.Logger): + state[name] = { + "level": logger.level, + "handlers": [ + type(h).__name__ + for h in logger.handlers + if not isinstance(h, logging.NullHandler) + ], + } + return state + + +def _describe(e: BaseException) -> dict[str, Any]: + missing = None + cursor: BaseException | None = e + while cursor is not None: + if isinstance(cursor, ModuleNotFoundError) and cursor.name: + missing = cursor.name + break + cursor = cursor.__cause__ or cursor.__context__ + return { + "type": type(e).__name__, + "message": str(e), + "missing": missing, + "traceback": traceback.format_exception(e)[-3:], + } + + +if __name__ == "__main__": + main() diff --git a/scripts/verifications/banned_imports.py b/scripts/verifications/banned_imports.py new file mode 100644 index 00000000..78dcd2ef --- /dev/null +++ b/scripts/verifications/banned_imports.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Bans test-only modules from shipped package code. + +Test frameworks and helpers are dev dependencies: customers don't have them. +An import of one in shipped code works in our venv and fails on install, or worse, +pulls a test framework into a production process. Enforced with ruff's +flake8-tidy-imports banned-api rule (TID251) so the ban can later move into +pyproject.toml unchanged. +""" + +import sys +from pathlib import Path + +from _common import Finding, main, packages, run + +_BANNED = { + "pytest": "test framework; not a runtime dependency", + "_pytest": "test framework; not a runtime dependency", + "freezegun": "test dependency", + "unittest.mock": "test-only; use a real object or a protocol", + "smithy_test": "test helpers package; not a runtime dependency", +} + + +def check() -> list[Finding]: + findings: list[Finding] = [] + ruff = str(Path(sys.executable).parent / "ruff") + for package in packages(): + overrides: list[str] = [] + for name, msg in _BANNED.items(): + overrides += [ + "--config", + f'lint.flake8-tidy-imports.banned-api."{name}".msg = "{msg}"', + ] + proc = run( + [ + ruff, + "check", + "--config", + str(package.dir / "pyproject.toml"), + *overrides, + "--select", + "TID251", + "--output-format", + "concise", + "--no-cache", + str(package.src), + ] + ) + for line in proc.stdout.splitlines(): + if ": TID251" in line: + findings.append(Finding(line)) + if proc.returncode not in (0, 1): + findings.append( + Finding(f"ruff failed for {package.dist}: {proc.stderr.strip()[-300:]}") + ) + return findings + + +if __name__ == "__main__": + main("banned imports", check) diff --git a/scripts/verifications/canonical_imports.py b/scripts/verifications/canonical_imports.py new file mode 100644 index 00000000..c9c3daca --- /dev/null +++ b/scripts/verifications/canonical_imports.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures our own code imports each of our symbols from the module that defines it. + +Importing through a re-export makes the importer depend on the re-exporting +module's import order and contents, which is how import cycles and accidental +public-surface dependencies creep in. It also hides where a symbol lives from +readers and refactoring tools. A re-export import is accepted under a private +local name (``import X as _X``), which marks it as a deliberate, module-internal +choice, and when the defining module is private, since the re-export is then the +symbol's public home. An import from another package's root is also accepted +when that root lists the symbol in ``__all__``, since the root's declared +surface is that package's public contract. + +Covers generated client and hand-written package sources. +""" + +from _common import ( + Finding, + canonical_definition, + main, + module_index, + owned_roots, + resolve_from, + runtime_modules, +) + + +def check() -> list[Finding]: + findings: list[Finding] = [] + index = module_index() + roots = owned_roots() + for module in runtime_modules(): + for imp in module.imports: + if imp.name is None or imp.name == "*" or imp.top not in roots: + continue + target, attr = resolve_from(imp) + if attr is None: + continue + source = index.get(target) + if source is None: + findings.append( + Finding( + f"cannot resolve module {target}", + module.path, + imp.line, + "warning", + ) + ) + continue + if source.model_data or attr in source.defined: + continue + if imp.asname is not None and imp.asname.startswith("_"): + continue + canonical = canonical_definition(target, attr) + if canonical is None: + findings.append( + Finding( + f"{attr} is not defined or imported in {target}", + module.path, + imp.line, + "warning", + ) + ) + continue + if any(part.startswith("_") for part in canonical.split(".")): + continue + if _is_declared_package_export( + module.name, target, attr, source.dunder_all + ): + continue + findings.append( + Finding( + f"imports {attr} from re-exporting {target}; defined in {canonical}", + module.path, + imp.line, + ) + ) + return findings + + +def _is_declared_package_export( + importer: str, target: str, attr: str, dunder_all: list[str] | None +) -> bool: + """Whether target is another package's root and lists attr in its __all__.""" + return ( + "." not in target + and importer.split(".")[0] != target + and dunder_all is not None + and attr in dunder_all + ) + + +if __name__ == "__main__": + main("canonical imports", check) diff --git a/scripts/verifications/declared_deps.py b/scripts/verifications/declared_deps.py new file mode 100644 index 00000000..290946cc --- /dev/null +++ b/scripts/verifications/declared_deps.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures each package declares every distribution it imports, and imports every one it declares. + +An import of a library the importing package never declared still works whenever +something else installed it: a transitive dependency, or a sibling in a shared +venv. Tests only reach the modules they exercise, so it can pass CI and then +fail for a customer who installs that package alone. +Conversely, a declared dependency nothing imports forces an unneeded install and +resolution constraint on every customer. Typing-only imports count as uses: type +checkers in a customer's environment need the dependency installed too. +""" + +from _common import Finding, import_to_dists, is_stdlib, main, packages, runtime_modules +from packaging.utils import canonicalize_name + + +def check() -> list[Finding]: + findings: list[Finding] = [] + mapping = import_to_dists() + by_package: dict[str, list] = {} + for module in runtime_modules(): + assert module.package is not None # noqa: S101 + by_package.setdefault(module.package.dist, []).extend(module.imports) + + for package in packages(): + own = set(package.roots) + required = {canonicalize_name(r.name) for r in package.dependencies} + optional = { + canonicalize_name(r.name) for reqs in package.extras.values() for r in reqs + } + declared = required | optional + used: set[str] = set() + reported: set[tuple[str, str]] = set() + for imp in by_package.get(package.dist, []): + top = imp.top + if not top or top in own or is_stdlib(top): + continue + # Type checkers resolve typing_extensions from typeshed's bundled stdlib stubs. + if top == "typing_extensions" and imp.context == "type_checking": + continue + dists = mapping.get(top) + if not dists: + key = (top, "unresolved") + if key not in reported: + reported.add(key) + findings.append( + Finding( + f"{package.dist}: import {top} maps to no installed distribution", + imp.module.path, + imp.line, + ) + ) + continue + used.update(dists) + if not dists & declared: + key = (top, "undeclared") + if key not in reported: + reported.add(key) + findings.append( + Finding( + f"{package.dist}: imports {top} ({', '.join(sorted(dists))}) " + f"without declaring it ({imp.context})", + imp.module.path, + imp.line, + ) + ) + for dist in sorted(declared - used): + where = "dependencies" if dist in required else "optional-dependencies" + findings.append( + Finding( + f"{package.dist}: declares {dist} in {where} but never imports it", + package.dir / "pyproject.toml", + ) + ) + return findings + + +if __name__ == "__main__": + main("declared dependencies", check) diff --git a/scripts/verifications/dep_specifiers.py b/scripts/verifications/dep_specifiers.py new file mode 100644 index 00000000..01d2f7f6 --- /dev/null +++ b/scripts/verifications/dep_specifiers.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every dependency specifier in the repo agrees, and in-repo ones track sibling versions. + +Each client and package resolves on its own, so diverging specifiers for the same +dependency go unnoticed in any one of them, but a customer installing a subset of +our packages gets whatever range happens to apply, and two packages may be +uninstallable together. Dev groups and build requirements diverging from each +other means different parts of the repo are tested against different +assumptions. In-repo dependencies of shipped packages must also be bounded and +satisfied by the sibling's current version, or a release can float onto a +sibling version that lacks the code it needs. +""" + +import tomllib +from collections.abc import Iterator +from pathlib import Path + +from _common import ROOT, Finding, main, package_by_dist, packages, run +from packaging.requirements import Requirement +from packaging.specifiers import SpecifierSet +from packaging.utils import canonicalize_name + + +def check() -> list[Finding]: + findings = _in_repo_bounds() + specs: dict[str, dict[str, list[str]]] = {} + for pyproject, where, req in _declarations(): + specs.setdefault(canonicalize_name(req.name), {}).setdefault( + str(req.specifier) or "(any)", [] + ).append(f"{pyproject.parent.relative_to(ROOT).as_posix() or '.'} {where}") + for name, variants in sorted(specs.items()): + if len(variants) > 1: + listed = "; ".join( + f"{s} ({', '.join(users)})" for s, users in sorted(variants.items()) + ) + findings.append(Finding(f"{name} has {len(variants)} specifiers: {listed}")) + return findings + + +def _in_repo_bounds() -> list[Finding]: + findings: list[Finding] = [] + siblings = package_by_dist() + for package in packages(): + reqs = [("dependencies", r) for r in package.dependencies] + [ + (f"extra {extra}", r) for extra, rs in package.extras.items() for r in rs + ] + for where, req in reqs: + name = canonicalize_name(req.name) + sibling = siblings.get(name) + if sibling is None or name == package.dist: + continue + version = sibling.current_version + pyproject = package.dir / "pyproject.toml" + if not req.specifier: + findings.append( + Finding( + f"{package.dist} {where}: in-repo {name} is unbounded", + pyproject, + ) + ) + elif version is not None and not SpecifierSet(str(req.specifier)).contains( + version, prereleases=True + ): + findings.append( + Finding( + f"{package.dist} {where}: in-repo {name}{req.specifier} excludes " + f"its current version {version}", + pyproject, + ) + ) + return findings + + +def _declarations() -> Iterator[tuple[Path, str, Requirement]]: + """Every requirement in every tracked pyproject.toml, with where it is declared.""" + tracked = run(["git", "ls-files", "*pyproject.toml"]).stdout.split() + for rel in tracked: + pyproject = ROOT / rel + data = tomllib.loads(pyproject.read_text()) + project = data.get("project", {}) + own = canonicalize_name(project.get("name", "")) + sources = [("build-system", data.get("build-system", {}).get("requires", []))] + sources.append(("dependencies", project.get("dependencies", []))) + sources += [ + (f"extra {name}", reqs) + for name, reqs in project.get("optional-dependencies", {}).items() + ] + sources += [ + (f"group {name}", reqs) + for name, reqs in data.get("dependency-groups", {}).items() + ] + for where, reqs in sources: + for req in reqs: + # Group entries may be {include-group = ...} tables, not requirements. + if not isinstance(req, str): + continue + requirement = Requirement(req) + # An extra that requires the package's own other extras is not a dependency. + if canonicalize_name(requirement.name) != own: + yield pyproject, where, requirement + + +if __name__ == "__main__": + main("dependency specifiers", check) diff --git a/scripts/verifications/docstrings.py b/scripts/verifications/docstrings.py new file mode 100644 index 00000000..24237c35 --- /dev/null +++ b/scripts/verifications/docstrings.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures public classes, functions, methods and constants have docstrings. + +Docstrings are the API reference customers read in their editor and in the +generated docs; an undocumented public symbol is a blank page there. Internal +symbols (underscore-prefixed, dunders, or anything in an underscore-prefixed +module) are exempt, since nobody outside the package reads them. Classes, +functions and methods are checked with ruff's pydocstyle rules. A method that +overrides a documented one is exempt, since editors and the generated docs show +the inherited docstring (``inspect.getdoc``) for it. Module-level +constants carry an attribute docstring (a string literal right after the +assignment), which no ruff rule checks, so that part is an AST pass. +""" + +import ast +import importlib +import inspect +import re +import sys +from pathlib import Path + +from _common import ( + ROOT, + Finding, + analyzed_modules, + main, + model_data_globs, + packages, + run, + runtime_modules, +) + +_SELECT = "D101,D102,D103,D106" +_CONSTANT = re.compile(r"^[A-Z][A-Z0-9_]*$") +_LOCATION = re.compile(r"^(?P[^:]+):(?P\d+):\d+: (?PD\d+) ") + + +def check() -> list[Finding]: + return [ + *(f for f in _ruff_findings() if not _inherits_docstring(f)), + *_constant_findings(), + ] + + +def _ruff_findings() -> list[Finding]: + ruff = str(Path(sys.executable).parent / "ruff") + findings: list[Finding] = [] + for package in packages(): + proc = run( + [ + ruff, + "check", + "--config", + str(package.dir / "pyproject.toml"), + "--select", + _SELECT, + "--output-format", + "concise", + "--no-cache", + *( + arg + for glob in model_data_globs(package) + for arg in ("--extend-exclude", glob) + ), + str(package.src), + ] + ) + findings.extend( + Finding(line) + for line in proc.stdout.splitlines() + if line.startswith(("/", "clients/", "packages/")) + ) + return findings + + +def _inherits_docstring(finding: Finding) -> bool: + """Whether a D102 method overrides one whose docstring editors and docs show.""" + match = _LOCATION.match(finding.message) + if match is None or match["code"] != "D102": + return False + path = (ROOT / match["path"]).resolve() + module = next((m for m in runtime_modules() if m.path.resolve() == path), None) + if module is None: + return False + qualname = _method_qualname(module.tree, int(match["line"])) + if qualname is None: + return False + for package in packages(): + if str(package.src) not in sys.path: + sys.path.insert(0, str(package.src)) + try: + obj: object = importlib.import_module(module.name) + for part in qualname: + obj = getattr(obj, part) + except Exception: # noqa: BLE001 + return False + return inspect.getdoc(obj) is not None + + +def _method_qualname(tree: ast.Module, line: int) -> list[str] | None: + def walk(body: list[ast.stmt], prefix: list[str]) -> list[str] | None: + for node in body: + if isinstance(node, ast.ClassDef): + if (found := walk(node.body, [*prefix, node.name])) is not None: + return found + elif ( + isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.lineno == line + and prefix + ): + return [*prefix, node.name] + return None + + return walk(tree.body, []) + + +def _constant_findings() -> list[Finding]: + findings: list[Finding] = [] + for module in analyzed_modules(): + if not module.public: + continue + body = module.tree.body + for index, node in enumerate(body): + if isinstance(node, ast.Assign) and len(node.targets) == 1: + target = node.targets[0] + elif isinstance(node, ast.AnnAssign): + target = node.target + else: + continue + if not isinstance(target, ast.Name) or not _CONSTANT.match(target.id): + continue + following = body[index + 1] if index + 1 < len(body) else None + documented = ( + isinstance(following, ast.Expr) + and isinstance(following.value, ast.Constant) + and isinstance(following.value.value, str) + ) + if not documented: + findings.append( + Finding( + f"public constant {target.id} has no docstring", + module.path, + node.lineno, + ) + ) + return findings + + +if __name__ == "__main__": + main("docstrings", check) diff --git a/scripts/verifications/dunder_all.py b/scripts/verifications/dunder_all.py new file mode 100644 index 00000000..1d63de06 --- /dev/null +++ b/scripts/verifications/dunder_all.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures each ``__all__`` lists the module's public definitions and its re-exports. + +Python never checks ``__all__``; only star imports read it. Yet type checkers, +documentation generators and IDEs treat it as the declared public surface, so a +public name missing from it is invisible to them, and a package ``__init__`` that +re-exports a name its source module doesn't export publishes something the owner +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. +""" + +import ast + +from _common import ( + Finding, + analyzed_modules, + main, + module_index, + owned_roots, + resolve_from, +) + +# A module-level logger is named for logging config, not imported by customers. +_NOT_API = frozenset({"logger"}) + + +def check() -> list[Finding]: + findings: list[Finding] = [] + index = module_index() + for module in analyzed_modules(): + exported = module.dunder_all + if exported is None: + continue + exported_set = set(exported) + for name in sorted(module.defined - _NOT_API): + if not name.startswith("_") and name not in exported_set: + findings.append( + Finding(f"public {name} is missing from __all__", module.path) + ) + if not module.is_package: + continue + used = _used_names(module.tree) + 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. + if name not in exported_set and ( + imp.context == "type_checking" or name in used + ): + continue + target, attr = resolve_from(imp) + if attr is None: + continue + if name not in exported_set: + findings.append( + Finding( + f"re-exported {name} (from {target}) is missing from __all__", + module.path, + imp.line, + "warning", + ) + ) + continue + source = index.get(target) + source_all = ( + source.dunder_all + if source is not None and not source.model_data + else None + ) + if source_all is not None and attr not in source_all: + findings.append( + Finding( + f"re-exports {attr} from {target}, which does not list it in __all__", + module.path, + imp.line, + ) + ) + return findings + + +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)) + return used + + +if __name__ == "__main__": + main("__all__ consistency", check) diff --git a/scripts/verifications/dynamic_imports.py b/scripts/verifications/dynamic_imports.py new file mode 100644 index 00000000..86964343 --- /dev/null +++ b/scripts/verifications/dynamic_imports.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Bans ``importlib.import_module`` and ``__import__`` with non-literal arguments in shipped code. + +An import whose target is computed at runtime is invisible to every static tool: +the dependency, cycle, import-direction and canonical-import verifications, +type checkers, bundlers and freezers all miss it. Requiring a string literal +keeps the import graph those tools see complete, since the shared import index +records literal calls as imports. +""" + +import ast + +from _common import ( + Finding, + analyzed_modules, + import_call_aliases, + is_import_call, + main, +) + + +def check() -> list[Finding]: + findings: list[Finding] = [] + for module in analyzed_modules(): + aliases = import_call_aliases(module.tree) + for node in ast.walk(module.tree): + if not isinstance(node, ast.Call) or not is_import_call(node.func, aliases): + continue + first = node.args[0] if node.args else None + if not (isinstance(first, ast.Constant) and isinstance(first.value, str)): + findings.append( + Finding( + f"dynamic import with non-literal target: {ast.unparse(node)[:120]}", + module.path, + node.lineno, + ) + ) + return findings + + +if __name__ == "__main__": + main("dynamic imports", check) diff --git a/scripts/verifications/exception_hierarchy.py b/scripts/verifications/exception_hierarchy.py new file mode 100644 index 00000000..799b0c7c --- /dev/null +++ b/scripts/verifications/exception_hierarchy.py @@ -0,0 +1,100 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures our exceptions share a root and shipped code doesn't raise generic ones. + +Customers handle runtime failures (service, transport, config, credentials) by +exception type, and need one ``except`` clause that catches every error the SDK +raises on purpose. That only works if every exception class we define derives +from the package's base error. ``Exception``, ``BaseException`` and +``RuntimeError`` give a caller nothing specific to catch, so raising them is +banned. Other builtins stay allowed: ``ValueError`` or ``TypeError`` for a bad +argument signals a bug in the caller, which nobody should catch, and protocol +methods must raise ``AttributeError``, ``KeyError`` and the like. + +Test helpers (``testing`` modules) are exempt from the base-class rule, since +customers never handle their errors. Raise sites are checked statically, so a +generic exception raised and caught internally is still reported. +""" + +import ast +import importlib +import inspect +import sys + +from _common import ( + Finding, + analyzed_modules, + main, + members, + packages, + runtime_modules, +) + +_BASE = "smithy_core.exceptions.SmithyError" +_GENERIC = frozenset({"Exception", "BaseException", "RuntimeError"}) + + +def check() -> list[Finding]: + for package in packages(): + sys.path.insert(0, str(package.src)) + return [*_class_findings(), *_raise_findings()] + + +def _class_findings() -> list[Finding]: + findings: list[Finding] = [] + base = _resolve(_BASE) + for module in runtime_modules(): + if "testing" in module.name.split("."): + continue + try: + loaded = importlib.import_module(module.name) + except Exception as e: + findings.append( + Finding( + f"cannot import {module.name}: {e}", module.path, severity="warning" + ) + ) + continue + for name, obj in members(module, loaded): + if ( + inspect.isclass(obj) + and issubclass(obj, BaseException) + and not issubclass(obj, Warning) + and obj.__module__ == module.name + and not issubclass(obj, base) + ): + findings.append( + Finding( + f"{name} does not derive from {base.__qualname__}", module.path + ) + ) + return findings + + +def _raise_findings() -> list[Finding]: + findings: list[Finding] = [] + for module in analyzed_modules(): + shadowed = set(module.defined) | set(module.imported) + for node in ast.walk(module.tree): + if not isinstance(node, ast.Raise) or node.exc is None: + continue + exc = node.exc.func if isinstance(node.exc, ast.Call) else node.exc + if ( + isinstance(exc, ast.Name) + and exc.id in _GENERIC + and exc.id not in shadowed + ): + findings.append( + Finding(f"raises generic {exc.id}", module.path, node.lineno) + ) + return findings + + +def _resolve(dotted: str) -> type: + module, _, name = dotted.rpartition(".") + return getattr(importlib.import_module(module), name) + + +if __name__ == "__main__": + main("exception hierarchy", check) diff --git a/scripts/verifications/export_names.py b/scripts/verifications/export_names.py new file mode 100644 index 00000000..415cda53 --- /dev/null +++ b/scripts/verifications/export_names.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures each exported class or function is exported under its own name. + +An export like ``Foo = Bar`` makes tracebacks, reprs, pickles, docs and log +lines say ``Bar`` while customers wrote ``Foo``, and leaves two names for one +object that must then both be kept stable. This is the export-side complement of +the canonical-imports verification. Both ``__name__`` and ``__qualname__`` are +compared, so an exported nested object is caught too. +""" + +import importlib +import inspect +import sys +import typing + +from _common import ( + Finding, + main, + owned_roots, + packages, + public_runtime_module_names, + runtime_modules, +) + + +def check() -> list[Finding]: + for package in packages(): + sys.path.insert(0, str(package.src)) + findings: list[Finding] = [] + by_name = {m.name: m for m in runtime_modules()} + paths = {name: m.path for name, m in by_name.items()} + roots = owned_roots() + for modname in public_runtime_module_names(): + try: + module = importlib.import_module(modname) + except Exception as e: + findings.append( + Finding( + f"cannot import {modname}: {e}", paths[modname], severity="warning" + ) + ) + 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 + ] + stub_only = [ + n + for n in by_name[modname].stub_names + if not n.startswith("_") and n not in names + ] + for name in [*names, *stub_only]: + obj = getattr(module, name, None) + if not _is_named(obj) or not _owned(obj, roots): + continue + actual = getattr(obj, "__name__", None) + qualname = getattr(obj, "__qualname__", actual) + if actual != name or qualname != name: + shown = ( + actual if qualname == actual else f"{actual} (qualname {qualname})" + ) + findings.append( + Finding( + f"{modname}.{name} is exported under a different name: {shown}", + paths[modname], + ) + ) + return findings + + +def _is_named(obj: object) -> bool: + return ( + inspect.isclass(obj) + or inspect.isfunction(obj) + or isinstance(obj, typing.TypeAliasType | typing.TypeVar) + ) + + +def _owned(obj: object, roots: frozenset[str]) -> bool: + module = getattr(obj, "__module__", None) or "" + return module.split(".")[0] in roots + + +if __name__ == "__main__": + main("export names", check) diff --git a/scripts/verifications/import-cost.json b/scripts/verifications/import-cost.json new file mode 100644 index 00000000..ec424b43 --- /dev/null +++ b/scripts/verifications/import-cost.json @@ -0,0 +1,66 @@ +{ + "aws_credentials_cognito": 1, + "aws_credentials_cognito (all public modules)": 1, + "aws_credentials_http": 5, + "aws_credentials_http (all public modules)": 9, + "aws_credentials_imds": 5, + "aws_credentials_imds (all public modules)": 9, + "aws_credentials_login": 1, + "aws_credentials_login (all public modules)": 1, + "aws_credentials_sso": 1, + "aws_credentials_sso (all public modules)": 1, + "aws_credentials_sts": 4, + "aws_credentials_sts (all public modules)": 7, + "aws_sdk_api_gateway": 1, + "aws_sdk_api_gateway (all public modules)": 15, + "aws_sdk_bedrock": 1, + "aws_sdk_bedrock (all public modules)": 13, + "aws_sdk_bedrock_agent": 1, + "aws_sdk_bedrock_agent (all public modules)": 13, + "aws_sdk_bedrock_agent_runtime": 1, + "aws_sdk_bedrock_agent_runtime (all public modules)": 13, + "aws_sdk_bedrock_agentcore": 1, + "aws_sdk_bedrock_agentcore (all public modules)": 13, + "aws_sdk_bedrock_agentcore_control": 1, + "aws_sdk_bedrock_agentcore_control (all public modules)": 13, + "aws_sdk_bedrock_data_automation": 1, + "aws_sdk_bedrock_data_automation (all public modules)": 13, + "aws_sdk_bedrock_data_automation_runtime": 1, + "aws_sdk_bedrock_data_automation_runtime (all public modules)": 13, + "aws_sdk_bedrock_runtime": 1, + "aws_sdk_bedrock_runtime (all public modules)": 13, + "aws_sdk_cloudtrail": 1, + "aws_sdk_cloudtrail (all public modules)": 13, + "aws_sdk_cognito_identity": 1, + "aws_sdk_cognito_identity (all public modules)": 13, + "aws_sdk_connecthealth": 1, + "aws_sdk_connecthealth (all public modules)": 13, + "aws_sdk_dynamodb": 1, + "aws_sdk_dynamodb (all public modules)": 15, + "aws_sdk_guardduty": 1, + "aws_sdk_guardduty (all public modules)": 13, + "aws_sdk_lambda": 1, + "aws_sdk_lambda (all public modules)": 13, + "aws_sdk_lex_runtime_v2": 1, + "aws_sdk_lex_runtime_v2 (all public modules)": 13, + "aws_sdk_polly": 1, + "aws_sdk_polly (all public modules)": 13, + "aws_sdk_python": 1, + "aws_sdk_python (all public modules)": 1, + "aws_sdk_qbusiness": 1, + "aws_sdk_qbusiness (all public modules)": 13, + "aws_sdk_sagemaker_runtime_http2": 1, + "aws_sdk_sagemaker_runtime_http2 (all public modules)": 13, + "aws_sdk_secrets_manager": 1, + "aws_sdk_secrets_manager (all public modules)": 13, + "aws_sdk_signin": 1, + "aws_sdk_signin (all public modules)": 13, + "aws_sdk_sns": 1, + "aws_sdk_sns (all public modules)": 13, + "aws_sdk_sqs": 1, + "aws_sdk_sqs (all public modules)": 13, + "aws_sdk_sts": 1, + "aws_sdk_sts (all public modules)": 13, + "aws_sdk_transcribe_streaming": 1, + "aws_sdk_transcribe_streaming (all public modules)": 13 +} diff --git a/scripts/verifications/import_cost.py b/scripts/verifications/import_cost.py new file mode 100644 index 00000000..b0fa2bc5 --- /dev/null +++ b/scripts/verifications/import_cost.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Records how many of our modules each top-level package pulls in on import, and fails when that changes. + +Import cost is paid on every cold start: every Lambda invocation that lands on a +new sandbox, every CLI run, every short-lived worker. It grows one convenient +import at a time, and nobody notices until it shows up as latency. The count of +our own modules loaded is fixed by our code alone, so it is snapshotted in +import-cost.json and any change must be reviewed like the API snapshot. Stdlib +and third-party module counts and wall time vary with dependency versions, +platform and host, so they are printed alongside but not gated. Our native +extensions are blocked so every host measures the same pure-Python fallback, +whether or not it has a compiled build. +""" + +import json +import os +import sys +from concurrent.futures import ThreadPoolExecutor + +from _common import ( + HERE, + Finding, + main, + owned_roots, + packages, + public_runtime_module_names, + run, +) + +SNAPSHOT = HERE / "import-cost.json" +_SCRIPT = """ +import importlib, importlib.machinery as machinery, sys, time + +roots = set(sys.argv[1].split(",")) + + +class BlockNative: + def find_spec(self, name, path=None, target=None): + if name.split(".")[0] not in roots: + return None + spec = machinery.PathFinder.find_spec(name, path) + if spec and spec.origin and spec.origin.endswith(tuple(machinery.EXTENSION_SUFFIXES)): + raise ImportError(name) + return None + + +sys.meta_path.insert(0, BlockNative()) +before = set(sys.modules) +start = time.perf_counter() +for m in sys.argv[2:]: + importlib.import_module(m) +elapsed = time.perf_counter() - start +new = set(sys.modules) - before +ours = sum(m.split(".")[0] in roots for m in new) +print(ours, len(new) - ours, elapsed) +""" + + +def check() -> list[Finding]: + findings: list[Finding] = [] + counts: dict[str, int] = {} + public = public_runtime_module_names() + roots = ",".join(sorted(owned_roots())) + scenarios: dict[str, list[str]] = {} + for package in packages(): + for root in package.roots: + # The root alone is often a thin facade; customers import submodules. + scenarios[root] = [root] + scenarios[f"{root} (all public modules)"] = [ + m for m in public if m.split(".")[0] == root + ] + # Wall times are informational, so measuring in parallel only adds noise to them. + with ThreadPoolExecutor(max_workers=os.cpu_count() or 4) as pool: + results = pool.map(lambda t: _measure(roots, t), scenarios.values()) + for label, measured in zip(scenarios, results, strict=True): + if isinstance(measured, str): + findings.append(Finding(f"import {label} failed: {measured}")) + continue + counts[label], others, fastest = measured + findings.append( + Finding( + f"{label}: {counts[label]} of our modules, {others} other modules, " + f"{fastest * 1000:.1f} ms", + severity="info", + ) + ) + SNAPSHOT.write_text(json.dumps(dict(sorted(counts.items())), indent=2) + "\n") + tracked = run(["git", "ls-files", "--error-unmatch", str(SNAPSHOT)]).returncode == 0 + if not tracked: + findings.append( + Finding( + "import-cost.json created; commit it as the baseline", + SNAPSHOT, + severity="warning", + ) + ) + elif run(["git", "diff", "--quiet", "--", str(SNAPSHOT)]).returncode: + diff = run(["git", "--no-pager", "diff", "--", str(SNAPSHOT)]).stdout + findings.append( + Finding( + f"import module counts changed; review and commit:\n{diff}", SNAPSHOT + ) + ) + return findings + + +def _measure(roots: str, targets: list[str]) -> tuple[int, int, float] | str: + """Our and other new module counts and the fastest of three wall times, or the error output.""" + samples: list[tuple[int, int, float]] = [] + for _ in range(3): + proc = run([sys.executable, "-c", _SCRIPT, roots, *targets]) + if proc.returncode != 0: + return proc.stderr.strip()[-300:] + ours, others, elapsed = proc.stdout.split() + samples.append((int(ours), int(others), float(elapsed))) + return samples[0][0], samples[0][1], min(s[2] for s in samples) + + +if __name__ == "__main__": + main("import cost", check) diff --git a/scripts/verifications/import_cycles.py b/scripts/verifications/import_cycles.py new file mode 100644 index 00000000..c2184a57 --- /dev/null +++ b/scripts/verifications/import_cycles.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Detects dependency cycles between packages and import cycles between modules. + +A package cycle means neither package can be released, installed or reasoned +about alone. A module import cycle makes import success depend on which module a +caller happens to import first, and surfaces as partially initialized module +errors far from the cause. Function-local imports are excluded since they run +after both modules finish initializing. Cycles that exist only through +``TYPE_CHECKING`` imports are reported as warnings: they break nothing at +runtime but still tangle the design. +""" + +from _common import ( + Finding, + main, + module_index, + packages, + resolve_from, + runtime_modules, + tarjan, +) +from packaging.utils import canonicalize_name + + +def check() -> list[Finding]: + findings: list[Finding] = [] + findings.extend(_package_cycles()) + findings.extend(_module_cycles()) + return findings + + +def _package_cycles() -> list[Finding]: + dists = {p.dist for p in packages()} + declared: dict[str, set[str]] = {} + for package in packages(): + reqs = [ + *package.dependencies, + *(r for rs in package.extras.values() for r in rs), + ] + # An extra that requires the package's own other extras is not a cycle. + declared[package.dist] = ({canonicalize_name(r.name) for r in reqs} & dists) - { + package.dist + } + + roots = {root: p.dist for p in packages() for root in p.roots} + imported: dict[str, set[str]] = {p.dist: set() for p in packages()} + for module in runtime_modules(): + assert module.package is not None # noqa: S101 + for imp in module.imports: + target = roots.get(imp.top) + if imp.context == "runtime" and target and target != module.package.dist: + imported[module.package.dist].add(target) + + findings: list[Finding] = [] + for label, graph in (("declared", declared), ("imported", imported)): + for cycle in tarjan(graph): + findings.append( + Finding(f"{label} package dependency cycle: {' <-> '.join(cycle)}") + ) + return findings + + +def _module_cycles() -> list[Finding]: + index = module_index() + runtime: dict[str, set[str]] = {} + typing: dict[str, set[str]] = {} + for module in runtime_modules(): + runtime.setdefault(module.name, set()) + typing.setdefault(module.name, set()) + for imp in module.imports: + if imp.context == "deferred": + continue + target, _ = resolve_from(imp) + if target not in index or target == module.name: + continue + typing[module.name].add(target) + if imp.context == "runtime": + runtime[module.name].add(target) + + findings: list[Finding] = [] + runtime_cycles = tarjan(runtime) + for cycle in runtime_cycles: + findings.append( + Finding( + f"module import cycle ({len(cycle)}): {' -> '.join(cycle)}", + index[cycle[0]].path, + ) + ) + covered = {frozenset(c) for c in runtime_cycles} + for cycle in tarjan(typing): + if frozenset(cycle) in covered: + continue + findings.append( + Finding( + f"module import cycle via TYPE_CHECKING ({len(cycle)}): {' -> '.join(cycle)}", + index[cycle[0]].path, + severity="warning", + ) + ) + return findings + + +if __name__ == "__main__": + main("import cycles", check) diff --git a/scripts/verifications/import_deprecations.py b/scripts/verifications/import_deprecations.py new file mode 100644 index 00000000..f77bbd51 --- /dev/null +++ b/scripts/verifications/import_deprecations.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures importing any shipped module emits no deprecation warnings. + +A deprecation warning at import time is a removal already scheduled for a future +Python or dependency release. Customers who run with warnings as errors (common +in test suites) break immediately, and everyone else breaks on the release that +removes the API. Each module is imported in a fresh interpreter so the warning is +attributed to the module that triggers it. +""" + +from _common import Finding, main, probe, runtime_modules + +_CATEGORIES = frozenset( + {"DeprecationWarning", "PendingDeprecationWarning", "FutureWarning"} +) + + +def check() -> list[Finding]: + findings: list[Finding] = [] + modules = runtime_modules() + reports = probe(m.name for m in modules) + seen: set[tuple[str, str]] = set() + for module in modules: + for warning in reports[module.name]["warnings"]: + if warning["category"] not in _CATEGORIES: + continue + key = (warning["where"], warning["message"]) + if key in seen: + continue + seen.add(key) + findings.append( + Finding( + f"importing {module.name}: {warning['category']} at {warning['where']}: " + f"{warning['message']}", + module.path, + ) + ) + return findings + + +if __name__ == "__main__": + main("import deprecations", check) diff --git a/scripts/verifications/import_hygiene.py b/scripts/verifications/import_hygiene.py new file mode 100644 index 00000000..050d7fdf --- /dev/null +++ b/scripts/verifications/import_hygiene.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every shipped module imports alone in a fresh interpreter, without optional deps at top level. + +The test suite imports modules in whatever order the tests happen to, so a module +that only imports after a sibling has warmed ``sys.modules`` passes CI and fails +for the customer who imports it first. An unguarded top-level import of an +optional dependency turns a missing extra into an ``ImportError`` for anyone who +imports a module that pulls it in. A module nothing else imports is the extra's +own integration point: only a customer who chose that extra imports it, and +optional-deps checks that it fails clearly without the extra. +""" + +from _common import ( + Finding, + import_to_dists, + main, + module_index, + probe, + resolve_from, + runtime_modules, +) +from packaging.utils import canonicalize_name + + +def check() -> list[Finding]: + findings: list[Finding] = [] + modules = runtime_modules() + reports = probe(m.name for m in modules) + mapping = import_to_dists() + imported = _imported_by_others() + for module in modules: + report = reports[module.name] + if not report["ok"]: + error = report["error"] + findings.append( + Finding( + f"{module.name} fails to import alone: {error['type']}: {error['message']}", + module.path, + ) + ) + if module.name not in imported: + continue + assert module.package is not None # noqa: S101 + optional = module.package.extras_only + for imp in module.imports: + if imp.context != "runtime" or imp.guarded: + continue + dists = {canonicalize_name(d) for d in mapping.get(imp.top, set())} + if dists & optional: + findings.append( + Finding( + f"unguarded top-level import of optional dependency {imp.top}", + module.path, + imp.line, + ) + ) + return findings + + +def _imported_by_others() -> set[str]: + """Modules some other shipped module imports unguarded at runtime.""" + index = module_index() + targets: set[str] = set() + for module in runtime_modules(): + for imp in module.imports: + if imp.context != "runtime" or imp.guarded: + continue + target, _ = resolve_from(imp) + if target in index and target != module.name: + targets.add(target) + return targets + + +if __name__ == "__main__": + main("import hygiene", check) diff --git a/scripts/verifications/import_side_effects.py b/scripts/verifications/import_side_effects.py new file mode 100644 index 00000000..a9a95f36 --- /dev/null +++ b/scripts/verifications/import_side_effects.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures importing a shipped module changes no process-global state. + +A library is imported into someone else's process. Configuring logging, reading +or writing environment variables, starting threads, installing warning filters, +signal handlers or excepthooks at import time overrides the application's own +choices, makes behavior depend on import order, and freezes configuration before +the application has set it. Environment access is attributed to the source line +in our code that made it, and warning filters a dependency installs itself are +not ours; the rest is attributed to the imported module. +""" + +from _common import Finding, main, probe, runtime_modules + + +def check() -> list[Finding]: + findings: list[Finding] = [] + modules = runtime_modules() + reports = probe(m.name for m in modules) + # An effect appears for every module that transitively imports its cause; + # the importer pulling in the fewest new modules is closest to the cause. + culprits: dict[tuple[str, str, str | None], tuple[int, str, int]] = {} + for module in modules: + report = reports[module.name] + for effect in report["side_effects"]: + key = (effect["kind"], effect["detail"], effect["where"]) + weight = report.get("new_modules", 0) + best = culprits.get(key) + if best is None or weight < best[0]: + culprits[key] = (weight, module.name, (best[2] + 1) if best else 1) + else: + culprits[key] = (best[0], best[1], best[2] + 1) + paths = {m.name: m.path for m in modules} + for (kind, detail, where), (_, name, count) in sorted( + culprits.items(), key=lambda i: i[1][1] + ): + located = f" at {where}" if where else "" + findings.append( + Finding( + f"importing {name}: {kind} {detail}{located} (seen from {count} module(s))", + paths[name], + ) + ) + return findings + + +if __name__ == "__main__": + main("import side effects", check) diff --git a/scripts/verifications/lowest_bounds.py b/scripts/verifications/lowest_bounds.py new file mode 100644 index 00000000..f6ce1463 --- /dev/null +++ b/scripts/verifications/lowest_bounds.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every package works on the lowest dependency versions it claims to support. + +The lockfile pins recent versions, so a lower bound that is too low is never +exercised: code starts using a newer API, the bound stays put, and a customer +whose environment holds an older release gets an ``AttributeError`` at runtime. +This installs every package with ``--resolution lowest-direct`` into a temporary +venv, imports every shipped module, and type-checks the hand-written sources +against it. Generated clients are only imported: type-checking the largest one +takes a minute. Requirements with no lower bound at all are reported directly, since the lowest +resolution of an unbounded requirement is an arbitrary ancient release. +""" + +import os +import sys +import tempfile +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +from _common import ( + ROOT, + Finding, + Package, + all_runtime_module_names, + main, + package_by_dist, + packages, + probe, + run, +) +from packaging.utils import canonicalize_name + +_SHOWN_PYRIGHT_ERRORS = 40 + + +def check() -> list[Finding]: + findings = _unbounded() + targets: list[str] = [] + for package in packages(): + extras = ",".join(sorted(package.extras)) + targets.append(f"{package.dir}[{extras}]" if extras else str(package.dir)) + with tempfile.TemporaryDirectory(prefix="lowest-bounds-") as tmp: + venv = Path(tmp) / "venv" + created = run(["uv", "venv", "--quiet", "--python", "3.12", str(venv)]) + if created.returncode != 0: + return [*findings, Finding(f"uv venv failed: {created.stderr[-500:]}")] + python = str(venv / "bin" / "python") + installed = run( + [ + "uv", + "pip", + "install", + "--python", + python, + "--resolution", + "lowest-direct", + *targets, + ] + ) + if installed.returncode != 0: + return [ + *findings, + Finding( + f"lowest-direct install failed:\n{installed.stderr.strip()[-3000:]}" + ), + ] + frozen = run(["uv", "pip", "freeze", "--python", python]).stdout + findings.append(Finding(f"resolved:\n{frozen.strip()}", severity="info")) + + for name, report in sorted(probe(all_runtime_module_names(), python).items()): + if not report["ok"]: + error = report["error"] + findings.append( + Finding( + f"{name} fails to import: {error['type']}: {error['message']}" + ) + ) + + pyright = str(Path(sys.executable).parent / "pyright") + + def typecheck(package: Package) -> list[str]: + checked = run( + [ + pyright, + "-p", + str(package.dir), + "--pythonpath", + python, + str(package.src), + ], + env={ + "VIRTUAL_ENV": str(venv), + "PATH": f"{venv / 'bin'}:{os.environ['PATH']}", + }, + ) + lines = checked.stdout.splitlines() + return [line for line in lines if " - error: " in line] + + handwritten = [p for p in packages() if p.dir.parent.name != "clients"] + errors: list[str] = [] + with ThreadPoolExecutor(max_workers=os.cpu_count() or 4) as pool: + for result in pool.map(typecheck, handwritten): + errors.extend(result) + for line in errors[:_SHOWN_PYRIGHT_ERRORS]: + findings.append( + Finding(f"pyright: {line.strip().replace(str(ROOT) + '/', '')}") + ) + if len(errors) > _SHOWN_PYRIGHT_ERRORS: + findings.append( + Finding( + f"pyright: ... {len(errors) - _SHOWN_PYRIGHT_ERRORS} more errors", + severity="info", + ) + ) + return findings + + +def _unbounded() -> list[Finding]: + siblings = package_by_dist() + findings: list[Finding] = [] + for package in packages(): + reqs = [("dependencies", r) for r in package.dependencies] + [ + (f"extra {extra}", r) for extra, rs in package.extras.items() for r in rs + ] + for where, req in reqs: + if canonicalize_name(req.name) in siblings: + continue + if not any( + s.operator in (">=", ">", "~=", "==", "===") for s in req.specifier + ): + findings.append( + Finding( + f"{package.dist} {where}: {req} has no lower bound", + package.dir / "pyproject.toml", + ) + ) + return findings + + +if __name__ == "__main__": + main("lowest bounds", check) diff --git a/scripts/verifications/optional_deps.py b/scripts/verifications/optional_deps.py new file mode 100644 index 00000000..5f326534 --- /dev/null +++ b/scripts/verifications/optional_deps.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every module works, or fails clearly, when no optional dependencies are installed. + +Our venv always has every extra installed, so nothing here ever runs the way a +customer's minimal install does. In a temporary venv holding every package but +no extras, each shipped module is imported. A module that needs no extra must +import. A module that needs one may fail, but only with an error that tells the +customer which extra to install (smithy-core's ``MissingDependencyError``, or an +``ImportError`` that names the extra); a bare ``No module named 'awscrt'`` leaves +them guessing. +""" + +import tempfile +from pathlib import Path + +from _common import ( + Finding, + all_runtime_module_names, + import_to_dists, + main, + make_venv, + packages, + probe, + runtime_modules, +) +from packaging.utils import canonicalize_name + + +def check() -> list[Finding]: + findings: list[Finding] = [] + mapping = import_to_dists() + modules = {m.name: m for m in runtime_modules()} + with tempfile.TemporaryDirectory(prefix="optional-deps-") as tmp: + editable = [arg for p in packages() for arg in ("-e", str(p.dir))] + python = make_venv(Path(tmp) / "venv", *editable) + reports = probe(all_runtime_module_names(), python) + for name, report in sorted(reports.items()): + if report["ok"]: + continue + module = modules[name] + assert module.package is not None # noqa: S101 + error = report["error"] + missing = (error.get("missing") or "").split(".")[0] + dists = {canonicalize_name(d) for d in mapping.get(missing, set())} + extras = sorted( + extra + for extra, reqs in module.package.extras.items() + if dists & {canonicalize_name(r.name) for r in reqs} + ) + if not extras: + findings.append( + Finding( + f"{name} fails without extras: {error['type']}: {error['message']}", + module.path, + ) + ) + continue + message = error["message"] + clear = error["type"] == "MissingDependencyError" or any( + e in message for e in extras + ) + if not clear: + findings.append( + Finding( + f"{name} needs extra {'/'.join(extras)} but fails with an unclear " + f"{error['type']}: {message}", + module.path, + ) + ) + return findings + + +if __name__ == "__main__": + main("optional dependencies", check) diff --git a/scripts/verifications/package_metadata.py b/scripts/verifications/package_metadata.py new file mode 100644 index 00000000..1dbf8252 --- /dev/null +++ b/scripts/verifications/package_metadata.py @@ -0,0 +1,95 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every package's pyproject.toml and layout follow the same conventions. + +Customers see each package's metadata on PyPI and in their resolver: a package +claiming a different Python floor, license, or supported versions than its +siblings is either wrong or an undeclared policy split. A package missing +``py.typed`` silently drops type information for everyone who installs it. Each +field is compared against the value most packages use, so a deliberate outlier +is a visible, reviewable exception rather than drift. +""" + +import json +from collections import Counter +from typing import Any + +from _common import Finding, Package, main, packages + + +def check() -> list[Finding]: + findings: list[Finding] = [] + rows = {p.dist: (p, _fields(p)) for p in packages()} + keys = sorted({k for _, fields in rows.values() for k in fields}) + for key in keys: + values = {dist: fields.get(key) for dist, (_, fields) in rows.items()} + counts = Counter(json.dumps(v, sort_keys=True) for v in values.values()) + majority, _ = counts.most_common(1)[0] + for dist, value in sorted(values.items()): + encoded = json.dumps(value, sort_keys=True) + if encoded != majority: + package = rows[dist][0] + findings.append( + Finding( + f"{dist}: {key} = {encoded} (most packages: {majority})", + package.dir / "pyproject.toml", + ) + ) + return findings + + +def _fields(package: Package) -> dict[str, Any]: + project = package.pyproject["project"] + tool = package.pyproject.get("tool", {}) + build = package.pyproject.get("build-system", {}) + roots = package.roots + root = roots[0] if roots else "" + + def templated(value: Any) -> Any: + text = json.dumps(value) + # Scoped to the path so the aws-sdk-python package doesn't match the repo name. + group = package.dir.parent.name + text = text.replace(f"{group}/{package.dir.name}", f"{group}/") + if root: + text = text.replace(root, "") + return json.loads(text) + + return { + "requires-python": project.get("requires-python"), + "license": project.get("license"), + "authors": project.get("authors"), + "readme": project.get("readme"), + "classifiers": sorted( + c + for c in project.get("classifiers", []) + if not c.startswith(_PER_PACKAGE_CLASSIFIERS) + ), + "urls": templated(project.get("urls")), + "version source": "dynamic" + if "version" in project.get("dynamic", []) + else "static", + "hatch version path": templated( + tool.get("hatch", {}).get("version", {}).get("path") + ), + "build backend": build.get("build-backend"), + "build requires": build.get("requires"), + "hatch build exclude": tool.get("hatch", {}).get("build", {}).get("exclude"), + "ruff src": tool.get("ruff", {}).get("src"), + "single import root": len(roots) == 1, + "py.typed": all((package.src / r / "py.typed").exists() for r in roots), + "README.md": (package.dir / "README.md").exists(), + "CHANGELOG.md": (package.dir / "CHANGELOG.md").exists(), + "NOTICE": (package.dir / "NOTICE").exists(), + "LICENSE": (package.dir / "LICENSE").exists(), + "tests/unit": (package.dir / "tests" / "unit").is_dir(), + ".changes": (package.dir / ".changes").is_dir(), + } + + +# Topic and audience describe what each package is, not repo-wide policy. +_PER_PACKAGE_CLASSIFIERS = ("Topic ::", "Intended Audience ::") + + +if __name__ == "__main__": + main("package metadata", check) diff --git a/scripts/verifications/secret_repr.py b/scripts/verifications/secret_repr.py new file mode 100644 index 00000000..cc3f3ffb --- /dev/null +++ b/scripts/verifications/secret_repr.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures objects holding secrets never render them in ``repr`` or ``str``. + +Credentials, tokens and keys end up in reprs through logging, tracebacks, debugger +output and error messages, where they are copied into log stores and tickets. +Dataclasses render every field by default, so a new secret field leaks unless +someone remembers to opt it out. Each of our classes with a secret-named +constructor parameter is built with sentinel values, and its ``repr`` and ``str`` +must not contain any of them. +""" + +import importlib +import inspect +import re +import sys +from typing import Any + +from _common import Finding, main, members, packages, runtime_modules + +_SECRET = re.compile( + r"(^|_)(secret\w*|\w*token|password|passphrase|api_key|private_key)$", re.IGNORECASE +) + + +def check() -> list[Finding]: + for package in packages(): + sys.path.insert(0, str(package.src)) + findings: list[Finding] = [] + seen: set[type] = set() + for module in runtime_modules(): + try: + loaded = importlib.import_module(module.name) + except Exception: # noqa: S112 + continue + for name, cls in members(module, loaded): + if not inspect.isclass(cls) or cls.__module__ != module.name or cls in seen: + continue + seen.add(cls) + if inspect.isabstract(cls) or getattr(cls, "_is_protocol", False): + continue + findings.extend(_check_class(module.path, name, cls)) + return findings + + +def _check_class(path: Any, name: str, cls: type) -> list[Finding]: + try: + params = inspect.signature(cls).parameters + except (TypeError, ValueError): + return [] + secrets = {p: f"SENTINEL-{p}-{id(cls)}" for p in params if _SECRET.search(p)} + if not secrets: + return [] + kwargs: dict[str, Any] = {} + for param in params.values(): + if param.kind in (param.VAR_POSITIONAL, param.VAR_KEYWORD): + continue + if param.name in secrets: + kwargs[param.name] = secrets[param.name] + elif param.default is param.empty: + kwargs[param.name] = _placeholder(param) + try: + instance = cls(**kwargs) + except Exception as e: + return [ + Finding( + f"{name}: could not construct to inspect {sorted(secrets)}: {type(e).__name__}: {e}", + path, + severity="warning", + ) + ] + findings: list[Finding] = [] + for render in (repr, str): + try: + text = render(instance) + except Exception: # noqa: S112 + continue + leaked = sorted(p for p, sentinel in secrets.items() if sentinel in text) + if leaked: + findings.append( + Finding(f"{name}: {render.__name__}() shows {', '.join(leaked)}", path) + ) + return findings + + +def _placeholder(param: inspect.Parameter) -> Any: + annotation = str(param.annotation) + if "str" in annotation: + return "placeholder" + if "int" in annotation: + return 0 + return None + + +if __name__ == "__main__": + main("secret repr", check) diff --git a/scripts/verifications/source_order.py b/scripts/verifications/source_order.py new file mode 100644 index 00000000..5d316e83 --- /dev/null +++ b/scripts/verifications/source_order.py @@ -0,0 +1,165 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures public definitions come before private ones, in modules and in classes. + +A reader opening a file should meet its API first and its machinery last. With a +fixed order, the public surface of any file or class is read top-down without +scanning, and review diffs that move a symbol between public and private stand +out. Within a class, dunders (lifecycle) come first, then public members, then +private ones. + +Python needs a name defined before code that runs at import time uses it, such +as a base class, a decorator, a default value, an eager annotation or a class +attribute. A private definition used that way by a later public one takes the +public one's place in the order. +""" + +import ast +from pathlib import Path + +from _common import Finding, analyzed_modules, main + +type _Def = ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef + + +def check() -> list[Finding]: + findings: list[Finding] = [] + for module in analyzed_modules(): + lazy = _lazy_annotations(module.tree) + findings.extend( + _order(module.path, module.tree.body, scope="module", lazy=lazy) + ) + for node in ast.walk(module.tree): + if isinstance(node, ast.ClassDef): + findings.extend( + _order( + module.path, node.body, scope=f"class {node.name}", lazy=lazy + ) + ) + return findings + + +_DEF_TYPES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef) + + +def _rank(name: str, *, in_class: bool) -> int: + if in_class and name.startswith("__") and name.endswith("__"): + return 0 + return 2 if name.startswith("_") else 1 + + +def _order( + path: Path, body: list[ast.stmt], *, scope: str, lazy: bool +) -> list[Finding]: + findings: list[Finding] = [] + in_class = scope != "module" + ranks = _effective_ranks(body, in_class=in_class, lazy=lazy) + highest = 0 + highest_name = "" + for node, effective in zip(body, ranks, strict=True): + if not isinstance(node, _DEF_TYPES): + continue + rank = _rank(node.name, in_class=in_class) + if rank < highest: + kind = {0: "dunder", 1: "public"}[rank] + findings.append( + Finding( + f"{scope}: {kind} {node.name} follows {highest_name}", + path, + node.lineno, + ) + ) + elif effective > highest: + highest, highest_name = effective, node.name + return findings + + +def _effective_ranks(body: list[ast.stmt], *, in_class: bool, lazy: bool) -> list[int]: + """Rank each statement, lowered to that of any later statement needing it.""" + # Other statements stay where they are, so they rank as the highest + # definition above them. + ranks: list[int] = [] + above = 0 + for s in body: + if isinstance(s, _DEF_TYPES): + ranks.append(_rank(s.name, in_class=in_class)) + above = max(above, ranks[-1]) + elif not in_class and _defines_public(s): + ranks.append(1) + else: + ranks.append(above) + eager = [_eager_names(s, lazy=lazy) for s in body] + changed = True + while changed: + changed = False + for i, stmt in enumerate(body): + if not isinstance(stmt, _DEF_TYPES): + continue + for j in range(i + 1, len(body)): + if stmt.name in eager[j] and ranks[j] < ranks[i]: + ranks[i] = ranks[j] + changed = True + return ranks + + +def _eager_names(stmt: ast.stmt, *, lazy: bool) -> set[str]: + """Names a statement reads when it executes, excluding function bodies.""" + if isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)): + args = stmt.args + nodes: list[ast.AST | None] = [ + *stmt.decorator_list, + *args.defaults, + *args.kw_defaults, + ] + if not lazy: + params = [*args.posonlyargs, *args.args, *args.kwonlyargs] + params += [a for a in (args.vararg, args.kwarg) if a is not None] + nodes += [p.annotation for p in params] + [stmt.returns] + return {n for node in nodes for n in _loads(node)} + if isinstance(stmt, ast.ClassDef): + names = { + n + for node in [*stmt.decorator_list, *stmt.bases, *stmt.keywords] + for n in _loads(node) + } + for member in stmt.body: + names |= _eager_names(member, lazy=lazy) + return names + if isinstance(stmt, ast.AnnAssign) and lazy: + return _loads(stmt.value) | _loads(stmt.target) + if isinstance(stmt, ast.TypeAlias): + return set() + return _loads(stmt) + + +def _defines_public(stmt: ast.stmt) -> bool: + return any( + isinstance(n, ast.Name) + and isinstance(n.ctx, ast.Store) + and not n.id.startswith("_") + for n in ast.walk(stmt) + ) + + +def _loads(node: ast.AST | None) -> set[str]: + if node is None: + return set() + return { + n.id + for n in ast.walk(node) + if isinstance(n, ast.Name) and isinstance(n.ctx, ast.Load) + } + + +def _lazy_annotations(tree: ast.Module) -> bool: + return any( + isinstance(s, ast.ImportFrom) + and s.module == "__future__" + and any(a.name == "annotations" for a in s.names) + for s in tree.body + ) + + +if __name__ == "__main__": + main("source order", check) diff --git a/scripts/verifications/type_completeness.py b/scripts/verifications/type_completeness.py new file mode 100644 index 00000000..3f16fc7b --- /dev/null +++ b/scripts/verifications/type_completeness.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures every symbol in each package's public API is fully typed. + +Shipping ``py.typed`` promises customers that our public API is typed. Pyright's +strict mode on our own sources doesn't verify that promise: inferred return +types, unannotated public attributes and partially unknown types type-check +internally but show up as ``Unknown`` in a customer's editor and type checker. +``pyright --verifytypes`` measures the public surface the way a consumer sees it. + +It only resolves installed (non-editable) packages, so the wheels are built and +installed into a temporary venv first. +""" + +import json +import os +import sys +import tempfile +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +from _common import ( + Finding, + build_wheels, + main, + make_venv, + packages, + run, + runtime_modules, +) + +_SHOWN_PER_PACKAGE = 20 + + +def check() -> list[Finding]: + findings: list[Finding] = [] + pyright = str(Path(sys.executable).parent / "pyright") + with tempfile.TemporaryDirectory(prefix="type-completeness-") as tmp: + work = Path(tmp) + wheels = build_wheels(work / "wheels") + venv = work / "venv" + make_venv(venv, *map(str, wheels)) + + def one(target: tuple[Path, str]) -> list[Finding]: + path, name = target + proc = run( + [pyright, "--verifytypes", name, "--ignoreexternal", "--outputjson"], + cwd=work, + env={ + "VIRTUAL_ENV": str(venv), + "PATH": f"{venv / 'bin'}:{os.environ['PATH']}", + }, + ) + return _report(path, name, proc.stdout, proc.stderr) + + with ThreadPoolExecutor(max_workers=os.cpu_count() or 4) as pool: + for result in pool.map(one, _targets()): + findings.extend(result) + return findings + + +def _targets() -> list[tuple[Path, str]]: + """Each package root, or each public non-model-data module of a generated client. + + Verifying a whole client walks its model data, which takes tens of seconds + for the largest clients. + """ + targets: list[tuple[Path, str]] = [] + for package in packages(): + if package.dir.parent.name != "clients": + targets.extend((package.src / r, r) for r in package.roots) + continue + mine = [m for m in runtime_modules() if m.package is package] + data = [m.name for m in mine if m.model_data] + # pyright verifies a package's submodules too, so skip packages holding model data. + targets.extend( + (m.path, m.name) + for m in mine + if m.public + and not m.model_data + and not any(d.startswith(m.name + ".") for d in data) + ) + return targets + + +def _report(path: Path, root: str, stdout: str, stderr: str) -> list[Finding]: + try: + completeness = json.loads(stdout)["typeCompleteness"] + except (json.JSONDecodeError, KeyError): + return [Finding(f"{root}: pyright failed: {stderr.strip()[-300:]}", path)] + if not completeness.get("packageRootDirectory"): + return [Finding(f"{root}: pyright did not resolve the installed package", path)] + score = completeness["completenessScore"] + if score >= 1: + return [] + counts = completeness["exportedSymbolCounts"] + total = sum(counts.values()) + findings = [ + Finding(f"{root}: {score:.1%} of {total} exported symbols fully typed", path) + ] + incomplete = [ + s for s in completeness["symbols"] if s["isExported"] and not s["isTypeKnown"] + ] + for symbol in incomplete[:_SHOWN_PER_PACKAGE]: + first = next( + ( + d["message"].splitlines()[0] + for d in symbol["diagnostics"] + if d["severity"] == "error" + ), + "type not known", + ) + findings.append(Finding(f" {symbol['name']}: {first}", path, severity="info")) + if len(incomplete) > _SHOWN_PER_PACKAGE: + findings.append( + Finding( + f" ... {len(incomplete) - _SHOWN_PER_PACKAGE} more", + path, + severity="info", + ) + ) + return findings + + +if __name__ == "__main__": + main("type completeness", check) diff --git a/scripts/verifications/version_consistency.py b/scripts/verifications/version_consistency.py new file mode 100644 index 00000000..b1c06d5a --- /dev/null +++ b/scripts/verifications/version_consistency.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures each package's ``__version__`` matches its distribution version. + +``__version__`` is what user-agent strings, bug reports and customer feature +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. +""" + +import importlib +import importlib.metadata + +from _common import Finding, main, packages + + +def check() -> list[Finding]: + findings: list[Finding] = [] + for package in packages(): + pyproject = package.dir / "pyproject.toml" + project = package.pyproject["project"] + dynamic = "version" in project.get("dynamic", []) + if not dynamic: + findings.append( + Finding( + f"{package.dist}: static version {project.get('version')!r} in pyproject.toml; " + "other packages read it from __version__", + pyproject, + ) + ) + elif package.version_file is None: + findings.append( + Finding( + f"{package.dist}: dynamic version without a hatch version path", + pyproject, + ) + ) + if package.source_version is None: + findings.append( + Finding( + f"{package.dist}: __version__ is not a literal in the package source", + pyproject, + severity="warning", + ) + ) + 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 + + +if __name__ == "__main__": + main("version consistency", check) diff --git a/scripts/verifications/wheel_contents.py b/scripts/verifications/wheel_contents.py new file mode 100644 index 00000000..243d94e0 --- /dev/null +++ b/scripts/verifications/wheel_contents.py @@ -0,0 +1,117 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures each built wheel ships exactly the package's source, plus ``py.typed``. + +Tests and CI import from the source tree, never from the wheel, so a module left +out by a build-config change, a missing ``py.typed``, or a test file swept into +the wheel all pass CI and reach customers. This builds the wheels and inspects +them. It also flags private modules that nothing imports: in a wheel those are +dead weight that can't be reached without reaching into private paths. +""" + +import tempfile +import zipfile +from pathlib import Path + +from _common import ( + Finding, + Package, + build_wheels, + main, + packages, + resolve_from, + runtime_modules, +) + +_ALLOWED_SUFFIXES = (".py", ".pyi", ".so", ".pyd") + + +def check() -> list[Finding]: + findings: list[Finding] = [] + with tempfile.TemporaryDirectory(prefix="wheel-contents-") as tmp: + wheels = build_wheels(Path(tmp)) + by_dist = {w.name.split("-")[0].replace("_", "-").lower(): w for w in wheels} + for package in packages(): + wheel = by_dist.get(package.dist) + if wheel is None: + findings.append(Finding(f"{package.dist}: no wheel built", package.dir)) + continue + with zipfile.ZipFile(wheel) as archive: + names = set(archive.namelist()) + findings.extend(_inspect(package, wheel.name, names)) + findings.extend(_unreferenced_private_modules()) + return findings + + +def _inspect(package: Package, wheel: str, names: set[str]) -> list[Finding]: + findings: list[Finding] = [] + expected = { + str(p.relative_to(package.src)) + for root in package.roots + for p in (package.src / root).rglob("*.py*") + if p.suffix in (".py", ".pyi") and "__pycache__" not in p.parts + } + for missing in sorted(expected - names): + findings.append(Finding(f"{wheel}: missing {missing}", package.src / missing)) + for root in package.roots: + if f"{root}/py.typed" not in names: + findings.append( + Finding(f"{wheel}: missing {root}/py.typed", package.src / root) + ) + for name in sorted(names): + if ".dist-info/" in name or name.endswith("/"): + continue + parts = name.split("/") + if ( + "tests" in parts + or parts[-1] == "conftest.py" + or parts[-1].startswith("test_") + ): + findings.append(Finding(f"{wheel}: ships test file {name}", package.dir)) + elif parts[-1] != "py.typed" and not parts[-1].endswith(_ALLOWED_SUFFIXES): + findings.append( + Finding( + f"{wheel}: ships unexpected file {name}", + package.dir, + severity="warning", + ) + ) + elif parts[0] not in package.roots: + findings.append( + Finding( + f"{wheel}: ships file outside its import roots: {name}", package.dir + ) + ) + return findings + + +def _unreferenced_private_modules() -> list[Finding]: + referenced: set[str] = set() + for module in runtime_modules(): + for imp in module.imports: + target, _ = resolve_from(imp) + parts = target.split(".") + referenced.update(".".join(parts[: i + 1]) for i in range(len(parts))) + # Model data imports aren't indexed, so the packages holding it count as used. + referenced.update(m.name for m in runtime_modules() if m.model_data) + findings: list[Finding] = [] + for module in runtime_modules(): + leaf = module.name.split(".")[-1] + if ( + module.public + or module.model_data + or module.name in referenced + or (leaf.startswith("__") and leaf.endswith("__")) + ): + continue + if any(other.startswith(module.name + ".") for other in referenced): + continue + findings.append( + Finding(f"private module {module.name} is never imported", module.path) + ) + return findings + + +if __name__ == "__main__": + main("wheel contents", check) From 228f88ba48f41f7bfa390e7475ae8fbb8a8f372c Mon Sep 17 00:00:00 2001 From: George Fu Date: Thu, 12 Nov 2026 15:47:01 +0000 Subject: [PATCH 2/4] Port verification rule updates from smithy-python --- Makefile | 14 +- scripts/aio-naming/aio_naming.py | 322 ++++++++++++++++++- scripts/api-snapshot/api-rex.json | 145 +++++++++ scripts/api-snapshot/api.json | 63 ++++ scripts/api-snapshot/api_snapshot.py | 238 ++++++++++++++ scripts/api-snapshot/mirrors.txt | 26 ++ scripts/verifications/dunder_all.py | 43 ++- scripts/verifications/export_names.py | 4 +- scripts/verifications/version_consistency.py | 110 +++++-- 9 files changed, 906 insertions(+), 59 deletions(-) create mode 100644 scripts/api-snapshot/api-rex.json create mode 100644 scripts/api-snapshot/api.json create mode 100644 scripts/api-snapshot/api_snapshot.py create mode 100644 scripts/api-snapshot/mirrors.txt diff --git a/Makefile b/Makefile index d2314ddf..956035fb 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ SMITHY_PYTHON ?= ../smithy-python BATCH_SIZE ?= 25 .PHONY: docs docs-serve docs-clean docs-install docs-lock venv gen clients fragments publish-codegen aio-check meta-package-check static-analysis test-unit test-functional test-integration \ - check-imports check-conventions check-api check-dependencies check-packaging check-import-stability verify-venv \ + check-imports check-conventions check-api check-dependencies check-packaging check-import-stability verify-venv api-snapshot \ canonical-imports dunder-all source-order dynamic-imports banned-imports docstrings declared-deps import-cycles \ dep-specifiers package-metadata version-consistency export-names exception-hierarchy secret-repr import-hygiene \ import-deprecations import-side-effects import-cost wheel-contents type-completeness optional-deps lowest-bounds @@ -33,7 +33,7 @@ static-analysis: uvx ruff@$(RUFF_VERSION) format --check --config $$p/pyproject.toml $$dirs; \ $(UV_RUN) $$p $$group --with pyright==$(PYRIGHT_VERSION) pyright -p $$p $$dirs; \ done - # TODO: enable once aws-credentials-sts passes the aio naming rules. + # TODO: enable once smithy-python codegen passes the aio naming rules. # $(MAKE) aio-check $(MAKE) check-imports $(MAKE) check-conventions @@ -95,8 +95,8 @@ $(eval $(filter-out gen,$(MAKECMDGOALS)):;@:) endif # Enforces aio naming and separation in package and client sources. -aio-check: - uv run --no-project --python $(PYTHON_VERSION) python scripts/aio-naming/aio_naming.py +aio-check: $(VERIFY_STAMP) + $(VERIFY_VENV)/bin/python scripts/aio-naming/aio_naming.py # Catches a client missing from aws-sdk-python's extras or in-repo sources, which # would otherwise resolve from PyPI and fail on any unpublished version bump. @@ -115,7 +115,8 @@ check-conventions: ## Holds shipped code to our ordering, docstring, error, and $(MAKE) exception-hierarchy # $(MAKE) secret-repr # TODO: enable once it passes -check-api: ## Keeps the public API surface typed and exported under its own names. +check-api: ## Keeps the public API surface reviewed, typed, and exported under its own names. + $(MAKE) api-snapshot $(MAKE) export-names $(MAKE) dunder-all # $(MAKE) type-completeness # TODO: enable once it passes @@ -150,6 +151,9 @@ $(VERIFY_STAMP): $(addsuffix /pyproject.toml,$(PROJECTS)) $(foreach p,$(PROJECTS),-e $(p) -r $(p)/pyproject.toml) touch $@ +api-snapshot: $(VERIFY_STAMP) ## Records the public API surface and dual-mode mirrors of each package and asserts no git diff. + $(VERIFY_VENV)/bin/python scripts/api-snapshot/api_snapshot.py + verify-venv: $(VERIFY_STAMP) ## Installs every client and package into the shared verification venv. canonical-imports: $(VERIFY_STAMP) ## Prevents re-export imports, which hide where symbols live and invite cycles. diff --git a/scripts/aio-naming/aio_naming.py b/scripts/aio-naming/aio_naming.py index 911c1b10..f871562a 100755 --- a/scripts/aio-naming/aio_naming.py +++ b/scripts/aio-naming/aio_naming.py @@ -7,35 +7,66 @@ ``aio`` subfolder holds all async code; everything else must be sync. Inside ``/aio``: - - classes must have at least one async method, own or inherited, and public - ones must be named ``Async*``. + - classes must be async, and public ones must be named ``Async*``, except the + transports with no sync twin in ``_NO_TWIN_NAMES``. A class is + async when it has an async method, own or inherited, or its annotations or + base class arguments name an ``Async*`` type. A class holding only data (no + methods besides dunders) also qualifies, since it serves the async code + beside it. - public functions (not methods) must be async and be named ``*_async``; an ``async_*`` name is accepted with a warning. Outside it: + - no imports from an ``aio`` package, including under ``TYPE_CHECKING``. - public classes must not be named ``Async*``. - functions and methods must not be named ``async_*`` or ``*_async``. - no ``async def``. +Public members a sync twin shares with its aio counterpart must take the same +argument names, so code switching calling styles keeps its keyword arguments. +Members on only one side are allowed, since some capabilities (event streams, +duplex streaming) are async-only by design. Only twins under ``packages/`` are +compared and listed, since generated clients' API is codegen's to keep stable. + A symbol is private when its name, an enclosing class, or a module path segment is ``_``-prefixed, or it is nested in a function. + +Output starts with each public aio symbol that has a sync twin +(``AsyncFoo``/``Foo``, ``foo_async``/``foo``) as a tree by folder, followed by +the findings. """ import argparse import ast +import importlib +import inspect import re import sys from collections.abc import Iterator from dataclasses import dataclass, field from pathlib import Path -from typing import Literal +from typing import Any, Literal ROOT = Path(__file__).resolve().parent.parent.parent +PACKAGES_DIR = ROOT / "packages" DEFAULT_SRC_GLOBS = ("packages/*/src", "clients/*/src") AIO = "aio" _ASYNC_CLASS = re.compile(r"^_*Async(?=[A-Z0-9_]|$)") +# Sync names that describe async things rather than being async themselves. +_SYNC_ASYNC_NAMES = frozenset({"AsyncBodyError"}) + +# aio classes with no sync twin that keep their established names. +_NO_TWIN_NAMES = frozenset( + { + "AIOHTTPClient", + "AIOHTTPClientConfig", + "AWSCRTHTTPClient", + "AWSCRTHTTPClientConfig", + } +) + type _Def = ast.FunctionDef | ast.AsyncFunctionDef @@ -65,13 +96,18 @@ def main() -> int: ) args = parser.parse_args() - src_dirs: list[Path] = args.src_dirs or sorted( - p for pattern in DEFAULT_SRC_GLOBS for p in ROOT.glob(pattern) if p.is_dir() - ) - findings = check([src.resolve() for src in src_dirs]) + src_dirs: list[Path] = args.src_dirs or _default_src_dirs() + findings, modules = _load([src.resolve() for src in src_dirs]) + findings.extend(_check_modules(modules)) + findings.extend(_check_parity(modules)) + sys.stdout.write(dual_mode_tree(modules)) + if findings: + sys.stdout.write("\n") for finding in findings: sys.stdout.write(f"{finding}\n") + # Flushed so the summary follows the report even when both streams share a pipe. + sys.stdout.flush() errors = sum(f.severity == "error" for f in findings) warnings = len(findings) - errors sys.stderr.write(f"aio naming: {errors} error(s), {warnings} warning(s).\n") @@ -80,6 +116,60 @@ def main() -> int: def check(src_dirs: list[Path]) -> list[Finding]: """Checks all package source roots under the given src directories.""" + findings, modules = _load(src_dirs) + return findings + _check_modules(modules) + _check_parity(modules) + + +def dual_mode_report() -> str: + """Render the dual-mode symbol tree for the default package source roots.""" + _, modules = _load([src.resolve() for src in _default_src_dirs()]) + return dual_mode_tree(modules) + + +def dual_mode_tree(modules: "list[_Module]") -> str: + """Render the public aio symbols that have a sync twin, as a tree by folder. + + Each aio symbol is listed under its module with its sync twin, which is shown + with its module when that isn't the aio module's mirror outside ``aio``. + """ + rows: dict[tuple[str, ...], list[tuple[str, str]]] = {} + for pair in _twin_pairs(modules): + mirror = ".".join(p for p in pair.aio.name.split(".") if p != AIO) + shown = ( + pair.twin + if pair.sync.name == mirror + else f"{pair.twin} ({pair.sync.name})" + ) + rows.setdefault(_display_path(pair.aio), []).append((pair.name, shown)) + + lines = ["Dual-mode symbols (async in aio, sync twin outside):"] + previous: tuple[str, ...] = () + count = 0 + for path in sorted(rows): + common = 0 + while ( + common < min(len(path), len(previous)) - 1 + and path[common] == previous[common] + ): + common += 1 + for depth in range(common, len(path)): + lines.append(" " * (depth + 1) + path[depth]) + width = max(len(name) for name, _ in rows[path]) + for name, shown in sorted(rows[path]): + lines.append(" " * (len(path) + 1) + f"{name.ljust(width)} {shown}") + count += 1 + previous = path + lines.append(f"{count} dual-mode symbol(s).") + return "\n".join(lines) + "\n" + + +def _default_src_dirs() -> list[Path]: + return sorted( + p for pattern in DEFAULT_SRC_GLOBS for p in ROOT.glob(pattern) if p.is_dir() + ) + + +def _load(src_dirs: list[Path]) -> "tuple[list[Finding], list[_Module]]": findings: list[Finding] = [] modules: list[_Module] = [] for src in src_dirs: @@ -100,15 +190,123 @@ def check(src_dirs: list[Path]) -> list[Finding]: f"syntax: {e.msg}", ) ) + return findings, modules + +def _check_modules(modules: "list[_Module]") -> list[Finding]: + findings: list[Finding] = [] index = _Index(modules) for module in modules: checker = (_AioChecker if module.in_aio else _SyncChecker)(module, index) checker.visit_body(module.tree.body, in_class=False, private=module.private) + if not module.in_aio: + findings.extend(_aio_imports(module)) findings.extend(checker.findings) return findings +def _check_parity(modules: "list[_Module]") -> list[Finding]: + findings: list[Finding] = [] + for pair in _twin_pairs(modules): + async_obj = getattr(importlib.import_module(pair.aio.name), pair.name) + sync_obj = getattr(importlib.import_module(pair.sync.name), pair.twin) + label = f"{pair.aio.name}.{pair.name} / {pair.sync.name}.{pair.twin}" + line = next( + ( + n.lineno + for n in pair.aio.tree.body + if isinstance(n, ast.ClassDef | ast.FunctionDef | ast.AsyncFunctionDef) + and n.name == pair.name + ), + 0, + ) + findings.extend( + Finding(pair.aio.path, line, 1, "error", message) + for message in _parity_differences(label, async_obj, sync_obj) + ) + return findings + + +@dataclass(frozen=True) +class _TwinPair: + aio: "_Module" + name: str + sync: "_Module" + twin: str + + +def _twin_pairs(modules: "list[_Module]") -> Iterator[_TwinPair]: + """Public aio symbols with a sync twin, preferring the twin in the aio module's mirror.""" + sync_defs: dict[tuple[str, str], list[_Module]] = {} + for module in modules: + if module.in_aio or module.private: + continue + for name in _public_defs(module): + sync_defs.setdefault((_root_of(module), name), []).append(module) + + for module in modules: + if not module.in_aio or module.private: + continue + if not module.path.is_relative_to(PACKAGES_DIR): + continue + mirror = ".".join(p for p in module.name.split(".") if p != AIO) + for name in _public_defs(module): + twin = _sync_name(name) + if twin is None: + continue + candidates = sync_defs.get((_root_of(module), twin)) + if not candidates: + continue + found = next((m for m in candidates if m.name == mirror), candidates[0]) + yield _TwinPair(module, name, found, twin) + + +def _parity_differences(label: str, async_obj: Any, sync_obj: Any) -> list[str]: + if inspect.isclass(async_obj) and inspect.isclass(sync_obj): + async_members = _public_members(async_obj) + sync_members = _public_members(sync_obj) + messages: list[str] = [] + for name in sorted(async_members.keys() & sync_members.keys()): + messages += _signature_difference( + f"{label}.{name}", async_members[name], sync_members[name] + ) + return messages + if callable(async_obj) and callable(sync_obj): + return _signature_difference(label, async_obj, sync_obj) + return [f"{label}: kinds differ"] + + +def _public_members(cls: type) -> dict[str, Any]: + members: dict[str, Any] = {} + for klass in reversed(cls.__mro__): + if klass is object or klass.__module__ in ("typing", "abc", "builtins"): + continue + for name, value in vars(klass).items(): + if not name.startswith("_"): + members[name] = value + return members + + +def _signature_difference(label: str, async_obj: Any, sync_obj: Any) -> list[str]: + async_sig, sync_sig = _signature(async_obj), _signature(sync_obj) + if async_sig is None or sync_sig is None or async_sig == sync_sig: + return [] + return [f"{label}: argument names differ: async {async_sig} / sync {sync_sig}"] + + +def _signature(obj: Any) -> tuple[str, ...] | None: + func = obj.fget if isinstance(obj, property) else getattr(obj, "__func__", obj) + if not callable(func): + return None + try: + sig = inspect.signature(func) + except (TypeError, ValueError): + return None + return tuple( + p.name for p in sig.parameters.values() if p.name not in ("self", "cls") + ) + + @dataclass class _Module: name: str @@ -189,9 +387,12 @@ def is_async(self, module: _Module, cls: ast.ClassDef) -> bool: if key not in self._async: # Seeded False so an inheritance cycle terminates. self._async[key] = False - self._async[key] = any( - isinstance(m, ast.AsyncFunctionDef) for m in _methods(cls) - ) or any(self._base_is_async(module, base) for base in cls.bases) + self._async[key] = ( + any(isinstance(m, ast.AsyncFunctionDef) for m in _methods(cls)) + or any(self._base_is_async(module, base) for base in cls.bases) + or _names_async_type(cls) + or _holds_only_data(cls) + ) return self._async[key] def _base_is_async(self, module: _Module, base: ast.expr) -> bool: @@ -290,7 +491,11 @@ def _report( class _AioChecker(_Checker): def check_class(self, node: ast.ClassDef, *, private: bool) -> None: - if not private and not _ASYNC_CLASS.match(node.name): + if ( + not private + and not _ASYNC_CLASS.match(node.name) + and node.name not in _NO_TWIN_NAMES + ): self._report( node, "error", f"aio class '{node.name}' must start with Async" ) @@ -321,7 +526,11 @@ def check_def(self, node: _Def, *, is_method: bool, private: bool) -> None: class _SyncChecker(_Checker): def check_class(self, node: ast.ClassDef, *, private: bool) -> None: - if not private and _ASYNC_CLASS.match(node.name): + if ( + not private + and _ASYNC_CLASS.match(node.name) + and node.name not in _SYNC_ASYNC_NAMES + ): self._report( node, "error", f"non-aio class '{node.name}' must not start with Async" ) @@ -341,6 +550,30 @@ def check_def(self, node: _Def, *, is_method: bool, private: bool) -> None: ) +def _aio_imports(module: "_Module") -> Iterator[Finding]: + is_package = module.path.stem == "__init__" + for node in ast.walk(module.tree): + match node: + case ast.Import(): + sources = [alias.name for alias in node.names] + case ast.ImportFrom(): + source = module._absolute(node, is_package=is_package) # pyright: ignore[reportPrivateUsage] + # `from pkg import aio` names the aio package itself. + sources = [f"{source}.{alias.name}" for alias in node.names] + case _: + continue + for source in sources: + if AIO in source.split("."): + yield Finding( + module.path, + node.lineno, + node.col_offset + 1, + "error", + f"non-aio module '{module.name}' must not import '{source}'", + ) + break + + def _misplaced_aio(root: Path) -> Iterator[Finding]: for directory in sorted(p for p in root.rglob(AIO) if p.is_dir()): if directory.parent != root: @@ -370,5 +603,70 @@ def _is_source_dir(path: Path) -> bool: return path.is_dir() and path.name != "__pycache__" and "." not in path.name +_ASYNC_TYPE_NAME = re.compile(r"(? bool: + """Whether the class's annotations or base class arguments name an Async* type.""" + exprs: list[ast.expr] = [b.slice for b in cls.bases if isinstance(b, ast.Subscript)] + for stmt in cls.body: + if isinstance(stmt, ast.AnnAssign): + exprs.append(stmt.annotation) + elif isinstance(stmt, ast.FunctionDef | ast.AsyncFunctionDef): + args = stmt.args + for arg in [*args.posonlyargs, *args.args, *args.kwonlyargs]: + if arg.annotation is not None: + exprs.append(arg.annotation) + if stmt.returns is not None: + exprs.append(stmt.returns) + for expr in exprs: + for node in ast.walk(expr): + text = ( + node.id + if isinstance(node, ast.Name) + else node.attr + if isinstance(node, ast.Attribute) + else node.value + if isinstance(node, ast.Constant) and isinstance(node.value, str) + else None + ) + if text is not None and _ASYNC_TYPE_NAME.search(text): + return True + return False + + +def _holds_only_data(cls: ast.ClassDef) -> bool: + return all(m.name.startswith("__") and m.name.endswith("__") for m in _methods(cls)) + + +def _public_defs(module: "_Module") -> list[str]: + return [ + node.name + for node in module.tree.body + if isinstance(node, ast.ClassDef | ast.FunctionDef | ast.AsyncFunctionDef) + and not node.name.startswith("_") + ] + + +def _sync_name(name: str) -> str | None: + if match := _ASYNC_CLASS.match(name): + return name[match.end() :] + if name.endswith("_async"): + return name.removesuffix("_async") + return None + + +def _root_of(module: "_Module") -> str: + return module.name.split(".")[0] + + +def _display_path(module: "_Module") -> tuple[str, ...]: + """Folders then file of a module, e.g. ("smithy_core/", "aio/", "client.py").""" + parts = module.name.split(".") + if module.path.stem == "__init__": + return (*(f"{p}/" for p in parts), "__init__.py") + return (*(f"{p}/" for p in parts[:-1]), f"{parts[-1]}.py") + + if __name__ == "__main__": sys.exit(main()) diff --git a/scripts/api-snapshot/api-rex.json b/scripts/api-snapshot/api-rex.json new file mode 100644 index 00000000..33f70417 --- /dev/null +++ b/scripts/api-snapshot/api-rex.json @@ -0,0 +1,145 @@ +{ + "aws_credentials_http": { + "ContainerCredentialsResolver": "rex-class", + "EcsContainerProvider": "rex-class", + "client": "rex-module", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_http.aio": { + "AsyncContainerCredentialsResolver": "rex-class[async]", + "AsyncEcsContainerProvider": "rex-class[async]", + "client": "rex-module", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_http.aio.client": { + "fetch_credentials": "rex-function" + }, + "aws_credentials_http.aio.providers": { + "AsyncContainerCredentialsResolver": "rex-class[async]", + "is_configured": "rex-function" + }, + "aws_credentials_http.aio.resolvers": { + "AsyncHttpCredentialsClient": "rex-class[async]", + "is_fresh": "rex-function", + "read_token_file": "rex-function", + "resolve_credentials": "rex-function" + }, + "aws_credentials_http.client": { + "fetch_credentials": "rex-function" + }, + "aws_credentials_http.providers": { + "ContainerCredentialsResolver": "rex-class", + "is_configured": "rex-function" + }, + "aws_credentials_http.resolvers": { + "HttpCredentialsClient": "rex-class", + "is_fresh": "rex-function", + "read_token_file": "rex-function", + "resolve_credentials": "rex-function" + }, + "aws_credentials_imds": { + "Ec2InstanceMetadataProvider": "rex-class", + "IMDSConfig": "rex-class", + "IMDSConfigurationError": "rex-exception", + "IMDSCredentialsResolver": "rex-class", + "client": "rex-module", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_imds.aio": { + "AsyncEc2InstanceMetadataProvider": "rex-class[async]", + "AsyncIMDSClient": "rex-class[async]", + "AsyncIMDSCredentialsResolver": "rex-class[async]", + "AsyncIMDSTokenCache": "rex-class[async]", + "client": "rex-module", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_imds.aio.client": { + "IMDSConfig": "rex-class", + "IMDSToken": "rex-class", + "fetch_metadata": "rex-function", + "fetch_token": "rex-function" + }, + "aws_credentials_imds.aio.providers": { + "AsyncIMDSCredentialsResolver": "rex-class[async]", + "provider_config": "rex-function" + }, + "aws_credentials_imds.aio.resolvers": { + "AsyncIMDSClient": "rex-class[async]", + "IMDSConfig": "rex-class", + "is_fresh": "rex-function", + "resolve_credentials": "rex-function" + }, + "aws_credentials_imds.client": { + "IMDSConfig": "rex-class", + "IMDSConfigurationError": "rex-exception", + "IMDSToken": "rex-class", + "fetch_metadata": "rex-function", + "fetch_token": "rex-function" + }, + "aws_credentials_imds.providers": { + "IMDSCredentialsResolver": "rex-class", + "provider_config": "rex-function" + }, + "aws_credentials_imds.resolvers": { + "IMDSClient": "rex-class", + "IMDSConfig": "rex-class", + "is_fresh": "rex-function", + "resolve_credentials": "rex-function" + }, + "aws_credentials_sts": { + "AssumeRoleConfigurationError": "rex-exception", + "AssumeRoleCredentialsResolver": "rex-class", + "ProfileAssumeRoleCredentialsResolver": "rex-class", + "ProfileAssumeRoleProvider": "rex-class", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_sts.aio": { + "AsyncAssumeRoleCredentialsResolver": "rex-class[async]", + "AsyncProfileAssumeRoleCredentialsResolver": "rex-class[async]", + "AsyncProfileAssumeRoleProvider": "rex-class[async]", + "providers": "rex-module", + "resolvers": "rex-module" + }, + "aws_credentials_sts.aio.providers": { + "AsyncProfileAssumeRoleCredentialsResolver": "rex-class[async]", + "has_role_arn": "rex-function" + }, + "aws_credentials_sts.aio.resolvers": { + "AssumeRolePlan": "rex-class", + "CredentialSource": "rex-class", + "StaticSource": "rex-class", + "assume_role_input": "rex-function", + "default_session_name": "rex-function", + "find_provider": "rex-function", + "first_resolver": "rex-function", + "identity_from_response": "rex-function", + "plan_profile": "rex-function", + "require_provider": "rex-function", + "resolve_sts_region": "rex-function", + "validate_profile": "rex-function" + }, + "aws_credentials_sts.providers": { + "ProfileAssumeRoleCredentialsResolver": "rex-class", + "has_role_arn": "rex-function" + }, + "aws_credentials_sts.resolvers": { + "AssumeRoleConfigurationError": "rex-exception", + "AssumeRolePlan": "rex-class", + "CredentialSource": "rex-class", + "StaticSource": "rex-class", + "assume_role_input": "rex-function", + "default_session_name": "rex-function", + "find_provider": "rex-function", + "first_resolver": "rex-function", + "identity_from_response": "rex-function", + "plan_profile": "rex-function", + "require_provider": "rex-function", + "resolve_sts_region": "rex-function", + "validate_profile": "rex-function" + } +} diff --git a/scripts/api-snapshot/api.json b/scripts/api-snapshot/api.json new file mode 100644 index 00000000..d3133e68 --- /dev/null +++ b/scripts/api-snapshot/api.json @@ -0,0 +1,63 @@ +{ + "aws_credentials_cognito": {}, + "aws_credentials_http": {}, + "aws_credentials_http.aio": {}, + "aws_credentials_http.aio.client": { + "AsyncHttpCredentialsClient": "class[async]" + }, + "aws_credentials_http.aio.providers": { + "AsyncEcsContainerProvider": "class[async]" + }, + "aws_credentials_http.aio.resolvers": { + "AsyncContainerCredentialsResolver": "class[async]" + }, + "aws_credentials_http.client": { + "HttpCredentialsClient": "class" + }, + "aws_credentials_http.providers": { + "EcsContainerProvider": "class" + }, + "aws_credentials_http.resolvers": { + "ContainerCredentialsResolver": "class" + }, + "aws_credentials_imds": {}, + "aws_credentials_imds.aio": {}, + "aws_credentials_imds.aio.client": { + "AsyncIMDSClient": "class[async]", + "AsyncIMDSTokenCache": "class[async]" + }, + "aws_credentials_imds.aio.providers": { + "AsyncEc2InstanceMetadataProvider": "class[async]" + }, + "aws_credentials_imds.aio.resolvers": { + "AsyncIMDSCredentialsResolver": "class[async]" + }, + "aws_credentials_imds.client": { + "IMDSClient": "class", + "IMDSTokenCache": "class" + }, + "aws_credentials_imds.providers": { + "Ec2InstanceMetadataProvider": "class" + }, + "aws_credentials_imds.resolvers": { + "IMDSCredentialsResolver": "class" + }, + "aws_credentials_login": {}, + "aws_credentials_sso": {}, + "aws_credentials_sts": {}, + "aws_credentials_sts.aio": {}, + "aws_credentials_sts.aio.providers": { + "AsyncProfileAssumeRoleProvider": "class[async]" + }, + "aws_credentials_sts.aio.resolvers": { + "AsyncAssumeRoleCredentialsResolver": "class[async]", + "AsyncProfileAssumeRoleCredentialsResolver": "class[async]" + }, + "aws_credentials_sts.providers": { + "ProfileAssumeRoleProvider": "class" + }, + "aws_credentials_sts.resolvers": { + "AssumeRoleCredentialsResolver": "class", + "ProfileAssumeRoleCredentialsResolver": "class" + } +} diff --git a/scripts/api-snapshot/api_snapshot.py b/scripts/api-snapshot/api_snapshot.py new file mode 100644 index 00000000..65cca038 --- /dev/null +++ b/scripts/api-snapshot/api_snapshot.py @@ -0,0 +1,238 @@ +#!/usr/bin/env python +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Ensures the Python API surface is reflected in the api.json and api-rex.json files. + +All changes to the surface must be intentional and reviewed in the PR making them. + +For every public module (no underscore-prefixed path segment), records each +reachable non-underscore symbol that we OWN. Symbols defined in the module's own +source go in api.json. Symbols re-exported from another of our modules (marked +with a ``rex-`` kind prefix) go in api-rex.json. Reachable re-exports of +third-party/stdlib names are excluded: they are not ours to keep stable. Then +fails if writing either file changed it from the committed version, so an +unreviewed surface change cannot pass CI. + +Also records the dual-mode symbols, each async symbol under ``aio`` with its sync +twin, in mirrors.txt, which fails the same way when it changes. +""" + +import ast +import importlib +import importlib.util +import inspect +import json +import subprocess +import sys +import types +import typing +from pathlib import Path + +HERE = Path(__file__).resolve().parent +ROOT = HERE.parent.parent +DATA_PATH = HERE / "api.json" +REX_PATH = HERE / "api-rex.json" +MIRRORS_PATH = HERE / "mirrors.txt" +PACKAGES_DIR = ROOT / "packages" + +# Top-level module names we own: one per packages/*/src/ or +# clients/*/src/. A reachable symbol defined under any of these is ours to +# keep stable; anything else (stdlib, typing, third-party) is excluded from the +# surface. Only packages are recorded, since codegen owns the clients' surface. +_OWNED_ROOTS = frozenset( + child.name + for src in [*PACKAGES_DIR.glob("*/src"), *ROOT.glob("clients/*/src")] + for child in src.iterdir() + if (child / "__init__.py").exists() +) + + +def main() -> int: + for src in sorted(PACKAGES_DIR.glob("*/src")): + sys.path.insert(0, str(src)) + + surface = _collect_surface() + declared = { + mod: {n: k for n, k in symbols.items() if not k.startswith("rex-")} + for mod, symbols in surface.items() + } + rex = { + mod: {n: k for n, k in symbols.items() if k.startswith("rex-")} + for mod, symbols in surface.items() + } + # api.json keeps every public module, even one with only re-exports, so a + # removed module still shows up in its diff. + _write_json(DATA_PATH, declared) + _write_json(REX_PATH, {mod: symbols for mod, symbols in rex.items() if symbols}) + MIRRORS_PATH.write_text(_mirrors()) + + paths = (DATA_PATH, REX_PATH, MIRRORS_PATH) + changed = [path for path in paths if _changed(path)] + if changed: + names = ", ".join(str(path.relative_to(ROOT)) for path in changed) + print( + f"\n{names} updated to reflect the current API surface. Review the " + "change above and commit it in the PR that made it.", + file=sys.stderr, + ) + return 1 + print("API snapshot is up to date.") + return 0 + + +def _write_json(path: Path, surface: dict[str, dict[str, str]]) -> None: + ordered = {k: dict(sorted(v.items())) for k, v in sorted(surface.items())} + path.write_text(json.dumps(ordered, indent=2) + "\n") + + +def _collect_surface() -> dict[str, dict[str, str]]: + surface: dict[str, dict[str, str]] = {} + for src in sorted(PACKAGES_DIR.glob("*/src")): + for modname in _public_module_paths(src): + surface[modname] = _module_surface(modname) + return surface + + +def _public_module_paths(src: Path) -> list[str]: + # A module path is public only when every segment is non-underscore; a single + # `_private`/`_foo` segment anywhere makes the whole subtree private. + paths: set[str] = set() + for init in src.rglob("__init__.py"): + parts = init.parent.relative_to(src).parts + if parts and not any(seg.startswith("_") for seg in parts): + paths.add(".".join(parts)) + for py in src.rglob("*.py"): + if py.name == "__init__.py": + continue + parts = py.relative_to(src).with_suffix("").parts + if not any(seg.startswith("_") for seg in parts): + paths.add(".".join(parts)) + return sorted(paths) + + +def _module_surface(modname: str) -> dict[str, str]: + module = importlib.import_module(modname) + defined = _defined_here(modname) + surface: dict[str, str] = {} + for name in dir(module): + if name.startswith("_"): + continue + obj = getattr(module, name) + if name in defined: + # Defined in this module's own source: our surface, plain kind. + surface[name] = _kind(obj) + elif _is_owned(obj): + # Reachable here but defined in another of our packages: we own and + # must track it, but mark it rex (re-export) so the defining module + # and the re-export are distinguishable. + surface[name] = f"rex-{_kind(obj)}" + # else: re-exported third-party/stdlib (Protocol, os, TYPE_CHECKING) -- + # reachable, but not ours to keep stable, so excluded from the surface. + return surface + + +def _defined_here(modname: str) -> set[str]: + # Top-level definitions in this module's own source. AST is the only way to + # attribute a bare constant (MAX_PAYLOAD_LENGTH = ...) to its defining + # module: the value object carries no assignment site, and __module__ on an + # int is "builtins". + path = _source_path(modname) + if path is None: + return set() + tree = ast.parse(path.read_text()) + names: set[str] = set() + for node in tree.body: + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): + names.add(node.name) + elif isinstance(node, ast.Assign): + names.update(t.id for t in node.targets if isinstance(t, ast.Name)) + elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + names.add(node.target.id) + return names + + +def _source_path(modname: str) -> Path | None: + for src in PACKAGES_DIR.glob("*/src"): + base = src.joinpath(*modname.split(".")) + for candidate in (base / "__init__.py", base.with_suffix(".py")): + if candidate.exists(): + return candidate + return None + + +def _is_owned(obj: object) -> bool: + mod = ( + obj.__name__ + if isinstance(obj, types.ModuleType) + else getattr(obj, "__module__", None) + ) + return bool(mod) and mod.split(".")[0] in _OWNED_ROOTS + + +def _kind(obj: object) -> str: + if isinstance(obj, types.ModuleType): + return "module" + if inspect.isclass(obj): + if getattr(obj, "_is_protocol", False): + base = "protocol" + elif issubclass(obj, Exception): + base = "exception" + else: + base = "class" + return f"{base}[async]" if _has_async_method(obj) else base + if inspect.iscoroutinefunction(obj): + return "function[async]" + if inspect.isfunction(obj) or inspect.isbuiltin(obj) or inspect.ismethod(obj): + return "function" + if _is_type_alias(obj): + return "type_alias" + if callable(obj): + return "callable" + return type(obj).__name__ + + +def _has_async_method(cls: type) -> bool: + # Reads raw __dict__ values across the MRO rather than getmembers, which would + # trigger descriptor/property execution on a class not meant to be introspected + # cold. Unwraps static/classmethod so an `async def` behind either is still seen. + for klass in getattr(cls, "__mro__", (cls,)): + for value in vars(klass).values(): + if inspect.iscoroutinefunction(getattr(value, "__func__", value)): + return True + return False + + +def _is_type_alias(obj: object) -> bool: + # typing.TypeAliasType (`type X = ...`), typing special forms (Literal/Union + # subscriptions), and PEP 585 builtin generics (`list[int]`) are all exported + # as type aliases, not runtime values. + if isinstance(obj, typing.TypeAliasType): + return True + if type(obj) is types.GenericAlias: + return True + return getattr(obj, "__module__", None) == "typing" and hasattr(obj, "__origin__") + + +def _mirrors() -> str: + path = ROOT / "scripts" / "aio-naming" / "aio_naming.py" + spec = importlib.util.spec_from_file_location("aio_naming", path) + assert spec is not None and spec.loader is not None # noqa: S101 + module = importlib.util.module_from_spec(spec) + # Registered first so the module's dataclasses can resolve their own module. + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module.dual_mode_report() + + +def _changed(path: Path) -> bool: + """Whether the file differs from git, counting a new untracked file as changed.""" + # Fixed argv, no shell: git resolved from PATH is the repo's own convention. + argv = ["git", "diff", "--exit-code", "--", str(path)] + diff = subprocess.run(argv, cwd=ROOT) # noqa: S603 + argv = ["git", "ls-files", "--others", "--exclude-standard", "--", str(path)] + untracked = subprocess.run(argv, cwd=ROOT, capture_output=True, text=True) # noqa: S603 + return diff.returncode != 0 or bool(untracked.stdout.strip()) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/api-snapshot/mirrors.txt b/scripts/api-snapshot/mirrors.txt new file mode 100644 index 00000000..31602ddd --- /dev/null +++ b/scripts/api-snapshot/mirrors.txt @@ -0,0 +1,26 @@ +Dual-mode symbols (async in aio, sync twin outside): + aws_credentials_http/ + aio/ + client.py + AsyncHttpCredentialsClient HttpCredentialsClient + providers.py + AsyncEcsContainerProvider EcsContainerProvider + resolvers.py + AsyncContainerCredentialsResolver ContainerCredentialsResolver + aws_credentials_imds/ + aio/ + client.py + AsyncIMDSClient IMDSClient + AsyncIMDSTokenCache IMDSTokenCache + providers.py + AsyncEc2InstanceMetadataProvider Ec2InstanceMetadataProvider + resolvers.py + AsyncIMDSCredentialsResolver IMDSCredentialsResolver + aws_credentials_sts/ + aio/ + providers.py + AsyncProfileAssumeRoleProvider ProfileAssumeRoleProvider + resolvers.py + AsyncAssumeRoleCredentialsResolver AssumeRoleCredentialsResolver + AsyncProfileAssumeRoleCredentialsResolver ProfileAssumeRoleCredentialsResolver +10 dual-mode symbol(s). diff --git a/scripts/verifications/dunder_all.py b/scripts/verifications/dunder_all.py index 1d63de06..4cf132ad 100644 --- a/scripts/verifications/dunder_all.py +++ b/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) diff --git a/scripts/verifications/export_names.py b/scripts/verifications/export_names.py index 415cda53..301ec63b 100644 --- a/scripts/verifications/export_names.py +++ b/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 diff --git a/scripts/verifications/version_consistency.py b/scripts/verifications/version_consistency.py index b1c06d5a..aa8e1c75 100644 --- a/scripts/verifications/version_consistency.py +++ b/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) From 82d28747bb0e2af6fa1ebfec94ef2c3429ad63df Mon Sep 17 00:00:00 2001 From: George Fu Date: Thu, 12 Nov 2026 16:02:48 +0000 Subject: [PATCH 3/4] Fail lowest-bounds when pyright exits abnormally --- scripts/verifications/lowest_bounds.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/verifications/lowest_bounds.py b/scripts/verifications/lowest_bounds.py index f6ce1463..011fc621 100644 --- a/scripts/verifications/lowest_bounds.py +++ b/scripts/verifications/lowest_bounds.py @@ -95,6 +95,11 @@ def typecheck(package: Package) -> list[str]: "PATH": f"{venv / 'bin'}:{os.environ['PATH']}", }, ) + # Exit codes above 1 mean pyright stopped before checking anything. + if checked.returncode not in (0, 1): + output = (checked.stderr or checked.stdout).strip()[-500:] + rel = package.dir.relative_to(ROOT) + return [f"{rel} exited {checked.returncode}: {output}"] lines = checked.stdout.splitlines() return [line for line in lines if " - error: " in line] From 5a7366c5a32e9d1f1939e8ab66372dbb3bb12beb Mon Sep 17 00:00:00 2001 From: George Fu Date: Thu, 12 Nov 2026 17:24:49 +0000 Subject: [PATCH 4/4] Add new clients to import-cost baseline --- scripts/verifications/import-cost.json | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/scripts/verifications/import-cost.json b/scripts/verifications/import-cost.json index ec424b43..0acf4ef1 100644 --- a/scripts/verifications/import-cost.json +++ b/scripts/verifications/import-cost.json @@ -11,8 +11,12 @@ "aws_credentials_sso (all public modules)": 1, "aws_credentials_sts": 4, "aws_credentials_sts (all public modules)": 7, + "aws_sdk_acm": 1, + "aws_sdk_acm (all public modules)": 13, "aws_sdk_api_gateway": 1, "aws_sdk_api_gateway (all public modules)": 15, + "aws_sdk_appconfigdata": 1, + "aws_sdk_appconfigdata (all public modules)": 13, "aws_sdk_bedrock": 1, "aws_sdk_bedrock (all public modules)": 13, "aws_sdk_bedrock_agent": 1, @@ -29,6 +33,12 @@ "aws_sdk_bedrock_data_automation_runtime (all public modules)": 13, "aws_sdk_bedrock_runtime": 1, "aws_sdk_bedrock_runtime (all public modules)": 13, + "aws_sdk_cloudformation": 1, + "aws_sdk_cloudformation (all public modules)": 13, + "aws_sdk_cloudfront": 1, + "aws_sdk_cloudfront (all public modules)": 13, + "aws_sdk_cloudfront_keyvaluestore": 1, + "aws_sdk_cloudfront_keyvaluestore (all public modules)": 13, "aws_sdk_cloudtrail": 1, "aws_sdk_cloudtrail (all public modules)": 13, "aws_sdk_cognito_identity": 1, @@ -37,8 +47,18 @@ "aws_sdk_connecthealth (all public modules)": 13, "aws_sdk_dynamodb": 1, "aws_sdk_dynamodb (all public modules)": 15, + "aws_sdk_ec2": 1, + "aws_sdk_ec2 (all public modules)": 13, + "aws_sdk_ecs": 1, + "aws_sdk_ecs (all public modules)": 13, "aws_sdk_guardduty": 1, "aws_sdk_guardduty (all public modules)": 13, + "aws_sdk_iam": 1, + "aws_sdk_iam (all public modules)": 13, + "aws_sdk_kinesis": 1, + "aws_sdk_kinesis (all public modules)": 13, + "aws_sdk_kms": 1, + "aws_sdk_kms (all public modules)": 13, "aws_sdk_lambda": 1, "aws_sdk_lambda (all public modules)": 13, "aws_sdk_lex_runtime_v2": 1, @@ -49,6 +69,10 @@ "aws_sdk_python (all public modules)": 1, "aws_sdk_qbusiness": 1, "aws_sdk_qbusiness (all public modules)": 13, + "aws_sdk_rds": 1, + "aws_sdk_rds (all public modules)": 13, + "aws_sdk_s3": 1, + "aws_sdk_s3 (all public modules)": 15, "aws_sdk_sagemaker_runtime_http2": 1, "aws_sdk_sagemaker_runtime_http2 (all public modules)": 13, "aws_sdk_secrets_manager": 1,