build(pyrefly): enable Pyrefly static type checking across Python targets - #4020
Open
rickeylev wants to merge 52 commits into
Open
build(pyrefly): enable Pyrefly static type checking across Python targets#4020rickeylev wants to merge 52 commits into
rickeylev wants to merge 52 commits into
Conversation
Integrates rules_pyrefly in MODULE.bazel, defines the Pyrefly aspect in tools/pyrefly.bzl with --config=pyrefly in .bazelrc, and addresses a type override suppression in python/runfiles/runfiles.py.
Relocates pyrefly.bzl into tools/private and updates the aspect target reference in .bazelrc.
Relocates pyrefly.bzl under tools/private/pyrefly and updates .bazelrc label reference.
Synchronizes BUILD.bazel with Gazelle generated rules.
Updates the aspect reference in .bazelrc to use the tools/private/pyrefly package path.
…ate BUILD files Cleans up unused pyrefly bzl_library definitions now that pyrefly.bzl lives in tools/private/pyrefly.
Removes the unnecessary bzl_library target and BUILD.bazel from tools/private/pyrefly, updating .bazelrc to reference //tools/private:pyrefly/pyrefly.bzl%pyrefly_aspect.
…ype checking Implements pyrefly_check in tools/private:pyrefly/pyrefly.bzl taking targets as a label list, and instantiates runfiles_check in tests/runfiles/BUILD.bazel targeting //python/runfiles:runfiles.
… update load labels
…d use default_visibility in tools/private/pyrefly/BUILD.bazel
Defines pyrefly_check_test with test = True so running bazel test //tests/runfiles/... automatically discovers and executes runfiles_check as a test target.
Restores pyrefly_check as a build rule in tools/private/pyrefly/pyrefly.bzl and tests it using standard build_test in tests/runfiles/BUILD.bazel.
Encapsulates pyrefly_check rule and build_test into a reusable pyrefly_check_test macro in tools/private/pyrefly/pyrefly.bzl.
Relocates the pyrefly test helper package from tools/private/pyrefly to tests/support/pyrefly and updates load references.
Replaces the custom pyrefly_check wrapper rule with rules_pyrefly's native opt_in_tags feature. Applies pyrefly_aspect globally via .bazelrc while tagging //python/runfiles:runfiles for opt-in static type checking.
Prevents WORKSPACE-mode (non-Bzlmod) builds from failing due to unresolved @rules_pyrefly repository references while preserving opt-in tag-based static type checking under Bzlmod.
Top-level if statements are forbidden in Bazel BUILD files. Moving the BZLMOD_ENABLED check inside the pyrefly_check_test macro definition in pyrefly.bzl ensures clean BUILD file evaluation while preserving Bzlmod-only target generation.
Adds rules_pyrefly repository definition to rules_python_internal_deps in internal_dev_deps.bzl so package loading succeeds during WORKSPACE mode builds. Removes non-existent bzl_library dependency target.
Clarifies in pyrefly.bzl that Pyrefly type checking does not support WORKSPACE mode and exits early when Bzlmod is not enabled.
Registers rules_pyrefly_stub in internal_dev_deps.bzl for WORKSPACE mode so load() statements resolve, and adds global aspect flag to .bazelrc to apply Pyrefly static type checking across tagged targets.
Removes the macro target invocation now that Pyrefly type checking is applied globally via aspect on tagged targets.
Removes the unused macro and rule definitions, leaving only pyrefly_aspect in tests/support/pyrefly/pyrefly.bzl.
Adds an always-on workspace rule prohibiting the use of auth tokens in command line arguments.
Enable Pyrefly type checking by default using opt-out enforcement. Configure rules_pyrefly aspect and toolchain setup in sphinxdocs module. Set --config=pyrefly by default in root and sphinxdocs .bazelrc files.
Enable Pyrefly type checking across tests and sphinxdocs libraries to ensure type safety, resolving typing diagnostics and adding necessary type ignores for generated proto stubs. - Remove no-pyrefly tags across sphinxdocs, test targets, and fixtures. - Add TypedDict definitions and modernize type annotations in sphinx_build and bzl.py. - Introduce _get_bzl_domain() helper in bzl.py and preserve @OverRide decorators. - Simplify variable assignment in dependency_resolver.py. - Add pyrefly enablement plan documenting findings and requirements.
…ule_directly The test asserts that importing a non-direct dependency raises ModuleNotFoundError at runtime; keep the import with type ignore.
…nd venv targets - Remove no-pyrefly tags and add # type: ignore annotations on dynamic or compiled extension imports in bootstrap_impls, build_data, py_extension, and venv_site_packages_libs tests. - Add explanatory comments above retained no-pyrefly tags in pytest_test. - Update Pyrefly enablement plan with latest resolutions.
rickeylev
enabled auto-merge
August 8, 2026 22:35
…erator and sphinxdocs
…tools and tests Add type narrowing assertions for runfiles resolution, refine container and parameter type annotations across release and dependency tooling, and annotate unsupported external imports.
… uv_lock_pypi_server
… main targets Setting no-pyrefly tag on both main and main_with_gen_data avoids unwanted dependency bundling in use_rule_with_dir_in_outs while preventing PyCompile action conflicts across targets sharing main.py.
rickeylev
marked this pull request as draft
August 8, 2026 23:51
auto-merge was automatically disabled
August 8, 2026 23:51
Pull request was converted to draft
Collaborator
Author
|
Apparently slopbot was lying about the tests it ran. I'll switch this back to non-draft once its sorted |
Update type checking and runfiles usage according to review comments: - Prefer in-file # type: ignore comments over target-level no-pyrefly tags - Add Runfiles.CreateOrRaise() helper and use Path API in toolchain tests - Factor out _compute_inert_node_types() in py_test_main_validator - Restore @OverRide annotations in sphinx_bzl - Clarify Python agent rules for type asserts and annotation consent
Add in-file # pyrefly: ignore[bad-override] annotations and # type: ignore for intersphinx import in sphinx_bzl/bzl.py to allow sphinxdocs type checking to pass without disabling target-level checking.
…ts_for_prs call Allow MockGitHub to pass self to resolve_merge_commits_for_prs without failing Pyrefly type checking on GitHub parameter type.
rickeylev
marked this pull request as ready for review
August 9, 2026 03:35
Collaborator
Author
|
Ready, PTAL |
dougthor42
reviewed
Aug 9, 2026
|
|
||
| ## Type Checking & Annotations | ||
| * **Target skipping vs in-file disables**: Prefer disabling specific errors in | ||
| source files (e.g. `# type: ignore[...]` / `# pyrefly: ignore[...]`) over |
Collaborator
There was a problem hiding this comment.
FYI pyrefly treats # type: ignore[...] as a blanket # type: ignore. I recommend against using it.
| source files (e.g. `# type: ignore[...]` / `# pyrefly: ignore[...]`) over | ||
| disabling type checking on targets (e.g. `tags = ["no-pyrefly"]`). | ||
| * **Type assertions**: When adding assertions for type narrowing, add an | ||
| end-of-line comment: `assert foo is not None # type assert`. |
Collaborator
There was a problem hiding this comment.
assert is removed when python is run with -O. I doubt this ever applies, but it can potentially cause issues.
Perhaps recommend if foo is not None: raise AssertionError(...)?
Non-blocking.
(Such asserts in test files are obviously just fine.)
| import examples.wheel.lib.module_with_data as module_with_data | ||
| import examples.wheel.lib.module_with_type_annotations as module_with_type_annotations | ||
| import examples.wheel.lib.simple_module as simple_module | ||
| import examples.wheel.lib.module_with_data as module_with_data # type: ignore[import-not-found] |
Collaborator
There was a problem hiding this comment.
Use # pyrefly: ignore[missing-import] instead because # type: ignore[...] is treated as a blanket ignore.
Further similar comments suppressed.
|
|
||
|
|
||
| def deserialize_structured_args(args: Dict[str, str]) -> Dict: | ||
| def deserialize_structured_args(args: dict[str, Any]) -> dict[str, Any]: |
Collaborator
There was a problem hiding this comment.
Just confirming that we've dropped support for py3.8 right? dict as a type hint was added in 3.9.
| # TODO: Update return type to Self when 3.11 is the min version | ||
| # https://peps.python.org/pep-0673/ | ||
| @staticmethod | ||
| def CreateOrRaise(env: Optional[Dict[str, str]] = None) -> "Runfiles": |
Collaborator
There was a problem hiding this comment.
New feature; should be added to news.
|
|
||
| @override | ||
| def resolve_xref( | ||
| def resolve_xref( # pyrefly: ignore[bad-override] |
Collaborator
There was a problem hiding this comment.
What's pyrefly complaining about here?
Adding a lot of ignore[bad-override] is code smell.
| if alt_names is not None: | ||
| alt_names = list(alt_names) | ||
| else: | ||
| alt_names = [] |
Collaborator
There was a problem hiding this comment.
Maybe just
alt_names = list(alt_names) if alt_names else []?
| from typing import Callable, Iterator, Optional, Sequence, TextIO, TypeVar | ||
|
|
||
| from stardoc.proto import stardoc_output_pb2 | ||
| from stardoc.proto import stardoc_output_pb2 # type: ignore |
Collaborator
There was a problem hiding this comment.
Prefer to not use blanket ignores.
Duplicate comments suppressed.
| def test_direct_import(self): | ||
| with self.assertRaises(ModuleNotFoundError): | ||
| import ext_pkg_test # buildifier: disable=g-import-not-at-top # noqa: F401 | ||
| import ext_pkg_test # type: ignore # buildifier: disable=g-import-not-at-top # noqa: F401 |
Collaborator
There was a problem hiding this comment.
Is the `buildifier: disable still needed?
| "@rules_python//python/config_settings:venvs_site_packages": "yes", | ||
| }, | ||
| # pytest_test uses a generated bootstrap runner (_boot.py) from template | ||
| # expansion which cannot be statically checked from source by Pyrefly. |
Collaborator
There was a problem hiding this comment.
This doesn't necessarily mean the entire target can't be type checked, right? Is it possible to use a more narrow-focused ignore?
Use specific error-code ignores with pyrefly syntax, annotate type narrowing assertions with '# type assert', replace @dev_pip with @pypi in release tool tests, and remove obsolete test tags.
Add assertions to narrow Optional[Runfiles] and include mypy-specific type ignores alongside pyrefly directives so both checkers pass.
Change return type from Self to Path to match Runfiles.root return type.
- Revert type ignores in debuggers.md docstring. - Safely extract readline.__doc__ to avoid unsupported-operation on None. - Use runfiles.CreateOrRaise() and Runfiles.root() in repl_template.py. - Cast obj to Path in runfiles.py __new__ and _make_child to eliminate attribute ignores and type:ignore[misc].
Dynamically lookup _make_child on super() and cast to Path so both mypy and pyrefly pass without explicit ignore comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enables Pyrefly static type checking across Python source and test
targets in rules_python and sphinxdocs.
Previously, static type analysis was not uniformly applied across
all Python targets, leaving potential type inconsistencies and
unbound variable edge cases uncaught during builds and CI runs.
This change configures the global Pyrefly aspect to evaluate Python
targets by default in Bzlmod mode, adds type annotations and
narrowing assertions on runfiles resolution, and annotates dynamic
imports (such as generated protobuf stubs and compiled C extensions)
with explicit type ignore comments. Targets utilizing dynamically
generated bootstrap wrappers retain opt-out tags with documented
rationales.