diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 07a90e3772949..96144c02d770e 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5517,6 +5517,30 @@ "category": "Message", "code": 95101 }, + "Add '@class' tag": { + "category": "Message", + "code": 95102 + }, + "Add '@this' tag": { + "category": "Message", + "code": 95103 + }, + "Add 'this' parameter.": { + "category": "Message", + "code": 95104 + }, + "Convert function expression '{0}' to arrow function": { + "category": "Message", + "code": 95105 + }, + "Convert function declaration '{0}' to arrow function": { + "category": "Message", + "code": 95106 + }, + "Fix all implicit-'this' errors": { + "category": "Message", + "code": 95107 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/compiler/factoryPublic.ts b/src/compiler/factoryPublic.ts index 04ec83a622231..8ffc3ee83918e 100644 --- a/src/compiler/factoryPublic.ts +++ b/src/compiler/factoryPublic.ts @@ -2466,6 +2466,12 @@ namespace ts { return tag; } + /* @internal */ + export function createJSDocClassTag(): JSDocClassTag { + return createJSDocTag(SyntaxKind.JSDocClassTag, "class"); + } + + /* @internal */ export function createJSDocComment(comment?: string | undefined, tags?: NodeArray | undefined) { const node = createSynthesizedNode(SyntaxKind.JSDocComment) as JSDoc; diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts new file mode 100644 index 0000000000000..891ebc319c813 --- /dev/null +++ b/src/services/codefixes/fixImplicitThis.ts @@ -0,0 +1,61 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "fixImplicitThis"; + const errorCodes = [Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation.code]; + registerCodeFix({ + errorCodes, + getCodeActions(context) { + const { sourceFile, program, span } = context; + let diagnostic: DiagnosticAndArguments | undefined; + const changes = textChanges.ChangeTracker.with(context, t => { + diagnostic = doChange(t, sourceFile, span.start, program.getTypeChecker()); + }); + return diagnostic ? [createCodeFixAction(fixId, changes, diagnostic, fixId, Diagnostics.Fix_all_implicit_this_errors)] : emptyArray; + }, + fixIds: [fixId], + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => { + doChange(changes, diag.file, diag.start, context.program.getTypeChecker()); + }), + }); + + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, checker: TypeChecker): DiagnosticAndArguments | undefined { + const token = getTokenAtPosition(sourceFile, pos); + Debug.assert(token.kind === SyntaxKind.ThisKeyword); + + const fn = getThisContainer(token, /*includeArrowFunctions*/ false); + if (!isFunctionDeclaration(fn) && !isFunctionExpression(fn)) return undefined; + + if (!isSourceFile(getThisContainer(fn, /*includeArrowFunctions*/ false))) { // 'this' is defined outside, convert to arrow function + const fnKeyword = Debug.assertDefined(findChildOfKind(fn, SyntaxKind.FunctionKeyword, sourceFile)); + const { name } = fn; + const body = Debug.assertDefined(fn.body); // Should be defined because the function contained a 'this' expression + if (isFunctionExpression(fn)) { + if (name && FindAllReferences.Core.isSymbolReferencedInFile(name, checker, sourceFile, body)) { + // Function expression references itself. To fix we would have to extract it to a const. + return undefined; + } + + // `function() {}` --> `() => {}` + changes.delete(sourceFile, fnKeyword); + if (name) { + changes.delete(sourceFile, name); + } + changes.insertText(sourceFile, body.pos, " =>"); + return [Diagnostics.Convert_function_expression_0_to_arrow_function, name ? name.text : ANONYMOUS]; + } + else { + // `function f() {}` => `const f = () => {}` + // `name` should be defined because we only do this in inner contexts, and name is only undefined for `export default function() {}`. + changes.replaceNode(sourceFile, fnKeyword, createToken(SyntaxKind.ConstKeyword)); + changes.insertText(sourceFile, name!.end, " = "); + changes.insertText(sourceFile, body.pos, " =>"); + return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text]; + } + } + // No outer 'this', add a @class tag if in a JS constructor function + else if (isSourceFileJS(sourceFile) && isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent)) { + addJSDocTags(changes, sourceFile, fn, [createJSDocClassTag()]); + return Diagnostics.Add_class_tag; + } + } +} diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index c92ad7365580c..0af71bbbb9ca0 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -351,7 +351,7 @@ namespace ts.codefix { addJSDocTags(changes, sourceFile, signature, paramTags); } - function addJSDocTags(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void { + export function addJSDocTags(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parent: HasJSDoc, newTags: readonly JSDocTag[]): void { const comments = mapDefined(parent.jsDoc, j => j.comment); const oldTags = flatMapToMutable(parent.jsDoc, j => j.tags); const unmergedNewTags = newTags.filter(newTag => !oldTags || !oldTags.some((tag, i) => { diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 4682bd29fe3a3..89a9f69979287 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -1174,16 +1174,16 @@ namespace ts.FindAllReferences { } /** Used as a quick check for whether a symbol is used at all in a file (besides its definition). */ - export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile): boolean { - return eachSymbolReferenceInFile(definition, checker, sourceFile, () => true) || false; + export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, searchContainer: Node = sourceFile): boolean { + return eachSymbolReferenceInFile(definition, checker, sourceFile, () => true, searchContainer) || false; } - export function eachSymbolReferenceInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined { + export function eachSymbolReferenceInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T, searchContainer: Node = sourceFile): T | undefined { const symbol = isParameterPropertyDeclaration(definition.parent, definition.parent.parent) ? first(checker.getSymbolsOfParameterPropertyDeclaration(definition.parent, definition.text)) : checker.getSymbolAtLocation(definition); if (!symbol) return undefined; - for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) { + for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name, searchContainer)) { if (!isIdentifier(token) || token === definition || token.escapedText !== definition.escapedText) continue; const referenceSymbol: Symbol = checker.getSymbolAtLocation(token)!; // See GH#19955 for why the type annotation is necessary if (referenceSymbol === symbol diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 799a44ea525be..30dcaeaebfaf6 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -677,7 +677,7 @@ namespace ts.refactor.extractSymbol { case SyntaxKind.FunctionDeclaration: return scope.name ? `function '${scope.name.text}'` - : "anonymous function"; + : ANONYMOUS; case SyntaxKind.ArrowFunction: return "arrow function"; case SyntaxKind.MethodDeclaration: diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 748fe0ca8e2d4..006bc8031deae 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -338,8 +338,18 @@ namespace ts.textChanges { }); } + public insertFirstParameter(sourceFile: SourceFile, parameters: NodeArray, newParam: ParameterDeclaration): void { + const p0 = firstOrUndefined(parameters); + if (p0) { + this.insertNodeBefore(sourceFile, p0, newParam); + } + else { + this.insertNodeAt(sourceFile, parameters.pos, newParam); + } + } + public insertNodeBefore(sourceFile: SourceFile, before: Node, newNode: Node, blankLineBetween = false): void { - this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}), newNode, this.getOptionsForInsertNodeBefore(before, blankLineBetween)); + this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}), newNode, this.getOptionsForInsertNodeBefore(before, newNode, blankLineBetween)); } public insertModifierBefore(sourceFile: SourceFile, modifier: SyntaxKind, before: Node): void { @@ -426,7 +436,7 @@ namespace ts.textChanges { this.insertNodesAt(sourceFile, start, typeParameters, { prefix: "<", suffix: ">" }); } - private getOptionsForInsertNodeBefore(before: Node, doubleNewlines: boolean): InsertNodeOptions { + private getOptionsForInsertNodeBefore(before: Node, inserted: Node, doubleNewlines: boolean): InsertNodeOptions { if (isStatement(before) || isClassElement(before)) { return { suffix: doubleNewlines ? this.newLineCharacter + this.newLineCharacter : this.newLineCharacter }; } @@ -434,7 +444,7 @@ namespace ts.textChanges { return { suffix: ", " }; } else if (isParameter(before)) { - return {}; + return isParameter(inserted) ? { suffix: ", " } : {}; } else if (isStringLiteral(before) && isImportDeclaration(before.parent) || isNamedImports(before)) { return { suffix: ", " }; @@ -1278,15 +1288,23 @@ namespace ts.textChanges { deleteImportBinding(changes, sourceFile, node as NamespaceImport); break; + case SyntaxKind.SemicolonToken: + deleteNode(changes, sourceFile, node, { trailingTriviaOption: TrailingTriviaOption.Exclude }); + break; + + case SyntaxKind.FunctionKeyword: + deleteNode(changes, sourceFile, node, { leadingTriviaOption: LeadingTriviaOption.Exclude }); + break; + default: if (isImportClause(node.parent) && node.parent.name === node) { deleteDefaultImport(changes, sourceFile, node.parent); } - else if (isCallLikeExpression(node.parent)) { + else if (isCallExpression(node.parent) && contains(node.parent.arguments, node)) { deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } else { - deleteNode(changes, sourceFile, node, node.kind === SyntaxKind.SemicolonToken ? { trailingTriviaOption: TrailingTriviaOption.Exclude } : undefined); + deleteNode(changes, sourceFile, node); } } } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index e135463bd5268..f2dc06e4597a8 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -55,7 +55,6 @@ "codefixes/addMissingInvocationForDecorator.ts", "codefixes/addNameToNamelessParameter.ts", "codefixes/annotateWithTypeFromJSDoc.ts", - "codefixes/inferFromUsage.ts", "codefixes/convertFunctionToEs6Class.ts", "codefixes/convertToAsyncFunction.ts", "codefixes/convertToEs6Module.ts", @@ -63,6 +62,7 @@ "codefixes/convertToTypeOnlyExport.ts", "codefixes/fixClassIncorrectlyImplementsInterface.ts", "codefixes/importFixes.ts", + "codefixes/fixImplicitThis.ts", "codefixes/fixSpelling.ts", "codefixes/fixAddMissingMember.ts", "codefixes/fixAddMissingNewOperator.ts", @@ -82,6 +82,7 @@ "codefixes/fixJSDocTypes.ts", "codefixes/fixMissingCallParentheses.ts", "codefixes/fixAwaitInSyncFunction.ts", + "codefixes/inferFromUsage.ts", "codefixes/disableJsDiagnostics.ts", "codefixes/helpers.ts", "codefixes/fixInvalidImportSyntax.ts", diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 7f675b2209fc0..c7539dd96de7d 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2396,6 +2396,8 @@ namespace ts { return checker.getTypeAtLocation(caseClause.parent.parent.expression); } + export const ANONYMOUS = "anonymous function"; + export function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, program: Program, host: LanguageServiceHost): TypeNode | undefined { const checker = program.getTypeChecker(); let typeIsAccessible = true; diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_all.ts b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts new file mode 100644 index 0000000000000..f99518afa1366 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts @@ -0,0 +1,36 @@ +/// + +// @allowJs: true +// @checkJs: true +// @noImplicitThis: true + +// @Filename: /a.js +////function f() { +//// this.x = 1; +////} +////class C { +//// m() { +//// function h() { +//// this; +//// } +//// } +////} + +verify.codeFixAll({ + fixId: "fixImplicitThis", + fixAllDescription: "Fix all implicit-'this' errors", + newFileContent: +`/** + * @class + */ +function f() { + this.x = 1; +} +class C { + m() { + const h = () => { + this; + } + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts b/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts new file mode 100644 index 0000000000000..52e7930b481d7 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts @@ -0,0 +1,22 @@ +/// + +// @allowJs: true +// @checkJs: true +// @noImplicitThis: true + +// @Filename: /a.js +////function f() { +//// this.x = 1; +////} + +verify.codeFix({ + description: "Add '@class' tag", + index: 0, + newFileContent: +`/** + * @class + */ +function f() { + this.x = 1; +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_classTag_addToComment.ts b/tests/cases/fourslash/codeFixImplicitThis_js_classTag_addToComment.ts new file mode 100644 index 0000000000000..9b684512568b3 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_classTag_addToComment.ts @@ -0,0 +1,24 @@ +/// + +// @allowJs: true +// @checkJs: true +// @noImplicitThis: true + +// @Filename: /a.js +/////** Doc */ +////function f() { +//// this.x = 1; +////} + +verify.codeFix({ + description: "Add '@class' tag", + index: 0, + newFileContent: +`/** + * Doc + * @class + */ +function f() { + this.x = 1; +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts new file mode 100644 index 0000000000000..768cf6b987672 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts @@ -0,0 +1,30 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// function f() { +//// this; +//// } +//// const g = function() { +//// this; +//// }; +//// } +////} + +verify.codeFixAll({ + fixId: "fixImplicitThis", + fixAllDescription: "Fix all implicit-'this' errors", + newFileContent: +`class C { + m() { + const f = () => { + this; + } + const g = () => { + this; + }; + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_cantFixNonFunction.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_cantFixNonFunction.ts new file mode 100644 index 0000000000000..127a0b8f98cda --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_cantFixNonFunction.ts @@ -0,0 +1,7 @@ +/// + +// @noImplicitThis: true + +////this; + +verify.codeFixAvailable([]); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionDeclaration.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionDeclaration.ts new file mode 100644 index 0000000000000..0a7c761f82b76 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionDeclaration.ts @@ -0,0 +1,26 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// function f() { +//// this; +//// f(); // self-reference OK +//// } +//// } +////} + +verify.codeFix({ + description: "Convert function declaration 'f' to arrow function", + index: 0, + newFileContent: +`class C { + m() { + const f = () => { + this; + f(); // self-reference OK + } + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression.ts new file mode 100644 index 0000000000000..3a4b9782879cb --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression.ts @@ -0,0 +1,24 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// return function f() { +//// return this; +//// }; +//// } +////} + +verify.codeFix({ + description: "Convert function expression 'f' to arrow function", + index: 0, + newFileContent: +`class C { + m() { + return () => { + return this; + }; + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts new file mode 100644 index 0000000000000..c3bb18f5fb157 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts @@ -0,0 +1,24 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// const f = function() { +//// this; +//// }; +//// } +////} + +verify.codeFix({ + description: "Convert function expression 'anonymous function' to arrow function", + index: 0, + newFileContent: +`class C { + m() { + const f = () => { + this; + }; + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts new file mode 100644 index 0000000000000..f99c4f5fa2294 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts @@ -0,0 +1,17 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// return function g() { +//// this; +//// g(); +//// }; +//// } +////} + +// note that no implicitThis fix is available, only a inferFromUsage one: +verify.codeFixAvailable([{ + description: "Infer 'this' type of 'g' from usage", +}]);