From 920bd5ebf766f41057fd6cbb4138020c17244d69 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 8 Oct 2018 15:49:54 -0700 Subject: [PATCH 1/2] Use host to improve SymbolTracker implementation --- src/services/codefixes/inferFromUsage.ts | 50 ++++++++++++++---------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index ed8939079e3ec..28c35c2f21eb9 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -25,24 +25,24 @@ namespace ts.codefix { registerCodeFix({ errorCodes, getCodeActions(context) { - const { sourceFile, program, span: { start }, errorCode, cancellationToken } = context; + const { sourceFile, program, span: { start }, errorCode, cancellationToken, host } = context; if (isSourceFileJS(sourceFile)) { return undefined; // TODO: GH#20113 } const token = getTokenAtPosition(sourceFile, start); let declaration!: Declaration | undefined; - const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeenseen*/ returnTrue); }); + const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeen*/ returnTrue, host); }); const name = declaration && getNameOfDeclaration(declaration); return !name || changes.length === 0 ? undefined : [createCodeFixAction(fixId, changes, [getDiagnostic(errorCode, token), name.getText(sourceFile)], fixId, Diagnostics.Infer_all_types_from_usage)]; }, fixIds: [fixId], getAllCodeActions(context) { - const { sourceFile, program, cancellationToken } = context; + const { sourceFile, program, cancellationToken, host } = context; const markSeen = nodeSeenTracker(); return codeFixAll(context, errorCodes, (changes, err) => { - doChange(changes, sourceFile, getTokenAtPosition(err.file, err.start), err.code, program, cancellationToken, markSeen); + doChange(changes, sourceFile, getTokenAtPosition(err.file, err.start), err.code, program, cancellationToken, markSeen, host); }); }, }); @@ -58,7 +58,7 @@ namespace ts.codefix { } } - function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, markSeen: NodeSeenTracker): Declaration | undefined { + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, markSeen: NodeSeenTracker, host: LanguageServiceHost): Declaration | undefined { if (!isParameterPropertyModifier(token.kind) && token.kind !== SyntaxKind.Identifier && token.kind !== SyntaxKind.DotDotDotToken) { return undefined; } @@ -69,7 +69,7 @@ namespace ts.codefix { 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: if ((isVariableDeclaration(parent) && markSeen(parent)) || isPropertyDeclaration(parent) || isPropertySignature(parent)) { // handle bad location - annotateVariableDeclaration(changes, sourceFile, parent, program, cancellationToken); + annotateVariableDeclaration(changes, sourceFile, parent, program, host, cancellationToken); return parent; } return undefined; @@ -77,7 +77,7 @@ namespace ts.codefix { case Diagnostics.Variable_0_implicitly_has_an_1_type.code: { const symbol = program.getTypeChecker().getSymbolAtLocation(token); if (symbol && symbol.valueDeclaration && isVariableDeclaration(symbol.valueDeclaration) && markSeen(symbol.valueDeclaration)) { - annotateVariableDeclaration(changes, sourceFile, symbol.valueDeclaration, program, cancellationToken); + annotateVariableDeclaration(changes, sourceFile, symbol.valueDeclaration, program, host, cancellationToken); return symbol.valueDeclaration; } return undefined; @@ -93,14 +93,14 @@ namespace ts.codefix { // Parameter declarations case Diagnostics.Parameter_0_implicitly_has_an_1_type.code: if (isSetAccessor(containingFunction)) { - annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken); + annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken); return containingFunction; } // falls through case Diagnostics.Rest_parameter_0_implicitly_has_an_any_type.code: if (markSeen(containingFunction)) { const param = cast(parent, isParameter); - annotateParameters(changes, param, containingFunction, sourceFile, program, cancellationToken); + annotateParameters(changes, param, containingFunction, sourceFile, program, host, cancellationToken); return param; } return undefined; @@ -109,7 +109,7 @@ namespace ts.codefix { 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: if (isGetAccessor(containingFunction) && isIdentifier(containingFunction.name)) { - annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program); + annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program, host); return containingFunction; } return undefined; @@ -117,7 +117,7 @@ namespace ts.codefix { // Set Accessor declarations case Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation.code: if (isSetAccessor(containingFunction)) { - annotateSetAccessor(changes, sourceFile, containingFunction, program, cancellationToken); + annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken); return containingFunction; } return undefined; @@ -127,9 +127,9 @@ namespace ts.codefix { } } - function annotateVariableDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: VariableDeclaration | PropertyDeclaration | PropertySignature, program: Program, cancellationToken: CancellationToken): void { + function annotateVariableDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: VariableDeclaration | PropertyDeclaration | PropertySignature, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void { if (isIdentifier(declaration.name)) { - annotate(changes, sourceFile, declaration, inferTypeForVariableFromUsage(declaration.name, program, cancellationToken), program); + annotate(changes, sourceFile, declaration, inferTypeForVariableFromUsage(declaration.name, program, cancellationToken), program, host); } } @@ -145,7 +145,7 @@ namespace ts.codefix { return false; } - function annotateParameters(changes: textChanges.ChangeTracker, parameterDeclaration: ParameterDeclaration, containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): void { + function annotateParameters(changes: textChanges.ChangeTracker, parameterDeclaration: ParameterDeclaration, containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void { if (!isIdentifier(parameterDeclaration.name) || !isApplicableFunctionForInference(containingFunction)) { return; } @@ -159,26 +159,27 @@ namespace ts.codefix { zipWith(containingFunction.parameters, types, (parameter, type) => { if (!parameter.type && !parameter.initializer) { - annotate(changes, sourceFile, parameter, type, program); + annotate(changes, sourceFile, parameter, type, program, host); } }); } - function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, cancellationToken: CancellationToken): void { + function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, host: LanguageServiceHost, 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); + annotate(changes, sourceFile, param, type, program, host); } } - function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type | undefined, program: Program): void { - const typeNode = type && getTypeNodeIfAccessible(type, declaration, program.getTypeChecker()); + function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type | undefined, program: Program, host: LanguageServiceHost): void { + const typeNode = type && getTypeNodeIfAccessible(type, declaration, program, host); if (typeNode) changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode); } - function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, checker: TypeChecker): TypeNode | undefined { + function getTypeNodeIfAccessible(type: Type, enclosingScope: Node, program: Program, host: LanguageServiceHost): TypeNode | undefined { + const checker = program.getTypeChecker(); let typeIsAccessible = true; const notAccessible = () => { typeIsAccessible = false; }; const res = checker.typeToTypeNode(type, enclosingScope, /*flags*/ undefined, { @@ -189,6 +190,15 @@ namespace ts.codefix { reportInaccessibleThisError: notAccessible, reportPrivateInBaseOfClassExpression: notAccessible, reportInaccessibleUniqueSymbolError: notAccessible, + moduleResolverHost: { + directoryExists: host.directoryExists, + fileExists: host.fileExists, + getCommonSourceDirectory: program.getCommonSourceDirectory, + getCurrentDirectory: program.getCurrentDirectory, + getSourceFiles: program.getSourceFiles, + readFile: host.readFile, + useCaseSensitiveFileNames: host.useCaseSensitiveFileNames, + } }); return typeIsAccessible ? res : undefined; } From fb01832773a3459a11d3b05d5950e7f2c7a4567f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 8 Oct 2018 15:58:19 -0700 Subject: [PATCH 2/2] inferFromUsage: Provide a better moduleResolverHost This produces better paths on import types. --- src/services/codefixes/inferFromUsage.ts | 9 ++++----- .../codeFixInferFromUsageSetterWithInaccessibleType.ts | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 28c35c2f21eb9..a8ec514c549a1 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -191,13 +191,12 @@ namespace ts.codefix { reportPrivateInBaseOfClassExpression: notAccessible, reportInaccessibleUniqueSymbolError: notAccessible, moduleResolverHost: { - directoryExists: host.directoryExists, + readFile: host.readFile, fileExists: host.fileExists, - getCommonSourceDirectory: program.getCommonSourceDirectory, - getCurrentDirectory: program.getCurrentDirectory, + directoryExists: host.directoryExists, getSourceFiles: program.getSourceFiles, - readFile: host.readFile, - useCaseSensitiveFileNames: host.useCaseSensitiveFileNames, + getCurrentDirectory: program.getCurrentDirectory, + getCommonSourceDirectory: program.getCommonSourceDirectory, } }); return typeIsAccessible ? res : undefined; diff --git a/tests/cases/fourslash/codeFixInferFromUsageSetterWithInaccessibleType.ts b/tests/cases/fourslash/codeFixInferFromUsageSetterWithInaccessibleType.ts index b56c6e05f6954..269f67f7cf7de 100644 --- a/tests/cases/fourslash/codeFixInferFromUsageSetterWithInaccessibleType.ts +++ b/tests/cases/fourslash/codeFixInferFromUsageSetterWithInaccessibleType.ts @@ -18,7 +18,7 @@ verify.codeFix({ description: "Infer type of 'x' from usage", newFileContent: `export class C { - set x(val: Promise) { val; } + set x(val: Promise) { val; } method() { this.x = import("./a"); } }`, });