Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4687,5 +4687,29 @@
"Generate types for all packages without types": {
"category": "Message",
"code": 95068
},
"Add '@class' tag": {
"category": "Message",
"code": 95069
},
"Add '@this' tag": {
"category": "Message",
"code": 95070
},
"Add 'this' parameter.": {
"category": "Message",
"code": 95071
},
"Convert function expression '{0}' to arrow function": {
"category": "Message",
"code": 95072
},
"Convert function declaration '{0}' to arrow function": {
"category": "Message",
"code": 95073
},
"Fix all implicit-'this' errors": {
"category": "Message",
"code": 95074
}
}
26 changes: 26 additions & 0 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,32 @@ namespace ts {
return node;
}

/* @internal */
export function createJSDocTypeExpression(type: TypeNode): JSDocTypeExpression {
const node = createSynthesizedNode(SyntaxKind.JSDocTypeExpression) as JSDocTypeExpression;
node.type = type;
return node;
}

/* @internal */
export function createJSDocThisTag(typeExpression: JSDocTypeExpression | TypeNode | undefined): JSDocThisTag {
const node = createJsDocTag<JSDocThisTag>(SyntaxKind.JSDocThisTag, "this");
node.typeExpression = typeExpression && (isJSDocTypeExpression(typeExpression) ? typeExpression : createJSDocTypeExpression(typeExpression));
return node;
}

/* @internal */
export function createJSDocClassTag(): JSDocClassTag {
return createJsDocTag<JSDocClassTag>(SyntaxKind.JSDocClassTag, "class");
}

function createJsDocTag<T extends JSDocTag>(kind: T["kind"], tagName: string): T {
const node = createSynthesizedNode(kind) as T;
node.atToken = createToken(SyntaxKind.AtToken);
node.tagName = createIdentifier(tagName);
return node;
}

export function updateFunctionDeclaration(
node: FunctionDeclaration,
decorators: ReadonlyArray<Decorator> | undefined,
Expand Down
68 changes: 68 additions & 0 deletions src/services/codefixes/fixImplicitThis.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/* @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);

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Author

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.

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 (fn.name && FindAllReferences.Core.isSymbolReferencedInFile(fn.name, checker, sourceFile, body)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of confusing to refer to the same property as both name and fn.name in this function.

// 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, " =>");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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>"];

@amcasey Andrew Casey (amcasey) Oct 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a constant for "<anonymous>" somewhere? It seems unlikely that this is the first time we've had to name an anonymous symbol.

}
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];
}
}
else { // No outer 'this', must add an annotation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or a parameter?

if (isSourceFileJS(sourceFile)) {
const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent);
changes.insertJsdocCommentBefore(sourceFile, fn,
addClassTag ? createJSDocClassTag() : createJSDocThisTag(createKeywordTypeNode(SyntaxKind.AnyKeyword)));
return addClassTag ? Diagnostics.Add_class_tag : Diagnostics.Add_this_tag;
}
else {
changes.insertNodeAt(sourceFile, fn.parameters.pos, makeParameter("this", createKeywordTypeNode(SyntaxKind.AnyKeyword)));
return Diagnostics.Add_this_parameter;
}
}
}
}
2 changes: 1 addition & 1 deletion src/services/codefixes/generateTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ namespace ts {
];
return { parameters, returnType: hasReturn ? anyType() : createKeywordTypeNode(SyntaxKind.VoidKeyword) };
}
function makeParameter(name: string, type: TypeNode): ParameterDeclaration {
export function makeParameter(name: string, type: TypeNode): ParameterDeclaration {
return createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, name, /*questionToken*/ undefined, type);
}
function makeRestParameter(): ParameterDeclaration {
Expand Down
8 changes: 4 additions & 4 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -826,14 +826,14 @@ namespace ts.FindAllReferences.Core {
}

/** 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<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined {
export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T, searchContainer: Node = sourceFile): T | undefined {
const symbol = 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
Expand Down
73 changes: 58 additions & 15 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,22 @@ namespace ts.textChanges {
this.insertText(sourceFile, token.getStart(sourceFile), text);
}

public insertJsdocCommentBefore(sourceFile: SourceFile, fn: FunctionDeclaration | FunctionExpression, tag: JSDocTag): void {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work right for function expressions immediately preceded by text.

const existingJsdoc = fn.jsDoc && firstOrUndefined(fn.jsDoc);
if (existingJsdoc) {
// `/** foo */` --> `/**\n * @constructor\n * foo */`
const jsdocStart = existingJsdoc.getStart(sourceFile);
const indent = getIndent(sourceFile, jsdocStart);
const indentAsterisk = `${this.newLineCharacter}${indent} *`;
this.insertNodeAt(sourceFile, jsdocStart + "/**".length, tag, { prefix: `${indentAsterisk} `, suffix: indentAsterisk });
}
else {
const fnStart = fn.getStart(sourceFile);
const indent = getIndent(sourceFile, fnStart);
this.insertNodeAt(sourceFile, fnStart, tag, { prefix: "/** ", suffix: ` */${this.newLineCharacter}${indent}` });
}
}

public replaceRangeWithText(sourceFile: SourceFile, range: TextRange, text: string) {
this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
}
Expand Down Expand Up @@ -720,6 +736,11 @@ namespace ts.textChanges {
}
}

function getIndent(sourceFile: SourceFile, position: number): string {
const lineStart = getStartPositionOfLine(getLineAndCharacterOfPosition(sourceFile, position).line, sourceFile);
return sourceFile.text.slice(lineStart, position);
}

// find first non-whitespace position in the leading trivia of the node
function startPositionToDeleteNodeInList(sourceFile: SourceFile, node: Node): number {
return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, {}, Position.FullStart), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true);
Expand Down Expand Up @@ -788,21 +809,35 @@ namespace ts.textChanges {
}

/** Note: this may mutate `nodeIn`. */
function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
if (validate) validate(node, text);
const { options: formatOptions } = formatContext;
const initialIndentation =
indentation !== undefined
? indentation
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
if (delta === undefined) {
delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
export function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this used now that there is an export here?

// Emitter doesn't handle JSDoc, so generate that here.
if (isJSDocTag(nodeIn)) {
switch (nodeIn.kind) {
case SyntaxKind.JSDocClassTag:
return "@class";
case SyntaxKind.JSDocThisTag:
const { typeExpression } = nodeIn as JSDocThisTag;
return typeExpression ? `@this {${getNonformattedText(typeExpression.type, sourceFile, newLineCharacter).text}}` : "@this";
default:
return Debug.fail(); // TODO (if this is needed)
}
}
else {
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
if (validate) validate(node, text);
const { options: formatOptions } = formatContext;
const initialIndentation =
indentation !== undefined
? indentation
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
if (delta === undefined) {
delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
}

const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
return applyChanges(text, changes);
const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
return applyChanges(text, changes);
}
}

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

case SyntaxKind.SemicolonToken:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be miscounting the scopes, but why would deleteDeclaration be called on a semicolon?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convertToEs6Module deletes semicolons when changing an assignment statement to a function declaration.

deleteNode(changes, sourceFile, node, { useNonAdjustedEndPosition: true });
break;

case SyntaxKind.FunctionKeyword:
deleteNode(changes, sourceFile, node, { useNonAdjustedStartPosition: true });
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 ? { useNonAdjustedEndPosition: true } : undefined);
deleteNode(changes, sourceFile, node);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"codefixes/correctQualifiedNameToIndexedAccessType.ts",
"codefixes/fixClassIncorrectlyImplementsInterface.ts",
"codefixes/importFixes.ts",
"codefixes/fixImplicitThis.ts",
"codefixes/fixSpelling.ts",
"codefixes/fixAddMissingMember.ts",
"codefixes/fixCannotFindModule.ts",
Expand Down
41 changes: 41 additions & 0 deletions tests/cases/fourslash/codeFixImplicitThis_js_all.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/// <reference path='fourslash.ts' />

// @allowJs: true
// @checkJs: true
// @noImplicitThis: true

// @Filename: /a.js
////function f() {
//// this.x = 1;
////}
////function g() {
//// this;
////}
////class C {
//// m() {
//// function h() {
//// this;
//// }
//// }
////}

verify.codeFixAll({
fixId: "fixImplicitThis",
fixAllDescription: "Fix all implicit-'this' errors",
newFileContent:
`/** @class */
function f() {
this.x = 1;
}
/** @this {any} */
function g() {
this;
}
class C {
m() {
const h = () => {
this;
}
}
}`,
});
20 changes: 20 additions & 0 deletions tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <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,23 @@
/// <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:
`/**
* @class
* Doc */
function f() {
this.x = 1;
}`,
});
20 changes: 20 additions & 0 deletions tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />

// @allowJs: true
// @checkJs: true
// @noImplicitThis: true

// @Filename: /a.js
////function f() {
//// this;
////}

verify.codeFix({
description: "Add '@this' tag",
index: 0,
newFileContent:
`/** @this {any} */
function f() {
this;
}`,
});
Loading