Skip to content

Do not narrow enum equality comparisons when member values are not statically known - #11692

Open
Nilesh Patil (nileshpatil6) wants to merge 2 commits into
microsoft:mainfrom
nileshpatil6:fix/enum-narrowing-unknown-values
Open

Nilesh Patil (nileshpatil6) wants to merge 2 commits into
microsoft:mainfrom
nileshpatil6:fix/enum-narrowing-unknown-values

Conversation

@nileshpatil6

Copy link
Copy Markdown
Contributor

Bug

#11622 added equality narrowing for primitive-backed enums, so a member of an IntEnum or StrEnum correctly compares equal to its underlying value. When a member's value is not statically known, though, unwrapPrimitiveBackedEnumLiteral falls back to returning the enum literal unchanged. isLiteralValueEqualAtRuntime then compares an EnumLiteral against an int or str literal, which is never equal, so the comparison is treated as statically false and the positive branch is eliminated.

auto() is the common way to hit this:

from enum import IntEnum, auto

class Color(IntEnum):
    RED = auto()
    GREEN = auto()

c: Color = Color.RED
if c == 1:
    reveal_type(c)  # Never, but Color.RED == 1 is True at runtime

The same happens in reverse (x: Literal[1, 3] compared against x == Color.RED), and in a match statement a case on a member whose value is known can unsoundly eliminate siblings whose values are not.

Any computed member value has the same effect, auto() is just the most common form.

Change

isLiteralValueEqualAtRuntime now returns boolean | undefined, where undefined means the comparison cannot be decided statically because a member's underlying value is unknown. An indeterminate result narrows nothing in either direction and eliminates no members, which is the conservative behaviour these narrowing paths already use elsewhere.

All determinate behaviour from #11622 is unchanged: members with explicit values still narrow exactly as before.

Tests

Cases added to enumNarrowing1.py covering auto() members in both comparison directions and the match-statement elimination case. They report 3 errors without the change and 0 with it.

typeEvaluator1 (159 tests) and typeEvaluator3 including EnumNarrowing1 pass, and tsc --noEmit is clean.

@bschnurr

Bill Schnurr (bschnurr) commented Sep 1, 2026 •

Copy link
Copy Markdown
Member

🔒 Automated review in progress — Bill Schnurr (@bschnurr) is auto-reviewing this PR.

@bschnurr

Copy link
Copy Markdown
Member

Please add the direct match-statement regression described in the PR so the sibling-elimination path is explicitly covered.

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Result: could-not-verify

Summary: Verification could not run because no trusted sandbox image is configured for `microsoft/pyright`, and local execution was not authorized. The PR adds five scenarios to the existing `EnumNarrowing1` sample test, covering unknown `auto()` and computed enum values in both comparison directions. No executable test result was obtained.

Test runs: 1 not run

  • ⚠️ Not run | Targeted test discovery and dependency preflight | git --no-pager status --short && printf '\n--- changed files ---\n' && git --no-pager diff --name-status upstream/main...HEAD && printf '\n--- test diff ---\n' && git --no-pager diff --unified=3 upstream/main...HEAD -- packages/pyright-internal/src/tests packages/pyright-internal/src/analyzer/typeGuards.ts && printf '\n--- dependency/profile check ---\n' && printf 'AUTOMATION_SANDBOX_PROFILE=%s\n' "$AUTOMATION_SANDBOX_PROFILE" && test -x packages/pyright-internal/node_modules/.bin/jest && echo 'package jest: present' || echo 'package jest: missing' && test -f packages/pyright-internal/package.json && node -e "const p=require('./packages/pyright-internal/package.json'); console.log(JSON.stringify({build:p.scripts.build,test:p.scripts.test},null,2))"
⚠️ Targeted test discovery and dependency preflight diagnostic output
Container verification could not start and local execution was not authorized for this PR HEAD: No trusted sandbox image is configured for microsoft/pyright.

@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.

@bschnurr Bill Schnurr (bschnurr) added the review-auto:approved Automated review: no blocking findings (approval posted). label Sep 1, 2026
@nileshpatil6

Copy link
Copy Markdown
Contributor Author

Added the match statement cases in 50ff887.

test_match_int_literal_pattern covers the sibling elimination path directly: ComputedValue.SECOND has a known value of 40 while FIRST is computed, so a case 40: pattern must keep FIRST in play rather than eliminating it. test_match_auto_int_literal_pattern covers the same path when no member value is known.

Both assert that neither branch narrows. On main they report errors (the matching case eliminates the unknown-valued member); with the change the sample is clean.

EnumNarrowing1 passes and tsc --noEmit is clean.

@bschnurr

Copy link
Copy Markdown
Member

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Result: could-not-verify

Summary: No tests executed because the verification container could not start: no trusted `microsoft/pyright` sandbox image was configured, and local execution was unauthorized. The PR adds seven scenarios to the existing `EnumNarrowing1` sample covering unknown `IntEnum` values, reverse comparisons, computed values, and match narrowing. Runtime confidence is therefore limited.

Test runs: 1 not run

  • ⚠️ Not run | Verification environment preflight and test discovery | git status --short && git diff --name-status upstream/main...HEAD && git diff --unified=0 upstream/main...HEAD -- packages/pyright-internal/src/tests && if exist packages\pyright-internal\node_modules\jest\bin\jest.js (echo JEST_PRESENT) else (echo JEST_MISSING) && node -e "const p=require('./packages/pyright-internal/package.json'); console.log(JSON.stringify(p.scripts,null,2))"
⚠️ Verification environment preflight and test discovery diagnostic output
Container verification could not start: no trusted sandbox image is configured for microsoft/pyright, and local execution was not authorized.

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) commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

🟢 Pyright CLI QA — 50ff8870 — verdict: green

Automated CLI checks compared this change with its base revision.
6 focused check(s) completed.

No new behavior difference was detected.

@heejaechang

Copy link
Copy Markdown
Collaborator

The added test_match_int_literal_pattern and test_match_auto_int_literal_pattern scenarios pass on both the pre-PR baseline (5a958ca8) and PR head (50ff8870), so they don’t demonstrate the claimed match-statement regression.

The distinction is between a direct pattern and an equality comparison in a guard:

# Direct literal pattern: uses the separate pattern-matching narrowing path.
match value:
    case 40:
        ...

# Equality in a guard: exercises the equality helper changed by this PR.
match value:
    case _ if value == 40:
        ...

Could you clarify which match behavior this PR fixes and add a regression test that fails on the baseline and passes with the change?

If the direct-pattern tests are intended as unchanged-behavior controls, keeping them makes sense, but the description should identify them as such.

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

sympy (https://github.com/sympy/sympy)
-   .../projects/sympy/sympy/simplify/radsimp.py:939:20 - error: Operator "/" not supported for types "Literal[1]" and "Unknown | Expr | None"
+   .../projects/sympy/sympy/simplify/radsimp.py:939:20 - error: Operator "/" not supported for types "Literal[1]" and "Expr | Unknown | None"
-   .../projects/sympy/sympy/simplify/radsimp.py:950:36 - error: Argument of type "Unknown | Expr | Self@Basic | bool | Basic | None" cannot be assigned to parameter "expr" of type "Expr" in function "make_args"
+   .../projects/sympy/sympy/simplify/radsimp.py:950:36 - error: Argument of type "Expr | Unknown | Self@Basic | bool | Basic | None" cannot be assigned to parameter "expr" of type "Expr" in function "make_args"
-     Type "Unknown | Expr | Basic* | bool | Basic | None" is not assignable to type "Expr"
+     Type "Expr | Unknown | Basic* | bool | Basic | None" is not assignable to type "Expr"
-   .../projects/sympy/sympy/simplify/radsimp.py:999:36 - error: Operator "/" not supported for types "Literal[1]" and "Unknown | Expr | Self@Basic | bool | Basic | Self@AssocOp | None"
+   .../projects/sympy/sympy/simplify/radsimp.py:999:36 - error: Operator "/" not supported for types "Literal[1]" and "Expr | Unknown | Self@Basic | bool | Basic | Self@AssocOp | None"
-   .../projects/sympy/sympy/simplify/radsimp.py:1196:14 - error: Operator "*" not supported for types "Unknown | GaussianRational | MPQ | GaussianInteger | MPZ | Expr | Rational | NaN | ComplexInfinity | One | NegativeOne | Zero | Integer | Infinity | NegativeInfinity | Float | Number | Poly" and "Unknown | GaussianRational | MPQ | GaussianInteger | MPZ | Expr | Rational | NaN | ComplexInfinity | One | NegativeOne | Zero | Integer | Infinity | NegativeInfinity | Float | Number | Poly"
-     Operator "*" not supported for types "GaussianRational" and "MPQ"
-     Operator "*" not supported for types "GaussianRational" and "GaussianInteger"
-     Operator "*" not supported for types "GaussianRational" and "MPZ"
-     Operator "*" not supported for types "GaussianRational" and "Expr"
-     Operator "*" not supported for types "GaussianRational" and "Rational"
-     Operator "*" not supported for types "GaussianRational" and "NaN"
-     Operator "*" not supported for types "GaussianRational" and "ComplexInfinity"
-     Operator "*" not supported for types "GaussianRational" and "Infinity"
-     ... (reportOperatorIssue)
+   .../projects/sympy/sympy/simplify/ratsimp.py:31:17 - error: Argument of type "Expr | Poly" cannot be assigned to parameter "args" of type "Expr | complex" in function "__new__"
+     Type "Expr | Poly" is not assignable to type "Expr | complex"
+       Type "Poly" is not assignable to type "Expr | complex"
+         "Poly" is not assignable to "Expr"
+         "Poly" is not assignable to "complex" (reportArgumentType)
-   .../projects/sympy/sympy/simplify/simplify.py:164:20 - error: Argument of type "dict[str, Unknown] | dict[Unknown, list[Unknown]] | Unknown | None" cannot be assigned to parameter "b" of type "Expr | complex" in function "__new__"
+   .../projects/sympy/sympy/simplify/simplify.py:164:20 - error: Argument of type "dict[str, Unknown] | dict[Unknown, list[Unknown]] | Expr | Abs | Unknown | None" cannot be assigned to parameter "b" of type "Expr | complex" in function "__new__"
-     Type "dict[str, Unknown] | dict[Unknown, list[Unknown]] | Unknown | None" is not assignable to type "Expr | complex"
+     Type "dict[str, Unknown] | dict[Unknown, list[Unknown]] | Expr | Abs | Unknown | None" is not assignable to type "Expr | complex"
+   .../projects/sympy/sympy/simplify/simplify.py:732:13 - error: "has" is not a known attribute of "None" (reportOptionalMemberAccess)
-   .../projects/sympy/sympy/simplify/simplify.py:765:25 - error: Cannot access attribute "as_numer_denom" for class "Basic*"
-     Attribute "as_numer_denom" is unknown (reportAttributeAccessIssue)
+   .../projects/sympy/sympy/simplify/sqrtdenest.py:157:19 - error: Operator "**" not supported for types "Basic" and "Literal[2]" (reportOperatorIssue)
+   .../projects/sympy/sympy/simplify/sqrtdenest.py:200:48 - error: Argument of type "Basic" cannot be assigned to parameter "args" of type "Expr | complex" in function "__new__"
+     Type "Basic" is not assignable to type "Expr | complex"
+       "Basic" is not assignable to "Expr"
+       "Basic" is not assignable to "complex" (reportArgumentType)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:207:26 - error: Operator "**" not supported for types "Unknown | Self@AssocOp | AssocOp*" and "Literal[2]"
+   .../projects/sympy/sympy/simplify/sqrtdenest.py:207:26 - error: Operator "**" not supported for types "Unknown | Self@AssocOp | Basic | AssocOp*" and "Literal[2]"
+     Operator "**" not supported for types "Basic" and "Literal[2]"
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:288:16 - error: Operator "*" not supported for types "Expr" and "Unknown | None"
+   .../projects/sympy/sympy/simplify/sqrtdenest.py:288:16 - error: Operator "*" not supported for types "Expr" and "Unknown | Expr | None"
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:309:10 - error: Operator "+" not supported for types "Expr" and "Unknown | Expr | Self@AssocOp | None"
-     Operator "+" not supported for types "Expr" and "None"
-     Operator "+" not supported for types "Expr" and "AssocOp*" (reportOperatorIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:340:19 - error: Operator "-" not supported for types "Expr | Unknown | GaussianRational | One | NegativeOne | Zero | Integer | MPQ | GaussianInteger | MPZ | Poly | NotImplementedType" and "Expr | Unknown | GaussianRational | One | NegativeOne | Zero | Integer | NaN | ComplexInfinity | Rational | MPQ | GaussianInteger | MPZ | Poly"
-     Operator "-" not supported for types "Expr" and "GaussianRational"
-     Operator "-" not supported for types "Expr" and "MPQ"
-     Operator "-" not supported for types "Expr" and "GaussianInteger"
-     Operator "-" not supported for types "Expr" and "MPZ"
-     Operator "-" not supported for types "GaussianRational" and "Expr"
-     Operator "-" not supported for types "GaussianRational" and "NaN"
-     Operator "-" not supported for types "GaussianRational" and "ComplexInfinity"
-     Operator "-" not supported for types "GaussianRational" and "Rational"
-     ... (reportOperatorIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:340:26 - error: Operator "*" not supported for types "Expr | Unknown | GaussianRational | One | NegativeOne | Zero | Integer | MPQ | GaussianInteger | MPZ | Poly | NotImplementedType" and "Expr | Unknown | GaussianRational | One | NegativeOne | Zero | Integer | NaN | ComplexInfinity | Rational | MPQ | GaussianInteger | MPZ | Infinity | NegativeInfinity | Poly | Float | Number"
-     Operator "*" not supported for types "Expr" and "GaussianRational"
-     Operator "*" not supported for types "Expr" and "MPQ"
-     Operator "*" not supported for types "Expr" and "GaussianInteger"
-     Operator "*" not supported for types "Expr" and "MPZ"
-     Operator "*" not supported for types "GaussianRational" and "Expr"
-     Operator "*" not supported for types "GaussianRational" and "NaN"
-     Operator "*" not supported for types "GaussianRational" and "ComplexInfinity"
-     Operator "*" not supported for types "GaussianRational" and "Rational"
-     ... (reportOperatorIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:341:11 - error: Cannot access attribute "is_Rational" for class "GaussianRational"
-     Attribute "is_Rational" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:341:11 - error: Cannot access attribute "is_Rational" for class "MPQ"
-     Attribute "is_Rational" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:341:11 - error: Cannot access attribute "is_Rational" for class "GaussianInteger"
-     Attribute "is_Rational" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:341:11 - error: Cannot access attribute "is_Rational" for class "MPZ"
-     Attribute "is_Rational" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:342:15 - error: Cannot access attribute "is_positive" for class "GaussianRational"
-     Attribute "is_positive" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:342:15 - error: Cannot access attribute "is_positive" for class "MPQ"
-     Attribute "is_positive" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:342:15 - error: Cannot access attribute "is_positive" for class "GaussianInteger"
-     Attribute "is_positive" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:342:15 - error: Cannot access attribute "is_positive" for class "MPZ"
-     Attribute "is_positive" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/simplify/sqrtdenest.py:350:28 - error: Operator "*" not supported for types "Unknown | Expr | GaussianRational | NegativeOne | One | Zero | Integer | NaN | ComplexInfinity | Rational | NegativeInfinity | Infinity | Float | Number | MPQ | GaussianInteger | MPZ | Poly" and "Expr | Unknown | GaussianRational | One | NegativeOne | Zero | Integer | NaN | ComplexInfinity | Rational | MPQ | GaussianInteger | MPZ | Infinity | NegativeInfinity | Poly | Float | Number"
-     Operator "*" not supported for types "Expr" and "GaussianRational"
-     Operator "*" not supported for types "Expr" and "MPQ"
-     Operator "*" not supported for types "Expr" and "GaussianInteger"
-     Operator "*" not supported for types "Expr" and "MPZ"
-     Operator "*" not supported for types "GaussianRational" and "Expr"
-     Operator "*" not supported for types "GaussianRational" and "NaN"

... (truncated 625 lines) ...

@github-actions

Copy link
Copy Markdown
Contributor

mypy_primer analysis

Advisory AI analysis of primer run 33527767591, attempt 2
for commit 50ff88706d2e2e62d7c1a300206a019bb1756028. This is not an approval or proof of correctness.

Only SymPy changed. These differences are treated as non-blocking primer noise; no other project changed.

Recorded changes: 258 added / 64 removed diagnostic headers; 245 added / 142 removed detail lines.

Full evidence and limitations: download mypy-primer-analysis-report from this analysis run.
The original raw primer comment is unchanged. Treat uncertainty and possible regressions as requests for human review.

This branch has not been deployed

No deployments
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.

3 participants