Merge pull request #885 from dhellmann/mypy-coverage-2

feat(mypy): enable comprehensive strict type checking across codebase

Author: LalatenduMohanty

AGENTS.md

@@ -18,8 +18,6 @@
 
 ### Do
 
-- **Type annotations REQUIRED** on ALL functions including tests. Use syntax compatible with Python 3.11+
-- Use `X | None` not `Optional[X]`
 - Add docstrings on all public functions and classes
 - Use file-scoped commands for fast feedback (see below)
 - Follow existing patterns - search codebase for similar code
@@ -29,8 +27,6 @@
 
 ### Don't
 
-- Don't use `Optional[X]` syntax (use `X | None`)
-- Don't omit type annotations or return types
 - Don't run full test suite for small changes (use file-scoped)
 - Don't create temporary helper scripts or workarounds
 - Don't commit without running quality checks

CONTRIBUTING.md

@@ -44,6 +44,9 @@ git remote add upstream https://github.com/python-wheel-build/fromager.git
 
 # 4. Create development environment
 hatch env create
+
+# 5. Install pre-commit hook (optional but recommended)
+./scripts/setup-pre-commit-hook.sh
 ```
 
 ### Contribution Workflow
@@ -72,6 +75,25 @@ git push origin feat/<short-description>
 # 7. Create a pull request on GitHub
 ```
 
+### Pre-commit Hook (Recommended)
+
+To ensure quality checks run automatically before each commit, install the pre-commit hook that runs both linting and mypy type checking:
+
+```bash
+# Install the pre-commit hook (run once after cloning)
+./scripts/setup-pre-commit-hook.sh
+
+# The hook automatically runs before each commit:
+# - hatch run lint:check
+# - hatch run mypy:check
+
+# If the hook fails, it will prevent the commit and show helpful messages
+# You can fix issues automatically with:
+hatch run lint:fix
+```
+
+The pre-commit hook prevents commits that would fail CI quality checks, saving time and ensuring consistent code quality.
+
 ---
 
 ## Coding Standards

e2e/flit_core_override/src/package_plugins/flit_core.py

@@ -1,11 +1,23 @@
 import logging
+import pathlib
+import typing
 
-from fromager import external_commands
+from packaging.requirements import Requirement
+from packaging.version import Version
+
+from fromager import build_environment, context, external_commands
 
 logger = logging.getLogger(__name__)
 
 
-def build_wheel(ctx, build_env, extra_environ, req, sdist_root_dir, version):
+def build_wheel(
+    ctx: context.WorkContext,
+    build_env: build_environment.BuildEnvironment,
+    extra_environ: dict[str, str],
+    req: Requirement,
+    sdist_root_dir: pathlib.Path,
+    version: Version,
+) -> None:
     # flit_core is a basic build system dependency for several
     # packages. It is capable of building its own wheels, so we use the
     # bootstrapping instructions to do that and put the wheel in the
@@ -15,8 +27,8 @@ def build_wheel(ctx, build_env, extra_environ, req, sdist_root_dir, version):
     # https://flit.pypa.io/en/stable/bootstrap.html
     logger.info('using override to build flit_core wheel in %s', sdist_root_dir)
     external_commands.run(
-        [build_env.python, '-m', 'flit_core.wheel',
-         '--outdir', ctx.wheels_build],
-        cwd=sdist_root_dir,
+        [str(build_env.python), '-m', 'flit_core.wheel',
+         '--outdir', str(ctx.wheels_build)],
+        cwd=str(sdist_root_dir),
         extra_environ=extra_environ,
     )

e2e/fromager_hooks/src/package_plugins/hooks.py

@@ -15,7 +15,7 @@ def after_build_wheel(
     dist_version: str,
     sdist_filename: pathlib.Path,
     wheel_filename: pathlib.Path,
-):
+) -> None:
     logger.info(
         f"running post build hook in {__name__} for {sdist_filename} and {wheel_filename}"
     )
@@ -31,7 +31,7 @@ def after_bootstrap(
     dist_version: str,
     sdist_filename: pathlib.Path | None,
     wheel_filename: pathlib.Path | None,
-):
+) -> None:
     logger.info(
         f"running post bootstrap hook in {__name__} for {sdist_filename} and {wheel_filename}"
     )
@@ -46,7 +46,7 @@ def after_prebuilt_wheel(
     dist_name: str,
     dist_version: str,
     wheel_filename: pathlib.Path,
-):
+) -> None:
     logger.info(
         f"running post build hook in {__name__} for {wheel_filename}"
     )

e2e/setup_coverage.py

@@ -5,7 +5,7 @@
 import sys
 import site
 
-def setup_coverage():
+def setup_coverage() -> None:
     """Create coverage.pth file for subprocess coverage collection."""
     # Get the virtual environment root
     venv_root = pathlib.Path(sys.prefix)

pyproject.toml

@@ -187,20 +187,18 @@ ignore = [
 known-first-party = ["fromager"]
 
 [tool.mypy]
-mypy_path = ["src"]
+mypy_path = ["src", "tests", "e2e"]
 # TODO: tighten type checks
-# check_untyped_defs = true
-# disallow_incomplete_defs = true
-# disallow_untyped_defs = true
-# warn_return_any = true
+check_untyped_defs = true
+disallow_incomplete_defs = true
+disallow_untyped_defs = true
+warn_return_any = true
 
 # TODO: remove excludes and silent follow
 follow_imports = "silent"
 exclude = [
-    "^src/fromager/sources\\.py$",
-    "^src/fromager/sdist\\.py$",
-    "^src/fromager/commands/build\\.py$",
-    "^src/fromager/settings\\.py$",
+    "^e2e/.*/build/.*$",
+    "^e2e/pyo3_test/.*$",
 ]
 
 [[tool.mypy.overrides]]
@@ -274,6 +272,7 @@ features = ["mypy"]
 check = [
   "mypy -p fromager",
   "mypy tests/",
+  "mypy e2e/",
 ]
 
 [tool.hatch.envs.docs]

scripts/pre-commit

@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+#
+# Git pre-commit hook to run quality checks
+# This ensures all commits pass linting and type checking
+#
+
+set -e  # Exit immediately if any command fails
+
+echo "Running pre-commit checks..."
+
+# Run lint check
+echo "🔍 Running lint check..."
+if ! hatch run lint:check; then
+    echo "❌ Lint check failed. Please fix linting issues and try again."
+    echo "   You can run 'hatch run lint:fix' to automatically fix many issues."
+    exit 1
+fi
+
+# Run mypy type checking
+echo "🔍 Running mypy type check..."
+if ! hatch run mypy:check; then
+    echo "❌ MyPy type check failed. Please fix type annotations and try again."
+    exit 1
+fi
+
+echo "✅ All pre-commit checks passed!"

scripts/setup-pre-commit-hook.sh

@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+#
+# Setup script to install the pre-commit hook for fromager development
+#
+# This script copies the pre-commit hook to .git/hooks/ and makes it executable
+#
+
+set -e
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+HOOKS_DIR="$REPO_ROOT/.git/hooks"
+PRE_COMMIT_HOOK="$HOOKS_DIR/pre-commit"
+
+echo "🔧 Setting up pre-commit hook for fromager..."
+
+# Check if we're in a git repository
+if [ ! -d "$HOOKS_DIR" ]; then
+    echo "❌ Error: Not in a git repository or .git/hooks directory not found"
+    exit 1
+fi
+
+# Copy the pre-commit hook script
+cp "$SCRIPT_DIR/pre-commit" "$PRE_COMMIT_HOOK"
+
+# Make it executable
+chmod +x "$PRE_COMMIT_HOOK"
+
+echo "✅ Pre-commit hook installed successfully!"
+echo ""
+echo "The hook will now run 'hatch run lint:check' and 'hatch run mypy:check'"
+echo "before every commit to ensure code quality."
+echo ""
+echo "To bypass the hook for a specific commit (not recommended):"
+echo "  git commit --no-verify -m \"message\""

src/fromager/__main__.py

@@ -258,7 +258,7 @@ def main(
     main.add_command(cmd)
 
 
-def _format_exception(exc):
+def _format_exception(exc: BaseException) -> str:
     if exc.__cause__:
         cause = _format_exception(exc.__cause__)
         return f"{exc} because {cause}"

src/fromager/bootstrapper.py

@@ -292,7 +292,7 @@ def _build_sdist(
             self.ctx, self.ctx.sdists_builds, req, str(resolved_version)
         )
         if not find_sdist_result:
-            sdist_filename = sources.build_sdist(
+            sdist_filename: pathlib.Path = sources.build_sdist(
                 ctx=self.ctx,
                 req=req,
                 version=resolved_version,
@@ -1015,7 +1015,9 @@ def _resolve_from_version_source(
             logger.debug(f"could not resolve {req} from {version_source}: {err}")
             return None
 
-    def _create_unpack_dir(self, req: Requirement, resolved_version: Version):
+    def _create_unpack_dir(
+        self, req: Requirement, resolved_version: Version
+    ) -> pathlib.Path:
         unpack_dir = self.ctx.work_dir / f"{req.name}-{resolved_version}"
         unpack_dir.mkdir(parents=True, exist_ok=True)
         return unpack_dir