Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 13.7k
Add codefix for --noImplicitThis#27565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
cb306228ae463fe74d5ee87f791f665ffc0dc299468074c74d0b95051a347bf7fb1bd2File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, " =>"); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't there a code fix or refactoring for converting a function to an error function? Can/should we share code? Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There will be in #28250, we can combine these then. | ||
| 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; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -338,8 +338,18 @@ namespace ts.textChanges { | ||
| }); | ||
| } | ||
| public insertFirstParameter(sourceFile: SourceFile, parameters: NodeArray<ParameterDeclaration>, 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,15 +436,15 @@ 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 }; | ||
| } | ||
| else if (isVariableDeclaration(before)) { // insert `x = 1, ` into `const x = 1, y = 2; | ||
| 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: | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could be miscounting the scopes, but why would Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| 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); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @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; | ||
| } | ||
| } | ||
| }`, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @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; | ||
| }`, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @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; | ||
| }`, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @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; | ||
| }; | ||
| } | ||
| }`, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @noImplicitThis: true | ||
| ////this; | ||
| verify.codeFixAvailable([]); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
| // @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 | ||
| } | ||
| } | ||
| }`, | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we be in a default argument of the function? Does that matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a parameter initializer, 'this' refers to the same 'this' as inside the body of the function, not to 'this' of an outer function. So this will work just as well in that case.