Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5c2eeb2

Browse filesBrowse 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
Copy full SHA for 5c2eeb2

17 files changed

+328-55Lines changed: 328 additions & 55 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/checker.ts‎

Copy file name to clipboardExpand all lines: src/compiler/checker.ts
+3-3Lines 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
Collapse file

‎src/compiler/utilities.ts‎

Copy file name to clipboardExpand all lines: src/compiler/utilities.ts
+18-31Lines 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
/**
Collapse file

‎src/services/utilities.ts‎

Copy file name to clipboardExpand all lines: src/services/utilities.ts
+1-1Lines 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);
Collapse file

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

Copy file name to clipboardExpand all lines: tests/baselines/reference/api/tsserverlibrary.d.ts
+2-1Lines 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,
Collapse file

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

Copy file name to clipboardExpand all lines: tests/baselines/reference/api/typescript.d.ts
+2-1Lines 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,
Collapse file

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

Copy file name to clipboard
+67Lines 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+
Collapse file

‎tests/baselines/reference/contextuallyTypedBindingInitializerNegative.types‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/contextuallyTypedBindingInitializerNegative.types
+7-7Lines 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

Collapse file

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

Copy file name to clipboard
+42Lines 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+
Collapse file

‎tests/baselines/reference/destructuringParameterDeclaration8.js‎

Copy file name to clipboard
+33Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.