add more tests and support more cases

This commit is contained in:
Gabriela Araujo Britto
2021-09-20 17:44:47 -07:00
parent 4d2d476962
commit c608a6eee9
3 changed files with 165 additions and 73 deletions

View File

@@ -18,7 +18,7 @@ namespace ts.codefix {
const classMembers = classDeclaration.symbol.members!;
for (const symbol of possiblyMissingSymbols) {
if (!classMembers.has(symbol.escapedName)) {
addNewNodeForMemberSymbol(symbol, classDeclaration, sourceFile, context, preferences, importAdder, addClassElement);
addNewNodeForMemberSymbol(symbol, classDeclaration, sourceFile, context, preferences, importAdder, addClassElement, /* body */ undefined);
}
}
}
@@ -39,6 +39,7 @@ namespace ts.codefix {
/**
* `addClassElement` will not be called if we can't figure out a representation for `symbol` in `enclosingDeclaration`.
* @param body If defined, this will be the body of the member node passed to `addClassElement`. Otherwise, the body will default to a stub.
*/
export function addNewNodeForMemberSymbol(
symbol: Symbol,
@@ -48,6 +49,7 @@ namespace ts.codefix {
preferences: UserPreferences,
importAdder: ImportAdder | undefined,
addClassElement: (node: AddNode) => void,
body: Block | undefined,
): void {
const declarations = symbol.getDeclarations();
if (!(declarations && declarations.length)) {
@@ -106,7 +108,7 @@ namespace ts.codefix {
name,
emptyArray,
typeNode,
ambient ? undefined : createStubbedMethodBody(quotePreference)));
ambient ? undefined : body || createStubbedMethodBody(quotePreference)));
}
else {
Debug.assertNode(accessor, isSetAccessorDeclaration, "The counterpart to a getter should be a setter");
@@ -117,7 +119,7 @@ namespace ts.codefix {
modifiers,
name,
createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false),
ambient ? undefined : createStubbedMethodBody(quotePreference)));
ambient ? undefined : body || createStubbedMethodBody(quotePreference)));
}
}
break;
@@ -139,7 +141,7 @@ namespace ts.codefix {
if (declarations.length === 1) {
Debug.assert(signatures.length === 1, "One declaration implies one signature");
const signature = signatures[0];
outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : createStubbedMethodBody(quotePreference));
outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : body || createStubbedMethodBody(quotePreference));
break;
}
@@ -151,11 +153,11 @@ namespace ts.codefix {
if (!ambient) {
if (declarations.length > signatures.length) {
const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!;
outputMethod(quotePreference, signature, modifiers, name, createStubbedMethodBody(quotePreference));
outputMethod(quotePreference, signature, modifiers, name, body || createStubbedMethodBody(quotePreference));
}
else {
Debug.assert(declarations.length === signatures.length, "Declarations and signatures should match count");
addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional, modifiers, quotePreference));
addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional, modifiers, quotePreference, body));
}
}
break;
@@ -365,6 +367,7 @@ namespace ts.codefix {
optional: boolean,
modifiers: readonly Modifier[] | undefined,
quotePreference: QuotePreference,
body: Block | undefined,
): MethodDeclaration {
/** This is *a* signature with the maximal number of arguments,
* such that if there is a "maximal" signature without rest arguments,
@@ -406,7 +409,8 @@ namespace ts.codefix {
/*typeParameters*/ undefined,
parameters,
getReturnTypeFromSignatures(signatures, checker, context, enclosingDeclaration),
quotePreference);
quotePreference,
body);
}
function getReturnTypeFromSignatures(signatures: readonly Signature[], checker: TypeChecker, context: TypeConstructionContext, enclosingDeclaration: ClassLikeDeclaration): TypeNode | undefined {
@@ -423,7 +427,8 @@ namespace ts.codefix {
typeParameters: readonly TypeParameterDeclaration[] | undefined,
parameters: readonly ParameterDeclaration[],
returnType: TypeNode | undefined,
quotePreference: QuotePreference
quotePreference: QuotePreference,
body: Block | undefined
): MethodDeclaration {
return factory.createMethodDeclaration(
/*decorators*/ undefined,
@@ -434,7 +439,7 @@ namespace ts.codefix {
typeParameters,
parameters,
returnType,
createStubbedMethodBody(quotePreference));
body || createStubbedMethodBody(quotePreference));
}
function createStubbedMethodBody(quotePreference: QuotePreference) {

View File

@@ -391,7 +391,8 @@ namespace ts.Completions {
host: LanguageServiceHost,
program: Program,
compilerOptions: CompilerOptions,
log: Log, completionData: CompletionData,
log: Log,
completionData: CompletionData,
preferences: UserPreferences,
): CompletionInfo | undefined {
const {
@@ -719,6 +720,7 @@ namespace ts.Completions {
}
// >> TODO: Find better location for code
// >> TODO: update this to `isMemberCompletion`... ?
function isMethodOverrideCompletion(symbol: Symbol, location: Node): boolean {
return !!(symbol.flags & SymbolFlags.Method) && isPropertyDeclaration(location.parent);
// >> TODO: add more checks. e.g. the method suggestion could come from an interface the class implements,
@@ -748,6 +750,14 @@ namespace ts.Completions {
omitTrailingSemicolon: true,
});
const importAdder = codefix.createImportAdder(sourceFile, program, preferences, host);
let body;
if (preferences.includeCompletionsWithSnippetText) {
isSnippet = true;
body = factory.createBlock([], /* multiline */ true); // TODO: add tabstop
}
else {
body = factory.createBlock([], /* multiline */ true);
}
codefix.addNewNodeForMemberSymbol(
symbol,
classLikeDeclaration,
@@ -755,66 +765,34 @@ namespace ts.Completions {
{ program, host },
preferences,
importAdder,
/* addClassElement */ nodeToEntry);
function nodeToEntry(node: PropertyDeclaration | GetAccessorDeclaration | SetAccessorDeclaration | MethodDeclaration | FunctionExpression | ArrowFunction): void {
if (!isPropertyDeclaration(node) && node.body && isBlock(node.body)) { // Declaration has body, so we might need to transform this completion into a snippet.
factory.updateBlock(node.body, []); // TODO: add tabstop if editor supports snippets
if (preferences.includeCompletionsWithSnippetText) {
// TODO: add tabstop if editor supports snippets
isSnippet = true;
node => {
if (isClassDeclaration(classLikeDeclaration) && hasAbstractModifier(classLikeDeclaration)) {
// Add `abstract` modifier
node = factory.updateModifiers(
node,
concatenate([factory.createModifier(SyntaxKind.AbstractKeyword)], node.modifiers),
);
if (isMethodDeclaration(node)) {
// Remove method body
// >> TODO: maybe move this up, when creating the body above?
node = factory.updateMethodDeclaration(
node,
node.decorators,
node.modifiers,
node.asteriskToken,
node.name,
node.questionToken,
node.typeParameters,
node.parameters,
node.type,
/* body */ undefined,
);
}
}
insertText = printer.printNode(EmitHint.Unspecified, node, sourceFile);
}
else { // Declaration has no body or body is not a block.
insertText = printer.printNode(EmitHint.Unspecified, node, sourceFile);
}
}
// ** ----- ** //
},
body);
// const methodDeclarations = symbol.declarations?.filter(isMethodDeclaration);
// // >> TODO: what should we do if we have more than 1 signature? when could that happen?
// if (methodDeclarations?.length === 1) {
// const originalDeclaration = methodDeclarations[0];
// const modifiers = originalDeclaration.modifiers?.filter(modifier => {
// switch (modifier.kind) { // >> Simplify this if we only need to filter out "abstract" modifier.
// case SyntaxKind.AbstractKeyword:
// return false;
// default:
// return true;
// }
// });
// if (options.noImplicitOverride) {
// modifiers?.push(factory.createModifier(SyntaxKind.OverrideKeyword));
// // Assuming it's ok if this modifier is duplicated.
// }
// const tabStop = preferences.includeCompletionsWithSnippetText ? "$1" : "";
// const tabStopStatement = factory.createExpressionStatement(factory.createIdentifier(tabStop));
// const completionDeclaration = factory.createMethodDeclaration(
// /*decorators*/ undefined, // I'm guessing we don't want to deal with decorators?
// /*modifiers*/ modifiers,
// /*asteriskToken*/ originalDeclaration.asteriskToken,
// /*name*/ name,
// /*questionToken*/ originalDeclaration.questionToken,
// /*typeParameters*/ originalDeclaration.typeParameters,
// /*parameters*/ originalDeclaration.parameters,
// /*type*/ originalDeclaration.type,
// /*body*/ factory.createBlock([tabStopStatement], /*multiLine*/ true));
// const printer = createPrinter({
// removeComments: true,
// module: options.module,
// target: options.target,
// omitTrailingSemicolon: true,
// });
// const insertText = printer.printNode(EmitHint.Unspecified, completionDeclaration, location.getSourceFile());
// return {
// insertText,
// isSnippet: preferences.includeCompletionsWithSnippetText ? true as const : undefined,
// };
// }
return { insertText, isSnippet };
}

View File

@@ -1,11 +1,53 @@
/// <reference path="fourslash.ts" />
////abstract class Base {
// @Filename: a.ts
// Case: Concrete class implements abstract method
////abstract class ABase {
//// abstract foo(param1: string, param2: boolean): Promise<void>;
////}
////
////class Sub extends Base {
//// f/*a*/
////class ASub extends ABase {
//// f/*a*/
////}
// @Filename: b.ts
// Case: Concrete class overrides concrete method
////class BBase {
//// foo(a: string, b: string): string {
//// return a + b;
//// }
////}
////
////class BSub extends BBase {
//// f/*b*/
////}
// @Filename: c.ts
// Case: Multiple overrides, concrete class overrides concrete method
////class CBase {
//// foo(a: string | number): string {
//// return a + "";
//// }
////}
////
////class CSub extends CBase {
//// foo(a: string): string {
//// return add;
//// }
////}
////
////class CSub2 extends CSub {
//// f/*c*/
////}
// @Filename: d.ts
// Case: Abstract class extends abstract class
////abstract class DBase {
//// abstract foo(a: string): string;
////}
////
////abstract class DSub extends DBase {
//// f/*d*/
////}
// format.setFormatOptions({
@@ -20,20 +62,87 @@ verify.completions({
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: true,
},
// exact: [
// ],
includes: [
{
name: "foo",
isRecommended: true,
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
isSnippet: true,
insertText:
"foo(param1: string, param2: boolean): Promise<void> {\r\n}",
}
],
});
verify.completions({
marker: "b",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: true,
},
includes: [
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
isSnippet: true,
insertText:
"foo(a: string, b: string): string {\r\n}",
}
],
});
verify.completions({
marker: "c",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: true,
},
includes: [
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
isSnippet: true,
insertText:
"foo(a: string): string {\r\n}",
}
],
});
verify.completions({
marker: "d",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: true,
},
includes: [
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
isSnippet: true,
insertText:
"abstract foo(a: string): string;", // Currently fails because no trailing semicolon
}
],
});