Skip to content

Commit f3bc891

Browse files
author
Andy Hanson
committed
fixAddMissingMember: Improve deduplication in code-fix-all
1 parent 38d6491 commit f3bc891

File tree

4 files changed

+149
-36
lines changed

4 files changed

+149
-36
lines changed

‎src/services/codeFixProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ namespace ts {
7171
return fixIdToRegistration.get(cast(context.fixId, isString))!.getAllCodeActions!(context);
7272
}
7373

74-
function createCombinedCodeActions(changes: FileTextChanges[], commands?: CodeActionCommand[]): CombinedCodeActions {
74+
export function createCombinedCodeActions(changes: FileTextChanges[], commands?: CodeActionCommand[]): CombinedCodeActions {
7575
return { changes, commands };
7676
}
7777

@@ -89,7 +89,7 @@ namespace ts {
8989
return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands);
9090
}
9191

92-
function eachDiagnostic({ program, sourceFile, cancellationToken }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void {
92+
export function eachDiagnostic({ program, sourceFile, cancellationToken }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void {
9393
for (const diag of program.getSemanticDiagnostics(sourceFile, cancellationToken).concat(computeSuggestionDiagnostics(sourceFile, program, cancellationToken))) {
9494
if (contains(errorCodes, diag.code)) {
9595
cb(diag as DiagnosticWithLocation);

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 83 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,55 +13,104 @@ namespace ts.codefix {
1313
if (!info) return undefined;
1414

1515
if (info.kind === InfoKind.enum) {
16-
const { token, enumDeclaration } = info;
17-
const changes = textChanges.ChangeTracker.with(context, t => addEnumMemberDeclaration(t, context.program.getTypeChecker(), token, enumDeclaration));
16+
const { token, parentDeclaration } = info;
17+
const changes = textChanges.ChangeTracker.with(context, t => addEnumMemberDeclaration(t, context.program.getTypeChecker(), token, parentDeclaration));
1818
return singleElementArray(createCodeFixAction(fixName, changes, [Diagnostics.Add_missing_enum_member_0, token.text], fixId, Diagnostics.Add_all_missing_enum_members));
1919
}
20-
const { classDeclaration, classDeclarationSourceFile, inJs, makeStatic, token, call } = info;
21-
const methodCodeAction = call && getActionForMethodDeclaration(context, classDeclarationSourceFile, classDeclaration, token, call, makeStatic, inJs, context.preferences);
20+
const { parentDeclaration, classDeclarationSourceFile, inJs, makeStatic, token, call } = info;
21+
const methodCodeAction = call && getActionForMethodDeclaration(context, classDeclarationSourceFile, parentDeclaration, token, call, makeStatic, inJs, context.preferences);
2222
const addMember = inJs ?
23-
singleElementArray(getActionsForAddMissingMemberInJavaScriptFile(context, classDeclarationSourceFile, classDeclaration, token.text, makeStatic)) :
24-
getActionsForAddMissingMemberInTypeScriptFile(context, classDeclarationSourceFile, classDeclaration, token, makeStatic);
23+
singleElementArray(getActionsForAddMissingMemberInJavaScriptFile(context, classDeclarationSourceFile, parentDeclaration, token.text, makeStatic)) :
24+
getActionsForAddMissingMemberInTypeScriptFile(context, classDeclarationSourceFile, parentDeclaration, token, makeStatic);
2525
return concatenate(singleElementArray(methodCodeAction), addMember);
2626
},
2727
fixIds: [fixId],
2828
getAllCodeActions: context => {
29-
const seenNames = createMap<true>();
30-
return codeFixAll(context, errorCodes, (changes, diag) => {
31-
const { program, preferences } = context;
32-
const checker = program.getTypeChecker();
33-
const info = getInfo(diag.file, diag.start, checker);
34-
if (!info || !addToSeen(seenNames, info.token.text)) {
35-
return;
36-
}
37-
38-
if (info.kind === InfoKind.enum) {
39-
const { token, enumDeclaration } = info;
40-
addEnumMemberDeclaration(changes, checker, token, enumDeclaration);
41-
}
42-
else {
43-
const { classDeclaration, classDeclarationSourceFile, inJs, makeStatic, token, call } = info;
44-
// Always prefer to add a method declaration if possible.
45-
if (call) {
46-
addMethodDeclaration(context, changes, classDeclarationSourceFile, classDeclaration, token, call, makeStatic, inJs, preferences);
29+
const { program, preferences } = context;
30+
const checker = program.getTypeChecker();
31+
const seen = createMap<true>();
32+
33+
const classToMembers = new NodeMap<ClassLikeDeclaration, ClassInfo[]>();
34+
35+
return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => {
36+
eachDiagnostic(context, errorCodes, diag => {
37+
const checker = program.getTypeChecker();
38+
const info = getInfo(diag.file, diag.start, checker);
39+
if (!info || !addToSeen(seen, getNodeId(info.parentDeclaration) + "#" + info.token.text)) {
40+
return;
41+
}
42+
43+
if (info.kind === InfoKind.enum) {
44+
const { token, parentDeclaration } = info;
45+
addEnumMemberDeclaration(changes, checker, token, parentDeclaration);
4746
}
4847
else {
49-
if (inJs) {
50-
addMissingMemberInJs(changes, classDeclarationSourceFile, classDeclaration, token.text, makeStatic);
48+
const { parentDeclaration, token } = info;
49+
const infos = classToMembers.getOrUpdate(parentDeclaration, () => []);
50+
if (!infos.some(i => i.token.text === token.text)) infos.push(info);
51+
}
52+
});
53+
54+
classToMembers.forEach((infos, classDeclaration) => {
55+
const superClasses = getAllSuperClasses(classDeclaration, checker);
56+
for (const info of infos) {
57+
// If some superclass added this property, don't add it again.
58+
if (superClasses.some(superClass => {
59+
const superInfos = classToMembers.get(superClass);
60+
return !!superInfos && superInfos.some(({ token }) => token.text === info.token.text);
61+
})) continue;
62+
63+
const { parentDeclaration, classDeclarationSourceFile, inJs, makeStatic, token, call } = info;
64+
65+
// Always prefer to add a method declaration if possible.
66+
if (call) {
67+
addMethodDeclaration(context, changes, classDeclarationSourceFile, parentDeclaration, token, call, makeStatic, inJs, preferences);
5168
}
5269
else {
53-
const typeNode = getTypeNode(program.getTypeChecker(), classDeclaration, token);
54-
addPropertyDeclaration(changes, classDeclarationSourceFile, classDeclaration, token.text, typeNode, makeStatic);
70+
if (inJs) {
71+
addMissingMemberInJs(changes, classDeclarationSourceFile, parentDeclaration, token.text, makeStatic);
72+
}
73+
else {
74+
const typeNode = getTypeNode(program.getTypeChecker(), parentDeclaration, token);
75+
addPropertyDeclaration(changes, classDeclarationSourceFile, parentDeclaration, token.text, typeNode, makeStatic);
76+
}
5577
}
5678
}
57-
}
58-
});
79+
});
80+
}));
5981
},
6082
});
6183

84+
function getAllSuperClasses(cls: ClassLikeDeclaration | undefined, checker: TypeChecker): ReadonlyArray<ClassLikeDeclaration> {
85+
const res: ClassLikeDeclaration[] = [];
86+
while (cls) {
87+
const superElement = getClassExtendsHeritageElement(cls);
88+
const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression);
89+
const superDecl = superSymbol && find(superSymbol.declarations, isClassLike);
90+
if (superDecl) { res.push(superDecl); }
91+
cls = superDecl;
92+
}
93+
return res;
94+
}
95+
96+
interface InfoBase {
97+
readonly kind: InfoKind;
98+
readonly token: Identifier;
99+
readonly parentDeclaration: EnumDeclaration | ClassLikeDeclaration;
100+
}
62101
enum InfoKind { enum, class }
63-
interface EnumInfo { kind: InfoKind.enum; token: Identifier; enumDeclaration: EnumDeclaration; }
64-
interface ClassInfo { kind: InfoKind.class; token: Identifier; classDeclaration: ClassLikeDeclaration; makeStatic: boolean; classDeclarationSourceFile: SourceFile; inJs: boolean; call: CallExpression | undefined; }
102+
interface EnumInfo extends InfoBase {
103+
readonly kind: InfoKind.enum;
104+
readonly parentDeclaration: EnumDeclaration;
105+
}
106+
interface ClassInfo extends InfoBase {
107+
readonly kind: InfoKind.class;
108+
readonly parentDeclaration: ClassLikeDeclaration;
109+
readonly makeStatic: boolean;
110+
readonly classDeclarationSourceFile: SourceFile;
111+
readonly inJs: boolean;
112+
readonly call: CallExpression | undefined;
113+
}
65114
type Info = EnumInfo | ClassInfo;
66115

67116
function getInfo(tokenSourceFile: SourceFile, tokenPos: number, checker: TypeChecker): Info | undefined {
@@ -86,11 +135,11 @@ namespace ts.codefix {
86135
const classDeclarationSourceFile = classDeclaration.getSourceFile();
87136
const inJs = isSourceFileJavaScript(classDeclarationSourceFile);
88137
const call = tryCast(parent.parent, isCallExpression);
89-
return { kind: InfoKind.class, token, classDeclaration, makeStatic, classDeclarationSourceFile, inJs, call };
138+
return { kind: InfoKind.class, token, parentDeclaration: classDeclaration, makeStatic, classDeclarationSourceFile, inJs, call };
90139
}
91140
const enumDeclaration = find(symbol.declarations, isEnumDeclaration);
92141
if (enumDeclaration) {
93-
return { kind: InfoKind.enum, token, enumDeclaration };
142+
return { kind: InfoKind.enum, token, parentDeclaration: enumDeclaration };
94143
}
95144
return undefined;
96145
}

‎src/services/utilities.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,6 +1351,40 @@ namespace ts {
13511351
}
13521352
}
13531353

1354+
export interface ReadonlyNodeMap<TNode extends Node, TValue> {
1355+
get(node: TNode): TValue | undefined;
1356+
has(node: TNode): boolean;
1357+
}
1358+
1359+
export class NodeMap<TNode extends Node, TValue> implements ReadonlyNodeMap<TNode, TValue> {
1360+
private map = createMap<{ node: TNode, value: TValue }>();
1361+
1362+
get(node: TNode): TValue | undefined {
1363+
const res = this.map.get(String(getNodeId(node)));
1364+
return res && res.value;
1365+
}
1366+
1367+
getOrUpdate(node: TNode, setValue: () => TValue): TValue {
1368+
const res = this.get(node);
1369+
if (res) return res;
1370+
const value = setValue();
1371+
this.set(node, value);
1372+
return value;
1373+
}
1374+
1375+
set(node: TNode, value: TValue): void {
1376+
this.map.set(String(getNodeId(node)), { node, value });
1377+
}
1378+
1379+
has(node: TNode): boolean {
1380+
return this.map.has(String(getNodeId(node)));
1381+
}
1382+
1383+
forEach(cb: (value: TValue, node: TNode) => void): void {
1384+
this.map.forEach(({ node, value }) => cb(value, node));
1385+
}
1386+
}
1387+
13541388
export function getParentNodeInSpan(node: Node | undefined, file: SourceFile, span: TextSpan): Node | undefined {
13551389
if (!node) return undefined;
13561390

‎tests/cases/fourslash/codeFixAddMissingMember_all.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@
77
//// this.x = "";
88
//// }
99
////}
10+
////
11+
////class D extends C {}
12+
////class E extends D {
13+
//// method() {
14+
//// this.x = 0;
15+
//// this.ex = 0;
16+
//// }
17+
////}
18+
////
19+
////class Unrelated {
20+
//// method() {
21+
//// this.x = 0;
22+
//// }
23+
////}
1024

1125
verify.codeFixAll({
1226
fixId: "addMissingMember",
@@ -22,5 +36,21 @@ verify.codeFixAll({
2236
y(): any {
2337
throw new Error("Method not implemented.");
2438
}
39+
}
40+
41+
class D extends C {}
42+
class E extends D {
43+
ex: number;
44+
method() {
45+
this.x = 0;
46+
this.ex = 0;
47+
}
48+
}
49+
50+
class Unrelated {
51+
x: number;
52+
method() {
53+
this.x = 0;
54+
}
2555
}`,
2656
});

0 commit comments

Comments
 (0)