Skip to content

Commit 5c2eeb2

Browse files
sandersnsandersn
authored
Destructuring declaration prefers type annotation type (microsoft#25282)
* Destructuring declaration prefers type annotation type Previously, getTypeForBindingElement would always union the declarations type and the type of the default initializer. Now, if the declaration has a type annotation, it does not union with the initializer type. The type annotation's type is the one used. * Small cleanup in parentDeclarationHasTypeAnnotation * Refactoring based on PR comments * Combine getCombined*Flags into a single helper function Retain the individual functions since they are used a lot. * Remove unneeded temp
1 parent 950593b commit 5c2eeb2

17 files changed

Lines changed: 328 additions & 55 deletions

‎src/compiler/checker.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4161,7 +4161,7 @@ namespace ts {
41614161
}
41624162
const parent = getDeclarationContainer(node);
41634163
// If the node is not exported or it is not ambient module element (except import declaration)
4164-
if (!(getCombinedModifierFlags(node) & ModifierFlags.Export) &&
4164+
if (!(getCombinedModifierFlags(node as Declaration) & ModifierFlags.Export) &&
41654165
!(node.kind !== SyntaxKind.ImportEqualsDeclaration && parent.kind !== SyntaxKind.SourceFile && parent.flags & NodeFlags.Ambient)) {
41664166
return isGlobalSourceFile(parent);
41674167
}
@@ -4525,7 +4525,7 @@ namespace ts {
45254525
if (strictNullChecks && declaration.initializer && !(getFalsyFlags(checkExpressionCached(declaration.initializer)) & TypeFlags.Undefined)) {
45264526
type = getTypeWithFacts(type, TypeFacts.NEUndefined);
45274527
}
4528-
return declaration.initializer ?
4528+
return declaration.initializer && !getEffectiveTypeAnnotationNode(walkUpBindingElementsAndPatterns(declaration)) ?
45294529
getUnionType([type, checkExpressionCached(declaration.initializer)], UnionReduction.Subtype) :
45304530
type;
45314531
}
@@ -22096,7 +22096,7 @@ namespace ts {
2209622096
return hasModifier(node, ModifierFlags.Private) && !!(node.flags & NodeFlags.Ambient);
2209722097
}
2209822098

22099-
function getEffectiveDeclarationFlags(n: Node, flagsToCheck: ModifierFlags): ModifierFlags {
22099+
function getEffectiveDeclarationFlags(n: Declaration, flagsToCheck: ModifierFlags): ModifierFlags {
2210022100
let flags = getCombinedModifierFlags(n);
2210122101

2210222102
// children of classes (even ambient classes) should not be marked as ambient or export

‎src/compiler/utilities.ts‎

Lines changed: 18 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,7 @@ namespace ts {
903903

904904
export function isConst(node: Node): boolean {
905905
return !!(getCombinedNodeFlags(node) & NodeFlags.Const)
906-
|| !!(getCombinedModifierFlags(node) & ModifierFlags.Const);
906+
|| !!(isDeclaration(node) && getCombinedModifierFlags(node) & ModifierFlags.Const);
907907
}
908908

909909
export function isLet(node: Node): boolean {
@@ -4605,33 +4605,36 @@ namespace ts {
46054605
return isEmptyBindingPattern(node.name);
46064606
}
46074607

4608-
function walkUpBindingElementsAndPatterns(node: Node): Node {
4609-
while (node && (node.kind === SyntaxKind.BindingElement || isBindingPattern(node))) {
4610-
node = node.parent;
4608+
export function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration {
4609+
let node = binding.parent;
4610+
while (isBindingElement(node.parent)) {
4611+
node = node.parent.parent;
46114612
}
4612-
4613-
return node;
4613+
return node.parent;
46144614
}
46154615

4616-
export function getCombinedModifierFlags(node: Node): ModifierFlags {
4617-
node = walkUpBindingElementsAndPatterns(node);
4618-
let flags = getModifierFlags(node);
4616+
function getCombinedFlags(node: Node, getFlags: (n: Node) => number): number {
4617+
if (isBindingElement(node)) {
4618+
node = walkUpBindingElementsAndPatterns(node);
4619+
}
4620+
let flags = getFlags(node);
46194621
if (node.kind === SyntaxKind.VariableDeclaration) {
46204622
node = node.parent;
46214623
}
4622-
46234624
if (node && node.kind === SyntaxKind.VariableDeclarationList) {
4624-
flags |= getModifierFlags(node);
4625+
flags |= getFlags(node);
46254626
node = node.parent;
46264627
}
4627-
46284628
if (node && node.kind === SyntaxKind.VariableStatement) {
4629-
flags |= getModifierFlags(node);
4629+
flags |= getFlags(node);
46304630
}
4631-
46324631
return flags;
46334632
}
46344633

4634+
export function getCombinedModifierFlags(node: Declaration): ModifierFlags {
4635+
return getCombinedFlags(node, getModifierFlags);
4636+
}
4637+
46354638
// Returns the node flags for this node and all relevant parent nodes. This is done so that
46364639
// nodes like variable declarations and binding elements can returned a view of their flags
46374640
// that includes the modifiers from their container. i.e. flags like export/declare aren't
@@ -4640,23 +4643,7 @@ namespace ts {
46404643
// list. By calling this function, all those flags are combined so that the client can treat
46414644
// the node as if it actually had those flags.
46424645
export function getCombinedNodeFlags(node: Node): NodeFlags {
4643-
node = walkUpBindingElementsAndPatterns(node);
4644-
4645-
let flags = node.flags;
4646-
if (node.kind === SyntaxKind.VariableDeclaration) {
4647-
node = node.parent;
4648-
}
4649-
4650-
if (node && node.kind === SyntaxKind.VariableDeclarationList) {
4651-
flags |= node.flags;
4652-
node = node.parent;
4653-
}
4654-
4655-
if (node && node.kind === SyntaxKind.VariableStatement) {
4656-
flags |= node.flags;
4657-
}
4658-
4659-
return flags;
4646+
return getCombinedFlags(node, n => n.flags);
46604647
}
46614648

46624649
/**

‎src/services/utilities.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,7 @@ namespace ts {
10441044
}
10451045

10461046
export function getNodeModifiers(node: Node): string {
1047-
const flags = getCombinedModifierFlags(node);
1047+
const flags = isDeclaration(node) ? getCombinedModifierFlags(node) : ModifierFlags.None;
10481048
const result: string[] = [];
10491049

10501050
if (flags & ModifierFlags.Private) result.push(ScriptElementKindModifier.privateMemberModifier);

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6677,7 +6677,8 @@ declare namespace ts {
66776677
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
66786678
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
66796679
function isEmptyBindingElement(node: BindingElement): boolean;
6680-
function getCombinedModifierFlags(node: Node): ModifierFlags;
6680+
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;
6681+
function getCombinedModifierFlags(node: Declaration): ModifierFlags;
66816682
function getCombinedNodeFlags(node: Node): NodeFlags;
66826683
/**
66836684
* Checks to see if the locale is in the appropriate format,

‎tests/baselines/reference/api/typescript.d.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3175,7 +3175,8 @@ declare namespace ts {
31753175
function isParameterPropertyDeclaration(node: Node): node is ParameterPropertyDeclaration;
31763176
function isEmptyBindingPattern(node: BindingName): node is BindingPattern;
31773177
function isEmptyBindingElement(node: BindingElement): boolean;
3178-
function getCombinedModifierFlags(node: Node): ModifierFlags;
3178+
function walkUpBindingElementsAndPatterns(binding: BindingElement): VariableDeclaration | ParameterDeclaration;
3179+
function getCombinedModifierFlags(node: Declaration): ModifierFlags;
31793180
function getCombinedNodeFlags(node: Node): NodeFlags;
31803181
/**
31813182
* Checks to see if the locale is in the appropriate format,

‎tests/baselines/reference/contextuallyTypedBindingInitializerNegative.errors.txt‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(4,20): error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
2+
Type 'number' is not assignable to type 'string'.
3+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(5,23): error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
4+
Type 'number' is not assignable to type 'string'.
5+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(6,25): error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
6+
Type 'number' is not assignable to type 'string'.
7+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(11,23): error TS2322: Type '{ show: (v: number) => number; }' is not assignable to type 'Show'.
8+
Types of property 'show' are incompatible.
9+
Type '(v: number) => number' is not assignable to type '(x: number) => string'.
10+
Type 'number' is not assignable to type 'string'.
11+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(16,23): error TS2322: Type '(arg: string) => number' is not assignable to type '(s: string) => string'.
12+
Type 'number' is not assignable to type 'string'.
13+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(21,14): error TS2322: Type '[number, number]' is not assignable to type '[string, number]'.
14+
Type 'number' is not assignable to type 'string'.
15+
tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts(26,14): error TS2322: Type '"baz"' is not assignable to type '"foo" | "bar"'.
16+
17+
18+
==== tests/cases/conformance/types/contextualTypes/methodDeclarations/contextuallyTypedBindingInitializerNegative.ts (7 errors) ====
19+
interface Show {
20+
show: (x: number) => string;
21+
}
22+
function f({ show: showRename = v => v }: Show) {}
23+
~~~~~~~~~~
24+
!!! error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
25+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
26+
function f2({ "show": showRename = v => v }: Show) {}
27+
~~~~~~~~~~
28+
!!! error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
29+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
30+
function f3({ ["show"]: showRename = v => v }: Show) {}
31+
~~~~~~~~~~
32+
!!! error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
33+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
34+
35+
interface Nested {
36+
nested: Show
37+
}
38+
function ff({ nested: nestedRename = { show: v => v } }: Nested) {}
39+
~~~~~~~~~~~~
40+
!!! error TS2322: Type '{ show: (v: number) => number; }' is not assignable to type 'Show'.
41+
!!! error TS2322: Types of property 'show' are incompatible.
42+
!!! error TS2322: Type '(v: number) => number' is not assignable to type '(x: number) => string'.
43+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
44+
45+
interface StringIdentity {
46+
stringIdentity(s: string): string;
47+
}
48+
let { stringIdentity: id = arg => arg.length }: StringIdentity = { stringIdentity: x => x};
49+
~~
50+
!!! error TS2322: Type '(arg: string) => number' is not assignable to type '(s: string) => string'.
51+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
52+
53+
interface Tuples {
54+
prop: [string, number];
55+
}
56+
function g({ prop = [101, 1234] }: Tuples) {}
57+
~~~~
58+
!!! error TS2322: Type '[number, number]' is not assignable to type '[string, number]'.
59+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
60+
61+
interface StringUnion {
62+
prop: "foo" | "bar";
63+
}
64+
function h({ prop = "baz" }: StringUnion) {}
65+
~~~~
66+
!!! error TS2322: Type '"baz"' is not assignable to type '"foo" | "bar"'.
67+

‎tests/baselines/reference/contextuallyTypedBindingInitializerNegative.types‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ interface Show {
99
function f({ show: showRename = v => v }: Show) {}
1010
>f : ({ show: showRename }: Show) => void
1111
>show : any
12-
>showRename : ((x: number) => string) | ((v: number) => number)
12+
>showRename : (x: number) => string
1313
>v => v : (v: number) => number
1414
>v : number
1515
>v : number
1616
>Show : Show
1717

1818
function f2({ "show": showRename = v => v }: Show) {}
1919
>f2 : ({ "show": showRename }: Show) => void
20-
>showRename : ((x: number) => string) | ((v: number) => number)
20+
>showRename : (x: number) => string
2121
>v => v : (v: number) => number
2222
>v : number
2323
>v : number
@@ -26,7 +26,7 @@ function f2({ "show": showRename = v => v }: Show) {}
2626
function f3({ ["show"]: showRename = v => v }: Show) {}
2727
>f3 : ({ ["show"]: showRename }: Show) => void
2828
>"show" : "show"
29-
>showRename : ((x: number) => string) | ((v: number) => number)
29+
>showRename : (x: number) => string
3030
>v => v : (v: number) => number
3131
>v : number
3232
>v : number
@@ -42,7 +42,7 @@ interface Nested {
4242
function ff({ nested: nestedRename = { show: v => v } }: Nested) {}
4343
>ff : ({ nested: nestedRename }: Nested) => void
4444
>nested : any
45-
>nestedRename : Show | { show: (v: number) => number; }
45+
>nestedRename : Show
4646
>{ show: v => v } : { show: (v: number) => number; }
4747
>show : (v: number) => number
4848
>v => v : (v: number) => number
@@ -59,7 +59,7 @@ interface StringIdentity {
5959
}
6060
let { stringIdentity: id = arg => arg.length }: StringIdentity = { stringIdentity: x => x};
6161
>stringIdentity : any
62-
>id : ((s: string) => string) | ((arg: string) => number)
62+
>id : (s: string) => string
6363
>arg => arg.length : (arg: string) => number
6464
>arg : string
6565
>arg.length : number
@@ -80,7 +80,7 @@ interface Tuples {
8080
}
8181
function g({ prop = [101, 1234] }: Tuples) {}
8282
>g : ({ prop }: Tuples) => void
83-
>prop : [string, number] | [number, number]
83+
>prop : [string, number]
8484
>[101, 1234] : [number, number]
8585
>101 : 101
8686
>1234 : 1234
@@ -94,7 +94,7 @@ interface StringUnion {
9494
}
9595
function h({ prop = "baz" }: StringUnion) {}
9696
>h : ({ prop }: StringUnion) => void
97-
>prop : "foo" | "bar" | "baz"
97+
>prop : "foo" | "bar"
9898
>"baz" : "baz"
9999
>StringUnion : StringUnion
100100

‎tests/baselines/reference/destructuringParameterDeclaration8.errors.txt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration8.ts(4,5): error TS2322: Type '"z"' is not assignable to type '"x" | "y"'.
2+
tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration8.ts(5,15): error TS2322: Type '"c"' is not assignable to type '"a" | "b"'.
3+
tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration8.ts(17,6): error TS2345: Argument of type '{ method: "z"; nested: { p: "b"; }; }' is not assignable to parameter of type '{ method?: "x" | "y"; nested?: { p: "a" | "b"; }; }'.
4+
Types of property 'method' are incompatible.
5+
Type '"z"' is not assignable to type '"x" | "y"'.
6+
tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration8.ts(18,6): error TS2345: Argument of type '{ method: "one"; nested: { p: "a"; }; }' is not assignable to parameter of type '{ method?: "x" | "y"; nested?: { p: "a" | "b"; }; }'.
7+
Types of property 'method' are incompatible.
8+
Type '"one"' is not assignable to type '"x" | "y"'.
9+
10+
11+
==== tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration8.ts (4 errors) ====
12+
// explicit type annotation should cause `method` to have type 'x' | 'y'
13+
// both inside and outside `test`.
14+
function test({
15+
method = 'z',
16+
~~~~~~
17+
!!! error TS2322: Type '"z"' is not assignable to type '"x" | "y"'.
18+
nested: { p = 'c' }
19+
~
20+
!!! error TS2322: Type '"c"' is not assignable to type '"a" | "b"'.
21+
}: {
22+
method?: 'x' | 'y',
23+
nested?: { p: 'a' | 'b' }
24+
})
25+
{
26+
method
27+
p
28+
}
29+
30+
test({});
31+
test({ method: 'x', nested: { p: 'a' } })
32+
test({ method: 'z', nested: { p: 'b' } })
33+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
34+
!!! error TS2345: Argument of type '{ method: "z"; nested: { p: "b"; }; }' is not assignable to parameter of type '{ method?: "x" | "y"; nested?: { p: "a" | "b"; }; }'.
35+
!!! error TS2345: Types of property 'method' are incompatible.
36+
!!! error TS2345: Type '"z"' is not assignable to type '"x" | "y"'.
37+
test({ method: 'one', nested: { p: 'a' } })
38+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
39+
!!! error TS2345: Argument of type '{ method: "one"; nested: { p: "a"; }; }' is not assignable to parameter of type '{ method?: "x" | "y"; nested?: { p: "a" | "b"; }; }'.
40+
!!! error TS2345: Types of property 'method' are incompatible.
41+
!!! error TS2345: Type '"one"' is not assignable to type '"x" | "y"'.
42+

‎tests/baselines/reference/destructuringParameterDeclaration8.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//// [destructuringParameterDeclaration8.ts]
2+
// explicit type annotation should cause `method` to have type 'x' | 'y'
3+
// both inside and outside `test`.
4+
function test({
5+
method = 'z',
6+
nested: { p = 'c' }
7+
}: {
8+
method?: 'x' | 'y',
9+
nested?: { p: 'a' | 'b' }
10+
})
11+
{
12+
method
13+
p
14+
}
15+
16+
test({});
17+
test({ method: 'x', nested: { p: 'a' } })
18+
test({ method: 'z', nested: { p: 'b' } })
19+
test({ method: 'one', nested: { p: 'a' } })
20+
21+
22+
//// [destructuringParameterDeclaration8.js]
23+
// explicit type annotation should cause `method` to have type 'x' | 'y'
24+
// both inside and outside `test`.
25+
function test(_a) {
26+
var _b = _a.method, method = _b === void 0 ? 'z' : _b, _c = _a.nested.p, p = _c === void 0 ? 'c' : _c;
27+
method;
28+
p;
29+
}
30+
test({});
31+
test({ method: 'x', nested: { p: 'a' } });
32+
test({ method: 'z', nested: { p: 'b' } });
33+
test({ method: 'one', nested: { p: 'a' } });

0 commit comments

Comments
 (0)