diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7d6e58ad9a4a4..2b2f29eedaff2 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2793,6 +2793,7 @@ namespace ts { /** Note that the resulting nodes cannot be checked. */ typeToTypeNode(type: Type, enclosingDeclaration?: Node, flags?: NodeBuilderFlags): TypeNode; + /* @internal */ typeToTypeNode(type: Type, enclosingDeclaration?: Node, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode; // tslint:disable-line unified-signatures /** Note that the resulting nodes cannot be checked. */ signatureToSignatureDeclaration(signature: Signature, kind: SyntaxKind, enclosingDeclaration?: Node, flags?: NodeBuilderFlags): SignatureDeclaration & {typeArguments?: NodeArray} | undefined; /** Note that the resulting nodes cannot be checked. */ diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 18df488e979a7..74ad2113dccc7 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -24,27 +24,26 @@ namespace ts.codefix { ]; registerCodeFix({ errorCodes, - getCodeActions({ sourceFile, program, span: { start }, errorCode, cancellationToken }) { + getCodeActions(context) { + const { sourceFile, program, span: { start }, errorCode, cancellationToken } = context; if (isSourceFileJavaScript(sourceFile)) { return undefined; // TODO: GH#20113 } const token = getTokenAtPosition(sourceFile, start, /*includeJsDocComment*/ false); - const fix = getFix(sourceFile, token, errorCode, program, cancellationToken); - if (!fix) return undefined; - - const { declaration, textChanges } = fix; - const name = getNameOfDeclaration(declaration); - const description = formatStringFromArgs(getLocaleSpecificMessage(getDiagnostic(errorCode, token)), [name.getText()]); - return [{ description, changes: [{ fileName: sourceFile.fileName, textChanges }], fixId }]; + let declaration!: Declaration; + const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken); }); + if (changes.length === 0) return undefined; + const name = getNameOfDeclaration(declaration).getText(); + const description = formatStringFromArgs(getLocaleSpecificMessage(getDiagnostic(errorCode, token)), [name]); + return [{ description, changes, fixId }]; }, fixIds: [fixId], getAllCodeActions(context) { const { sourceFile, program, cancellationToken } = context; const seenFunctions = createMap(); - return codeFixAllWithTextChanges(context, errorCodes, (changes, err) => { - const fix = getFix(sourceFile, getTokenAtPosition(err.file!, err.start!, /*includeJsDocComment*/ false), err.code, program, cancellationToken, seenFunctions); - if (fix) changes.push(...fix.textChanges); + return codeFixAll(context, errorCodes, (changes, err) => { + doChange(changes, sourceFile, getTokenAtPosition(err.file!, err.start!, /*includeJsDocComment*/ false), err.code, program, cancellationToken, seenFunctions); }); }, }); @@ -60,12 +59,7 @@ namespace ts.codefix { } } - interface Fix { - readonly declaration: Declaration; - readonly textChanges: TextChange[]; - } - - function getFix(sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, seenFunctions?: Map): Fix | undefined { + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, seenFunctions?: Map): Declaration | undefined { if (!isAllowedTokenKind(token.kind)) { return undefined; } @@ -74,11 +68,15 @@ namespace ts.codefix { // Variable and Property declarations case Diagnostics.Member_0_implicitly_has_an_1_type.code: case Diagnostics.Variable_0_implicitly_has_type_1_in_some_locations_where_its_type_cannot_be_determined.code: - return getCodeActionForVariableDeclaration(token.parent, program, cancellationToken); + annotateVariableDeclaration(changes, sourceFile, token.parent, program, cancellationToken); + return token.parent as Declaration; case Diagnostics.Variable_0_implicitly_has_an_1_type.code: { const symbol = program.getTypeChecker().getSymbolAtLocation(token); - return symbol && symbol.valueDeclaration && getCodeActionForVariableDeclaration(symbol.valueDeclaration, program, cancellationToken); + if (symbol && symbol.valueDeclaration) { + annotateVariableDeclaration(changes, sourceFile, symbol.valueDeclaration, program, cancellationToken); + return symbol.valueDeclaration; + } } } @@ -91,22 +89,34 @@ namespace ts.codefix { // Parameter declarations case Diagnostics.Parameter_0_implicitly_has_an_1_type.code: if (isSetAccessor(containingFunction)) { - return getCodeActionForSetAccessor(containingFunction, program, cancellationToken); + annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken); + return containingFunction; } // falls through case Diagnostics.Rest_parameter_0_implicitly_has_an_any_type.code: - return !seenFunctions || addToSeen(seenFunctions, getNodeId(containingFunction)) - ? getCodeActionForParameters(cast(token.parent, isParameter), containingFunction, sourceFile, program, cancellationToken) - : undefined; + if (!seenFunctions || addToSeen(seenFunctions, getNodeId(containingFunction))) { + const param = cast(token.parent, isParameter); + annotateParameters(changes, param, containingFunction, sourceFile, program, cancellationToken); + return param; + } + return undefined; // Get Accessor declarations case Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation.code: case Diagnostics._0_which_lacks_return_type_annotation_implicitly_has_an_1_return_type.code: - return isGetAccessor(containingFunction) ? getCodeActionForGetAccessor(containingFunction, sourceFile, program, cancellationToken) : undefined; + if (isGetAccessor(containingFunction) && isIdentifier(containingFunction.name)) { + annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program); + return containingFunction; + } + return undefined; // Set Accessor declarations case Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation.code: - return isSetAccessor(containingFunction) ? getCodeActionForSetAccessor(containingFunction, program, cancellationToken) : undefined; + if (isSetAccessor(containingFunction)) { + annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken); + return containingFunction; + } + return undefined; default: return Debug.fail(String(errorCode)); @@ -127,10 +137,10 @@ namespace ts.codefix { } } - function getCodeActionForVariableDeclaration(declaration: VariableDeclaration | PropertyDeclaration | PropertySignature, program: Program, cancellationToken: CancellationToken): Fix | undefined { - if (!isIdentifier(declaration.name)) return undefined; - const type = inferTypeForVariableFromUsage(declaration.name, program, cancellationToken); - return makeFix(declaration, declaration.name.getEnd(), type, program); + function annotateVariableDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: VariableDeclaration | PropertyDeclaration | PropertySignature, program: Program, cancellationToken: CancellationToken): void { + if (isIdentifier(declaration.name)) { + annotate(changes, sourceFile, declaration, inferTypeForVariableFromUsage(declaration.name, program, cancellationToken), program); + } } function isApplicableFunctionForInference(declaration: FunctionLike): declaration is MethodDeclaration | FunctionDeclaration | ConstructorDeclaration { @@ -145,54 +155,51 @@ namespace ts.codefix { return false; } - function getCodeActionForParameters(parameterDeclaration: ParameterDeclaration, containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): Fix | undefined { + function annotateParameters(changes: textChanges.ChangeTracker, parameterDeclaration: ParameterDeclaration, containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): void { if (!isIdentifier(parameterDeclaration.name) || !isApplicableFunctionForInference(containingFunction)) { - return undefined; + return; } const types = inferTypeForParametersFromUsage(containingFunction, sourceFile, program, cancellationToken) || containingFunction.parameters.map(p => isIdentifier(p.name) ? inferTypeForVariableFromUsage(p.name, program, cancellationToken) : undefined); - if (!types) return undefined; - // We didn't actually find a set of type inference positions matching each parameter position - if (containingFunction.parameters.length !== types.length) { - return undefined; + if (!types || containingFunction.parameters.length !== types.length) { + return; } - const textChanges = arrayFrom(mapDefinedIterator(zipToIterator(containingFunction.parameters, types), ([parameter, type]) => - type && !parameter.type && !parameter.initializer ? makeChange(containingFunction, parameter.end, type, program) : undefined)); - return textChanges.length ? { declaration: parameterDeclaration, textChanges } : undefined; - } - - function getCodeActionForSetAccessor(setAccessorDeclaration: SetAccessorDeclaration, program: Program, cancellationToken: CancellationToken): Fix | undefined { - const setAccessorParameter = setAccessorDeclaration.parameters[0]; - if (!setAccessorParameter || !isIdentifier(setAccessorDeclaration.name) || !isIdentifier(setAccessorParameter.name)) { - return undefined; - } - - const type = inferTypeForVariableFromUsage(setAccessorDeclaration.name, program, cancellationToken) || - inferTypeForVariableFromUsage(setAccessorParameter.name, program, cancellationToken); - return makeFix(setAccessorParameter, setAccessorParameter.name.getEnd(), type, program); + zipWith(containingFunction.parameters, types, (parameter, type) => { + if (!parameter.type && !parameter.initializer) { + annotate(changes, sourceFile, parameter, type, program); + } + }); } - function getCodeActionForGetAccessor(getAccessorDeclaration: GetAccessorDeclaration, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): Fix | undefined { - if (!isIdentifier(getAccessorDeclaration.name)) { - return undefined; + function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, cancellationToken: CancellationToken): void { + const param = firstOrUndefined(setAccessorDeclaration.parameters); + if (param && isIdentifier(setAccessorDeclaration.name) && isIdentifier(param.name)) { + const type = inferTypeForVariableFromUsage(setAccessorDeclaration.name, program, cancellationToken) || + inferTypeForVariableFromUsage(param.name, program, cancellationToken); + annotate(changes, sourceFile, param, type, program); } - - const type = inferTypeForVariableFromUsage(getAccessorDeclaration.name, program, cancellationToken); - const closeParenToken = findChildOfKind(getAccessorDeclaration, SyntaxKind.CloseParenToken, sourceFile); - return makeFix(getAccessorDeclaration, closeParenToken.getEnd(), type, program); } - function makeFix(declaration: Declaration, start: number, type: Type | undefined, program: Program): Fix | undefined { - const change = makeChange(declaration, start, type, program); - return change && { declaration, textChanges: [change] }; + function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type | undefined, program: Program): void { + const typeNode = type && getTypeNodeIfAccessible(type, declaration, program.getTypeChecker()); + if (typeNode) changes.insertTypeAnnotation(sourceFile, declaration, typeNode); } - function makeChange(declaration: Declaration, start: number, type: Type | undefined, program: Program): TextChange | undefined { - const typeString = type && typeToString(type, declaration, program.getTypeChecker()); - return typeString === undefined ? undefined : createTextChangeFromStartLength(start, 0, `: ${typeString}`); + function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, checker: TypeChecker): TypeNode | undefined { + let typeIsAccessible = true; + const notAccessible = () => { typeIsAccessible = false; }; + const res = checker.typeToTypeNode(type, enclosingScope, /*flags*/ undefined, { + trackSymbol: (symbol, declaration, meaning) => { + typeIsAccessible = typeIsAccessible && checker.isSymbolAccessible(symbol, declaration, meaning, /*shouldComputeAliasToMarkVisible*/ false).accessibility === SymbolAccessibility.Accessible; + }, + reportInaccessibleThisError: notAccessible, + reportPrivateInBaseOfClassExpression: notAccessible, + reportInaccessibleUniqueSymbolError: notAccessible, + }); + return typeIsAccessible ? res : undefined; } function getReferences(token: PropertyName | Token, program: Program, cancellationToken: CancellationToken): ReadonlyArray { @@ -221,51 +228,6 @@ namespace ts.codefix { } } - function getTypeAccessiblityWriter(checker: TypeChecker): EmitTextWriter { - let str = ""; - let typeIsAccessible = true; - - const writeText: (text: string) => void = text => str += text; - return { - getText: () => typeIsAccessible ? str : undefined, - writeKeyword: writeText, - writeOperator: writeText, - writePunctuation: writeText, - writeSpace: writeText, - writeStringLiteral: writeText, - writeParameter: writeText, - writeProperty: writeText, - writeSymbol: writeText, - write: writeText, - writeTextOfNode: writeText, - rawWrite: writeText, - writeLiteral: writeText, - getTextPos: () => 0, - getLine: () => 0, - getColumn: () => 0, - getIndent: () => 0, - isAtStartOfLine: () => false, - writeLine: () => writeText(" "), - increaseIndent: noop, - decreaseIndent: noop, - clear: () => { str = ""; typeIsAccessible = true; }, - trackSymbol: (symbol, declaration, meaning) => { - if (checker.isSymbolAccessible(symbol, declaration, meaning, /*shouldComputeAliasToMarkVisible*/ false).accessibility !== SymbolAccessibility.Accessible) { - typeIsAccessible = false; - } - }, - reportInaccessibleThisError: () => { typeIsAccessible = false; }, - reportPrivateInBaseOfClassExpression: () => { typeIsAccessible = false; }, - reportInaccessibleUniqueSymbolError: () => { typeIsAccessible = false; } - }; - } - - function typeToString(type: Type, enclosingDeclaration: Declaration, checker: TypeChecker): string { - const writer = getTypeAccessiblityWriter(checker); - checker.writeType(type, enclosingDeclaration, /*flags*/ undefined, writer); - return writer.getText(); - } - namespace InferFromReference { interface CallContext { argumentTypes: Type[]; diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 0e75158c4d1c3..f2a5d6ddb3caf 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -199,6 +199,8 @@ namespace ts.textChanges { formatContext: formatting.FormatContext; } + export type TypeAnnotatable = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertyDeclaration | PropertySignature; + export class ChangeTracker { private readonly changes: Change[] = []; private readonly deletedNodesInLists: true[] = []; // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`. @@ -343,6 +345,14 @@ namespace ts.textChanges { this.replaceRange(sourceFile, { pos, end: pos }, createToken(modifier), { suffix: " " }); } + /** Prefer this over replacing a node with another that has a type annotation, as it avoids reformatting the other parts of the node. */ + public insertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void { + const end = (isFunctionLike(node) + ? findChildOfKind(node, SyntaxKind.CloseParenToken, sourceFile)! + : node.kind !== SyntaxKind.VariableDeclaration && node.questionToken ? node.questionToken : node.name).end; + this.insertNodeAt(sourceFile, end, type, { prefix: ": " }); + } + private getOptionsForInsertNodeBefore(before: Node, doubleNewlines: boolean): ChangeNodeOptions { if (isStatement(before) || isClassElement(before)) { return { suffix: doubleNewlines ? this.newLineCharacter + this.newLineCharacter : this.newLineCharacter }; diff --git a/tests/cases/fourslash/codeFixInferFromUsageInaccessibleTypes.ts b/tests/cases/fourslash/codeFixInferFromUsageInaccessibleTypes.ts index ae9106be8c4f0..e522d8fa62e39 100644 --- a/tests/cases/fourslash/codeFixInferFromUsageInaccessibleTypes.ts +++ b/tests/cases/fourslash/codeFixInferFromUsageInaccessibleTypes.ts @@ -1,15 +1,15 @@ /// // @noImplicitAny: true -////function f1([|a |]) { } +////function f1(a) { } ////function h1() { //// class C { p: number }; //// f1({ ofTypeC: new C() }); ////} //// -////function f2([|a |]) { } +////function f2(a) { } ////function h2() { -//// interface I { a: number } +//// interface I { a: number } //// var i: I = {a : 1}; //// f2(i); //// f2(2); diff --git a/tests/cases/fourslash/typeToStringCrashInCodeFix.ts b/tests/cases/fourslash/typeToStringCrashInCodeFix.ts index 05cf00cb4cec9..261f082440641 100644 --- a/tests/cases/fourslash/typeToStringCrashInCodeFix.ts +++ b/tests/cases/fourslash/typeToStringCrashInCodeFix.ts @@ -3,4 +3,4 @@ // @noImplicitAny: true //// function f(y, z = { p: y[ -verify.getAndApplyCodeFix(); +verify.not.codeFixAvailable();