Skip to content

Commit ca00b32

Browse files
Josh Goldbergsandersn
Josh Goldberg
authored and
sandersn
committed
Added --noImplicitThis code fix for functions used as object properties (#31138)
* Added --noImplicitThis code fix for functions used as object properties Before trying out all the various possibilities for where these functions could be used, I figured I'd start out with a relatively simple use case to verify this is the right approach. Is it? 😄 Starts on #28964. * Fixed function expression names; included new baselines * Got JSDocs to work, hooray! * Added test for 'any' case of no function uses * Refactored for inferFunctionReferencesFromUsage * Fixed inference bug: undefined references cause parameters to default * Removed dead code comments
1 parent 7f65be4 commit ca00b32

26 files changed

+520
-73
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4990,7 +4990,10 @@
49904990
"category": "Message",
49914991
"code": 95079
49924992
},
4993-
4993+
"Infer 'this' type of '{0}' from usage": {
4994+
"category": "Message",
4995+
"code": 95080
4996+
},
49944997
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
49954998
"category": "Error",
49964999
"code": 18004

‎src/compiler/factory.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,6 +2204,13 @@ namespace ts {
22042204
return tag;
22052205
}
22062206

2207+
/** @internal */
2208+
export function createJSDocThisTag(typeExpression?: JSDocTypeExpression): JSDocThisTag {
2209+
const tag = createJSDocTag<JSDocThisTag>(SyntaxKind.JSDocThisTag, "this");
2210+
tag.typeExpression = typeExpression;
2211+
return tag;
2212+
}
2213+
22072214
/* @internal */
22082215
export function createJSDocParamTag(name: EntityName, isBracketed: boolean, typeExpression?: JSDocTypeExpression, comment?: string): JSDocParameterTag {
22092216
const tag = createJSDocTag<JSDocParameterTag>(SyntaxKind.JSDocParameterTag, "param");

‎src/compiler/utilities.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5187,6 +5187,9 @@ namespace ts {
51875187
return node.parent.left.name;
51885188
}
51895189
}
5190+
else if (isVariableDeclaration(node.parent) && isIdentifier(node.parent.name)) {
5191+
return node.parent.name;
5192+
}
51905193
}
51915194

51925195
/**

‎src/services/codefixes/inferFromUsage.ts

Lines changed: 100 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ namespace ts.codefix {
4242

4343
// Property declarations
4444
Diagnostics.Member_0_implicitly_has_an_1_type_but_a_better_type_may_be_inferred_from_usage.code,
45+
46+
// Function expressions and declarations
47+
Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation.code,
4548
];
4649
registerCodeFix({
4750
errorCodes,
@@ -73,6 +76,8 @@ namespace ts.codefix {
7376
case Diagnostics.Rest_parameter_0_implicitly_has_an_any_type.code:
7477
case Diagnostics.Rest_parameter_0_implicitly_has_an_any_type_but_a_better_type_may_be_inferred_from_usage.code:
7578
return Diagnostics.Infer_parameter_types_from_usage;
79+
case Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation.code:
80+
return Diagnostics.Infer_this_type_of_0_from_usage;
7681
default:
7782
return Diagnostics.Infer_type_of_0_from_usage;
7883
}
@@ -176,6 +181,14 @@ namespace ts.codefix {
176181
}
177182
return undefined;
178183

184+
// Function 'this'
185+
case Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation.code:
186+
if (textChanges.isThisTypeAnnotatable(containingFunction) && markSeen(containingFunction)) {
187+
annotateThis(changes, sourceFile, containingFunction, program, host, cancellationToken);
188+
return containingFunction;
189+
}
190+
return undefined;
191+
179192
default:
180193
return Debug.fail(String(errorCode));
181194
}
@@ -191,7 +204,9 @@ namespace ts.codefix {
191204
if (!isIdentifier(parameterDeclaration.name)) {
192205
return;
193206
}
194-
const parameterInferences = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) ||
207+
208+
const references = inferFunctionReferencesFromUsage(containingFunction, sourceFile, program, cancellationToken);
209+
const parameterInferences = InferFromReference.inferTypeForParametersFromReferences(references, containingFunction, program, cancellationToken) ||
195210
containingFunction.parameters.map<ParameterInference>(p => ({
196211
declaration: p,
197212
type: isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : program.getTypeChecker().getAnyType()
@@ -213,6 +228,36 @@ namespace ts.codefix {
213228
}
214229
}
215230

231+
function annotateThis(changes: textChanges.ChangeTracker, sourceFile: SourceFile, containingFunction: textChanges.ThisTypeAnnotatable, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken) {
232+
const references = inferFunctionReferencesFromUsage(containingFunction, sourceFile, program, cancellationToken);
233+
if (!references) {
234+
return;
235+
}
236+
237+
const thisInference = InferFromReference.inferTypeForThisFromReferences(references, program, cancellationToken);
238+
if (!thisInference) {
239+
return;
240+
}
241+
242+
const typeNode = getTypeNodeIfAccessible(thisInference, containingFunction, program, host);
243+
if (!typeNode) {
244+
return;
245+
}
246+
247+
if (isInJSFile(containingFunction)) {
248+
annotateJSDocThis(changes, sourceFile, containingFunction, typeNode);
249+
}
250+
else {
251+
changes.tryInsertThisTypeAnnotation(sourceFile, containingFunction, typeNode);
252+
}
253+
}
254+
255+
function annotateJSDocThis(changes: textChanges.ChangeTracker, sourceFile: SourceFile, containingFunction: FunctionLike, typeNode: TypeNode) {
256+
addJSDocTags(changes, sourceFile, containingFunction, [
257+
createJSDocThisTag(createJSDocTypeExpression(typeNode)),
258+
]);
259+
}
260+
216261
function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void {
217262
const param = firstOrUndefined(setAccessorDeclaration.parameters);
218263
if (param && isIdentifier(setAccessorDeclaration.name) && isIdentifier(param.name)) {
@@ -317,7 +362,7 @@ namespace ts.codefix {
317362
return InferFromReference.unifyFromContext(types, checker);
318363
}
319364

320-
function inferTypeForParametersFromUsage(containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined {
365+
function inferFunctionReferencesFromUsage(containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): ReadonlyArray<Identifier> | undefined {
321366
let searchToken;
322367
switch (containingFunction.kind) {
323368
case SyntaxKind.Constructor:
@@ -335,9 +380,12 @@ namespace ts.codefix {
335380
searchToken = containingFunction.name;
336381
break;
337382
}
338-
if (searchToken) {
339-
return InferFromReference.inferTypeForParametersFromReferences(getReferences(searchToken, program, cancellationToken), containingFunction, program, cancellationToken);
383+
384+
if (!searchToken) {
385+
return undefined;
340386
}
387+
388+
return getReferences(searchToken, program, cancellationToken);
341389
}
342390

343391
interface ParameterInference {
@@ -364,6 +412,7 @@ namespace ts.codefix {
364412
constructContexts?: CallContext[];
365413
numberIndexContext?: UsageContext;
366414
stringIndexContext?: UsageContext;
415+
candidateThisTypes?: Type[];
367416
}
368417

369418
export function inferTypesFromReferences(references: ReadonlyArray<Identifier>, checker: TypeChecker, cancellationToken: CancellationToken): Type[] {
@@ -375,15 +424,12 @@ namespace ts.codefix {
375424
return inferFromContext(usageContext, checker);
376425
}
377426

378-
export function inferTypeForParametersFromReferences(references: ReadonlyArray<Identifier>, declaration: FunctionLike, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined {
379-
const checker = program.getTypeChecker();
380-
if (references.length === 0) {
381-
return undefined;
382-
}
383-
if (!declaration.parameters) {
427+
export function inferTypeForParametersFromReferences(references: ReadonlyArray<Identifier> | undefined, declaration: FunctionLike, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined {
428+
if (references === undefined || references.length === 0 || !declaration.parameters) {
384429
return undefined;
385430
}
386431

432+
const checker = program.getTypeChecker();
387433
const usageContext: UsageContext = {};
388434
for (const reference of references) {
389435
cancellationToken.throwIfCancellationRequested();
@@ -421,6 +467,22 @@ namespace ts.codefix {
421467
});
422468
}
423469

470+
export function inferTypeForThisFromReferences(references: ReadonlyArray<Identifier>, program: Program, cancellationToken: CancellationToken) {
471+
if (references.length === 0) {
472+
return undefined;
473+
}
474+
475+
const checker = program.getTypeChecker();
476+
const usageContext: UsageContext = {};
477+
478+
for (const reference of references) {
479+
cancellationToken.throwIfCancellationRequested();
480+
inferTypeFromContext(reference, checker, usageContext);
481+
}
482+
483+
return unifyFromContext(usageContext.candidateThisTypes || emptyArray, checker);
484+
}
485+
424486
function inferTypeFromContext(node: Expression, checker: TypeChecker, usageContext: UsageContext): void {
425487
while (isRightSideOfQualifiedNameOrPropertyAccess(node)) {
426488
node = <Expression>node.parent;
@@ -455,6 +517,13 @@ namespace ts.codefix {
455517
case SyntaxKind.ElementAccessExpression:
456518
inferTypeFromPropertyElementExpressionContext(<ElementAccessExpression>node.parent, node, checker, usageContext);
457519
break;
520+
case SyntaxKind.PropertyAssignment:
521+
case SyntaxKind.ShorthandPropertyAssignment:
522+
inferTypeFromPropertyAssignment(<PropertyAssignment | ShorthandPropertyAssignment>node.parent, checker, usageContext);
523+
break;
524+
case SyntaxKind.PropertyDeclaration:
525+
inferTypeFromPropertyDeclaration(<PropertyDeclaration>node.parent, checker, usageContext);
526+
break;
458527
case SyntaxKind.VariableDeclaration: {
459528
const { name, initializer } = node.parent as VariableDeclaration;
460529
if (node === name) {
@@ -647,6 +716,21 @@ namespace ts.codefix {
647716
}
648717
}
649718

719+
function inferTypeFromPropertyAssignment(assignment: PropertyAssignment | ShorthandPropertyAssignment, checker: TypeChecker, usageContext: UsageContext) {
720+
const objectLiteral = isShorthandPropertyAssignment(assignment) ?
721+
assignment.parent :
722+
assignment.parent.parent;
723+
const nodeWithRealType = isVariableDeclaration(objectLiteral.parent) ?
724+
objectLiteral.parent :
725+
objectLiteral;
726+
727+
addCandidateThisType(usageContext, checker.getTypeAtLocation(nodeWithRealType));
728+
}
729+
730+
function inferTypeFromPropertyDeclaration(declaration: PropertyDeclaration, checker: TypeChecker, usageContext: UsageContext) {
731+
addCandidateThisType(usageContext, checker.getTypeAtLocation(declaration.parent));
732+
}
733+
650734
interface Priority {
651735
high: (t: Type) => boolean;
652736
low: (t: Type) => boolean;
@@ -841,6 +925,12 @@ namespace ts.codefix {
841925
}
842926
}
843927

928+
function addCandidateThisType(context: UsageContext, type: Type | undefined) {
929+
if (type && !(type.flags & TypeFlags.Any) && !(type.flags & TypeFlags.Never)) {
930+
(context.candidateThisTypes || (context.candidateThisTypes = [])).push(type);
931+
}
932+
}
933+
844934
function hasCallContext(usageContext: UsageContext | undefined): boolean {
845935
return !!usageContext && !!usageContext.callContexts;
846936
}

‎src/services/textChanges.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,12 @@ namespace ts.textChanges {
222222

223223
export type TypeAnnotatable = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertyDeclaration | PropertySignature;
224224

225+
export type ThisTypeAnnotatable = FunctionDeclaration | FunctionExpression;
226+
227+
export function isThisTypeAnnotatable(containingFunction: FunctionLike): containingFunction is ThisTypeAnnotatable {
228+
return isFunctionExpression(containingFunction) || isFunctionDeclaration(containingFunction);
229+
}
230+
225231
export class ChangeTracker {
226232
private readonly changes: Change[] = [];
227233
private readonly newFiles: { readonly oldFile: SourceFile | undefined, readonly fileName: string, readonly statements: ReadonlyArray<Statement> }[] = [];
@@ -393,6 +399,13 @@ namespace ts.textChanges {
393399
this.insertNodeAt(sourceFile, endNode.end, type, { prefix: ": " });
394400
}
395401

402+
public tryInsertThisTypeAnnotation(sourceFile: SourceFile, node: ThisTypeAnnotatable, type: TypeNode): void {
403+
const start = findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile)!.getStart(sourceFile) + 1;
404+
const suffix = node.parameters.length ? ", " : "";
405+
406+
this.insertNodeAt(sourceFile, start, type, { prefix: "this: ", suffix });
407+
}
408+
396409
public insertTypeParameters(sourceFile: SourceFile, node: SignatureDeclaration, typeParameters: ReadonlyArray<TypeParameterDeclaration>): void {
397410
// If no `(`, is an arrow function `x => x`, so use the pos of the first parameter
398411
const start = (findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile) || first(node.parameters)).getStart(sourceFile);

‎tests/baselines/reference/blockScopedVariablesUseBeforeDef.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,10 @@ function foo3() {
128128
}
129129
function foo4() {
130130
var y = /** @class */ (function () {
131-
function class_1() {
131+
function y() {
132132
}
133-
class_1.prototype.m = function () { return x; };
134-
return class_1;
133+
y.prototype.m = function () { return x; };
134+
return y;
135135
}());
136136
var x;
137137
}
@@ -156,19 +156,19 @@ function foo7() {
156156
}
157157
function foo8() {
158158
var y = /** @class */ (function () {
159-
function class_2() {
159+
function class_1() {
160160
this.a = x;
161161
}
162-
return class_2;
162+
return class_1;
163163
}());
164164
var x;
165165
}
166166
function foo9() {
167167
var _a;
168168
var y = (_a = /** @class */ (function () {
169-
function class_3() {
169+
function class_2() {
170170
}
171-
return class_3;
171+
return class_2;
172172
}()),
173173
_a.a = x,
174174
_a);
@@ -187,9 +187,9 @@ function foo11() {
187187
function f() {
188188
var _a;
189189
var y = (_a = /** @class */ (function () {
190-
function class_4() {
190+
function class_3() {
191191
}
192-
return class_4;
192+
return class_3;
193193
}()),
194194
_a.a = x,
195195
_a);
@@ -199,10 +199,10 @@ function foo11() {
199199
function foo12() {
200200
function f() {
201201
var y = /** @class */ (function () {
202-
function class_5() {
202+
function class_4() {
203203
this.a = x;
204204
}
205-
return class_5;
205+
return class_4;
206206
}());
207207
}
208208
var x;

‎tests/baselines/reference/classExpression4.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ let x = (new C).foo();
99

1010
//// [classExpression4.js]
1111
var C = /** @class */ (function () {
12-
function class_1() {
12+
function C() {
1313
}
14-
class_1.prototype.foo = function () {
14+
C.prototype.foo = function () {
1515
return new C();
1616
};
17-
return class_1;
17+
return C;
1818
}());
1919
var x = (new C).foo();

‎tests/baselines/reference/classExpressionExtendingAbstractClass.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ var A = /** @class */ (function () {
2828
return A;
2929
}());
3030
var C = /** @class */ (function (_super) {
31-
__extends(class_1, _super);
32-
function class_1() {
31+
__extends(C, _super);
32+
function C() {
3333
return _super !== null && _super.apply(this, arguments) || this;
3434
}
35-
return class_1;
35+
return C;
3636
}(A));

‎tests/baselines/reference/emitClassExpressionInDeclarationFile.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ var __extends = (this && this.__extends) || (function () {
4747
})();
4848
exports.__esModule = true;
4949
exports.simpleExample = /** @class */ (function () {
50-
function class_1() {
50+
function simpleExample() {
5151
}
52-
class_1.getTags = function () { };
53-
class_1.prototype.tags = function () { };
54-
return class_1;
52+
simpleExample.getTags = function () { };
53+
simpleExample.prototype.tags = function () { };
54+
return simpleExample;
5555
}());
5656
exports.circularReference = /** @class */ (function () {
5757
function C() {
@@ -70,13 +70,13 @@ var FooItem = /** @class */ (function () {
7070
exports.FooItem = FooItem;
7171
function WithTags(Base) {
7272
return /** @class */ (function (_super) {
73-
__extends(class_2, _super);
74-
function class_2() {
73+
__extends(class_1, _super);
74+
function class_1() {
7575
return _super !== null && _super.apply(this, arguments) || this;
7676
}
77-
class_2.getTags = function () { };
78-
class_2.prototype.tags = function () { };
79-
return class_2;
77+
class_1.getTags = function () { };
78+
class_1.prototype.tags = function () { };
79+
return class_1;
8080
}(Base));
8181
}
8282
exports.WithTags = WithTags;

0 commit comments

Comments
 (0)