Skip to content

Commit 5224a6a

Browse files
weswighamweswigham
committed
Merge branch 'master' into noConstraint-is-unknown
2 parents a560fd6 + bb5eb02 commit 5224a6a

23 files changed

+1457
-103
lines changed

‎src/compiler/checker.ts‎

Lines changed: 52 additions & 46 deletions
Large diffs are not rendered by default.

‎src/compiler/utilities.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,6 +1330,10 @@ namespace ts {
13301330
return findAncestor(node.parent, isFunctionLike);
13311331
}
13321332

1333+
export function getContainingFunctionDeclaration(node: Node): FunctionLikeDeclaration | undefined {
1334+
return findAncestor(node.parent, isFunctionLikeDeclaration);
1335+
}
1336+
13331337
export function getContainingClass(node: Node): ClassLikeDeclaration | undefined {
13341338
return findAncestor(node.parent, isClassLike);
13351339
}

‎src/services/refactors/convertParamsToDestructuredObject.ts‎

Lines changed: 81 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ namespace ts.refactor.convertParamsToDestructuredObject {
9090
function groupReferences(referenceEntries: ReadonlyArray<FindAllReferences.Entry>): GroupedReferences {
9191
const classReferences: ClassReferences = { accessExpressions: [], typeUsages: [] };
9292
const groupedReferences: GroupedReferences = { functionCalls: [], declarations: [], classReferences, valid: true };
93-
const functionSymbols = map(functionNames, checker.getSymbolAtLocation);
94-
const classSymbols = map(classNames, checker.getSymbolAtLocation);
93+
const functionSymbols = map(functionNames, getSymbolTargetAtLocation);
94+
const classSymbols = map(classNames, getSymbolTargetAtLocation);
9595
const isConstructor = isConstructorDeclaration(functionDeclaration);
9696

9797
for (const entry of referenceEntries) {
@@ -111,7 +111,11 @@ namespace ts.refactor.convertParamsToDestructuredObject {
111111
So we need to add a special case for this because when calling a constructor of a class through one of its subclasses,
112112
the symbols are going to be different.
113113
*/
114-
if (contains(functionSymbols, checker.getSymbolAtLocation(entry.node), symbolComparer) || isNewExpressionTarget(entry.node)) {
114+
if (contains(functionSymbols, getSymbolTargetAtLocation(entry.node)) || isNewExpressionTarget(entry.node)) {
115+
const importOrExportReference = entryToImportOrExport(entry);
116+
if (importOrExportReference) {
117+
continue;
118+
}
115119
const decl = entryToDeclaration(entry);
116120
if (decl) {
117121
groupedReferences.declarations.push(decl);
@@ -125,7 +129,12 @@ namespace ts.refactor.convertParamsToDestructuredObject {
125129
}
126130
}
127131
// if the refactored function is a constructor, we must also check if the references to its class are valid
128-
if (isConstructor && contains(classSymbols, checker.getSymbolAtLocation(entry.node), symbolComparer)) {
132+
if (isConstructor && contains(classSymbols, getSymbolTargetAtLocation(entry.node))) {
133+
const importOrExportReference = entryToImportOrExport(entry);
134+
if (importOrExportReference) {
135+
continue;
136+
}
137+
129138
const decl = entryToDeclaration(entry);
130139
if (decl) {
131140
groupedReferences.declarations.push(decl);
@@ -153,10 +162,27 @@ namespace ts.refactor.convertParamsToDestructuredObject {
153162

154163
return groupedReferences;
155164
}
165+
166+
function getSymbolTargetAtLocation(node: Node) {
167+
const symbol = checker.getSymbolAtLocation(node);
168+
return symbol && getSymbolTarget(symbol, checker);
169+
}
156170
}
157171

158-
function symbolComparer(a: Symbol, b: Symbol): boolean {
159-
return getSymbolTarget(a) === getSymbolTarget(b);
172+
function entryToImportOrExport(entry: FindAllReferences.NodeEntry): Node | undefined {
173+
const node = entry.node;
174+
175+
if (isImportSpecifier(node.parent)
176+
|| isImportClause(node.parent)
177+
|| isImportEqualsDeclaration(node.parent)
178+
|| isNamespaceImport(node.parent)) {
179+
return node;
180+
}
181+
182+
if (isExportSpecifier(node.parent) || isExportAssignment(node.parent)) {
183+
return node;
184+
}
185+
return undefined;
160186
}
161187

162188
function entryToDeclaration(entry: FindAllReferences.NodeEntry): Node | undefined {
@@ -171,37 +197,31 @@ namespace ts.refactor.convertParamsToDestructuredObject {
171197
const functionReference = entry.node;
172198
const parent = functionReference.parent;
173199
switch (parent.kind) {
174-
// Function call (foo(...) or super(...))
200+
// foo(...) or super(...) or new Foo(...)
175201
case SyntaxKind.CallExpression:
176-
const callExpression = tryCast(parent, isCallExpression);
177-
if (callExpression && callExpression.expression === functionReference) {
178-
return callExpression;
179-
}
180-
break;
181-
// Constructor call (new Foo(...))
182202
case SyntaxKind.NewExpression:
183-
const newExpression = tryCast(parent, isNewExpression);
184-
if (newExpression && newExpression.expression === functionReference) {
185-
return newExpression;
203+
const callOrNewExpression = tryCast(parent, isCallOrNewExpression);
204+
if (callOrNewExpression && callOrNewExpression.expression === functionReference) {
205+
return callOrNewExpression;
186206
}
187207
break;
188-
// Method call (x.foo(...))
208+
// x.foo(...)
189209
case SyntaxKind.PropertyAccessExpression:
190210
const propertyAccessExpression = tryCast(parent, isPropertyAccessExpression);
191211
if (propertyAccessExpression && propertyAccessExpression.parent && propertyAccessExpression.name === functionReference) {
192-
const callExpression = tryCast(propertyAccessExpression.parent, isCallExpression);
193-
if (callExpression && callExpression.expression === propertyAccessExpression) {
194-
return callExpression;
212+
const callOrNewExpression = tryCast(propertyAccessExpression.parent, isCallOrNewExpression);
213+
if (callOrNewExpression && callOrNewExpression.expression === propertyAccessExpression) {
214+
return callOrNewExpression;
195215
}
196216
}
197217
break;
198-
// Method call (x["foo"](...))
218+
// x["foo"](...)
199219
case SyntaxKind.ElementAccessExpression:
200220
const elementAccessExpression = tryCast(parent, isElementAccessExpression);
201221
if (elementAccessExpression && elementAccessExpression.parent && elementAccessExpression.argumentExpression === functionReference) {
202-
const callExpression = tryCast(elementAccessExpression.parent, isCallExpression);
203-
if (callExpression && callExpression.expression === elementAccessExpression) {
204-
return callExpression;
222+
const callOrNewExpression = tryCast(elementAccessExpression.parent, isCallOrNewExpression);
223+
if (callOrNewExpression && callOrNewExpression.expression === elementAccessExpression) {
224+
return callOrNewExpression;
205225
}
206226
}
207227
break;
@@ -244,7 +264,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
244264

245265
function getFunctionDeclarationAtPosition(file: SourceFile, startPosition: number, checker: TypeChecker): ValidFunctionDeclaration | undefined {
246266
const node = getTouchingToken(file, startPosition);
247-
const functionDeclaration = getContainingFunction(node);
267+
const functionDeclaration = getContainingFunctionDeclaration(node);
248268

249269
// don't offer refactor on top-level JSDoc
250270
if (isTopLevelJSDoc(node)) return undefined;
@@ -267,25 +287,21 @@ namespace ts.refactor.convertParamsToDestructuredObject {
267287
}
268288

269289
function isValidFunctionDeclaration(
270-
functionDeclaration: SignatureDeclaration,
290+
functionDeclaration: FunctionLikeDeclaration,
271291
checker: TypeChecker): functionDeclaration is ValidFunctionDeclaration {
272292
if (!isValidParameterNodeArray(functionDeclaration.parameters, checker)) return false;
273293
switch (functionDeclaration.kind) {
274294
case SyntaxKind.FunctionDeclaration:
295+
return hasNameOrDefault(functionDeclaration) && isSingleImplementation(functionDeclaration, checker);
275296
case SyntaxKind.MethodDeclaration:
276-
return !!functionDeclaration.name
277-
&& !!functionDeclaration.body
278-
&& !checker.isImplementationOfOverload(functionDeclaration);
297+
return isSingleImplementation(functionDeclaration, checker);
279298
case SyntaxKind.Constructor:
280299
if (isClassDeclaration(functionDeclaration.parent)) {
281-
return !!functionDeclaration.body
282-
&& !!functionDeclaration.parent.name
283-
&& !checker.isImplementationOfOverload(functionDeclaration);
300+
return hasNameOrDefault(functionDeclaration.parent) && isSingleImplementation(functionDeclaration, checker);
284301
}
285302
else {
286303
return isValidVariableDeclaration(functionDeclaration.parent.parent)
287-
&& !!functionDeclaration.body
288-
&& !checker.isImplementationOfOverload(functionDeclaration);
304+
&& isSingleImplementation(functionDeclaration, checker);
289305
}
290306
case SyntaxKind.FunctionExpression:
291307
case SyntaxKind.ArrowFunction:
@@ -294,6 +310,18 @@ namespace ts.refactor.convertParamsToDestructuredObject {
294310
return false;
295311
}
296312

313+
function isSingleImplementation(functionDeclaration: FunctionLikeDeclaration, checker: TypeChecker): boolean {
314+
return !!functionDeclaration.body && !checker.isImplementationOfOverload(functionDeclaration);
315+
}
316+
317+
function hasNameOrDefault(functionOrClassDeclaration: FunctionDeclaration | ClassDeclaration): boolean {
318+
if (!functionOrClassDeclaration.name) {
319+
const defaultKeyword = findModifier(functionOrClassDeclaration, SyntaxKind.DefaultKeyword);
320+
return !!defaultKeyword;
321+
}
322+
return true;
323+
}
324+
297325
function isValidParameterNodeArray(
298326
parameters: NodeArray<ParameterDeclaration>,
299327
checker: TypeChecker): parameters is ValidParameterNodeArray {
@@ -488,11 +516,17 @@ namespace ts.refactor.convertParamsToDestructuredObject {
488516
return getTextOfIdentifierOrLiteral(paramDeclaration.name);
489517
}
490518

491-
function getClassNames(constructorDeclaration: ValidConstructor): Identifier[] {
519+
function getClassNames(constructorDeclaration: ValidConstructor): (Identifier | Modifier)[] {
492520
switch (constructorDeclaration.parent.kind) {
493521
case SyntaxKind.ClassDeclaration:
494522
const classDeclaration = constructorDeclaration.parent;
495-
return [classDeclaration.name];
523+
if (classDeclaration.name) return [classDeclaration.name];
524+
// If the class declaration doesn't have a name, it should have a default modifier.
525+
// We validated this in `isValidFunctionDeclaration` through `hasNameOrDefault`
526+
const defaultModifier = Debug.assertDefined(
527+
findModifier(classDeclaration, SyntaxKind.DefaultKeyword),
528+
"Nameless class declaration should be a default export");
529+
return [defaultModifier];
496530
case SyntaxKind.ClassExpression:
497531
const classExpression = constructorDeclaration.parent;
498532
const variableDeclaration = constructorDeclaration.parent.parent;
@@ -505,10 +539,19 @@ namespace ts.refactor.convertParamsToDestructuredObject {
505539
function getFunctionNames(functionDeclaration: ValidFunctionDeclaration): Node[] {
506540
switch (functionDeclaration.kind) {
507541
case SyntaxKind.FunctionDeclaration:
542+
if (functionDeclaration.name) return [functionDeclaration.name];
543+
// If the function declaration doesn't have a name, it should have a default modifier.
544+
// We validated this in `isValidFunctionDeclaration` through `hasNameOrDefault`
545+
const defaultModifier = Debug.assertDefined(
546+
findModifier(functionDeclaration, SyntaxKind.DefaultKeyword),
547+
"Nameless function declaration should be a default export");
548+
return [defaultModifier];
508549
case SyntaxKind.MethodDeclaration:
509550
return [functionDeclaration.name];
510551
case SyntaxKind.Constructor:
511-
const ctrKeyword = findChildOfKind(functionDeclaration, SyntaxKind.ConstructorKeyword, functionDeclaration.getSourceFile())!;
552+
const ctrKeyword = Debug.assertDefined(
553+
findChildOfKind(functionDeclaration, SyntaxKind.ConstructorKeyword, functionDeclaration.getSourceFile()),
554+
"Constructor declaration should have constructor keyword");
512555
if (functionDeclaration.parent.kind === SyntaxKind.ClassExpression) {
513556
const variableDeclaration = functionDeclaration.parent.parent;
514557
return [variableDeclaration.name, ctrKeyword];
@@ -532,13 +575,12 @@ namespace ts.refactor.convertParamsToDestructuredObject {
532575
}
533576

534577
interface ValidConstructor extends ConstructorDeclaration {
535-
parent: (ClassDeclaration & { name: Identifier }) | (ClassExpression & { parent: ValidVariableDeclaration });
578+
parent: ClassDeclaration | (ClassExpression & { parent: ValidVariableDeclaration });
536579
parameters: NodeArray<ValidParameterDeclaration>;
537580
body: FunctionBody;
538581
}
539582

540583
interface ValidFunction extends FunctionDeclaration {
541-
name: Identifier;
542584
parameters: NodeArray<ValidParameterDeclaration>;
543585
body: FunctionBody;
544586
}

‎src/services/utilities.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,10 +1664,15 @@ namespace ts {
16641664
return ensureScriptKind(fileName, host && host.getScriptKind && host.getScriptKind(fileName));
16651665
}
16661666

1667-
export function getSymbolTarget(symbol: Symbol): Symbol {
1667+
export function getSymbolTarget(symbol: Symbol, checker: TypeChecker): Symbol {
16681668
let next: Symbol = symbol;
1669-
while (isTransientSymbol(next) && next.target) {
1670-
next = next.target;
1669+
while (isAliasSymbol(next) || (isTransientSymbol(next) && next.target)) {
1670+
if (isTransientSymbol(next) && next.target) {
1671+
next = next.target;
1672+
}
1673+
else {
1674+
next = skipAlias(next, checker);
1675+
}
16711676
}
16721677
return next;
16731678
}
@@ -1676,6 +1681,10 @@ namespace ts {
16761681
return (symbol.flags & SymbolFlags.Transient) !== 0;
16771682
}
16781683

1684+
function isAliasSymbol(symbol: Symbol): boolean {
1685+
return (symbol.flags & SymbolFlags.Alias) !== 0;
1686+
}
1687+
16791688
export function getUniqueSymbolId(symbol: Symbol, checker: TypeChecker) {
16801689
return getSymbolId(skipAlias(symbol, checker));
16811690
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
tests/cases/compiler/errorMessageOnIntersectionsWithDiscriminants01.ts(8,1): error TS2322: Type 'A' is not assignable to type 'B'.
2+
Type '{ test: true; } & { foo: 1; }' is not assignable to type 'B'.
3+
Type '{ test: true; } & { foo: 1; }' is not assignable to type '{ test: true; } & { bar: 1; }'.
4+
Property 'bar' is missing in type '{ test: true; } & { foo: 1; }' but required in type '{ bar: 1; }'.
5+
6+
7+
==== tests/cases/compiler/errorMessageOnIntersectionsWithDiscriminants01.ts (1 errors) ====
8+
export type Common = { test: true } | { test: false };
9+
export type A = Common & { foo: 1 };
10+
export type B = Common & { bar: 1 };
11+
12+
declare const a: A;
13+
declare let b: B;
14+
15+
b = a;
16+
~
17+
!!! error TS2322: Type 'A' is not assignable to type 'B'.
18+
!!! error TS2322: Type '{ test: true; } & { foo: 1; }' is not assignable to type 'B'.
19+
!!! error TS2322: Type '{ test: true; } & { foo: 1; }' is not assignable to type '{ test: true; } & { bar: 1; }'.
20+
!!! error TS2322: Property 'bar' is missing in type '{ test: true; } & { foo: 1; }' but required in type '{ bar: 1; }'.
21+
!!! related TS2728 tests/cases/compiler/errorMessageOnIntersectionsWithDiscriminants01.ts:3:28: 'bar' is declared here.
22+

‎tests/baselines/reference/errorMessageOnIntersectionsWithDiscriminants01.symbols‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
=== tests/cases/compiler/errorMessageOnIntersectionsWithDiscriminants01.ts ===
2+
export type Common = { test: true } | { test: false };
3+
>Common : Symbol(Common, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 0))
4+
>test : Symbol(test, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 22))
5+
>test : Symbol(test, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 39))
6+
7+
export type A = Common & { foo: 1 };
8+
>A : Symbol(A, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 54))
9+
>Common : Symbol(Common, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 0))
10+
>foo : Symbol(foo, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 1, 26))
11+
12+
export type B = Common & { bar: 1 };
13+
>B : Symbol(B, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 1, 36))
14+
>Common : Symbol(Common, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 0))
15+
>bar : Symbol(bar, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 2, 26))
16+
17+
declare const a: A;
18+
>a : Symbol(a, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 4, 13))
19+
>A : Symbol(A, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 0, 54))
20+
21+
declare let b: B;
22+
>b : Symbol(b, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 5, 11))
23+
>B : Symbol(B, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 1, 36))
24+
25+
b = a;
26+
>b : Symbol(b, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 5, 11))
27+
>a : Symbol(a, Decl(errorMessageOnIntersectionsWithDiscriminants01.ts, 4, 13))
28+

‎tests/baselines/reference/errorMessageOnIntersectionsWithDiscriminants01.types‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/errorMessageOnIntersectionsWithDiscriminants01.ts ===
2+
export type Common = { test: true } | { test: false };
3+
>Common : Common
4+
>test : true
5+
>true : true
6+
>test : false
7+
>false : false
8+
9+
export type A = Common & { foo: 1 };
10+
>A : A
11+
>foo : 1
12+
13+
export type B = Common & { bar: 1 };
14+
>B : B
15+
>bar : 1
16+
17+
declare const a: A;
18+
>a : A
19+
20+
declare let b: B;
21+
>b : B
22+
23+
b = a;
24+
>b = a : A
25+
>b : B
26+
>a : A
27+

‎tests/baselines/reference/genericCallWithConstraintsTypeArgumentInference2.types‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ var r = foo(1); // ok
1919
>1 : 1
2020

2121
var r2 = foo(null); // {}
22-
>r2 : unknown
23-
>foo(null) : unknown
22+
>r2 : any
23+
>foo(null) : any
2424
>foo : <T, U extends T>(t: T) => U
2525
>null : null
2626

‎tests/baselines/reference/genericCallWithObjectTypeArgsAndConstraints4.types‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ var r8 = foo(() => { }, () => { });
9696
>() => { } : () => void
9797

9898
var r9 = foo(() => { }, () => 1);
99-
>r9 : (x: () => void) => () => 1
100-
>foo(() => { }, () => 1) : (x: () => void) => () => 1
99+
>r9 : (x: () => void) => () => number
100+
>foo(() => { }, () => 1) : (x: () => void) => () => number
101101
>foo : <T, U extends T>(t: T, t2: U) => (x: T) => U
102102
>() => { } : () => void
103-
>() => 1 : () => 1
103+
>() => 1 : () => number
104104
>1 : 1
105105

106106
function other<T, U extends T>() {

0 commit comments

Comments
 (0)