Apply type narrowing in ternary expressions#11369
Apply type narrowing in ternary expressions#11369gramster wants to merge 2 commits intomicrosoft:mainfrom
Conversation
|
Diff from mypy_primer, showing the effect of this PR on open source code: apprise (https://github.com/caronc/apprise)
- Return type mismatch: base method returns type "Literal[1]", override returns type "int"
+ Return type mismatch: base method returns type "Literal[1]", override returns type "Literal[0]"
- "int" is not assignable to type "Literal[1]" (reportIncompatibleMethodOverride)
+ "Literal[0]" is not assignable to type "Literal[1]" (reportIncompatibleMethodOverride)
+ .../projects/apprise/apprise/attachment/memory.py:210:9 - error: Method "__len__" overrides class "AttachBase" in an incompatible manner
+ Return type mismatch: base method returns type "Literal[0]", override returns type "int"
+ "int" is not assignable to type "Literal[0]" (reportIncompatibleMethodOverride)
- 4262 errors, 247 warnings, 0 informations
+ 4263 errors, 247 warnings, 0 informations
sympy (https://github.com/sympy/sympy)
- .../projects/sympy/sympy/solvers/ode/nonhomogeneous.py:468:28 - error: Argument of type "Unknown | None" cannot be assigned to parameter "expr" of type "Expr" in function "make_args"
+ .../projects/sympy/sympy/solvers/ode/nonhomogeneous.py:468:28 - error: Argument of type "Expr | Unknown | None" cannot be assigned to parameter "expr" of type "Expr" in function "make_args"
- Type "Unknown | None" is not assignable to type "Expr"
+ Type "Expr | Unknown | None" is not assignable to type "Expr"
+ Attribute "pop" is unknown (reportAttributeAccessIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:610:22 - error: Cannot access attribute "pop" for class "tuple[()]"
+ Attribute "pop" is unknown (reportAttributeAccessIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:610:22 - error: Cannot access attribute "pop" for class "tuple[str, ...]"
+ .../projects/sympy/sympy/solvers/ode/ode.py:1516:38 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1517:38 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1527:9 - error: No overloads for "update" match the provided arguments (reportCallIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1527:12 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1527:19 - error: Argument of type "Unknown | dict[Unknown, Unknown] | None" cannot be assigned to parameter "m" of type "Iterable[tuple[str, Unknown]]" in function "update"
+ Type "Unknown | dict[Unknown, Unknown] | None" is not assignable to type "Iterable[tuple[str, Unknown]]"
+ "None" is incompatible with protocol "Iterable[tuple[str, Unknown]]"
+ "__iter__" is not present (reportArgumentType)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1534:43 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1534:64 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1540:9 - error: No overloads for "update" match the provided arguments (reportCallIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1540:12 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1540:19 - error: Argument of type "Unknown | dict[Unknown, Unknown] | None" cannot be assigned to parameter "m" of type "Iterable[tuple[str, Unknown]]" in function "update"
+ Type "Unknown | dict[Unknown, Unknown] | None" is not assignable to type "Iterable[tuple[str, Unknown]]"
+ "None" is incompatible with protocol "Iterable[tuple[str, Unknown]]"
+ "__iter__" is not present (reportArgumentType)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1547:43 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1547:74 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1553:9 - error: No overloads for "update" match the provided arguments (reportCallIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1553:12 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1553:19 - error: Argument of type "Unknown | dict[Unknown, Unknown] | None" cannot be assigned to parameter "m" of type "Iterable[tuple[str, Unknown]]" in function "update"
+ Type "Unknown | dict[Unknown, Unknown] | None" is not assignable to type "Iterable[tuple[str, Unknown]]"
+ "None" is incompatible with protocol "Iterable[tuple[str, Unknown]]"
+ "__iter__" is not present (reportArgumentType)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1560:49 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:1560:70 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
- .../projects/sympy/sympy/solvers/ode/ode.py:1690:36 - error: Cannot access attribute "lhs" for class "Expr"
- Attribute "lhs" is unknown (reportAttributeAccessIssue)
- .../projects/sympy/sympy/solvers/ode/ode.py:1690:55 - error: Cannot access attribute "rhs" for class "Expr"
- Attribute "rhs" is unknown (reportAttributeAccessIssue)
- .../projects/sympy/sympy/solvers/ode/ode.py:1691:36 - error: Cannot access attribute "lhs" for class "Expr"
- Attribute "lhs" is unknown (reportAttributeAccessIssue)
- .../projects/sympy/sympy/solvers/ode/ode.py:1692:17 - error: No overloads for "__setitem__" match the provided arguments (reportCallIssue)
- .../projects/sympy/sympy/solvers/ode/ode.py:1692:17 - error: Argument of type "Equality | BooleanFalse | BooleanTrue | Unknown | Expr" cannot be assigned to parameter "value" of type "Equality | BooleanFalse | BooleanTrue" in function "__setitem__"
- Type "Equality | BooleanFalse | BooleanTrue | Unknown | Expr" is not assignable to type "Equality | BooleanFalse | BooleanTrue"
- Type "Expr" is not assignable to type "Equality | BooleanFalse | BooleanTrue"
- "Expr" is not assignable to "Equality"
- "Expr" is not assignable to "BooleanFalse"
- "Expr" is not assignable to "BooleanTrue" (reportArgumentType)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3379:7 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3379:38 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3380:7 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3380:38 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3381:14 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3382:14 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3383:14 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3395:50 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3396:50 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3397:50 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3437:5 - error: No overloads for "update" match the provided arguments (reportCallIssue)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3437:7 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3437:14 - error: Argument of type "Unknown | dict[Unknown, Unknown] | None" cannot be assigned to parameter "m" of type "Iterable[tuple[str, Unknown]]" in function "update"
+ Type "Unknown | dict[Unknown, Unknown] | None" is not assignable to type "Iterable[tuple[str, Unknown]]"
+ "None" is incompatible with protocol "Iterable[tuple[str, Unknown]]"
+ "__iter__" is not present (reportArgumentType)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3438:18 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3438:43 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3439:9 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3439:16 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3439:24 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3439:31 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3440:9 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3440:15 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3440:23 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3440:30 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3441:7 - error: "update" is not a known attribute of "None" (reportOptionalMemberAccess)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3441:46 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3441:54 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3442:9 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3442:19 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3442:29 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
+ .../projects/sympy/sympy/solvers/ode/ode.py:3443:10 - error: Object of type "None" is not subscriptable (reportOptionalSubscript)
... (truncated 335 lines) ...
pandera (https://github.com/pandera-dev/pandera)
- .../projects/pandera/pandera/api/pandas/model.py:251:13 - error: Argument of type "dict[str, Any | None]" cannot be assigned to parameter "dtype" of type "AstypeArg | Mapping[Any, Dtype] | Series[Any]" in function "astype"
- Type "dict[str, Any | None]" is not assignable to type "AstypeArg | Mapping[Any, Dtype] | Series[Any]"
- Type "dict[str, Any | None]" is not assignable to type "type[bool]"
- "dict[str, Any | None]" is not assignable to "BooleanDtype"
- Type "dict[str, Any | None]" is not assignable to type "type[numpy.bool[builtins.bool]]"
- Type "dict[str, Any | None]" is not assignable to type "type[int]"
- "dict[str, Any | None]" is not assignable to "Int8Dtype"
- "dict[str, Any | None]" is not assignable to "Int16Dtype"
- "dict[str, Any | None]" is not assignable to "Int32Dtype"
- ... (reportArgumentType)
- 1995 errors, 122 warnings, 0 informations
+ 1994 errors, 122 warnings, 0 informations
dd-trace-py (https://github.com/DataDog/dd-trace-py)
- .../projects/dd-trace-py/tests/debugging/exploration/_profiler.py:58:39 - error: Cannot access attribute "__code__" for class "FullyNamedFunction"
- Attribute "__code__" is unknown (reportAttributeAccessIssue)
- Attribute "_encoders" is unknown (reportAttributeAccessIssue)
- .../projects/dd-trace-py/tests/debugging/exploration/debugger.py:143:23 - error: Cannot assign to attribute "_encoders" for class "NoopSnapshotJsonEncoder"
- 13738 errors, 685 warnings, 0 informations
+ 13736 errors, 685 warnings, 0 informations
|
| @@ -765,6 +765,33 @@ export function getTypeOfUnaryOperation( | |||
| return { type, isIncomplete, magicMethodDeprecationInfo: deprecatedInfo }; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Copilot generated:
[High] The isBoolLiteralName guard only checks for bool literals, but the same reassignment concern applies to any literal-typed Name. For example:
count = 42 # flow-narrowed to Literal[42]
result = "yes" if count else "no"canBeFalsy(Literal[42]) returns false, so the else branch is pruned — but count is just as reassignable as a bool flag. Either generalize the guard to all literal-typed Names or add a comment explaining why bool is uniquely special here.
| @@ -765,6 +765,33 @@ export function getTypeOfUnaryOperation( | |||
| return { type, isIncomplete, magicMethodDeprecationInfo: deprecatedInfo }; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Copilot generated:
[Medium] The guard fires for any Name with a Literal[True/False] type, including legitimately flow-narrowed bools. Consider def f(x: bool): if x: return 1 if x else "no" — here x is narrowed to Literal[True] via control flow (not assignment), so the else branch should be prunable, but the guard blocks it. The guard can't distinguish "narrowed by flow" from "literal from assignment." This is worth a test case to document the accepted trade-off and a comment clarifying the limitation.
| // simple name references with literal bool types because the variable could | ||
| // be reassigned, even though the type is a literal. This also applies to | ||
| // `not <Name>` expressions to maintain consistency. | ||
| // Note: This guard is specific to ternary expressions. The and/or operators |
There was a problem hiding this comment.
Copilot generated:
[Medium] The comment explains that and/or operators "don't need this guard because they operate on already-evaluated types from their operands." Both the Skeptic and Architect note this distinction is debatable — and/or's leftType is also flow-narrowed via getTypeOfExpression. The observable inconsistency: maybe = True; maybe and 1 or "no" narrows (no guard), but 1 if maybe else "no" does not (guard fires). Consider acknowledging this inconsistency in the comment rather than asserting a structural distinction.
| @@ -0,0 +1,106 @@ | |||
| # This sample tests type narrowing for conditional expressions (ternary operator) | |||
There was a problem hiding this comment.
Copilot generated:
[Medium] Missing test case: def f(x: Literal[True]): return 1 if x else "no" — where the literal type comes from an explicit annotation, not assignment. This would document whether the guard's behavior for explicitly-typed parameters is accepted or surprising. Also consider adding a test for nested ternaries (a if x else (b if y else c)) to confirm correct narrowing propagation.
| # However, val could still be an empty list (falsy), so the else branch | ||
| # is still reachable. Both branches should contribute to the type. | ||
| ts1: list[int] | str = val if val else "empty" | ||
| assert_type(val if val else "empty", list[int] | str) |
There was a problem hiding this comment.
Copilot generated:
[High] func_bool_literal and func_bool_literal_not are vacuous — they pass identically with the isBoolLiteralName guard deleted. Pyright widens maybe = True to bool (not Literal[True]), so isBoolLiteralName returns false and the guard never fires. canBeTruthy(bool) and canBeFalsy(bool) both return true, producing int | str regardless.
Replace these with tests that actually produce Literal[True] via flow narrowing:
def func_guard_flow_narrowed(x: bool):
if x:
# x is Literal[True] here — guard should fire
ts: int | str = 1 if x else "no"
assert_type(1 if x else "no", int | str)
def func_guard_flow_narrowed_not(x: bool):
if x:
ts: int | str = 1 if not x else "yes"
assert_type(1 if not x else "yes", int | str)
| @@ -0,0 +1,106 @@ | |||
| # This sample tests type narrowing for conditional expressions (ternary operator) | |||
There was a problem hiding this comment.
Copilot generated:
[Medium] (Unresolved prior feedback) Missing test cases that would strengthen coverage: (1) def f(x: Literal[True]): return 1 if x else "no" — documents the guard's behavior for explicitly-annotated parameters where no reassignment occurred. (2) A nested ternary a if x else (b if y else c) for regression coverage. (3) A custom class without __bool__/__len__ to verify canBeFalsy is the deciding factor (not isNodeReachable).
| @@ -765,6 +765,33 @@ export function getTypeOfUnaryOperation( | |||
| return { type, isIncomplete, magicMethodDeprecationInfo: deprecatedInfo }; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Copilot generated:
[Medium] (Unresolved prior feedback) isBoolLiteralName only guards bool literals, not other literal types like Literal[42]. The Advocate's defense that bool is uniquely common as a mutable flag pattern is reasonable, but this design choice should be documented. Add a comment explaining why only bool is guarded — e.g., "Only bool is guarded because bool literals are the overwhelmingly common case for mutable flag variables used as ternary conditions; non-bool literals like Literal[42] are rarely used as ternary conditions where reassignment would cause over-narrowing."
| // simple name references with literal bool types because the variable could | ||
| // be reassigned, even though the type is a literal. This also applies to | ||
| // `not <Name>` expressions to maintain consistency. | ||
| // Note: This guard is specific to ternary expressions. The and/or operators |
There was a problem hiding this comment.
Copilot generated:
[Medium] (Unresolved prior feedback) The comment "and/or operators don't need this guard because they operate on already-evaluated types from their operands" asserts a structural distinction that all four reviewers agree is inaccurate — and/or's leftType is also flow-narrowed via getTypeOfExpression. Revise to something like: "The and/or operators historically apply narrowing without this guard. The guard is added here as conservative behavior for the new ternary narrowing, since ternary conditions are more commonly simple name references that may have been reassigned."
3 participants
No description provided.