Skip to content

Commit

Permalink
feat(16755): show QF to declare missing properties in a call expressi…
Browse files Browse the repository at this point in the history 
…on with an object literal argument
  • Loading branch information
@a-tarasyuk
a-tarasyuk committed Jun 28, 2021
1 parent a7a0d25 commit 63991bb
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 31 deletions.
62 changes: 36 additions & 26 deletions src/services/codefixes/fixAddMissingMember.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ namespace ts.codefix {
Diagnostics.Property_0_is_missing_in_type_1_but_required_in_type_2.code,
Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2.code,
Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2_and_3_more.code,
Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code,
Diagnostics.Cannot_find_name_0.code
];

registerCodeFix({
errorCodes,
getCodeActions(context) {
const typeChecker = context.program.getTypeChecker();
const info = getInfo(context.sourceFile, context.span.start, typeChecker, context.program);
const info = getInfo(context.sourceFile, context.span.start, context.errorCode, typeChecker, context.program);
if (!info) {
return undefined;
}
Expand All @@ -44,7 +45,7 @@ namespace ts.codefix {

return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => {
eachDiagnostic(context, errorCodes, diag => {
const info = getInfo(diag.file, diag.start, checker, context.program);
const info = getInfo(diag.file, diag.start, diag.code, checker, context.program);
if (!info || !addToSeen(seen, getNodeId(info.parentDeclaration) + "#" + info.token.text)) {
return;
}
Expand Down Expand Up @@ -135,38 +136,48 @@ namespace ts.codefix {
readonly token: Identifier;
readonly properties: Symbol[];
readonly parentDeclaration: ObjectLiteralExpression;
readonly indentation?: number;
}

function getInfo(sourceFile: SourceFile, tokenPos: number, checker: TypeChecker, program: Program): Info | undefined {
function getInfo(sourceFile: SourceFile, tokenPos: number, errorCode: number, checker: TypeChecker, program: Program): Info | undefined {
// The identifier of the missing property. eg:
// this.missing = 1;
// ^^^^^^^
const token = getTokenAtPosition(sourceFile, tokenPos);
if (!isIdentifier(token) && !isPrivateIdentifier(token)) {
return undefined;
const parent = token.parent;

if (errorCode === Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1.code) {
if (!(token.kind === SyntaxKind.OpenBraceToken && isObjectLiteralExpression(parent) && isCallExpression(parent.parent))) return undefined;

const argIndex = findIndex(parent.parent.arguments, arg => arg === parent);
if (argIndex < 0) return undefined;

const signature = singleOrUndefined(checker.getSignaturesOfType(checker.getTypeAtLocation(parent.parent.expression), SignatureKind.Call));
if (!(signature && signature.declaration && signature.parameters[argIndex])) return undefined;

const param = signature.parameters[argIndex].valueDeclaration;
if (!(param && isParameter(param) && isIdentifier(param.name))) return undefined;

const properties = arrayFrom(checker.getUnmatchedProperties(checker.getTypeAtLocation(parent), checker.getTypeAtLocation(param), /* requireOptionalProperties */ false, /* matchDiscriminantProperties */ false));
return length(properties) ? { kind: InfoKind.ObjectLiteral, token: param.name, properties, indentation: 0, parentDeclaration: parent } : undefined;;
}

const { parent } = token;
if (!isMemberName(token)) return undefined;

if (isIdentifier(token) && hasInitializer(parent) && parent.initializer && isObjectLiteralExpression(parent.initializer)) {
const properties = arrayFrom(checker.getUnmatchedProperties(checker.getTypeAtLocation(parent.initializer), checker.getTypeAtLocation(token), /* requireOptionalProperties */ false, /* matchDiscriminantProperties */ false));
if (length(properties)) {
return { kind: InfoKind.ObjectLiteral, token, properties, parentDeclaration: parent.initializer };
}
return length(properties) ? { kind: InfoKind.ObjectLiteral, token, properties, indentation: undefined, parentDeclaration: parent.initializer } : undefined;
}

if (isIdentifier(token) && isCallExpression(parent)) {
return { kind: InfoKind.Function, token, call: parent, sourceFile, modifierFlags: ModifierFlags.None, parentDeclaration: sourceFile };
}

if (!isPropertyAccessExpression(parent)) {
return undefined;
}
if (!isPropertyAccessExpression(parent)) return undefined;

const leftExpressionType = skipConstraint(checker.getTypeAtLocation(parent.expression));
const { symbol } = leftExpressionType;
if (!symbol || !symbol.declarations) {
return undefined;
}
const symbol = leftExpressionType.symbol;
if (!symbol || !symbol.declarations) return undefined;

if (isIdentifier(token) && isCallExpression(parent.parent)) {
const moduleDeclaration = find(symbol.declarations, isModuleDeclaration);
Expand All @@ -176,9 +187,7 @@ namespace ts.codefix {
}

const moduleSourceFile = find(symbol.declarations, isSourceFile);
if (sourceFile.commonJsModuleIndicator) {
return;
}
if (sourceFile.commonJsModuleIndicator) return undefined;

if (moduleSourceFile && !program.isSourceFileFromExternalLibrary(moduleSourceFile)) {
return { kind: InfoKind.Function, token, call: parent.parent, sourceFile: moduleSourceFile, modifierFlags: ModifierFlags.Export, parentDeclaration: moduleSourceFile };
Expand All @@ -187,17 +196,13 @@ namespace ts.codefix {

const classDeclaration = find(symbol.declarations, isClassLike);
// Don't suggest adding private identifiers to anything other than a class.
if (!classDeclaration && isPrivateIdentifier(token)) {
return undefined;
}
if (!classDeclaration && isPrivateIdentifier(token)) return undefined;

// Prefer to change the class instead of the interface if they are merged
const classOrInterface = classDeclaration || find(symbol.declarations, isInterfaceDeclaration);
if (classOrInterface && !program.isSourceFileFromExternalLibrary(classOrInterface.getSourceFile())) {
const makeStatic = ((leftExpressionType as TypeReference).target || leftExpressionType) !== checker.getDeclaredTypeOfSymbol(symbol);
if (makeStatic && (isPrivateIdentifier(token) || isInterfaceDeclaration(classOrInterface))) {
return undefined;
}
if (makeStatic && (isPrivateIdentifier(token) || isInterfaceDeclaration(classOrInterface))) return undefined;

const declSourceFile = classOrInterface.getSourceFile();
const modifierFlags = (makeStatic ? ModifierFlags.Static : 0) | (startsWithUnderscore(token.text) ? ModifierFlags.Private : 0);
Expand Down Expand Up @@ -438,7 +443,12 @@ namespace ts.codefix {
const initializer = prop.valueDeclaration ? tryGetInitializerValueFromType(context, checker, importAdder, quotePreference, checker.getTypeAtLocation(prop.valueDeclaration)) : createUndefined();
return factory.createPropertyAssignment(prop.name, initializer);
});
changes.replaceNode(context.sourceFile, info.parentDeclaration, factory.createObjectLiteralExpression([...info.parentDeclaration.properties, ...props], /*multiLine*/ true));
const options = {
leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude,
trailingTriviaOption: textChanges.TrailingTriviaOption.Exclude,
indentation: info.indentation
};
changes.replaceNode(context.sourceFile, info.parentDeclaration, factory.createObjectLiteralExpression([...info.parentDeclaration.properties, ...props], /*multiLine*/ true), options);
}

function tryGetInitializerValueFromType(context: CodeFixContextBase, checker: TypeChecker, importAdder: ImportAdder, quotePreference: QuotePreference, type: Type): Expression {
Expand Down
18 changes: 18 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingProperties12.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

////interface Foo {
//// a: number;
//// b: number;
////}
////function f(foo: Foo) {}
////[|f({})|];

verify.codeFix({
index: 0,
description: ts.Diagnostics.Add_missing_properties.message,
newRangeContent:
`f({
a: 0,
b: 0
})`
});
22 changes: 22 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingProperties13.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/// <reference path='fourslash.ts' />

////interface Foo {
//// a: number;
//// b: number;
//// c: () => void;
////}
////function f(foo: Foo) {}
////[|f({ a: 10 })|];

verify.codeFix({
index: 0,
description: ts.Diagnostics.Add_missing_properties.message,
newRangeContent:
`f({
a: 10,
b: 0,
c: function(): void {
throw new Error("Function not implemented.");
}
})`
});
18 changes: 18 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingProperties14.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

////interface Foo {
//// a: number;
//// b: number;
////}
////function f(a: number, b: number, c: Foo) {}
////[|f(1, 2, {})|];

verify.codeFix({
index: 0,
description: ts.Diagnostics.Add_missing_properties.message,
newRangeContent:
`f(1, 2, {
a: 0,
b: 0
})`
});
24 changes: 19 additions & 5 deletions tests/cases/fourslash/codeFixAddMissingProperties_all.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
////class C {
//// public c: I1 = {};
////}
////function fn(foo: I2 = {}) {
////}
////function fn1(foo: I2 = {}) {}
////function fn2(a: I1) {}
////fn2({});

verify.codeFixAll({
fixId: "fixMissingProperties",
Expand Down Expand Up @@ -70,9 +71,22 @@ class C {
}
};
}
function fn(foo: I2 = {
function fn1(foo: I2 = {
a: undefined,
b: undefined
}) {
}`
}) {}
function fn2(a: I1) {}
fn2({
a: 0,
b: "",
c: 1,
d: "d",
e: "e1",
f: function(x: number, y: number): void {
throw new Error("Function not implemented.");
},
g: function(x: number, y: number): void {
throw new Error("Function not implemented.");
}
});`
});

0 comments on commit 63991bb

Please sign in to comment.