Skip to content

Retain Sentinel types for non-CONSTANT_NAME bindings - #11655

Open
Henry Su (hsusul) wants to merge 1 commit into
microsoft:mainfrom
hsusul:fix/sentinel-non-constant-narrowing
Open

Retain Sentinel types for non-CONSTANT_NAME bindings#11655
Henry Su (hsusul) wants to merge 1 commit into
microsoft:mainfrom
hsusul:fix/sentinel-non-constant-narrowing

Conversation

@hsusul

Copy link
Copy Markdown
Contributor

Summary

  • Sentinel objects are unique singletons, so pyright can keep their literal types even when the binding is not a CONSTANT_NAME or Final.
  • That makes is / is not narrowing work for names like Empty = Sentinel("Empty"), matching the behavior already used for all-caps sentinels.

Fixes #10744

Test plan

  • pnpm exec jest typeEvaluator2.test.ts -t "Sentinel" --forceExit in packages/pyright-internal

Sentinel values are unique objects, so `is`/`is not` narrowing is sound even when the name is not CONSTANT_NAME or Final.
@heejaechang

Heejae Chang (heejaechang) commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — Heejae Chang (@heejaechang) is auto-reviewing this PR.

from typing_extensions import Sentinel # pyright: ignore[reportMissingModuleSource]


Empty = Sentinel("Empty")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

📍 packages/pyright-internal/src/tests/samples/sentinel3.py:8
[unverified] The new policy relies on singleton identity while this non-Final binding remains mutable. Add adversarial coverage for reassignment and aliasing—and preferably lowercase local and module-attribute bindings—to establish that narrowing remains sound across the affected flow paths.

[verified]


// Sentinel objects are singletons; retaining the literal is sound
// even when the binding is not a CONSTANT_NAME or Final.
if (isSentinelLiteral(type)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

📍 packages/pyright-internal/src/analyzer/typeEvaluator.ts:24658
[unverified] The corresponding Pylance async-evaluator widening branches do not contain this exception, so IDE behavior may diverge when async evaluation is enabled. Verify the scenario with a downstream async-mode Pylance regression test and mirror the evaluator change only if that path owns the behavior rather than TSP.

[verified]

!isSentinelLiteral(destType)
) {
destType = stripTypeForm(stripLiteralValue(destType));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

The new sample exercises module-level names but not the member-access widening path modified here. Add a class or instance sentinel-member regression case so this distinct retention path is covered.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved via Review Center.

@heejaechang Heejae Chang (heejaechang) added the review-auto:approved Automated review: no blocking findings (approval posted). label Aug 18, 2026

@rchiodo Rich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved via Review Center.

@bschnurr Bill Schnurr (bschnurr) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved via Review Center.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentinel instances only work as constants

4 participants