Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Narrow generic conditional and indexed access return types when checking return statements #56941

Merged
merged 102 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 98 commits
Commits
Show all changes
102 commits
Select commit Hold shift + click to select a range
0d393e3
WIP: custom instantiation
gabritto May 16, 2023
a1d9f89
fix more things
gabritto May 26, 2023
b9495a9
latest fixes
gabritto May 31, 2023
81cdee5
Merge branch 'main' into gabritto/d2
gabritto May 31, 2023
af57b34
fix bare type parameter instantiation
gabritto May 31, 2023
01152a0
change binder. BROKEN
gabritto Jun 23, 2023
eac3192
fix binder change
gabritto Jul 17, 2023
199ecd0
clean up + tests
gabritto Aug 5, 2023
d0e85d0
WIP
gabritto Sep 1, 2023
23e3d33
Merge branch 'main' into gabritto/d2
gabritto Sep 2, 2023
d9ac102
defer conditional unless narrowing
gabritto Sep 5, 2023
b4910f4
don't skip jsdoc type assertions
gabritto Sep 6, 2023
6258bc0
more fixes
gabritto Oct 4, 2023
de3428d
WIP: account for pre-existing mapping
gabritto Nov 7, 2023
04980d3
only narrow when we take a true branch of a conditional
gabritto Nov 28, 2023
e60a625
small fix + new tests
gabritto Dec 2, 2023
ee2d1ba
fix optional parameter handling and shadowing
gabritto Dec 7, 2023
b8ce490
fix skip parentheses
gabritto Dec 21, 2023
2439767
fix updates to binder, cleanup in checker
gabritto Dec 22, 2023
3f504f4
Merge branch 'main' into gabritto/d2
gabritto Dec 22, 2023
5e8ca3e
HasFlowNode update & fixes
gabritto Dec 22, 2023
534cc38
cleanup; always check conditional expression's condition inside retur…
gabritto Dec 23, 2023
1dc9ab5
update test after merging
gabritto Jan 4, 2024
a94bc23
maybe make binder change faster
gabritto Jan 4, 2024
046b53c
don't narrow if conditional has infer type params
gabritto Jan 4, 2024
5a83feb
don't walk up parents when binding conditional expression
gabritto Jan 4, 2024
1ac4a58
add promise and generator tests
gabritto Jan 4, 2024
db219a3
get rid of cast
gabritto Jan 4, 2024
56792a1
add indexed access tests
gabritto Jan 5, 2024
b292bd8
WIP
gabritto Dec 14, 2023
18464f3
WIP: detect expression to use for narrowing type parameter based on c…
gabritto Dec 21, 2023
2d30f38
use conditional expression in return stmt's flow nodes to collect ref…
gabritto Dec 21, 2023
764c4b6
fix cast
gabritto Dec 23, 2023
c9abd24
update test
gabritto Jan 3, 2024
2ed4774
detect useless narrowing when narrowed type is simply type parameter
gabritto Jan 3, 2024
0d567af
fix visit of flow nodes
gabritto Jan 3, 2024
a33d99d
optimization
gabritto Jan 4, 2024
0b07a73
check if narrowable using couldContainTypeVariables
gabritto Jan 4, 2024
45772c7
fix problem with caching of flow types
gabritto Jan 6, 2024
dba73d6
cleanup
gabritto Jan 11, 2024
16c7d04
use objects as keys for maps related to flow node references
gabritto Jan 11, 2024
58f0d3a
don't narrow to any or error type
gabritto Jan 11, 2024
fea73f1
hasOwnProperty test
gabritto Jan 11, 2024
51a3516
get rid of closure
gabritto Jan 11, 2024
5a9022f
attach flow nodes directly to conditional expression
gabritto Jan 12, 2024
76f1899
update baselines
gabritto Jan 12, 2024
2c3624d
get rid of extra subtype check in conditional type resolution
gabritto Jan 12, 2024
bcaf504
WIP: stop narrowing if one of the distribution types can't be narrowed
gabritto Jan 16, 2024
85cdfad
get rid of aliasSymbol tracking for now; it's never used
gabritto Jan 17, 2024
7d24d34
move and add tests for indexed access return type narrowing
gabritto Jan 17, 2024
7d76043
add never test
gabritto Jan 17, 2024
c736383
fix optional param detection under exact optional property types
gabritto Jan 17, 2024
9fee0c0
remove unused type parameter property
gabritto Jan 23, 2024
0218430
WIP: fix breaking case where return stmt already assignable to unnarr…
gabritto Jan 26, 2024
1f45a76
Merge remote-tracking branch 'origin/gabritto/d2-detect' into gabritt…
gabritto Jan 26, 2024
c90c2d9
fix lint
gabritto Jan 26, 2024
6c52555
Merge branch 'main' into gabritto/d2-detect
gabritto Jan 26, 2024
96f879c
format
gabritto Jan 26, 2024
7b2c741
remove unused new parameter to mapTypes
gabritto Jan 26, 2024
ba18b41
renaming
gabritto Jan 27, 2024
edc2b66
support detecting aliased expressions
gabritto Jan 27, 2024
a2bb3b6
clean up + fix detection
gabritto Jan 30, 2024
816af88
undo spurious changes
gabritto Jan 30, 2024
64e1566
support switch true detection
gabritto Jan 30, 2024
747f740
add comment
gabritto Jan 31, 2024
31c9a00
don't call get type of expression
gabritto Feb 1, 2024
7c0d003
Merge branch 'main' into gabritto/d2-detect
gabritto Feb 5, 2024
23411b5
add out of scope test
gabritto Feb 9, 2024
9c26a9e
cleanup
gabritto Feb 13, 2024
f05fa4b
Merge branch 'main' into gabritto/d2-detect
gabritto Jul 15, 2024
c15eca8
fix tests
gabritto Jul 16, 2024
7cdf655
new validation for conditional type shape + updated tests
gabritto Aug 20, 2024
67ccf1a
minor fix
gabritto Aug 20, 2024
8a1417c
reuse instantiateType and other functions for narrowing instantiation
gabritto Aug 26, 2024
d74630c
use substitution types
gabritto Sep 7, 2024
10403cf
refactoring
gabritto Sep 9, 2024
99d3cd5
switch reference detection algorithm
gabritto Sep 11, 2024
39d2f2b
refactor and add more tests
gabritto Sep 13, 2024
42f7a2d
remove unused
gabritto Sep 13, 2024
678d2e0
Merge branch 'main' into gabritto/d2-detect
gabritto Sep 13, 2024
7e1fe01
update baselines
gabritto Sep 13, 2024
bbaf88f
refactor and fix validation of conditional return type
gabritto Oct 4, 2024
a8200ec
Merge branch 'main' into gabritto/d2-detect
gabritto Oct 4, 2024
47d8d3f
fmt
gabritto Oct 4, 2024
228148d
cache intermediate results for conditional validation
gabritto Oct 4, 2024
76718d8
take mapper into consideration when validating conditional type
gabritto Oct 7, 2024
20b79eb
refactor
gabritto Oct 7, 2024
3788ada
don't skip jsdoc type assertions
gabritto Oct 9, 2024
2569804
Merge branch 'main' into gabritto/d2-detect
gabritto Oct 9, 2024
827962f
experiment: always set flow node in cond expr
gabritto Oct 9, 2024
4e77373
support jsdoc
gabritto Oct 9, 2024
c076dda
add more jsdoc tests
gabritto Oct 9, 2024
6de9bcb
Merge branch 'main' into gabritto/d2-detect
gabritto Oct 16, 2024
eb88415
support arrow expressions with expression body, small fixes
gabritto Oct 18, 2024
9dc4228
Merge branch 'main' into gabritto/d2-detect
gabritto Oct 21, 2024
6ad3ee9
fix checking of arrow expression
gabritto Oct 22, 2024
ebd9c1b
fix not checking return expressions
gabritto Oct 22, 2024
a4ba333
remove commented out code
gabritto Oct 22, 2024
bd49834
refactor tests
gabritto Oct 29, 2024
03c9881
refactor: feedback
gabritto Oct 30, 2024
26f2283
short circuit conditional type validation
gabritto Oct 31, 2024
86079ac
use mapTypeToIntersection
gabritto Oct 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
var preSwitchCaseFlow: FlowNode | undefined;
var activeLabelList: ActiveLabel | undefined;
var hasExplicitReturn: boolean;
var inReturnPosition: boolean;
var hasFlowEffects: boolean;

// state used for emit helpers
Expand Down Expand Up @@ -622,6 +623,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
currentExceptionTarget = undefined;
activeLabelList = undefined;
hasExplicitReturn = false;
inReturnPosition = false;
hasFlowEffects = false;
inAssignmentPattern = false;
emitFlags = NodeFlags.None;
Expand Down Expand Up @@ -967,7 +969,9 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
const saveContainer = container;
const saveThisParentContainer = thisParentContainer;
const savedBlockScopeContainer = blockScopeContainer;
const savedInReturnPosition = inReturnPosition;

if (node.kind === SyntaxKind.ArrowFunction && node.body.kind !== SyntaxKind.Block) inReturnPosition = true;
// Depending on what kind of node this is, we may have to adjust the current container
// and block-container. If the current node is a container, then it is automatically
// considered the current block-container as well. Also, for containers that we know
Expand Down Expand Up @@ -1071,6 +1075,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
bindChildren(node);
}

inReturnPosition = savedInReturnPosition;
container = saveContainer;
thisParentContainer = saveThisParentContainer;
blockScopeContainer = savedBlockScopeContainer;
Expand Down Expand Up @@ -1571,7 +1576,10 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
}

function bindReturnOrThrow(node: ReturnStatement | ThrowStatement): void {
const savedInReturnPosition = inReturnPosition;
inReturnPosition = true;
bind(node.expression);
inReturnPosition = savedInReturnPosition;
if (node.kind === SyntaxKind.ReturnStatement) {
hasExplicitReturn = true;
if (currentReturnTarget) {
Expand Down Expand Up @@ -2016,10 +2024,16 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
hasFlowEffects = false;
bindCondition(node.condition, trueLabel, falseLabel);
currentFlow = finishFlowLabel(trueLabel);
if (inReturnPosition) {
node.flowNodeWhenTrue = currentFlow;
}
bind(node.questionToken);
bind(node.whenTrue);
addAntecedent(postExpressionLabel, currentFlow);
currentFlow = finishFlowLabel(falseLabel);
if (inReturnPosition) {
node.flowNodeWhenFalse = currentFlow;
}
bind(node.colonToken);
bind(node.whenFalse);
addAntecedent(postExpressionLabel, currentFlow);
Expand Down
440 changes: 395 additions & 45 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/compiler/factory/nodeFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3481,6 +3481,8 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
propagateChildFlags(node.whenTrue) |
propagateChildFlags(node.colonToken) |
propagateChildFlags(node.whenFalse);
node.flowNodeWhenFalse = undefined;
node.flowNodeWhenTrue = undefined;
return node;
}

Expand Down
7 changes: 7 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2735,6 +2735,10 @@ export interface ConditionalExpression extends Expression {
readonly whenTrue: Expression;
readonly colonToken: ColonToken;
readonly whenFalse: Expression;
/** @internal*/
flowNodeWhenTrue: FlowNode | undefined;
/** @internal */
flowNodeWhenFalse: FlowNode | undefined;
}

export type FunctionBody = Block;
Expand Down Expand Up @@ -6235,6 +6239,7 @@ export interface NodeLinks {
decoratorSignature?: Signature; // Signature for decorator as if invoked by the runtime.
spreadIndices?: { first: number | undefined, last: number | undefined }; // Indices of first and last spread elements in array literal
parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined".
contextualReturnType?: Type; // If the node is a return statement's expression, then this is the contextual return type.
fakeScopeForSignatureDeclaration?: "params" | "typeParams"; // If present, this is a fake scope injected into an enclosing declaration chain.
assertionExpressionType?: Type; // Cached type of the expression of a type assertion
potentialThisCollisions?: Node[];
Expand Down Expand Up @@ -6501,6 +6506,8 @@ export const enum ObjectFlags {
IsGenericIndexType = 1 << 23, // Union or intersection contains generic index type
/** @internal */
IsGenericType = IsGenericObjectType | IsGenericIndexType,
/** @internal */
IsNarrowedType = 1 << 24, // Substitution type that comes from type narrowing
gabritto marked this conversation as resolved.
Show resolved Hide resolved

// Flags that require TypeFlags.Union
/** @internal */
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/arrowExpressionJs.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [tests/cases/compiler/arrowExpressionJs.ts] ////

=== mytest.js ===
/**
* @template T
* @param {T|undefined} value value or not
* @returns {T} result value
*/
const cloneObjectGood = value => /** @type {T} */({ ...value });
>cloneObjectGood : Symbol(cloneObjectGood, Decl(mytest.js, 5, 5))
>value : Symbol(value, Decl(mytest.js, 5, 23))
>value : Symbol(value, Decl(mytest.js, 5, 23))

22 changes: 22 additions & 0 deletions tests/baselines/reference/arrowExpressionJs.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//// [tests/cases/compiler/arrowExpressionJs.ts] ////

=== mytest.js ===
/**
* @template T
* @param {T|undefined} value value or not
* @returns {T} result value
*/
const cloneObjectGood = value => /** @type {T} */({ ...value });
>cloneObjectGood : <T>(value: T | undefined) => T
> : ^ ^^ ^^ ^^^^^
>value => /** @type {T} */({ ...value }) : <T>(value: T | undefined) => T
> : ^ ^^ ^^ ^^^^^
>value : T | undefined
> : ^^^^^^^^^^^^^
>({ ...value }) : T
> : ^
>{ ...value } : {}
> : ^^
>value : T | undefined
> : ^^^^^^^^^^^^^

Loading