Skip to content

Commit 1a101f7

Browse files
weswighamweswigham
committed
Merge branch 'master' into broader-maybe-detection
2 parents b3856bd + 5280ba4 commit 1a101f7

23 files changed

+675
-52
lines changed

‎src/compiler/checker.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23162,10 +23162,15 @@ namespace ts {
2316223162
}
2316323163
}
2316423164

23165-
// If the candidate type is a subtype of the target type, narrow to the candidate type,
23166-
// if the target type is a subtype of the candidate type, narrow to the target type,
23167-
// otherwise, narrow to an intersection of the two types.
23168-
return isTypeSubtypeOf(candidate, type) ? candidate : isTypeSubtypeOf(type, candidate) ? type : getIntersectionType([type, candidate]);
23165+
// If the candidate type is a subtype of the target type, narrow to the candidate type.
23166+
// Otherwise, if the target type is assignable to the candidate type, keep the target type.
23167+
// Otherwise, if the candidate type is assignable to the target type, narrow to the candidate
23168+
// type. Otherwise, the types are completely unrelated, so narrow to an intersection of the
23169+
// two types.
23170+
return isTypeSubtypeOf(candidate, type) ? candidate :
23171+
isTypeAssignableTo(type, candidate) ? type :
23172+
isTypeAssignableTo(candidate, type) ? candidate :
23173+
getIntersectionType([type, candidate]);
2316923174
}
2317023175

2317123176
function narrowTypeByCallExpression(type: Type, callExpression: CallExpression, assumeTrue: boolean): Type {

‎src/server/project.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,8 +1581,8 @@ namespace ts.server {
15811581

15821582
protected enablePlugin(pluginConfigEntry: PluginImport, searchPaths: string[], pluginConfigOverrides: Map<any> | undefined) {
15831583
this.projectService.logger.info(`Enabling plugin ${pluginConfigEntry.name} from candidate paths: ${searchPaths.join(",")}`);
1584-
if (parsePackageName(pluginConfigEntry.name).rest) {
1585-
this.projectService.logger.info(`Skipped loading plugin ${pluginConfigEntry.name} because only package name is allowed plugin name`);
1584+
if (!pluginConfigEntry.name || parsePackageName(pluginConfigEntry.name).rest) {
1585+
this.projectService.logger.info(`Skipped loading plugin ${pluginConfigEntry.name || JSON.stringify(pluginConfigEntry)} because only package name is allowed plugin name`);
15861586
return;
15871587
}
15881588

‎src/services/refactors/convertParamsToDestructuredObject.ts

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,14 @@ namespace ts.refactor.convertParamsToDestructuredObject {
5151
changes: textChanges.ChangeTracker,
5252
functionDeclaration: ValidFunctionDeclaration,
5353
groupedReferences: GroupedReferences): void {
54-
const newParamDeclaration = map(createNewParameters(functionDeclaration, program, host), param => getSynthesizedDeepClone(param));
55-
changes.replaceNodeRangeWithNodes(
56-
sourceFile,
57-
first(functionDeclaration.parameters),
58-
last(functionDeclaration.parameters),
59-
newParamDeclaration,
60-
{ joiner: ", ",
61-
// indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter
62-
indentation: 0,
63-
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
64-
trailingTriviaOption: textChanges.TrailingTriviaOption.Include
65-
});
54+
const signature = groupedReferences.signature;
55+
const newFunctionDeclarationParams = map(createNewParameters(functionDeclaration, program, host), param => getSynthesizedDeepClone(param));
56+
57+
if (signature) {
58+
const newSignatureParams = map(createNewParameters(signature, program, host), param => getSynthesizedDeepClone(param));
59+
replaceParameters(signature, newSignatureParams);
60+
}
61+
replaceParameters(functionDeclaration, newFunctionDeclarationParams);
6662

6763
const functionCalls = sortAndDeduplicate(groupedReferences.functionCalls, /*comparer*/ (a, b) => compareValues(a.pos, b.pos));
6864
for (const call of functionCalls) {
@@ -76,6 +72,21 @@ namespace ts.refactor.convertParamsToDestructuredObject {
7672
{ leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll, trailingTriviaOption: textChanges.TrailingTriviaOption.Include });
7773
}
7874
}
75+
76+
function replaceParameters(declarationOrSignature: ValidFunctionDeclaration | ValidMethodSignature, parameterDeclarations: ParameterDeclaration[]) {
77+
changes.replaceNodeRangeWithNodes(
78+
sourceFile,
79+
first(declarationOrSignature.parameters),
80+
last(declarationOrSignature.parameters),
81+
parameterDeclarations,
82+
{
83+
joiner: ", ",
84+
// indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter
85+
indentation: 0,
86+
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
87+
trailingTriviaOption: textChanges.TrailingTriviaOption.Include
88+
});
89+
}
7990
}
8091

8192
function getGroupedReferences(functionDeclaration: ValidFunctionDeclaration, program: Program, cancellationToken: CancellationToken): GroupedReferences {
@@ -99,13 +110,41 @@ namespace ts.refactor.convertParamsToDestructuredObject {
99110
const functionSymbols = map(functionNames, getSymbolTargetAtLocation);
100111
const classSymbols = map(classNames, getSymbolTargetAtLocation);
101112
const isConstructor = isConstructorDeclaration(functionDeclaration);
113+
const contextualSymbols = map(functionNames, name => getSymbolForContextualType(name, checker));
102114

103115
for (const entry of referenceEntries) {
104-
if (entry.kind !== FindAllReferences.EntryKind.Node) {
116+
if (entry.kind === FindAllReferences.EntryKind.Span) {
105117
groupedReferences.valid = false;
106118
continue;
107119
}
108120

121+
/* Declarations in object literals may be implementations of method signatures which have a different symbol from the declaration
122+
For example:
123+
interface IFoo { m(a: number): void }
124+
const foo: IFoo = { m(a: number): void {} }
125+
In these cases we get the symbol for the signature from the contextual type.
126+
*/
127+
if (contains(contextualSymbols, getSymbolTargetAtLocation(entry.node))) {
128+
if (isValidMethodSignature(entry.node.parent)) {
129+
groupedReferences.signature = entry.node.parent;
130+
continue;
131+
}
132+
const call = entryToFunctionCall(entry);
133+
if (call) {
134+
groupedReferences.functionCalls.push(call);
135+
continue;
136+
}
137+
}
138+
139+
const contextualSymbol = getSymbolForContextualType(entry.node, checker);
140+
if (contextualSymbol && contains(contextualSymbols, contextualSymbol)) {
141+
const decl = entryToDeclaration(entry);
142+
if (decl) {
143+
groupedReferences.declarations.push(decl);
144+
continue;
145+
}
146+
}
147+
109148
/* We compare symbols because in some cases find all references wil return a reference that may or may not be to the refactored function.
110149
Example from the refactorConvertParamsToDestructuredObject_methodCallUnion.ts test:
111150
class A { foo(a: number, b: number) { return a + b; } }
@@ -175,6 +214,20 @@ namespace ts.refactor.convertParamsToDestructuredObject {
175214
}
176215
}
177216

217+
/**
218+
* Gets the symbol for the contextual type of the node if it is not a union or intersection.
219+
*/
220+
function getSymbolForContextualType(node: Node, checker: TypeChecker): Symbol | undefined {
221+
const element = getContainingObjectLiteralElement(node);
222+
if (element) {
223+
const contextualType = checker.getContextualTypeForObjectLiteralElement(<ObjectLiteralElementLike>element);
224+
const symbol = contextualType?.getSymbol();
225+
if (symbol && !(getCheckFlags(symbol) & CheckFlags.Synthetic)) {
226+
return symbol;
227+
}
228+
}
229+
}
230+
178231
function entryToImportOrExport(entry: FindAllReferences.NodeEntry): Node | undefined {
179232
const node = entry.node;
180233

@@ -292,6 +345,10 @@ namespace ts.refactor.convertParamsToDestructuredObject {
292345
return false;
293346
}
294347

348+
function isValidMethodSignature(node: Node): node is ValidMethodSignature {
349+
return isMethodSignature(node) && (isInterfaceDeclaration(node.parent) || isTypeLiteralNode(node.parent));
350+
}
351+
295352
function isValidFunctionDeclaration(
296353
functionDeclaration: FunctionLikeDeclaration,
297354
checker: TypeChecker): functionDeclaration is ValidFunctionDeclaration {
@@ -300,6 +357,11 @@ namespace ts.refactor.convertParamsToDestructuredObject {
300357
case SyntaxKind.FunctionDeclaration:
301358
return hasNameOrDefault(functionDeclaration) && isSingleImplementation(functionDeclaration, checker);
302359
case SyntaxKind.MethodDeclaration:
360+
if (isObjectLiteralExpression(functionDeclaration.parent)) {
361+
const contextualSymbol = getSymbolForContextualType(functionDeclaration.name, checker);
362+
// don't offer the refactor when there are multiple signatures since we won't know which ones the user wants to change
363+
return contextualSymbol?.declarations.length === 1 && isSingleImplementation(functionDeclaration, checker);
364+
}
303365
return isSingleImplementation(functionDeclaration, checker);
304366
case SyntaxKind.Constructor:
305367
if (isClassDeclaration(functionDeclaration.parent)) {
@@ -398,7 +460,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
398460
return objectLiteral;
399461
}
400462

401-
function createNewParameters(functionDeclaration: ValidFunctionDeclaration, program: Program, host: LanguageServiceHost): NodeArray<ParameterDeclaration> {
463+
function createNewParameters(functionDeclaration: ValidFunctionDeclaration | ValidMethodSignature, program: Program, host: LanguageServiceHost): NodeArray<ParameterDeclaration> {
402464
const checker = program.getTypeChecker();
403465
const refactorableParameters = getRefactorableParameters(functionDeclaration.parameters);
404466
const bindingElements = map(refactorableParameters, createBindingElementFromParameterDeclaration);
@@ -584,6 +646,10 @@ namespace ts.refactor.convertParamsToDestructuredObject {
584646
parameters: NodeArray<ValidParameterDeclaration>;
585647
}
586648

649+
interface ValidMethodSignature extends MethodSignature {
650+
parameters: NodeArray<ValidParameterDeclaration>;
651+
}
652+
587653
type ValidFunctionDeclaration = ValidConstructor | ValidFunction | ValidMethod | ValidArrowFunction | ValidFunctionExpression;
588654

589655
interface ValidParameterDeclaration extends ParameterDeclaration {
@@ -595,6 +661,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
595661
interface GroupedReferences {
596662
functionCalls: (CallExpression | NewExpression)[];
597663
declarations: Node[];
664+
signature?: ValidMethodSignature;
598665
classReferences?: ClassReferences;
599666
valid: boolean;
600667
}

‎src/testRunner/unittests/tsserver/plugins.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ namespace ts.projectSystem {
2424
const tsconfig: File = {
2525
path: "/tsconfig.json",
2626
content: JSON.stringify({
27-
compilerOptions: { plugins: [...expectedToLoad, ...notToLoad].map(name => ({ name })) }
27+
compilerOptions: {
28+
plugins: [
29+
...[...expectedToLoad, ...notToLoad].map(name => ({ name })),
30+
{ transform: "some-transform" }
31+
]
32+
}
2833
})
2934
};
3035
const { host, pluginsLoaded } = createHostWithPlugin([aTs, tsconfig, libFile]);

‎tests/baselines/reference/instanceOfAssignability.types

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ function fn2(x: Base) {
7070
// 1.5: y: Base
7171
// Want: y: Derived1
7272
let y = x;
73-
>y : Base & Derived1
74-
>x : Base & Derived1
73+
>y : Derived1
74+
>x : Derived1
7575
}
7676
}
7777

@@ -104,8 +104,8 @@ function fn4(x: Base|Derived2) {
104104
// 1.5: y: {}
105105
// Want: Derived1
106106
let y = x;
107-
>y : (Base | Derived2) & Derived1
108-
>x : (Base | Derived2) & Derived1
107+
>y : Derived1
108+
>x : Derived1
109109
}
110110
}
111111

‎tests/baselines/reference/narrowingAssignmentReadonlyRespectsAssertion.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//// [narrowingAssignmentReadonlyRespectsAssertion.ts]
2+
// https://github.com/microsoft/TypeScript/issues/41984
3+
4+
interface TestCase<T extends string | number> {
5+
readonly val1: T | ReadonlyArray<T>;
6+
readonly val2: ReadonlyArray<T>;
7+
}
8+
9+
interface MultiCaseFixture<T> {
10+
cases: T[];
11+
}
12+
13+
function subDataFunc(): TestCase<string | number>[] {
14+
return [
15+
{ val1: "a", val2: ["a", "b", "c"] },
16+
{ val1: 2, val2: [1, 2, 3] },
17+
{ val1: ["a", "z"], val2: ["x", "y", "z"] },
18+
{ val1: [5, 10], val2: [10, 100, 1000] },
19+
];
20+
}
21+
22+
function dataFunc<T>(subFunc: () => T[]): MultiCaseFixture<T> {
23+
return { cases: subFunc() };
24+
}
25+
26+
function testFunc() {
27+
const fixture = dataFunc<TestCase<string | number>>(subDataFunc);
28+
fixture.cases.forEach(({ val1, val2 }) => {
29+
if (Array.isArray(val1)) {
30+
// This should retain val1 as being an array
31+
const reversedVal1 = val1.slice().reverse();
32+
console.log(reversedVal1);
33+
} else {
34+
console.log(val1);
35+
}
36+
console.log(val2);
37+
});
38+
}
39+
40+
testFunc();
41+
42+
43+
//// [narrowingAssignmentReadonlyRespectsAssertion.js]
44+
// https://github.com/microsoft/TypeScript/issues/41984
45+
function subDataFunc() {
46+
return [
47+
{ val1: "a", val2: ["a", "b", "c"] },
48+
{ val1: 2, val2: [1, 2, 3] },
49+
{ val1: ["a", "z"], val2: ["x", "y", "z"] },
50+
{ val1: [5, 10], val2: [10, 100, 1000] },
51+
];
52+
}
53+
function dataFunc(subFunc) {
54+
return { cases: subFunc() };
55+
}
56+
function testFunc() {
57+
var fixture = dataFunc(subDataFunc);
58+
fixture.cases.forEach(function (_a) {
59+
var val1 = _a.val1, val2 = _a.val2;
60+
if (Array.isArray(val1)) {
61+
// This should retain val1 as being an array
62+
var reversedVal1 = val1.slice().reverse();
63+
console.log(reversedVal1);
64+
}
65+
else {
66+
console.log(val1);
67+
}
68+
console.log(val2);
69+
});
70+
}
71+
testFunc();

0 commit comments

Comments
 (0)