Recognize typing_extensions.sentinel as a sentinel constructor#4254
Recognize typing_extensions.sentinel as a sentinel constructor#4254MannXo wants to merge 1 commit into
Conversation
typing_extensions 4.16.0 backported the lowercase PEP 661 `sentinel` (the
form Python 3.15 exposes as `builtins.sentinel`), but Pyrefly only treated
the lowercase name as a sentinel constructor when it came from `builtins`:
the `BuiltinsSentinel` special export was gated to that module alone.
Imported from `typing_extensions`, it fell through to a plain function
call, so `x = sentinel("x")` was typed as the function's return type
instead of a unique sentinel, diverging from the uppercase `Sentinel` it
is meant to replace.
Widen `BuiltinsSentinel`'s module gate to also accept `typing_extensions`.
Both names already route to the same sentinel-construction path, so the
gate was the only thing wrong.
Fixes facebook#4225
|
Hi @MannXo! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
Summary
typing_extensions4.16.0 backported the lowercase PEP 661sentinel(the form Python 3.15 exposes asbuiltins.sentinel), but Pyrefly only treated the lowercase name as a sentinel constructor when it came frombuiltins: theBuiltinsSentinelspecial export'sdefined_ingate listed only that module. Sofrom typing_extensions import sentinel; X = sentinel("X")fell through to a plain function call, andreveal_type(X)showedsentinelinstead of the unique sentinel type, diverging from the uppercaseSentinelit replaces. (As you noted on the issue, this just needed adding to the module gate.)The fix widens
BuiltinsSentinel'sdefined_into also allowtyping_extensions.SentinelandBuiltinsSentinelalready route to the same sentinel-construction path inbinding/stmt.rs, so the module gate was the only thing wrong.Fixes #4225
Test Plan
Added two cases to
pyrefly/lib/test/sentinel.rs:test_sentinel_lowercase_from_typing_extensions—from typing_extensions import sentinelnow narrows/assert_types like the uppercaseSentinel.test_sentinel_lowercase_from_typing_extensions_reveal— the issue's repro:reveal_typereports the sentinel type, notsentinel.cargo test -p pyrefly sentinel→ 33 passed, 0 failed. Confirmed both new tests fail without the one-line fix (2 failed, "expected 0 errors, but got 3").cargo clippy -p pyreflyandcargo fmt --checkare clean.Note: built and tested against the
1.95.0stable toolchain (this environment couldn't download the pinnedstable). I did not runtest.py's conformance regeneration — the change only widens a module gate fortyping_extensions.sentineland touches no behavior the python/typing conformance suite covers, so I don't expect generated changes, but flagging in case CI disagrees.Disclosure: this PR was prepared with the help of Claude Opus 4.8, an AI coding agent, under my direction and review.