Skip to content

fix(49964): Incorrect code generated by "add extends constraint" fix for multiple usages of the same type parameter #50004

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.

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 1 commit into from
Jul 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 81 additions & 27 deletions src/services/codefixes/fixAddMissingConstraint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,50 +16,104 @@ namespace ts.codefix {
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { sourceFile, span, program } = context;
const related = getDiagnosticRelatedInfo(program, sourceFile, span);
if (!related) {
return;
}
const changes = textChanges.ChangeTracker.with(context, t => addMissingConstraint(t, related));
const { sourceFile, span, program, preferences, host } = context;
const info = getInfo(program, sourceFile, span);
if (info === undefined) return;

const changes = textChanges.ChangeTracker.with(context, t => addMissingConstraint(t, program, preferences, host, sourceFile, info));
return [createCodeFixAction(fixId, changes, Diagnostics.Add_extends_constraint, fixId, Diagnostics.Add_extends_constraint_to_all_type_parameters)];
},
fixIds: [fixId],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
const info = getDiagnosticRelatedInfo(context.program, context.sourceFile, diag);
if (!info) return;
return addMissingConstraint(changes, info);
}),
getAllCodeActions: context => {
const { program, preferences, host } = context;
const seen = new Map<string, true>();

return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => {
eachDiagnostic(context, errorCodes, diag => {
const info = getInfo(program, diag.file, createTextSpan(diag.start, diag.length));
if (info) {
const id = getNodeId(info.declaration) + "#" + info.token.getText();
if (addToSeen(seen, id)) {
return addMissingConstraint(changes, program, preferences, host, diag.file, info);
}
}
return undefined;
});
}));
}
});

function getDiagnosticRelatedInfo(program: Program, sourceFile: SourceFile, span: TextSpan) {
interface Info {
constraint: Type | string;
declaration: TypeParameterDeclaration;
token: Node;
}

function getInfo(program: Program, sourceFile: SourceFile, span: TextSpan): Info | undefined {
const diag = find(program.getSemanticDiagnostics(sourceFile), diag => diag.start === span.start && diag.length === span.length);
if (!diag || !diag.relatedInformation) return;
if (diag === undefined || diag.relatedInformation === undefined) return;

const related = find(diag.relatedInformation, related => related.code === Diagnostics.This_type_parameter_might_need_an_extends_0_constraint.code);
if (!related) return;
return related;
}
if (related === undefined || related.file === undefined || related.start === undefined || related.length === undefined) return;

let declaration = findAncestorMatchingSpan(related.file, createTextSpan(related.start, related.length));
if (declaration === undefined) return;

function addMissingConstraint(changes: textChanges.ChangeTracker, related: DiagnosticRelatedInformation): void {
let decl = findAncestorMatchingSpan(related.file!, related as TextSpan);
if (!decl) return;
if (isIdentifier(decl) && isTypeParameterDeclaration(decl.parent)) {
decl = decl.parent;
if (isIdentifier(declaration) && isTypeParameterDeclaration(declaration.parent)) {
declaration = declaration.parent;
}
if (!isTypeParameterDeclaration(decl) || isMappedTypeNode(decl.parent)) return; // should only issue fix on type parameters written using `extends`
const newConstraint = flattenDiagnosticMessageText(related.messageText, "\n", 0).match(/`extends (.*)`/);
if (!newConstraint) return;
const newConstraintText = newConstraint[1];

changes.insertText(related.file!, decl.name.end, ` extends ${newConstraintText}`);
if (isTypeParameterDeclaration(declaration)) {
// should only issue fix on type parameters written using `extends`
if (isMappedTypeNode(declaration.parent)) return;

const token = getTokenAtPosition(sourceFile, span.start);
const checker = program.getTypeChecker();
const constraint = tryGetConstraintType(checker, token) || tryGetConstraintFromDiagnosticMessage(related.messageText);

return { constraint, declaration, token };
}
return undefined;
}

function addMissingConstraint(changes: textChanges.ChangeTracker, program: Program, preferences: UserPreferences, host: LanguageServiceHost, sourceFile: SourceFile, info: Info): void {
const { declaration, constraint } = info;
const checker = program.getTypeChecker();

if (isString(constraint)) {
changes.insertText(sourceFile, declaration.name.end, ` extends ${constraint}`);
}
else {
const scriptTarget = getEmitScriptTarget(program.getCompilerOptions());
const tracker = getNoopSymbolTrackerWithResolver({ program, host });
const importAdder = createImportAdder(sourceFile, program, preferences, host);
const typeNode = typeToAutoImportableTypeNode(checker, importAdder, constraint, /*contextNode*/ undefined, scriptTarget, /*flags*/ undefined, tracker);
if (typeNode) {
changes.replaceNode(sourceFile, declaration, factory.updateTypeParameterDeclaration(declaration, /*modifiers*/ undefined, declaration.name, typeNode, declaration.default));
importAdder.writeFixes(changes);
}
}
}

function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node {
let token = getTokenAtPosition(sourceFile, span.start);
const end = textSpanEnd(span);
let token = getTokenAtPosition(sourceFile, span.start);
while (token.end < end) {
token = token.parent;
}
return token;
}

function tryGetConstraintFromDiagnosticMessage(messageText: string | DiagnosticMessageChain) {
const [_, constraint] = flattenDiagnosticMessageText(messageText, "\n", 0).match(/`extends (.*)`/) || [];
return constraint;
}

function tryGetConstraintType(checker: TypeChecker, node: Node) {
if (isTypeNode(node.parent)) {
return checker.getTypeArgumentConstraint(node.parent);
}
const contextualType = isExpression(node) ? checker.getContextualType(node) : undefined;
return contextualType || checker.getTypeAtLocation(node);
}
}
24 changes: 24 additions & 0 deletions tests/cases/fourslash/quickfixAddMissingConstraint4.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/// <reference path="fourslash.ts" />

// @filename: /bar.ts
////export type Bar = Record<string, string>
////export function bar<T extends Bar>(obj: { prop: T }) {}

// @filename: /foo.ts
////import { bar } from "./bar";
////
////export function foo<T>(x: T) {
//// bar({ prop: x/**/ })
////}

goTo.marker("");
verify.codeFix({
index: 0,
description: "Add `extends` constraint.",
newFileContent:
`import { bar } from "./bar";

export function foo<T extends Bar>(x: T) {
bar({ prop: x })
}`
});
65 changes: 65 additions & 0 deletions tests/cases/fourslash/quickfixAddMissingConstraint_all.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/// <reference path="fourslash.ts" />

// @strict: true

// @filename: /bar.ts
////export type Bar = Record<string, string>
////export function bar<T extends Bar>(obj: { prop: T }) {}

// @filename: /foo.ts
////import { bar } from "./bar";
////
////export function f1<T>(x: T) {
//// bar({ prop: x })
////}
////
////function f2<T>(x: T) {
//// const y: `${number}` = x;
////}
////
////interface Fn<T extends string> {}
////function f3<T>(x: Fn<T>) {
////}
////
////function f4<T = `${number}`>(x: T) {
//// const y: `${number}` = x;
////}
////
////interface TypeRef<T extends {}> {
//// x: T
////}
////function f5<T>(): TypeRef</**/T> {
//// throw undefined as any as TypeRef<T>;
////}

goTo.file("/foo.ts");
verify.codeFixAll({
fixId: "addMissingConstraint",
fixAllDescription: ts.Diagnostics.Add_extends_constraint_to_all_type_parameters.message,
newFileContent:
"import { bar } from \"./bar\";\n\n" +

"export function f1<T extends Bar>(x: T) {\n" +
" bar({ prop: x })\n" +
"}\n\n" +

"function f2<T extends \`${number}\`>(x: T) {\n" +
" const y: `${number}` = x;\n" +
"}\n\n" +

"interface Fn<T extends string> {}\n" +
"function f3<T extends string>(x: Fn<T>) {\n" +
"}\n\n" +

"function f4<T extends `${number}` = `${number}`>(x: T) {\n" +
" const y: `${number}` = x;\n" +
"}\n\n" +

"interface TypeRef<T extends {}> {\n" +
" x: T\n" +
"}\n" +
"function f5<T extends {}>(): TypeRef<T> {\n" +
" throw undefined as any as TypeRef<T>;\n" +
"}"

});