Skip to content

Consider __all__.remove when determining explicit exports - #4638

Closed
kavix wants to merge 1 commit into
meta:mainfrom
kavix:fix-issue-4564
Closed

kavix wants to merge 1 commit into
meta:mainfrom
kavix:fix-issue-4564

Conversation

@kavix

@kavix kavix commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #4564.

When checking for implicit re-exports, Pyrefly previously filtered only DunderAllEntry::Name entries from explicit __all__ definitions, ignoring subsequent __all__.remove(...) mutations. As a result, symbols removed from __all__ were still treated as explicitly exported.

Changes

  • Added Exports::get_explicit_dunder_all_names to compute the final set of user-defined __all__ symbols by applying mutations in source order (inserting on Name, removing on Remove), consistent with get_partially_known_dunder_all and wildcard.
  • Stored explicit_dunder_all_names: Option<SmallSet<Name>> on Exports and updated get_explicit_dunder_all_names_iter, is_implicit_reexport, and changed_exports.
  • Added regression test test_implicit_reexport_removed_from_all in pyrefly/lib/test/imports.rs.

Test Plan

  • CARGO_BUILD_JOBS=1 cargo test -p pyrefly --lib test::imports::test_implicit_reexport_removed_from_all -- --exact --nocapture
  • CARGO_BUILD_JOBS=1 cargo check -p pyrefly --lib
  • CARGO_BUILD_JOBS=1 cargo check -p pyrefly --tests
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema
  • Direct CLI smoke test with [tool.pyrefly.errors] implicit-reexport = "error" verifying that removing a name triggers the diagnostic and appending it back silences it.

AI Disclosure

This PR was developed and verified with the assistance of an AI coding assistant (Google Antigravity).

When checking for implicit re-exports, Pyrefly previously filtered only
DunderAllEntry::Name entries from explicit __all__ definitions, ignoring
subsequent __all__.remove(...) mutations. As a result, symbols removed
from __all__ were still treated as explicitly exported.

Compute the final explicit __all__ names by replaying mutations in
source order (inserting on Name, removing on Remove), and use this
resulting set for explicit export queries and implicit-reexport checks.
@meta-codesync

meta-codesync Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D117137289. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@stroxler stroxler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@meta-codesync meta-codesync Bot closed this in 2c86c33 Aug 24, 2026
@meta-codesync

meta-codesync Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in 2c86c33.

@meta-codesync meta-codesync Bot added the Merged label Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implicit-reexport does not consider __all__.remove(...).

2 participants