From cb30622001876d518a8405f1b7a4696315abb7a1 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 4 Sep 2018 10:10:28 -0700 Subject: [PATCH 1/8] Add codefix for --noImplicitThis --- src/compiler/diagnosticMessages.json | 24 ++++++ src/compiler/factory.ts | 26 +++++++ src/services/codefixes/fixImplicitThis.ts | 68 +++++++++++++++++ src/services/codefixes/generateTypes.ts | 2 +- src/services/findAllReferences.ts | 8 +- src/services/textChanges.ts | 73 +++++++++++++++---- src/services/tsconfig.json | 1 + .../fourslash/codeFixImplicitThis_js_all.ts | 41 +++++++++++ .../codeFixImplicitThis_js_classTag.ts | 20 +++++ ...ixImplicitThis_js_classTag_addToComment.ts | 23 ++++++ .../codeFixImplicitThis_js_typeTag.ts | 20 +++++ .../fourslash/codeFixImplicitThis_ts_all.ts | 36 +++++++++ ...deFixImplicitThis_ts_cantFixNonFunction.ts | 7 ++ ...eFixImplicitThis_ts_functionDeclaration.ts | 26 +++++++ ...deFixImplicitThis_ts_functionExpression.ts | 24 ++++++ ...plicitThis_ts_functionExpression_noName.ts | 24 ++++++ ...s_ts_functionExpression_selfReferencing.ts | 14 ++++ .../codeFixImplicitThis_ts_thisParameter.ts | 16 ++++ 18 files changed, 433 insertions(+), 20 deletions(-) create mode 100644 src/services/codefixes/fixImplicitThis.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_js_all.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_js_classTag_addToComment.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_all.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_cantFixNonFunction.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_functionDeclaration.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 7baeeb7b65ddd..7222e2eb46550 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -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 } } diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 4b2855790a985..73fcebaf60b2d 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -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(SyntaxKind.JSDocThisTag, "this"); + node.typeExpression = typeExpression && (isJSDocTypeExpression(typeExpression) ? typeExpression : createJSDocTypeExpression(typeExpression)); + return node; + } + + /* @internal */ + export function createJSDocClassTag(): JSDocClassTag { + return createJsDocTag(SyntaxKind.JSDocClassTag, "class"); + } + + function createJsDocTag(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 | undefined, diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts new file mode 100644 index 0000000000000..40d50aee50adb --- /dev/null +++ b/src/services/codefixes/fixImplicitThis.ts @@ -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); + 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)) { + // 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 : ""]; + } + 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 + 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; + } + } + } +} diff --git a/src/services/codefixes/generateTypes.ts b/src/services/codefixes/generateTypes.ts index 6af72da562d44..b533aa6018f03 100644 --- a/src/services/codefixes/generateTypes.ts +++ b/src/services/codefixes/generateTypes.ts @@ -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 { diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 9fde734049c3f..fca187e7b3511 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -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(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 = 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/textChanges.ts b/src/services/textChanges.ts index abd153f0bacd7..b33eed9ac54a4 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -339,6 +339,22 @@ namespace ts.textChanges { this.insertText(sourceFile, token.getStart(sourceFile), text); } + public insertJsdocCommentBefore(sourceFile: SourceFile, fn: FunctionDeclaration | FunctionExpression, tag: JSDocTag): void { + 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 }); } @@ -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); @@ -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 { + // 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. */ @@ -1103,15 +1138,23 @@ namespace ts.textChanges { deleteImportBinding(changes, sourceFile, node as NamespaceImport); break; + case SyntaxKind.SemicolonToken: + 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); } } } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 6f718b43730f0..d2359f0c6c555 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -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", diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_all.ts b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts new file mode 100644 index 0000000000000..b44b2bb94aad4 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts @@ -0,0 +1,41 @@ +/// + +// @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; + } + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts b/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts new file mode 100644 index 0000000000000..e0a168abeebe2 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_classTag.ts @@ -0,0 +1,20 @@ +/// + +// @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..73ee28898b86e --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_classTag_addToComment.ts @@ -0,0 +1,23 @@ +/// + +// @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; +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts b/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts new file mode 100644 index 0000000000000..ae49a6a1ee95e --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts @@ -0,0 +1,20 @@ +/// + +// @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; +}`, +}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts new file mode 100644 index 0000000000000..c2d5bff7441cf --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts @@ -0,0 +1,36 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// function f() { +//// this; +//// } +//// const g = function() { +//// this; +//// }; +//// } +////} +////function h() { +//// this; +////} + +verify.codeFixAll({ + fixId: "fixImplicitThis", + fixAllDescription: "Fix all implicit-'this' errors", + newFileContent: +`class C { + m() { + const f = () => { + this; + } + const g = () => { + this; + }; + } +} +function h(this: any) { + 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..97e062f7b1a04 --- /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 '' 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..52ad0f4e981f3 --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts @@ -0,0 +1,14 @@ +/// + +// @noImplicitThis: true + +////class C { +//// m() { +//// return function g() { +//// this; +//// g(); +//// }; +//// } +////} + +verify.codeFixAvailable([]); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts new file mode 100644 index 0000000000000..8b29517b95ded --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts @@ -0,0 +1,16 @@ +/// + +// @noImplicitThis: true + +////function f() { +//// this; +////} + +verify.codeFix({ + description: "Add 'this' parameter.", + index: 0, + newFileContent: +`function f(this: any) { + this; +}`, +}); From e74d5eed16ded7fedefc1d6f2ac117bdf43972ff Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 13 Nov 2018 11:37:24 -0800 Subject: [PATCH 2/8] Code review --- src/services/codefixes/fixImplicitThis.ts | 8 ++++---- src/services/codefixes/generateTypes.ts | 1 + src/services/refactors/extractSymbol.ts | 2 +- src/services/textChanges.ts | 18 ++++++++++++++---- src/services/utilities.ts | 2 ++ ...mplicitThis_ts_functionExpression_noName.ts | 2 +- .../codeFixImplicitThis_ts_thisParameter2.ts | 16 ++++++++++++++++ 7 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts index 7bd23c93bb8d1..67ca3eb718d6a 100644 --- a/src/services/codefixes/fixImplicitThis.ts +++ b/src/services/codefixes/fixImplicitThis.ts @@ -30,7 +30,7 @@ namespace ts.codefix { 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)) { + 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; } @@ -41,7 +41,7 @@ namespace ts.codefix { changes.delete(sourceFile, name); } changes.insertText(sourceFile, body.pos, " =>"); - return [Diagnostics.Convert_function_expression_0_to_arrow_function, name ? name.text : ""]; + return [Diagnostics.Convert_function_expression_0_to_arrow_function, name ? name.text : ANONYMOUS]; } else { // `function f() {}` => `const f = () => {}` @@ -52,14 +52,14 @@ namespace ts.codefix { return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text]; } } - else { // No outer 'this', must add an annotation + else { // No outer 'this', must add a jsdoc tag / parameter if (isSourceFileJS(sourceFile)) { const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent); addJSDocTags(changes, 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))); + changes.insertFirstParameter(sourceFile, fn.parameters, makeParameter("this", createKeywordTypeNode(SyntaxKind.AnyKeyword))); return Diagnostics.Add_this_parameter; } } diff --git a/src/services/codefixes/generateTypes.ts b/src/services/codefixes/generateTypes.ts index dfa67a22a51eb..9dc4677818b3d 100644 --- a/src/services/codefixes/generateTypes.ts +++ b/src/services/codefixes/generateTypes.ts @@ -191,6 +191,7 @@ namespace ts { ]; return { parameters, returnType: hasReturn ? anyType() : createKeywordTypeNode(SyntaxKind.VoidKeyword) }; } + /* @internal */ export function makeParameter(name: string, type: TypeNode): ParameterDeclaration { return createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, name, /*questionToken*/ undefined, type); } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index c1915a66431df..f9a6fa39956dd 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -673,7 +673,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 e1f65b74be7de..02a3b8b2fb116 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -310,8 +310,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) { - this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}, Position.Start), newNode, this.getOptionsForInsertNodeBefore(before, blankLineBetween)); + this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}, Position.Start), newNode, this.getOptionsForInsertNodeBefore(before, newNode, blankLineBetween)); } public insertModifierBefore(sourceFile: SourceFile, modifier: SyntaxKind, before: Node): void { @@ -390,7 +400,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 }; } @@ -398,7 +408,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: ", " }; @@ -807,7 +817,7 @@ namespace ts.textChanges { } /** Note: this may mutate `nodeIn`. */ - export function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string { + function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string { // Emitter doesn't handle JSDoc, so generate that here. if (isJSDocTag(nodeIn)) { switch (nodeIn.kind) { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index e95e144d41ae0..69a62def63e58 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1908,4 +1908,6 @@ namespace ts { export function getSwitchedType(caseClause: CaseClause, checker: TypeChecker): Type | undefined { return checker.getTypeAtLocation(caseClause.parent.parent.expression); } + + export const ANONYMOUS = "anonymous function"; } diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts index 97e062f7b1a04..c3bb18f5fb157 100644 --- a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_noName.ts @@ -11,7 +11,7 @@ ////} verify.codeFix({ - description: "Convert function expression '' to arrow function", + description: "Convert function expression 'anonymous function' to arrow function", index: 0, newFileContent: `class C { diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts new file mode 100644 index 0000000000000..fe170e447eb4b --- /dev/null +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts @@ -0,0 +1,16 @@ +/// + +// @noImplicitThis: true + +////function f(x: number) { +//// this; +////} + +verify.codeFix({ + description: "Add 'this' parameter.", + index: 0, + newFileContent: +`function f(this: any, x: number) { + this; +}`, +}); From 665ffc0f47f316cee57acd6996ccfd9dc83dea35 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 16 Mar 2020 10:56:25 -0700 Subject: [PATCH 3/8] Back to building post-merge --- src/services/codefixes/fixImplicitThis.ts | 4 ++-- src/services/textChanges.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts index 67ca3eb718d6a..6c9da99bad32e 100644 --- a/src/services/codefixes/fixImplicitThis.ts +++ b/src/services/codefixes/fixImplicitThis.ts @@ -55,11 +55,11 @@ namespace ts.codefix { else { // No outer 'this', must add a jsdoc tag / parameter if (isSourceFileJS(sourceFile)) { const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent); - addJSDocTags(changes, sourceFile, fn, [addClassTag ? createJSDocClassTag() : createJSDocThisTag(createKeywordTypeNode(SyntaxKind.AnyKeyword))]); + addJSDocTags(changes, sourceFile, fn, [addClassTag ? createJSDocClassTag() : createJSDocThisTag(createJSDocTypeExpression(createKeywordTypeNode(SyntaxKind.AnyKeyword)))]); return addClassTag ? Diagnostics.Add_class_tag : Diagnostics.Add_this_tag; } else { - changes.insertFirstParameter(sourceFile, fn.parameters, makeParameter("this", createKeywordTypeNode(SyntaxKind.AnyKeyword))); + changes.insertFirstParameter(sourceFile, fn.parameters, createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, "this", /*questionToken*/ undefined, createKeywordTypeNode(SyntaxKind.AnyKeyword))); return Diagnostics.Add_this_parameter; } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 559b51d7bd6dc..3412bbaa4cdd4 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -1303,11 +1303,11 @@ namespace ts.textChanges { break; case SyntaxKind.SemicolonToken: - deleteNode(changes, sourceFile, node, { useNonAdjustedEndPosition: true }); + deleteNode(changes, sourceFile, node, { trailingTriviaOption: TrailingTriviaOption.Exclude }); break; case SyntaxKind.FunctionKeyword: - deleteNode(changes, sourceFile, node, { useNonAdjustedStartPosition: true }); + deleteNode(changes, sourceFile, node, { leadingTriviaOption: LeadingTriviaOption.Exclude }); break; default: From dc29946f1064f2a0d00a3aff4f0de8a7f2dc8e94 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 17 Mar 2020 08:33:39 -0700 Subject: [PATCH 4/8] Remove redundant functions + update tests Infer-from-usage also inserts `this: any` parameters when needed, so I removed that from fixImplicitThis. Otherwise, fixImplicitThis has better suggestions than inferFromUsage, so I moved inferFromUsage later in the suggestion order. --- src/services/codefixes/fixImplicitThis.ts | 6 ++---- src/services/tsconfig.json | 2 +- .../fourslash/codeFixImplicitThis_ts_all.ts | 6 ------ ...This_ts_functionExpression_selfReferencing.ts | 5 ++++- .../codeFixImplicitThis_ts_thisParameter.ts | 16 ---------------- .../codeFixImplicitThis_ts_thisParameter2.ts | 16 ---------------- ...romFunctionThisUsageJsDocExistingDocsClass.ts | 2 +- ...nferFromFunctionThisUsageJsDocNewDocsClass.ts | 2 +- ...mFunctionThisUsageJsDocNewDocsInaccessible.ts | 2 +- ...erFromFunctionThisUsageJsDocNewDocsLiteral.ts | 2 +- 10 files changed, 11 insertions(+), 48 deletions(-) delete mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts delete mode 100644 tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts index 6c9da99bad32e..0ec55315c37f6 100644 --- a/src/services/codefixes/fixImplicitThis.ts +++ b/src/services/codefixes/fixImplicitThis.ts @@ -25,6 +25,7 @@ namespace ts.codefix { const fn = getThisContainer(token, /*includeArrowFunctions*/ false); if (!isFunctionDeclaration(fn) && !isFunctionExpression(fn)) return undefined; + // TODO: Flip this conditional 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; @@ -52,16 +53,13 @@ namespace ts.codefix { return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text]; } } + // TODO: inline this conditional after the above is flipped else { // No outer 'this', must add a jsdoc tag / parameter if (isSourceFileJS(sourceFile)) { const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent); addJSDocTags(changes, sourceFile, fn, [addClassTag ? createJSDocClassTag() : createJSDocThisTag(createJSDocTypeExpression(createKeywordTypeNode(SyntaxKind.AnyKeyword)))]); return addClassTag ? Diagnostics.Add_class_tag : Diagnostics.Add_this_tag; } - else { - changes.insertFirstParameter(sourceFile, fn.parameters, createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, "this", /*questionToken*/ undefined, createKeywordTypeNode(SyntaxKind.AnyKeyword))); - return Diagnostics.Add_this_parameter; - } } } } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 0aae0867c24a1..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", @@ -83,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/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts index c2d5bff7441cf..768cf6b987672 100644 --- a/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_all.ts @@ -12,9 +12,6 @@ //// }; //// } ////} -////function h() { -//// this; -////} verify.codeFixAll({ fixId: "fixImplicitThis", @@ -29,8 +26,5 @@ verify.codeFixAll({ this; }; } -} -function h(this: any) { - this; }`, }); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts index 52ad0f4e981f3..f99c4f5fa2294 100644 --- a/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts +++ b/tests/cases/fourslash/codeFixImplicitThis_ts_functionExpression_selfReferencing.ts @@ -11,4 +11,7 @@ //// } ////} -verify.codeFixAvailable([]); +// note that no implicitThis fix is available, only a inferFromUsage one: +verify.codeFixAvailable([{ + description: "Infer 'this' type of 'g' from usage", +}]); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts deleted file mode 100644 index 8b29517b95ded..0000000000000 --- a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter.ts +++ /dev/null @@ -1,16 +0,0 @@ -/// - -// @noImplicitThis: true - -////function f() { -//// this; -////} - -verify.codeFix({ - description: "Add 'this' parameter.", - index: 0, - newFileContent: -`function f(this: any) { - this; -}`, -}); diff --git a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts b/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts deleted file mode 100644 index fe170e447eb4b..0000000000000 --- a/tests/cases/fourslash/codeFixImplicitThis_ts_thisParameter2.ts +++ /dev/null @@ -1,16 +0,0 @@ -/// - -// @noImplicitThis: true - -////function f(x: number) { -//// this; -////} - -verify.codeFix({ - description: "Add 'this' parameter.", - index: 0, - newFileContent: -`function f(this: any, x: number) { - this; -}`, -}); diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts index 20dfb17ea378f..fbb043cf08f1f 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts @@ -21,7 +21,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 0, + index: 1, newFileContent: `/** * @returns {string} * @this {Container} diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts index 0ea75ec0e8c77..8b2977fbcb198 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts @@ -18,7 +18,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 0, + index: 1, newFileContent: `/** * @this {Container} */ diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts index 14dc7fb4ae7b1..dd8b27d3b4bb5 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts @@ -27,7 +27,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 0, + index: 1, newFileContent: `/** * @this {any} */ diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts index 4a39ddbf09f3a..be14a27ea1174 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts @@ -18,7 +18,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 0, + index: 1, newFileContent: `/** * @this {{ member: string; returnThisMember: () => any; }} */ From 8074c74202a37c4ef30e5cd79e5d39d58775f6ca Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 17 Mar 2020 08:42:27 -0700 Subject: [PATCH 5/8] More redundancy removal Don't need to add `@this` anymore either since inferFromUsage will do that. --- src/services/codefixes/fixImplicitThis.ts | 12 ++++------ .../fourslash/codeFixImplicitThis_js_all.ts | 9 -------- .../codeFixImplicitThis_js_typeTag.ts | 22 ------------------- 3 files changed, 4 insertions(+), 39 deletions(-) delete mode 100644 tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts diff --git a/src/services/codefixes/fixImplicitThis.ts b/src/services/codefixes/fixImplicitThis.ts index 0ec55315c37f6..891ebc319c813 100644 --- a/src/services/codefixes/fixImplicitThis.ts +++ b/src/services/codefixes/fixImplicitThis.ts @@ -25,7 +25,6 @@ namespace ts.codefix { const fn = getThisContainer(token, /*includeArrowFunctions*/ false); if (!isFunctionDeclaration(fn) && !isFunctionExpression(fn)) return undefined; - // TODO: Flip this conditional 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; @@ -53,13 +52,10 @@ namespace ts.codefix { return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text]; } } - // TODO: inline this conditional after the above is flipped - else { // No outer 'this', must add a jsdoc tag / parameter - if (isSourceFileJS(sourceFile)) { - const addClassTag = isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent); - addJSDocTags(changes, sourceFile, fn, [addClassTag ? createJSDocClassTag() : createJSDocThisTag(createJSDocTypeExpression(createKeywordTypeNode(SyntaxKind.AnyKeyword)))]); - return addClassTag ? Diagnostics.Add_class_tag : Diagnostics.Add_this_tag; - } + // 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/tests/cases/fourslash/codeFixImplicitThis_js_all.ts b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts index 0d27c90781569..f99518afa1366 100644 --- a/tests/cases/fourslash/codeFixImplicitThis_js_all.ts +++ b/tests/cases/fourslash/codeFixImplicitThis_js_all.ts @@ -8,9 +8,6 @@ ////function f() { //// this.x = 1; ////} -////function g() { -//// this; -////} ////class C { //// m() { //// function h() { @@ -29,12 +26,6 @@ verify.codeFixAll({ function f() { this.x = 1; } -/** - * @this {any} - */ -function g() { - this; -} class C { m() { const h = () => { diff --git a/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts b/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts deleted file mode 100644 index 13f9d34aa3bce..0000000000000 --- a/tests/cases/fourslash/codeFixImplicitThis_js_typeTag.ts +++ /dev/null @@ -1,22 +0,0 @@ -/// - -// @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; -}`, -}); From d0b95057b21ea78f979ede705d39b230fceb2d83 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 17 Mar 2020 09:00:55 -0700 Subject: [PATCH 6/8] More baseline updates From moving inferFromUsage down in priority I think? --- .../codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts | 2 +- .../codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts | 2 +- ...codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts | 2 +- .../codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts index fbb043cf08f1f..20dfb17ea378f 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocExistingDocsClass.ts @@ -21,7 +21,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 1, + index: 0, newFileContent: `/** * @returns {string} * @this {Container} diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts index 8b2977fbcb198..0ea75ec0e8c77 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsClass.ts @@ -18,7 +18,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 1, + index: 0, newFileContent: `/** * @this {Container} */ diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts index dd8b27d3b4bb5..14dc7fb4ae7b1 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsInaccessible.ts @@ -27,7 +27,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 1, + index: 0, newFileContent: `/** * @this {any} */ diff --git a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts index be14a27ea1174..4a39ddbf09f3a 100644 --- a/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts +++ b/tests/cases/fourslash/codeFixInferFromFunctionThisUsageJsDocNewDocsLiteral.ts @@ -18,7 +18,7 @@ verify.codeFix({ description: "Infer 'this' type of 'returnThisMember' from usage", - index: 1, + index: 0, newFileContent: `/** * @this {{ member: string; returnThisMember: () => any; }} */ From 1a347bf88477d5db56a0783989884e13c3cb0a98 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 17 Mar 2020 11:28:30 -0700 Subject: [PATCH 7/8] remove now-redundant ad-hoc jsdoc emit --- src/services/textChanges.ts | 38 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 3412bbaa4cdd4..ab54ffa6d2ba6 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -915,34 +915,20 @@ 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 { - // 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) - } + const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter); + if (validate) validate(node, text); + const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile); + 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; } - else { - const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter); - if (validate) validate(node, text); - const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile); - 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, options: formatOptions }); - 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, options: formatOptions }); + return applyChanges(text, changes); } /** Note: output node may be mutated input node. */ From 7fb1bd2ee6e64b4af84a69c9e0b0c0022d7d456b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 17 Mar 2020 11:40:48 -0700 Subject: [PATCH 8/8] fix more bad merge --- src/services/textChanges.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index ab54ffa6d2ba6..006bc8031deae 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -920,10 +920,10 @@ namespace ts.textChanges { const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile); const initialIndentation = indentation !== undefined - ? indentation - : formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos); + ? 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; + delta = formatting.SmartIndenter.shouldIndentChildNode(formatOptions, nodeIn) ? (formatOptions.indentSize || 0) : 0; } const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };