Skip to content
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

feat(27615): fixAddMissingMember should work for type literal #47212

Merged
merged 1 commit into from
Mar 30, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
114 changes: 61 additions & 53 deletions src/services/codefixes/fixAddMissingMember.ts

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace ts.codefix {
const abstractAndNonPrivateExtendsSymbols = checker.getPropertiesOfType(instantiatedExtendsType).filter(symbolPointsToNonPrivateAndAbstractMember);

const importAdder = createImportAdder(sourceFile, context.program, preferences, context.host);
createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, sourceFile, context, preferences, importAdder, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member as ClassElement));
createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, sourceFile, context, preferences, importAdder, member => changeTracker.insertMemberAtStart(sourceFile, classDeclaration, member as ClassElement));
importAdder.writeFixes(changeTracker);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace ts.codefix {
changeTracker.insertNodeAfter(sourceFile, constructor, newElement);
}
else {
changeTracker.insertNodeAtClassStart(sourceFile, cls, newElement);
changeTracker.insertMemberAtStart(sourceFile, cls, newElement);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/generateAccessors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ namespace ts.codefix {
}

function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) {
isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertNodeAtClassStart(file, container as ClassLikeDeclaration, accessor) :
isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertMemberAtStart(file, container as ClassLikeDeclaration, accessor) :
isPropertyAssignment(declaration) ? changeTracker.insertNodeAfterComma(file, declaration, accessor) :
changeTracker.insertNodeAfter(file, declaration, accessor);
}
Expand Down
59 changes: 36 additions & 23 deletions src/services/codefixes/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ namespace ts.codefix {
}

export function createSignatureDeclarationFromCallExpression(
kind: SyntaxKind.MethodDeclaration | SyntaxKind.FunctionDeclaration,
kind: SyntaxKind.MethodDeclaration | SyntaxKind.FunctionDeclaration | SyntaxKind.MethodSignature,
context: CodeFixContextBase,
importAdder: ImportAdder,
call: CallExpression,
Expand Down Expand Up @@ -313,29 +313,42 @@ namespace ts.codefix {
? undefined
: checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);

if (kind === SyntaxKind.MethodDeclaration) {
return factory.createMethodDeclaration(
/*decorators*/ undefined,
modifiers,
asteriskToken,
name,
/*questionToken*/ undefined,
typeParameters,
parameters,
type,
isInterfaceDeclaration(contextNode) ? undefined : createStubbedMethodBody(quotePreference)
);
switch (kind) {
case SyntaxKind.MethodDeclaration:
return factory.createMethodDeclaration(
/*decorators*/ undefined,
modifiers,
asteriskToken,
name,
/*questionToken*/ undefined,
typeParameters,
parameters,
type,
createStubbedMethodBody(quotePreference)
);
case SyntaxKind.MethodSignature:
return factory.createMethodSignature(
modifiers,
name,
/*questionToken*/ undefined,
typeParameters,
parameters,
type
);
case SyntaxKind.FunctionDeclaration:
return factory.createFunctionDeclaration(
/*decorators*/ undefined,
modifiers,
asteriskToken,
name,
typeParameters,
parameters,
type,
createStubbedBody(Diagnostics.Function_not_implemented.message, quotePreference)
);
default:
Debug.fail("Unexpected kind");
}
return factory.createFunctionDeclaration(
/*decorators*/ undefined,
modifiers,
asteriskToken,
name,
typeParameters,
parameters,
type,
createStubbedBody(Diagnostics.Function_not_implemented.message, quotePreference)
);
}

export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
Expand Down
36 changes: 18 additions & 18 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,27 +618,27 @@ namespace ts.textChanges {
});
}

public insertNodeAtClassStart(sourceFile: SourceFile, cls: ClassLikeDeclaration | InterfaceDeclaration, newElement: ClassElement): void {
this.insertNodeAtStartWorker(sourceFile, cls, newElement);
public insertMemberAtStart(sourceFile: SourceFile, node: ClassLikeDeclaration | InterfaceDeclaration | TypeLiteralNode, newElement: ClassElement | PropertySignature | MethodSignature): void {
this.insertNodeAtStartWorker(sourceFile, node, newElement);
}

public insertNodeAtObjectStart(sourceFile: SourceFile, obj: ObjectLiteralExpression, newElement: ObjectLiteralElementLike): void {
this.insertNodeAtStartWorker(sourceFile, obj, newElement);
}

private insertNodeAtStartWorker(sourceFile: SourceFile, cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression, newElement: ClassElement | ObjectLiteralElementLike): void {
const indentation = this.guessIndentationFromExistingMembers(sourceFile, cls) ?? this.computeIndentationForNewMember(sourceFile, cls);
this.insertNodeAt(sourceFile, getMembersOrProperties(cls).pos, newElement, this.getInsertNodeAtStartInsertOptions(sourceFile, cls, indentation));
private insertNodeAtStartWorker(sourceFile: SourceFile, node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression | TypeLiteralNode, newElement: ClassElement | ObjectLiteralElementLike | PropertySignature | MethodSignature): void {
const indentation = this.guessIndentationFromExistingMembers(sourceFile, node) ?? this.computeIndentationForNewMember(sourceFile, node);
this.insertNodeAt(sourceFile, getMembersOrProperties(node).pos, newElement, this.getInsertNodeAtStartInsertOptions(sourceFile, node, indentation));
}

/**
* Tries to guess the indentation from the existing members of a class/interface/object. All members must be on
* new lines and must share the same indentation.
*/
private guessIndentationFromExistingMembers(sourceFile: SourceFile, cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression) {
private guessIndentationFromExistingMembers(sourceFile: SourceFile, node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression | TypeLiteralNode) {
let indentation: number | undefined;
let lastRange: TextRange = cls;
for (const member of getMembersOrProperties(cls)) {
let lastRange: TextRange = node;
for (const member of getMembersOrProperties(node)) {
if (rangeStartPositionsAreOnSameLine(lastRange, member, sourceFile)) {
// each indented member must be on a new line
return undefined;
Expand All @@ -657,13 +657,13 @@ namespace ts.textChanges {
return indentation;
}

private computeIndentationForNewMember(sourceFile: SourceFile, cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression) {
const clsStart = cls.getStart(sourceFile);
return formatting.SmartIndenter.findFirstNonWhitespaceColumn(getLineStartPositionForPosition(clsStart, sourceFile), clsStart, sourceFile, this.formatContext.options)
private computeIndentationForNewMember(sourceFile: SourceFile, node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression | TypeLiteralNode) {
const nodeStart = node.getStart(sourceFile);
return formatting.SmartIndenter.findFirstNonWhitespaceColumn(getLineStartPositionForPosition(nodeStart, sourceFile), nodeStart, sourceFile, this.formatContext.options)
+ (this.formatContext.options.indentSize ?? 4);
}

private getInsertNodeAtStartInsertOptions(sourceFile: SourceFile, cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression, indentation: number): InsertNodeOptions {
private getInsertNodeAtStartInsertOptions(sourceFile: SourceFile, node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression | TypeLiteralNode, indentation: number): InsertNodeOptions {
// Rules:
// - Always insert leading newline.
// - For object literals:
Expand All @@ -674,11 +674,11 @@ namespace ts.textChanges {
// - Only insert a trailing newline if body is single-line and there are no other insertions for the node.
// NOTE: This is handled in `finishClassesWithNodesInsertedAtStart`.

const members = getMembersOrProperties(cls);
const members = getMembersOrProperties(node);
const isEmpty = members.length === 0;
const isFirstInsertion = addToSeen(this.classesWithNodesInsertedAtStart, getNodeId(cls), { node: cls, sourceFile });
const insertTrailingComma = isObjectLiteralExpression(cls) && (!isJsonSourceFile(sourceFile) || !isEmpty);
const insertLeadingComma = isObjectLiteralExpression(cls) && isJsonSourceFile(sourceFile) && isEmpty && !isFirstInsertion;
const isFirstInsertion = addToSeen(this.classesWithNodesInsertedAtStart, getNodeId(node), { node, sourceFile });
const insertTrailingComma = isObjectLiteralExpression(node) && (!isJsonSourceFile(sourceFile) || !isEmpty);
const insertLeadingComma = isObjectLiteralExpression(node) && isJsonSourceFile(sourceFile) && isEmpty && !isFirstInsertion;
return {
indentation,
prefix: (insertLeadingComma ? "," : "") + this.newLineCharacter,
Expand Down Expand Up @@ -998,8 +998,8 @@ namespace ts.textChanges {
const close = findChildOfKind(cls, SyntaxKind.CloseBraceToken, sourceFile);
return [open?.end, close?.end];
}
function getMembersOrProperties(cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression): NodeArray<Node> {
return isObjectLiteralExpression(cls) ? cls.properties : cls.members;
function getMembersOrProperties(node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression | TypeLiteralNode): NodeArray<Node> {
return isObjectLiteralExpression(node) ? node.properties : node.members;
}

export type ValidateNonFormattedText = (node: Node, text: string) => void;
Expand Down
15 changes: 15 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingMember22.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

////[|type Foo = {};|]
////function f(foo: Foo) {
//// foo.y;
////}

verify.codeFix({
description: [ts.Diagnostics.Declare_property_0.message, "y"],
index: 0,
newRangeContent:
`type Foo = {
y: any;
};`
});
15 changes: 15 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingMember23.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

////[|type Foo = {};|]
////function f(foo: Foo) {
//// foo.y = 1;
////}

verify.codeFix({
description: [ts.Diagnostics.Declare_property_0.message, "y"],
index: 0,
newRangeContent:
`type Foo = {
y: number;
};`
});
15 changes: 15 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingMember24.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

////[|type Foo = {};|]
////function f(foo: Foo) {
//// foo.test(1, 1, "");
////}

verify.codeFix({
description: [ts.Diagnostics.Declare_method_0.message, "test"],
index: 0,
newRangeContent:
`type Foo = {
test(arg0: number, arg1: number, arg2: string);
};`
});
18 changes: 18 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingMember25.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

////[|type Foo = {
//// y: number;
////};|]
////function f(foo: Foo) {
//// foo.x = 1;
////}

verify.codeFix({
description: [ts.Diagnostics.Declare_property_0.message, "x"],
index: 0,
newRangeContent:
`type Foo = {
x: number;
y: number;
};`
});
15 changes: 15 additions & 0 deletions tests/cases/fourslash/codeFixAddMissingMember26.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

////[|type Foo = {};|]
////function f(foo: Foo) {
//// foo.x = 1;
////}

verify.codeFix({
description: [ts.Diagnostics.Add_index_signature_for_property_0.message, "x"],
index: 1,
newRangeContent:
`type Foo = {
[x: string]: number;
};`
});
20 changes: 19 additions & 1 deletion tests/cases/fourslash/codeFixAddMissingMember_all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@
////
////enum En {}
////En.A;
////
////type T = {};
////function foo(t: T) {
//// t.x;
//// t.y = 1;
//// t.test(1, 2);
////}

verify.codeFixAll({
fixId: "fixMissingMember",
Expand Down Expand Up @@ -60,5 +67,16 @@ class Unrelated {
enum En {
A
}
En.A;`,
En.A;

type T = {
x: any;
y: number;
test(arg0: number, arg1: number);
};
function foo(t: T) {
t.x;
t.y = 1;
t.test(1, 2);
}`,
});