Skip to content

Fix #11463: Incorrect is behaviour with NewType - #11496

Open
rchiodo wants to merge 2 commits into
microsoft:mainfrom
rchiodo:fix11463
Open

Fix #11463: Incorrect is behaviour with NewType#11496
rchiodo wants to merge 2 commits into
microsoft:mainfrom
rchiodo:fix11463

Conversation

@rchiodo

@rchiodo rchiodo commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes incorrect is narrowing for NewType instances. At runtime a NewType value is simply an instance of its base type, but Pyright treated NewType instances as never overlapping with None or bool literals. This caused x is None / x is True / x is False branches to be wrongly marked unreachable (and narrowing to be skipped) when x was a NewType over NoneType, bool, etc.

How you figured out what to do

The reported symptom was that comparing a NewType instance against None/bool literals never narrowed and the guarded branch was flagged unreachable. Tracing the is narrowing logic in typeGuards.ts showed that both the None check (narrowTypeForIsNone) and the literal-comparison check compared the declared NewType class against the target, which never overlaps. The fix is to test the underlying base type instead, matching runtime behavior.

Implementation

This branch was reconciled with the fix that landed independently on main. The narrowing uses the on-disk helper getInnermostNewTypeBaseInstance in typeGuards.ts, which recursively unwraps NewType classes down to the innermost base type instance (or returns undefined if the type isn't a NewType):

  • narrowTypeForIsNone: uses the unwrapped innermost base instance when testing for overlap with None. The negative branch is eliminated (returns undefined) only when the innermost base is exactly None (isNoneInstance), so a NewType over None correctly makes the is not None branch unreachable, while a None-compatible base like NewType("Obj", object) stays reachable.
  • Literal comparison narrowing preserves the NewType brand on the positive branch (return isClassInstance(subtype) && ClassType.isNewTypeClass(subtype) ? subtype : literalType;), so b is True narrows to the NewType (e.g. Banana) rather than Literal[True], avoiding a spurious reportArgumentType on a later construction.

Non-NewType types fall back to the original subtype, so existing behavior is unchanged.

Testing

Regression coverage is provided by sample newTypeIsLiteral1.py, wired up as NewType8 in typeEvaluator2.test.ts. The test runs with reportUnreachable = 'error' and asserts validateResults(analysisResults, 0) — any branch wrongly greyed out would surface as a reportUnreachable error and fail. It covers:

  • NewType over NoneType narrowed by is None / is not None (exactly-None positive and negative directions).
  • Literal narrowing (is True / is False) that preserves the NewType brand.
  • None-/ellipsis-compatible bases (NewType("Fig", object), NewType("Guava", object)) that must stay reachable on is not None / is not ... and not collapse to Never.

Addresses

Fixes #11463

…literals

A NewType instance is, at runtime, an instance of its base type. When narrowing
`x is None` or `x is <literal>` for a NewType-typed value, the assignability
check used the NewType subclass, so the base type (e.g. None or bool) was not
considered assignable and the branch was incorrectly marked unreachable.

Unwrap NewType instances to their underlying base type (recursively for nested
NewTypes) when testing for None/literal overlap in narrowTypeForIsNone and
narrowTypeForLiteralComparison.

Fixes microsoft#11463

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@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/stats/random_matrix_models.py:111:36 - error: Operator "*" not supported for types "Expr" and "tuple[Unknown, ...] | Unknown | Product | Basic | BooleanFalse | BooleanTrue | ComplexInfinity | Equality | Expr | Float | Infinity | Integer | NaN | NegativeInfinity | NegativeOne | Number | One | Rational | Zero"
+   .../projects/sympy/sympy/stats/random_matrix_models.py:111:36 - error: Operator "*" not supported for types "Expr" and "tuple[Unknown, ...] | Unknown | Product | Basic | BooleanFalse | BooleanTrue | Equality | Expr | NaN | One | Zero"
-   .../projects/sympy/sympy/stats/rv.py:473:25 - error: Argument of type "Expr | Lambda | Zero | One | Integral | Unknown | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | NaN | Piecewise | Basic | NegativeOne | Integer | ComplexInfinity | Rational | Infinity | NegativeInfinity | Float | Number | int" cannot be assigned to parameter "args" of type "Expr | complex" in function "__new__"
-     Type "Expr | Lambda | Zero | One | Integral | Unknown | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | NaN | Piecewise | Basic | NegativeOne | Integer | ComplexInfinity | Rational | Infinity | NegativeInfinity | Float | Number | int" is not assignable to type "Expr | complex"
+   .../projects/sympy/sympy/stats/rv.py:473:25 - error: Argument of type "Expr | Lambda | Zero | One | Integral | Unknown | NegativeOne | Integer | NaN | ComplexInfinity | Rational | Infinity | Number | NegativeInfinity | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | int" cannot be assigned to parameter "args" of type "Expr | complex" in function "__new__"
+     Type "Expr | Lambda | Zero | One | Integral | Unknown | NegativeOne | Integer | NaN | ComplexInfinity | Rational | Infinity | Number | NegativeInfinity | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | int" is not assignable to type "Expr | complex"
+       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/stats/rv.py:473:25 - error: Argument of type "Mul | Expr | Lambda | Zero | One | Integral | Unknown | NegativeOne | Integer | NaN | ComplexInfinity | Rational | Infinity | Number | NegativeInfinity | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | int" cannot be assigned to parameter "args" of type "Expr | complex" in function "__new__"
+     Type "Mul | Expr | Lambda | Zero | One | Integral | Unknown | NegativeOne | Integer | NaN | ComplexInfinity | Rational | Infinity | Number | NegativeInfinity | Probability | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | int" is not assignable to type "Expr | complex"
-     Return type mismatch: base method returns type "Self@Basic", override returns type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | Self@Expectation | Literal[0]"
+     Return type mismatch: base method returns type "Self@Basic", override returns type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | Self@Expectation | Literal[0]"
-       Type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | Expectation* | Literal[0]" is not assignable to type "Basic"
+       Type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | Expectation* | Literal[0]" is not assignable to type "Basic"
-     Return type mismatch: base method returns type "Basic", override returns type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | Literal[0]"
+     Return type mismatch: base method returns type "Basic", override returns type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | Literal[0]"
-       Type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | Literal[0]" is not assignable to type "Basic"
+       Type "Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | Literal[0]" is not assignable to type "Basic"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:97:29 - error: Cannot access attribute "symbol" for class "ConditionalFiniteDomain"
+     Attribute "symbol" is unknown (reportAttributeAccessIssue)
-   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:367:12 - error: Operator "-" not supported for types "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" and "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:367:12 - error: Operator "-" not supported for types "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" and "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]"
-   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:415:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:415:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
-     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
+     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
-   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1005:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1005:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
-     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
+     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
-   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1060:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1060:21 - error: Argument of type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
-     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
+     Type "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" is not assignable to type "Basic"
-   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1297:61 - error: Operator "**" not supported for types "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Add | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" and "Literal[2]"
+   .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1297:61 - error: Operator "**" not supported for types "RandomSymbol | Basic | Expectation | Expr | tuple[Unknown, ...] | Unknown | Sum | ZeroMatrix | Zero | NaN | Piecewise | Integral | Any | ExpectationMatrix | Literal[0]" and "Literal[2]"

... (truncated 1476 lines) ...

Comment thread packages/pyright-internal/src/tests/typeEvaluator2.test.ts
Comment thread packages/pyright-internal/src/analyzer/typeGuards.ts
@rchiodo

rchiodo commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

Fix looks correct and the sample covers the key cases. Two non-blocking notes below: tighten the test to also assert the unreachable-diagnostic count, and clarify whether negative-direction (is not/else) narrowing for NewType is intentionally out of scope.

Comment thread packages/pyright-internal/src/analyzer/typeGuards.ts
Comment thread packages/pyright-internal/src/analyzer/typeGuards.ts
Comment thread packages/pyright-internal/src/analyzer/typeGuards.ts Outdated
Comment thread packages/pyright-internal/src/tests/samples/newType8.py Outdated
Comment thread packages/pyright-internal/src/tests/samples/newType8.py Outdated
@heejaechang

Copy link
Copy Markdown
Collaborator

This change overlaps with an already-landed fix on main: getInnermostNewTypeBaseInstance in typeGuards.ts already implements NewType is narrowing, and it diverges from this PR on two behaviors (handling of is not None for None-based NewTypes, and preserving the NewType brand on literal narrowing). Please reconcile to the single on-disk helper/semantic rather than landing a second near-duplicate, and update newType8.py expectations to match.

…o the on-disk getInnermostNewTypeBaseInstance helper

Adopts main's complete NewType identity-narrowing fix (None/ellipsis/literal, brand-preserving, correct negative branches) and removes the divergent duplicate getNewTypeBaseInstance helper and newType8.py sample. Resolves review feedback on PR microsoft#11496.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rchiodo

rchiodo commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

I reconciled this branch with the already-landed fix on main. I merged current upstream/main and dropped the divergent getNewTypeBaseInstance helper plus the newType8.py sample, so the branch now uses the single on-disk getInnermostNewTypeBaseInstance helper and main's complete narrowing semantics (None/ellipsis/literal, brand-preserving, correct negative branches; covered by newTypeIsLiteral1.py).

@heejaechang

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — @heejaechang is auto-reviewing this PR.

@heejaechang

Copy link
Copy Markdown
Collaborator

The narrowing fix itself looks correct and well-targeted. Two follow-ups before merge: add a Pylance-harness regression test per the pylance-pyright-split rule, and update the PR description to match the actual implementation (getInnermostNewTypeBaseInstance, newTypeIsLiteral1.py, test expects 0 errors).

@rchiodo

rchiodo commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks — addressed the two follow-ups:

  1. PR description — Updated to match the reconciled implementation: it now references getInnermostNewTypeBaseInstance (not the dropped getNewTypeBaseInstance), the newTypeIsLiteral1.py sample wired as NewType8, the reportUnreachable = 'error' + validateResults(analysisResults, 0) assertion, the brand-preserving literal narrowing, and the exactly-None vs. None-compatible negative-branch semantics. Also corrected the issue link to microsoft/pyright#11463.

  2. Pylance-harness regression test — Not applicable in this repo. This PR targets the standalone microsoft/pyright repo, which has no Pylance harness (packages/ contains only pyright, pyright-internal, and vscode-pyright). The pylance-pyright-split rule is a microsoft/pyrx monorepo convention and doesn't map here. The correct pyright-side regression test already exists — newTypeIsLiteral1.py (NewType8) — and enforces the behavior under reportUnreachable = 'error' (0 errors), covering both positive and negative is None/is not None/literal directions.

Note: after merging current main, the net diff of this branch vs main is empty — the getInnermostNewTypeBaseInstance fix and newTypeIsLiteral1.py test have fully landed on main already, so there are no remaining code changes on this branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect is behaviour with NewType

2 participants