Skip to content

Commit eca51a3

Browse files
Copilotjakebailey
Copilot
and
jakebailey
authored
Fix isolatedDeclarations errors for arrow functions with default parameters (#3521)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
1 parent 23ec44f commit eca51a3

7 files changed

Lines changed: 244 additions & 29 deletions

‎internal/pseudochecker/lookup.go‎

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,9 @@ func typeNodeCouldReferToUndefined(node *ast.Node) bool {
574574
return true
575575
case ast.KindTypePredicate: // suspect - always refers to `never` or `boolean`, depending on kind - considered possibly-`undefined` referencing for strada compat
576576
return true
577-
default: // all keywords (why is `undefined` not excluded???), literal types, function-y types, array/tuple types, type literals, template types, this types
577+
case ast.KindUndefinedKeyword:
578+
return true
579+
default: // all other keywords, literal types, function-y types, array/tuple types, type literals, template types, this types
578580
return false
579581
}
580582
}
@@ -607,6 +609,20 @@ func isOptionalInitializedOrRestParameter(node *ast.ParameterDeclarationNode) bo
607609
return false
608610
}
609611

612+
// lastRequiredParamIndex returns the index just past the last required parameter
613+
// in the list. A parameter is "required" if it has no question token, no initializer,
614+
// and no rest token. This is computed in a single reverse pass so callers can
615+
// determine "has required parameter after index i" with `i+1 < lastRequired`
616+
// (equivalently, `i < lastRequired-1`) in O(1).
617+
func lastRequiredParamIndex(params []*ast.Node) int {
618+
for i := len(params) - 1; i >= 0; i-- {
619+
if !isOptionalInitializedOrRestParameter(params[i]) {
620+
return i + 1
621+
}
622+
}
623+
return 0
624+
}
625+
610626
func addUndefinedIfDefinitelyRequired(expr *PseudoType) *PseudoType {
611627
// If `expr` doesn't already contain `| undefined` or a direct/inferred type that may contain `undefined`, add `| undefined`
612628
// in Strada, this reached into the checker to see if `undefined` was necessary, using `isRequiredOptionalParameter` from the emit resolver,
@@ -624,25 +640,45 @@ func (ch *PseudoChecker) typeFromParameter(node *ast.ParameterDeclaration) *Pseu
624640
if parent.Kind == ast.KindSetAccessor {
625641
return ch.GetTypeOfAccessor(parent)
626642
}
643+
// Fast path: no initializer means we never need parameter position info.
644+
if node.Initializer == nil {
645+
if node.Type != nil {
646+
return NewPseudoTypeDirect(node.Type)
647+
}
648+
return NewPseudoTypeNoResult(node.AsNode())
649+
}
650+
p := parent.Parameters()
651+
selfIdx := slices.Index(p, node.AsNode())
652+
lastRequired := lastRequiredParamIndex(p)
653+
return ch.typeFromParameterWorker(node, selfIdx, lastRequired)
654+
}
655+
656+
func (ch *PseudoChecker) typeFromParameterWorker(node *ast.ParameterDeclaration, selfIdx int, lastRequired int) *PseudoType {
657+
parent := node.Parent
658+
if parent.Kind == ast.KindSetAccessor {
659+
return ch.GetTypeOfAccessor(parent)
660+
}
661+
hasRequiredAfter := selfIdx < lastRequired-1
627662
declaredType := node.Type
628663
if declaredType != nil {
629-
return NewPseudoTypeDirect(declaredType)
664+
result := NewPseudoTypeDirect(declaredType)
665+
// When the parameter has an initializer and strict null checks are enabled,
666+
// check if `| undefined` needs to be added because there are required parameters after this one.
667+
// This mirrors the checker's getTypeOfParameter which adds optionality for initialized parameters.
668+
if ch.strictNullChecks && node.Initializer != nil && hasRequiredAfter {
669+
return addUndefinedIfDefinitelyRequired(result)
670+
}
671+
return result
630672
}
631673
if node.Initializer != nil && ast.IsIdentifier(node.Name()) && !isContextuallyTyped(node.AsNode()) {
632674
expr := ch.typeFromExpression(node.Initializer)
633675
if !ch.strictNullChecks {
634676
return expr
635677
}
636-
p := node.Parent.Parameters()
637-
selfIdx := slices.Index(p, node.AsNode())
638-
if selfIdx == len(p)-1 {
678+
if !hasRequiredAfter {
639679
return expr
640680
}
641681
// if there is a non-optional parameter after this one, a `| undefined` will need to explicitly be emitted on this parameter, if it's not already there
642-
remainingParams := node.Parent.Parameters()[selfIdx+1:]
643-
if core.Every(remainingParams, isOptionalInitializedOrRestParameter) {
644-
return expr
645-
}
646682
return addUndefinedIfDefinitelyRequired(expr)
647683
}
648684
// TODO: In strada, the ID checker doesn't infer a parameter type from binding pattern names, but the real checker _does_!
@@ -659,13 +695,22 @@ func (ch *PseudoChecker) cloneParameters(nodes *ast.NodeList) []*PseudoParameter
659695
if len(nodes.Nodes) == 0 {
660696
return nil
661697
}
698+
lastRequired := lastRequiredParamIndex(nodes.Nodes)
662699
result := make([]*PseudoParameter, 0, len(nodes.Nodes))
663-
for _, e := range nodes.Nodes {
700+
for i, e := range nodes.Nodes {
701+
p := e.AsParameterDeclaration()
702+
optional := p.QuestionToken != nil
703+
if !optional && p.Initializer != nil {
704+
// A parameter with an initializer is optional only if all subsequent
705+
// parameters are also optional/have initializers/are rest parameters.
706+
// This matches the checker's isOptionalParameter semantics.
707+
optional = i >= lastRequired-1
708+
}
664709
result = append(result, NewPseudoParameter(
665-
e.AsParameterDeclaration().DotDotDotToken != nil,
710+
p.DotDotDotToken != nil,
666711
e.Name(),
667-
e.AsParameterDeclaration().QuestionToken != nil || e.AsParameterDeclaration().Initializer != nil,
668-
ch.typeFromParameter(e.AsParameterDeclaration()),
712+
optional,
713+
ch.typeFromParameterWorker(p, i, lastRequired),
669714
))
670715
}
671716
return result

‎testdata/baselines/reference/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//// [tests/cases/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.ts] ////
2+
3+
//// [isolatedDeclarationsArrowFunctionDefaultParameter.ts]
4+
export const foo = (one: string = "DEFAULT", two: string): string => {
5+
return one + " " + two;
6+
};
7+
8+
export const bar = (one: string | undefined = "DEFAULT", two: string): string => {
9+
return one + " " + two;
10+
};
11+
12+
// Arrow function where the default parameter is the last one (should be optional)
13+
export const baz = (one: string, two: string | undefined = "DEFAULT"): string => {
14+
return one + " " + two;
15+
};
16+
17+
// Arrow function where all parameters have defaults
18+
export const qux = (one: string = "DEFAULT", two: string = "DEFAULT"): string => {
19+
return one + " " + two;
20+
};
21+
22+
23+
//// [isolatedDeclarationsArrowFunctionDefaultParameter.js]
24+
export const foo = (one = "DEFAULT", two) => {
25+
return one + " " + two;
26+
};
27+
export const bar = (one = "DEFAULT", two) => {
28+
return one + " " + two;
29+
};
30+
// Arrow function where the default parameter is the last one (should be optional)
31+
export const baz = (one, two = "DEFAULT") => {
32+
return one + " " + two;
33+
};
34+
// Arrow function where all parameters have defaults
35+
export const qux = (one = "DEFAULT", two = "DEFAULT") => {
36+
return one + " " + two;
37+
};
38+
39+
40+
//// [isolatedDeclarationsArrowFunctionDefaultParameter.d.ts]
41+
export declare const foo: (one: string | undefined, two: string) => string;
42+
export declare const bar: (one: string | undefined, two: string) => string;
43+
export declare const baz: (one: string, two?: string | undefined) => string;
44+
export declare const qux: (one?: string, two?: string) => string;

‎testdata/baselines/reference/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.symbols‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//// [tests/cases/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.ts] ////
2+
3+
=== isolatedDeclarationsArrowFunctionDefaultParameter.ts ===
4+
export const foo = (one: string = "DEFAULT", two: string): string => {
5+
>foo : Symbol(foo, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 0, 12))
6+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 0, 20))
7+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 0, 44))
8+
9+
return one + " " + two;
10+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 0, 20))
11+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 0, 44))
12+
13+
};
14+
15+
export const bar = (one: string | undefined = "DEFAULT", two: string): string => {
16+
>bar : Symbol(bar, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 4, 12))
17+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 4, 20))
18+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 4, 56))
19+
20+
return one + " " + two;
21+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 4, 20))
22+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 4, 56))
23+
24+
};
25+
26+
// Arrow function where the default parameter is the last one (should be optional)
27+
export const baz = (one: string, two: string | undefined = "DEFAULT"): string => {
28+
>baz : Symbol(baz, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 9, 12))
29+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 9, 20))
30+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 9, 32))
31+
32+
return one + " " + two;
33+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 9, 20))
34+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 9, 32))
35+
36+
};
37+
38+
// Arrow function where all parameters have defaults
39+
export const qux = (one: string = "DEFAULT", two: string = "DEFAULT"): string => {
40+
>qux : Symbol(qux, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 14, 12))
41+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 14, 20))
42+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 14, 44))
43+
44+
return one + " " + two;
45+
>one : Symbol(one, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 14, 20))
46+
>two : Symbol(two, Decl(isolatedDeclarationsArrowFunctionDefaultParameter.ts, 14, 44))
47+
48+
};
49+

‎testdata/baselines/reference/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.types‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
//// [tests/cases/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.ts] ////
2+
3+
=== isolatedDeclarationsArrowFunctionDefaultParameter.ts ===
4+
export const foo = (one: string = "DEFAULT", two: string): string => {
5+
>foo : (one: string | undefined, two: string) => string
6+
>(one: string = "DEFAULT", two: string): string => { return one + " " + two;} : (one: string | undefined, two: string) => string
7+
>one : string
8+
>"DEFAULT" : "DEFAULT"
9+
>two : string
10+
11+
return one + " " + two;
12+
>one + " " + two : string
13+
>one + " " : string
14+
>one : string
15+
>" " : " "
16+
>two : string
17+
18+
};
19+
20+
export const bar = (one: string | undefined = "DEFAULT", two: string): string => {
21+
>bar : (one: string | undefined, two: string) => string
22+
>(one: string | undefined = "DEFAULT", two: string): string => { return one + " " + two;} : (one: string | undefined, two: string) => string
23+
>one : string | undefined
24+
>"DEFAULT" : "DEFAULT"
25+
>two : string
26+
27+
return one + " " + two;
28+
>one + " " + two : string
29+
>one + " " : string
30+
>one : string
31+
>" " : " "
32+
>two : string
33+
34+
};
35+
36+
// Arrow function where the default parameter is the last one (should be optional)
37+
export const baz = (one: string, two: string | undefined = "DEFAULT"): string => {
38+
>baz : (one: string, two?: string | undefined) => string
39+
>(one: string, two: string | undefined = "DEFAULT"): string => { return one + " " + two;} : (one: string, two?: string | undefined) => string
40+
>one : string
41+
>two : string | undefined
42+
>"DEFAULT" : "DEFAULT"
43+
44+
return one + " " + two;
45+
>one + " " + two : string
46+
>one + " " : string
47+
>one : string
48+
>" " : " "
49+
>two : string
50+
51+
};
52+
53+
// Arrow function where all parameters have defaults
54+
export const qux = (one: string = "DEFAULT", two: string = "DEFAULT"): string => {
55+
>qux : (one?: string, two?: string) => string
56+
>(one: string = "DEFAULT", two: string = "DEFAULT"): string => { return one + " " + two;} : (one?: string, two?: string) => string
57+
>one : string
58+
>"DEFAULT" : "DEFAULT"
59+
>two : string
60+
>"DEFAULT" : "DEFAULT"
61+
62+
return one + " " + two;
63+
>one + " " + two : string
64+
>one + " " : string
65+
>one : string
66+
>" " : " "
67+
>two : string
68+
69+
};
70+

‎testdata/baselines/reference/submodule/compiler/isolatedDeclarationsAddUndefined.errors.txt‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
file2.ts(1,26): error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
21
file2.ts(4,27): error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
32

43

@@ -11,11 +10,8 @@ file2.ts(4,27): error TS9025: Declaration emit for this parameter requires impli
1110
f = 2;
1211
}
1312

14-
==== file2.ts (2 errors) ====
13+
==== file2.ts (1 errors) ====
1514
export function foo(p = (ip = 10, v: number): void => {}): void{
16-
~~~~~~~
17-
!!! error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
18-
!!! related TS9028 file2.ts:1:26: Add a type annotation to the parameter ip.
1915
}
2016
type T = number
2117
export function foo2(p = (ip = 10 as T, v: number): void => {}): void{}

‎testdata/baselines/reference/submoduleTriaged/compiler/isolatedDeclarationsAddUndefined.errors.txt.diff‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,11 @@
22
+++ new.isolatedDeclarationsAddUndefined.errors.txt
33
@@= skipped -0, +0 lines =@@
44
-file2.ts(4,38): error TS9011: Parameter must have an explicit type annotation with --isolatedDeclarations.
5-
+file2.ts(1,26): error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
65
+file2.ts(4,27): error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
76

87

98
==== file1.ts (0 errors) ====
10-
@@= skipped -9, +10 lines =@@
11-
f = 2;
12-
}
13-
14-
-==== file2.ts (1 errors) ====
15-
+==== file2.ts (2 errors) ====
16-
export function foo(p = (ip = 10, v: number): void => {}): void{
17-
+ ~~~~~~~
18-
+!!! error TS9025: Declaration emit for this parameter requires implicitly adding undefined to its type. This is not supported with --isolatedDeclarations.
19-
+!!! related TS9028 file2.ts:1:26: Add a type annotation to the parameter ip.
9+
@@= skipped -14, +14 lines =@@
2010
}
2111
type T = number
2212
export function foo2(p = (ip = 10 as T, v: number): void => {}): void{}

‎testdata/tests/cases/compiler/isolatedDeclarationsArrowFunctionDefaultParameter.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// @declaration: true
2+
// @isolatedDeclarations: true
3+
// @strict: true
4+
5+
export const foo = (one: string = "DEFAULT", two: string): string => {
6+
return one + " " + two;
7+
};
8+
9+
export const bar = (one: string | undefined = "DEFAULT", two: string): string => {
10+
return one + " " + two;
11+
};
12+
13+
// Arrow function where the default parameter is the last one (should be optional)
14+
export const baz = (one: string, two: string | undefined = "DEFAULT"): string => {
15+
return one + " " + two;
16+
};
17+
18+
// Arrow function where all parameters have defaults
19+
export const qux = (one: string = "DEFAULT", two: string = "DEFAULT"): string => {
20+
return one + " " + two;
21+
};

0 commit comments

Comments
 (0)