Skip to content

Allow inferFromUsage to do auto-imports #33915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 14, 2019
Merged

Allow inferFromUsage to do auto-imports #33915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
01617e3
Add test
andrewbranch Oct 4, 2019
9eddf83
Auto-import instead of using ImportTypeNodes
andrewbranch Oct 9, 2019
282961c
Write more tests and fix namespace case
andrewbranch Oct 9, 2019
d789e96
Remove unused enum memmber
andrewbranch Oct 9, 2019
f92fab9
Update API baselines
andrewbranch Oct 10, 2019
a035c2d
Lint
andrewbranch Oct 10, 2019
052a9b2
Style nits and util consolidation
andrewbranch Oct 10, 2019
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
17 changes: 0 additions & 17 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32168,23 +32168,6 @@ namespace ts {
}
}

function getFirstIdentifier(node: EntityNameOrEntityNameExpression): Identifier {
switch (node.kind) {
case SyntaxKind.Identifier:
return node;
case SyntaxKind.QualifiedName:
do {
node = node.left;
} while (node.kind !== SyntaxKind.Identifier);
return node;
case SyntaxKind.PropertyAccessExpression:
do {
node = node.expression;
} while (node.kind !== SyntaxKind.Identifier);
return node;
}
}

function getFirstNonModuleExportsIdentifier(node: EntityNameOrEntityNameExpression): Identifier {
switch (node.kind) {
case SyntaxKind.Identifier:
Expand Down
17 changes: 17 additions & 0 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4179,6 +4179,23 @@ namespace ts {
return node.kind === SyntaxKind.Identifier || isPropertyAccessEntityNameExpression(node);
}

export function getFirstIdentifier(node: EntityNameOrEntityNameExpression): Identifier {
switch (node.kind) {
case SyntaxKind.Identifier:
return node;
case SyntaxKind.QualifiedName:
do {
node = node.left;
} while (node.kind !== SyntaxKind.Identifier);
return node;
case SyntaxKind.PropertyAccessExpression:
do {
node = node.expression;
} while (node.kind !== SyntaxKind.Identifier);
return node;
}
}

export function isDottedName(node: Expression): boolean {
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
Expand Down
96 changes: 77 additions & 19 deletions src/services/codefixes/inferFromUsage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,21 @@ namespace ts.codefix {
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { sourceFile, program, span: { start }, errorCode, cancellationToken, host } = context;
const { sourceFile, program, span: { start }, errorCode, cancellationToken, host, formatContext, preferences } = context;

const token = getTokenAtPosition(sourceFile, start);
let declaration!: Declaration | undefined;
const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeen*/ returnTrue, host); });
const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeen*/ returnTrue, host, formatContext, preferences); });
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, host } = context;
const { sourceFile, program, cancellationToken, host, formatContext, preferences } = context;
const markSeen = nodeSeenTracker();
return codeFixAll(context, errorCodes, (changes, err) => {
doChange(changes, sourceFile, getTokenAtPosition(err.file, err.start), err.code, program, cancellationToken, markSeen, host);
doChange(changes, sourceFile, getTokenAtPosition(err.file, err.start), err.code, program, cancellationToken, markSeen, host, formatContext, preferences);
});
},
});
Expand Down Expand Up @@ -106,7 +106,7 @@ namespace ts.codefix {
return errorCode;
}

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, markSeen: NodeSeenTracker, host: LanguageServiceHost): Declaration | undefined {
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, errorCode: number, program: Program, cancellationToken: CancellationToken, markSeen: NodeSeenTracker, host: LanguageServiceHost, formatContext: formatting.FormatContext, preferences: UserPreferences): Declaration | undefined {
if (!isParameterPropertyModifier(token.kind) && token.kind !== SyntaxKind.Identifier && token.kind !== SyntaxKind.DotDotDotToken && token.kind !== SyntaxKind.ThisKeyword) {
return undefined;
}
Expand All @@ -118,7 +118,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, host, cancellationToken);
annotateVariableDeclaration(changes, sourceFile, parent, program, host, cancellationToken, formatContext, preferences);
return parent;
}
if (isPropertyAccessExpression(parent)) {
Expand All @@ -136,7 +136,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, host, cancellationToken);
annotateVariableDeclaration(changes, sourceFile, symbol.valueDeclaration, program, host, cancellationToken, formatContext, preferences);
return symbol.valueDeclaration;
}
return undefined;
Expand All @@ -152,14 +152,14 @@ namespace ts.codefix {
// Parameter declarations
case Diagnostics.Parameter_0_implicitly_has_an_1_type.code:
if (isSetAccessorDeclaration(containingFunction)) {
annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken);
annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken, formatContext, preferences);
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, sourceFile, param, containingFunction, program, host, cancellationToken);
annotateParameters(changes, sourceFile, param, containingFunction, program, host, cancellationToken, formatContext, preferences);
return param;
}
return undefined;
Expand All @@ -168,15 +168,15 @@ 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 (isGetAccessorDeclaration(containingFunction) && isIdentifier(containingFunction.name)) {
annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program, host);
annotate(changes, sourceFile, containingFunction, inferTypeForVariableFromUsage(containingFunction.name, program, cancellationToken), program, host, formatContext, preferences);
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:
if (isSetAccessorDeclaration(containingFunction)) {
annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken);
annotateSetAccessor(changes, sourceFile, containingFunction, program, host, cancellationToken, formatContext, preferences);
return containingFunction;
}
return undefined;
Expand All @@ -194,13 +194,32 @@ namespace ts.codefix {
}
}

function annotateVariableDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: VariableDeclaration | PropertyDeclaration | PropertySignature, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void {
function annotateVariableDeclaration(
changes: textChanges.ChangeTracker,
sourceFile: SourceFile,
declaration: VariableDeclaration | PropertyDeclaration | PropertySignature,
program: Program,
host: LanguageServiceHost,
cancellationToken: CancellationToken,
formatContext: formatting.FormatContext,
preferences: UserPreferences,
): void {
if (isIdentifier(declaration.name)) {
annotate(changes, sourceFile, declaration, inferTypeForVariableFromUsage(declaration.name, program, cancellationToken), program, host);
annotate(changes, sourceFile, declaration, inferTypeForVariableFromUsage(declaration.name, program, cancellationToken), program, host, formatContext, preferences);
}
}

function annotateParameters(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parameterDeclaration: ParameterDeclaration, containingFunction: FunctionLike, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void {
function annotateParameters(
changes: textChanges.ChangeTracker,
sourceFile: SourceFile,
parameterDeclaration: ParameterDeclaration,
containingFunction: FunctionLike,
program: Program,
host: LanguageServiceHost,
cancellationToken: CancellationToken,
formatContext: formatting.FormatContext,
preferences: UserPreferences,
): void {
if (!isIdentifier(parameterDeclaration.name)) {
return;
}
Expand All @@ -216,7 +235,7 @@ namespace ts.codefix {
if (needParens) changes.insertNodeBefore(sourceFile, first(containingFunction.parameters), createToken(SyntaxKind.OpenParenToken));
for (const { declaration, type } of parameterInferences) {
if (declaration && !declaration.type && !declaration.initializer) {
annotate(changes, sourceFile, declaration, type, program, host);
annotate(changes, sourceFile, declaration, type, program, host, formatContext, preferences);
}
}
if (needParens) changes.insertNodeAfter(sourceFile, last(containingFunction.parameters), createToken(SyntaxKind.CloseParenToken));
Expand Down Expand Up @@ -248,7 +267,16 @@ namespace ts.codefix {
]);
}

function annotateSetAccessor(changes: textChanges.ChangeTracker, sourceFile: SourceFile, setAccessorDeclaration: SetAccessorDeclaration, program: Program, host: LanguageServiceHost, cancellationToken: CancellationToken): void {
function annotateSetAccessor(
changes: textChanges.ChangeTracker,
sourceFile: SourceFile,
setAccessorDeclaration: SetAccessorDeclaration,
program: Program,
host: LanguageServiceHost,
cancellationToken: CancellationToken,
formatContext: formatting.FormatContext,
preferences: UserPreferences,
): void {
const param = firstOrUndefined(setAccessorDeclaration.parameters);
if (param && isIdentifier(setAccessorDeclaration.name) && isIdentifier(param.name)) {
let type = inferTypeForVariableFromUsage(setAccessorDeclaration.name, program, cancellationToken);
Expand All @@ -259,12 +287,12 @@ namespace ts.codefix {
annotateJSDocParameters(changes, sourceFile, [{ declaration: param, type }], program, host);
}
else {
annotate(changes, sourceFile, param, type, program, host);
annotate(changes, sourceFile, param, type, program, host, formatContext, preferences);
}
}
}

function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type, program: Program, host: LanguageServiceHost): void {
function annotate(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type, program: Program, host: LanguageServiceHost, formatContext: formatting.FormatContext, preferences: UserPreferences): void {
const typeNode = getTypeNodeIfAccessible(type, declaration, program, host);
if (typeNode) {
if (isInJSFile(sourceFile) && declaration.kind !== SyntaxKind.PropertySignature) {
Expand All @@ -276,12 +304,42 @@ namespace ts.codefix {
const typeTag = isGetAccessorDeclaration(declaration) ? createJSDocReturnTag(typeExpression, "") : createJSDocTypeTag(typeExpression, "");
addJSDocTags(changes, sourceFile, parent, [typeTag]);
}
else {
else if (!tryReplaceImportTypeNodeWithAutoImport(typeNode, changes, sourceFile, declaration, type, program, host, formatContext, preferences)) {
changes.tryInsertTypeAnnotation(sourceFile, declaration, typeNode);
}
}
}

function tryReplaceImportTypeNodeWithAutoImport(typeNode: TypeNode, changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: textChanges.TypeAnnotatable, type: Type, program: Program, host: LanguageServiceHost, formatContext: formatting.FormatContext, preferences: UserPreferences): boolean {
if (isLiteralImportTypeNode(typeNode) && typeNode.qualifier && type.symbol) {
// Replace 'import("./a").SomeType' with 'SomeType' and an actual import if possible
const moduleSymbol = find(type.symbol.declarations, d => !!d.getSourceFile().externalModuleIndicator)?.getSourceFile().symbol;
// Symbol for the left-most thing after the dot
if (moduleSymbol) {
const symbol = getFirstIdentifier(typeNode.qualifier).symbol;
const action = getImportCompletionAction(
symbol,
moduleSymbol,
sourceFile,
symbol.name,
host,
program,
formatContext,
declaration.pos,
preferences,
);
if (action.codeAction.changes.length && changes.tryInsertTypeAnnotation(sourceFile, declaration, createTypeReferenceNode(typeNode.qualifier, typeNode.typeArguments))) {
for (const change of action.codeAction.changes) {
const file = sourceFile.fileName === change.fileName ? sourceFile : Debug.assertDefined(program.getSourceFile(change.fileName));
changes.pushRaw(file, change);
Copy link
Member

Choose a reason for hiding this comment

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

Is this the first codefix to run another codefix and merge its actions? Seems like something that would have a common utility inside the codefox folder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Completions calls functions from importFixes, but AFAIK no other codefix has called another codefix.

}
return true;
}
}
}
return false;
}

function annotateJSDocParameters(changes: textChanges.ChangeTracker, sourceFile: SourceFile, parameterInferences: readonly ParameterInference[], program: Program, host: LanguageServiceHost): void {
const signature = parameterInferences.length && parameterInferences[0].declaration.parent;
if (!signature) {
Expand Down
17 changes: 15 additions & 2 deletions src/services/textChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,18 @@ namespace ts.textChanges {
/** Public for tests only. Other callers should use `ChangeTracker.with`. */
constructor(private readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}

public pushRaw(sourceFile: SourceFile, change: FileTextChanges) {
Debug.assertEqual(sourceFile.fileName, change.fileName);
for (const c of change.textChanges) {
this.changes.push({
kind: ChangeKind.Text,
sourceFile,
text: c.newText,
range: createTextRangeFromSpan(c.span),
});
}
}

public deleteRange(sourceFile: SourceFile, range: TextRange): void {
this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
}
Expand Down Expand Up @@ -383,12 +395,12 @@ namespace ts.textChanges {
}

/** Prefer this over replacing a node with another that has a type annotation, as it avoids reformatting the other parts of the node. */
public tryInsertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void {
public tryInsertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): boolean {
let endNode: Node | undefined;
if (isFunctionLike(node)) {
endNode = findChildOfKind(node, SyntaxKind.CloseParenToken, sourceFile);
if (!endNode) {
if (!isArrowFunction(node)) return; // Function missing parentheses, give up
if (!isArrowFunction(node)) return false; // Function missing parentheses, give up
// If no `)`, is an arrow function `x => x`, so use the end of the first parameter
endNode = first(node.parameters);
}
Expand All @@ -398,6 +410,7 @@ namespace ts.textChanges {
}

this.insertNodeAt(sourceFile, endNode.end, type, { prefix: ": " });
return true;
}

public tryInsertThisTypeAnnotation(sourceFile: SourceFile, node: ThisTypeAnnotatable, type: TypeNode): void {
Expand Down
2 changes: 1 addition & 1 deletion src/services/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ namespace ts {

export interface FileTextChanges {
fileName: string;
textChanges: TextChange[];
textChanges: readonly TextChange[];
isNewFile?: boolean;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5194,7 +5194,7 @@ declare namespace ts {
}
interface FileTextChanges {
fileName: string;
textChanges: TextChange[];
textChanges: readonly TextChange[];
isNewFile?: boolean;
}
interface CodeAction {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5194,7 +5194,7 @@ declare namespace ts {
}
interface FileTextChanges {
fileName: string;
textChanges: TextChange[];
textChanges: readonly TextChange[];
isNewFile?: boolean;
}
interface CodeAction {
Expand Down
28 changes: 28 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageContextualImport1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path="fourslash.ts" />

// @strict: true
// @noImplicitAny: true
// @noLib: true

// @Filename: /a.ts
////export interface User {}
////export declare function getEmail(user: User): string;

// @Filename: /b.ts
////import { getEmail } from "./a";
////
////export function f([|user|]) {
//// getEmail(user);
////}

goTo.file("/b.ts");

verify.codeFix({
description: "Infer parameter types from usage",
newFileContent:
`import { getEmail, User } from "./a";

export function f(user: User) {
getEmail(user);
}`
});
Loading