Skip to content

Commit cb30622

Browse files
author
Andy Hanson
committed
Add codefix for --noImplicitThis
1 parent b185784 commit cb30622

18 files changed

+433
-20
lines changed

src/compiler/diagnosticMessages.json

+24
Original file line numberDiff line numberDiff line change
@@ -4687,5 +4687,29 @@
46874687
"Generate types for all packages without types": {
46884688
"category": "Message",
46894689
"code": 95068
4690+
},
4691+
"Add '@class' tag": {
4692+
"category": "Message",
4693+
"code": 95069
4694+
},
4695+
"Add '@this' tag": {
4696+
"category": "Message",
4697+
"code": 95070
4698+
},
4699+
"Add 'this' parameter.": {
4700+
"category": "Message",
4701+
"code": 95071
4702+
},
4703+
"Convert function expression '{0}' to arrow function": {
4704+
"category": "Message",
4705+
"code": 95072
4706+
},
4707+
"Convert function declaration '{0}' to arrow function": {
4708+
"category": "Message",
4709+
"code": 95073
4710+
},
4711+
"Fix all implicit-'this' errors": {
4712+
"category": "Message",
4713+
"code": 95074
46904714
}
46914715
}

src/compiler/factory.ts

+26
Original file line numberDiff line numberDiff line change
@@ -1788,6 +1788,32 @@ namespace ts {
17881788
return node;
17891789
}
17901790

1791+
/* @internal */
1792+
export function createJSDocTypeExpression(type: TypeNode): JSDocTypeExpression {
1793+
const node = createSynthesizedNode(SyntaxKind.JSDocTypeExpression) as JSDocTypeExpression;
1794+
node.type = type;
1795+
return node;
1796+
}
1797+
1798+
/* @internal */
1799+
export function createJSDocThisTag(typeExpression: JSDocTypeExpression | TypeNode | undefined): JSDocThisTag {
1800+
const node = createJsDocTag<JSDocThisTag>(SyntaxKind.JSDocThisTag, "this");
1801+
node.typeExpression = typeExpression && (isJSDocTypeExpression(typeExpression) ? typeExpression : createJSDocTypeExpression(typeExpression));
1802+
return node;
1803+
}
1804+
1805+
/* @internal */
1806+
export function createJSDocClassTag(): JSDocClassTag {
1807+
return createJsDocTag<JSDocClassTag>(SyntaxKind.JSDocClassTag, "class");
1808+
}
1809+
1810+
function createJsDocTag<T extends JSDocTag>(kind: T["kind"], tagName: string): T {
1811+
const node = createSynthesizedNode(kind) as T;
1812+
node.atToken = createToken(SyntaxKind.AtToken);
1813+
node.tagName = createIdentifier(tagName);
1814+
return node;
1815+
}
1816+
17911817
export function updateFunctionDeclaration(
17921818
node: FunctionDeclaration,
17931819
decorators: ReadonlyArray<Decorator> | undefined,
+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/* @internal */
2+
namespace ts.codefix {
3+
const fixId = "fixImplicitThis";
4+
const errorCodes = [Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation.code];
5+
registerCodeFix({
6+
errorCodes,
7+
getCodeActions(context) {
8+
const { sourceFile, program, span } = context;
9+
let diagnostic: DiagnosticAndArguments | undefined;
10+
const changes = textChanges.ChangeTracker.with(context, t => {
11+
diagnostic = doChange(t, sourceFile, span.start, program.getTypeChecker());
12+
});
13+
return diagnostic ? [createCodeFixAction(fixId, changes, diagnostic, fixId, Diagnostics.Fix_all_implicit_this_errors)] : emptyArray;
14+
},
15+
fixIds: [fixId],
16+
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
17+
doChange(changes, diag.file, diag.start, context.program.getTypeChecker());
18+
}),
19+
});
20+
21+
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, checker: TypeChecker): DiagnosticAndArguments | undefined {
22+
const token = getTokenAtPosition(sourceFile, pos);
23+
Debug.assert(token.kind === SyntaxKind.ThisKeyword);
24+
25+
const fn = getThisContainer(token, /*includeArrowFunctions*/ false);
26+
if (!isFunctionDeclaration(fn) && !isFunctionExpression(fn)) return undefined;
27+
28+
if (!isSourceFile(getThisContainer(fn, /*includeArrowFunctions*/ false))) { // 'this' is defined outside, convert to arrow function
29+
const fnKeyword = Debug.assertDefined(findChildOfKind(fn, SyntaxKind.FunctionKeyword, sourceFile));
30+
const { name } = fn;
31+
const body = Debug.assertDefined(fn.body); // Should be defined because the function contained a 'this' expression
32+
if (isFunctionExpression(fn)) {
33+
if (fn.name && FindAllReferences.Core.isSymbolReferencedInFile(fn.name, checker, sourceFile, body)) {
34+
// Function expression references itself. To fix we would have to extract it to a const.
35+
return undefined;
36+
}
37+
38+
// `function() {}` --> `() => {}`
39+
changes.delete(sourceFile, fnKeyword);
40+
if (name) {
41+
changes.delete(sourceFile, name);
42+
}
43+
changes.insertText(sourceFile, body.pos, " =>");
44+
return [Diagnostics.Convert_function_expression_0_to_arrow_function, name ? name.text : "<anonymous>"];
45+
}
46+
else {
47+
// `function f() {}` => `const f = () => {}`
48+
// `name` should be defined because we only do this in inner contexts, and name is only undefined for `export default function() {}`.
49+
changes.replaceNode(sourceFile, fnKeyword, createToken(SyntaxKind.ConstKeyword));
50+
changes.insertText(sourceFile, name!.end, " = ");
51+
changes.insertText(sourceFile, body.pos, " =>");
52+
return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text];
53+
}
54+
}
55+
else { // No outer 'this', must add an annotation
56+
if (isSourceFileJS(sourceFile)) {
57+
const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent);
58+
changes.insertJsdocCommentBefore(sourceFile, fn,
59+
addClassTag ? createJSDocClassTag() : createJSDocThisTag(createKeywordTypeNode(SyntaxKind.AnyKeyword)));
60+
return addClassTag ? Diagnostics.Add_class_tag : Diagnostics.Add_this_tag;
61+
}
62+
else {
63+
changes.insertNodeAt(sourceFile, fn.parameters.pos, makeParameter("this", createKeywordTypeNode(SyntaxKind.AnyKeyword)));
64+
return Diagnostics.Add_this_parameter;
65+
}
66+
}
67+
}
68+
}

src/services/codefixes/generateTypes.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ namespace ts {
167167
];
168168
return { parameters, returnType: hasReturn ? anyType() : createKeywordTypeNode(SyntaxKind.VoidKeyword) };
169169
}
170-
function makeParameter(name: string, type: TypeNode): ParameterDeclaration {
170+
export function makeParameter(name: string, type: TypeNode): ParameterDeclaration {
171171
return createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, name, /*questionToken*/ undefined, type);
172172
}
173173
function makeRestParameter(): ParameterDeclaration {

src/services/findAllReferences.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -826,14 +826,14 @@ namespace ts.FindAllReferences.Core {
826826
}
827827

828828
/** Used as a quick check for whether a symbol is used at all in a file (besides its definition). */
829-
export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile): boolean {
830-
return eachSymbolReferenceInFile(definition, checker, sourceFile, () => true) || false;
829+
export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, searchContainer: Node = sourceFile): boolean {
830+
return eachSymbolReferenceInFile(definition, checker, sourceFile, () => true, searchContainer) || false;
831831
}
832832

833-
export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined {
833+
export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T, searchContainer: Node = sourceFile): T | undefined {
834834
const symbol = checker.getSymbolAtLocation(definition);
835835
if (!symbol) return undefined;
836-
for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) {
836+
for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name, searchContainer)) {
837837
if (!isIdentifier(token) || token === definition || token.escapedText !== definition.escapedText) continue;
838838
const referenceSymbol: Symbol = checker.getSymbolAtLocation(token)!; // See GH#19955 for why the type annotation is necessary
839839
if (referenceSymbol === symbol

src/services/textChanges.ts

+58-15
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,22 @@ namespace ts.textChanges {
339339
this.insertText(sourceFile, token.getStart(sourceFile), text);
340340
}
341341

342+
public insertJsdocCommentBefore(sourceFile: SourceFile, fn: FunctionDeclaration | FunctionExpression, tag: JSDocTag): void {
343+
const existingJsdoc = fn.jsDoc && firstOrUndefined(fn.jsDoc);
344+
if (existingJsdoc) {
345+
// `/** foo */` --> `/**\n * @constructor\n * foo */`
346+
const jsdocStart = existingJsdoc.getStart(sourceFile);
347+
const indent = getIndent(sourceFile, jsdocStart);
348+
const indentAsterisk = `${this.newLineCharacter}${indent} *`;
349+
this.insertNodeAt(sourceFile, jsdocStart + "/**".length, tag, { prefix: `${indentAsterisk} `, suffix: indentAsterisk });
350+
}
351+
else {
352+
const fnStart = fn.getStart(sourceFile);
353+
const indent = getIndent(sourceFile, fnStart);
354+
this.insertNodeAt(sourceFile, fnStart, tag, { prefix: "/** ", suffix: ` */${this.newLineCharacter}${indent}` });
355+
}
356+
}
357+
342358
public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string) {
343359
this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
344360
}
@@ -720,6 +736,11 @@ namespace ts.textChanges {
720736
}
721737
}
722738

739+
function getIndent(sourceFile: SourceFile, position: number): string {
740+
const lineStart = getStartPositionOfLine(getLineAndCharacterOfPosition(sourceFile, position).line, sourceFile);
741+
return sourceFile.text.slice(lineStart, position);
742+
}
743+
723744
// find first non-whitespace position in the leading trivia of the node
724745
function startPositionToDeleteNodeInList(sourceFile: SourceFile, node: Node): number {
725746
return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, {}, Position.FullStart), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true);
@@ -788,21 +809,35 @@ namespace ts.textChanges {
788809
}
789810

790811
/** Note: this may mutate `nodeIn`. */
791-
function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
792-
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
793-
if (validate) validate(node, text);
794-
const { options: formatOptions } = formatContext;
795-
const initialIndentation =
796-
indentation !== undefined
797-
? indentation
798-
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
799-
if (delta === undefined) {
800-
delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
812+
export function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
813+
// Emitter doesn't handle JSDoc, so generate that here.
814+
if (isJSDocTag(nodeIn)) {
815+
switch (nodeIn.kind) {
816+
case SyntaxKind.JSDocClassTag:
817+
return "@class";
818+
case SyntaxKind.JSDocThisTag:
819+
const { typeExpression } = nodeIn as JSDocThisTag;
820+
return typeExpression ? `@this {${getNonformattedText(typeExpression.type, sourceFile, newLineCharacter).text}}` : "@this";
821+
default:
822+
return Debug.fail(); // TODO (if this is needed)
823+
}
801824
}
825+
else {
826+
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
827+
if (validate) validate(node, text);
828+
const { options: formatOptions } = formatContext;
829+
const initialIndentation =
830+
indentation !== undefined
831+
? indentation
832+
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
833+
if (delta === undefined) {
834+
delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
835+
}
802836

803-
const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
804-
const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
805-
return applyChanges(text, changes);
837+
const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
838+
const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
839+
return applyChanges(text, changes);
840+
}
806841
}
807842

808843
/** Note: output node may be mutated input node. */
@@ -1103,15 +1138,23 @@ namespace ts.textChanges {
11031138
deleteImportBinding(changes, sourceFile, node as NamespaceImport);
11041139
break;
11051140

1141+
case SyntaxKind.SemicolonToken:
1142+
deleteNode(changes, sourceFile, node, { useNonAdjustedEndPosition: true });
1143+
break;
1144+
1145+
case SyntaxKind.FunctionKeyword:
1146+
deleteNode(changes, sourceFile, node, { useNonAdjustedStartPosition: true });
1147+
break;
1148+
11061149
default:
11071150
if (isImportClause(node.parent) && node.parent.name === node) {
11081151
deleteDefaultImport(changes, sourceFile, node.parent);
11091152
}
1110-
else if (isCallLikeExpression(node.parent)) {
1153+
else if (isCallExpression(node.parent) && contains(node.parent.arguments, node)) {
11111154
deleteNodeInList(changes, deletedNodesInLists, sourceFile, node);
11121155
}
11131156
else {
1114-
deleteNode(changes, sourceFile, node, node.kind === SyntaxKind.SemicolonToken ? { useNonAdjustedEndPosition: true } : undefined);
1157+
deleteNode(changes, sourceFile, node);
11151158
}
11161159
}
11171160
}

src/services/tsconfig.json

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
"codefixes/correctQualifiedNameToIndexedAccessType.ts",
5252
"codefixes/fixClassIncorrectlyImplementsInterface.ts",
5353
"codefixes/importFixes.ts",
54+
"codefixes/fixImplicitThis.ts",
5455
"codefixes/fixSpelling.ts",
5556
"codefixes/fixAddMissingMember.ts",
5657
"codefixes/fixCannotFindModule.ts",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @noImplicitThis: true
6+
7+
// @Filename: /a.js
8+
////function f() {
9+
//// this.x = 1;
10+
////}
11+
////function g() {
12+
//// this;
13+
////}
14+
////class C {
15+
//// m() {
16+
//// function h() {
17+
//// this;
18+
//// }
19+
//// }
20+
////}
21+
22+
verify.codeFixAll({
23+
fixId: "fixImplicitThis",
24+
fixAllDescription: "Fix all implicit-'this' errors",
25+
newFileContent:
26+
`/** @class */
27+
function f() {
28+
this.x = 1;
29+
}
30+
/** @this {any} */
31+
function g() {
32+
this;
33+
}
34+
class C {
35+
m() {
36+
const h = () => {
37+
this;
38+
}
39+
}
40+
}`,
41+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @noImplicitThis: true
6+
7+
// @Filename: /a.js
8+
////function f() {
9+
//// this.x = 1;
10+
////}
11+
12+
verify.codeFix({
13+
description: "Add '@class' tag",
14+
index: 0,
15+
newFileContent:
16+
`/** @class */
17+
function f() {
18+
this.x = 1;
19+
}`,
20+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @noImplicitThis: true
6+
7+
// @Filename: /a.js
8+
/////** Doc */
9+
////function f() {
10+
//// this.x = 1;
11+
////}
12+
13+
verify.codeFix({
14+
description: "Add '@class' tag",
15+
index: 0,
16+
newFileContent:
17+
`/**
18+
* @class
19+
* Doc */
20+
function f() {
21+
this.x = 1;
22+
}`,
23+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @noImplicitThis: true
6+
7+
// @Filename: /a.js
8+
////function f() {
9+
//// this;
10+
////}
11+
12+
verify.codeFix({
13+
description: "Add '@this' tag",
14+
index: 0,
15+
newFileContent:
16+
`/** @this {any} */
17+
function f() {
18+
this;
19+
}`,
20+
});

0 commit comments

Comments
 (0)