Skip to content

Commit 5f7406e

Browse files
gabrittoandrewbranch
authored andcommitted
Add method signature completions (microsoft#46370)
* prototype creation for method override completion snippet * WIP: start using codefix `addNewNodeForMemberSymbol` to create method decl node * update type of addNewNodeForMemberSymbol * add more tests and support more cases * add more tests and fix some details * wip: more fixes and tests * expose check override modifier in checker * fix test * WIP: add snippet support * WIP: snippet support on emitter, adding snippets in completions * make add snippets work with overloads (not synced) * fix snippet adding * rebase * WIP: try to add snippet escaping in emitter * support escaping in snippets * small fixes; fixed tests * more tests and fixes * fix new tests * fix modifier inheritance for overloads * merge conflict fixes; remove comments * throw error if setOptions is called but not implemented * fix newline handling * fix weird stuff * fix tests * fix more tests * Fix unbound host.getNewLine * fix isParameterDeclaration changes * rename diagnostic to status and remove snippets from public api * rename emitter functions + fix indentation * check completion kind before calling isclasslikemembercompletion * fix missing type parameters * Revert "fix missing type parameters" This reverts commit 7bdeaa8. * add isAmbient flag to addNewNodeForMemberSymbol * add test for abstract overloads * refactor snippet escaping support * add user preference flag for enabling class member snippets * update API baseline * update tabstop order Co-authored-by: Andrew Branch <[email protected]>
1 parent c75f69c commit 5f7406e

28 files changed

+1457
-81
lines changed

src/compiler/checker.ts

+163-35
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ namespace ts {
729729
isDeclarationVisible,
730730
isPropertyAccessible,
731731
getTypeOnlyAliasDeclaration,
732+
getMemberOverrideModifierStatus,
732733
};
733734

734735
function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
@@ -38469,7 +38470,7 @@ namespace ts {
3846938470
}
3847038471
}
3847138472

38472-
checkMembersForMissingOverrideModifier(node, type, typeWithThis, staticType);
38473+
checkMembersForOverrideModifier(node, type, typeWithThis, staticType);
3847338474

3847438475
const implementedTypeNodes = getEffectiveImplementsTypeNodes(node);
3847538476
if (implementedTypeNodes) {
@@ -38506,8 +38507,7 @@ namespace ts {
3850638507
}
3850738508
}
3850838509

38509-
function checkMembersForMissingOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type, staticType: ObjectType) {
38510-
const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
38510+
function checkMembersForOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type, staticType: ObjectType) {
3851138511
const baseTypeNode = getEffectiveBaseTypeNode(node);
3851238512
const baseTypes = baseTypeNode && getBaseTypes(type);
3851338513
const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;
@@ -38521,77 +38521,163 @@ namespace ts {
3852138521
if (isConstructorDeclaration(member)) {
3852238522
forEach(member.parameters, param => {
3852338523
if (isParameterPropertyDeclaration(param, member)) {
38524-
checkClassMember(param, /*memberIsParameterProperty*/ true);
38524+
checkExistingMemberForOverrideModifier(
38525+
node,
38526+
staticType,
38527+
baseStaticType,
38528+
baseWithThis,
38529+
type,
38530+
typeWithThis,
38531+
param,
38532+
/* memberIsParameterProperty */ true
38533+
);
3852538534
}
3852638535
});
3852738536
}
38528-
checkClassMember(member);
38537+
checkExistingMemberForOverrideModifier(
38538+
node,
38539+
staticType,
38540+
baseStaticType,
38541+
baseWithThis,
38542+
type,
38543+
typeWithThis,
38544+
member,
38545+
/* memberIsParameterProperty */ false,
38546+
);
3852938547
}
38548+
}
3853038549

38531-
function checkClassMember(member: ClassElement | ParameterPropertyDeclaration, memberIsParameterProperty?: boolean) {
38532-
const hasOverride = hasOverrideModifier(member);
38533-
const hasStatic = isStatic(member);
38534-
const isJs = isInJSFile(member);
38535-
if (baseWithThis && (hasOverride || compilerOptions.noImplicitOverride)) {
38536-
const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
38537-
if (!declaredProp) {
38538-
return;
38539-
}
38540-
38541-
const thisType = hasStatic ? staticType : typeWithThis;
38542-
const baseType = hasStatic ? baseStaticType : baseWithThis;
38543-
const prop = getPropertyOfType(thisType, declaredProp.escapedName);
38544-
const baseProp = getPropertyOfType(baseType, declaredProp.escapedName);
38550+
/**
38551+
* @param member Existing member node to be checked.
38552+
* Note: `member` cannot be a synthetic node.
38553+
*/
38554+
function checkExistingMemberForOverrideModifier(
38555+
node: ClassLikeDeclaration,
38556+
staticType: ObjectType,
38557+
baseStaticType: Type,
38558+
baseWithThis: Type | undefined,
38559+
type: InterfaceType,
38560+
typeWithThis: Type,
38561+
member: ClassElement | ParameterPropertyDeclaration,
38562+
memberIsParameterProperty: boolean,
38563+
reportErrors = true,
38564+
): MemberOverrideStatus {
38565+
const declaredProp = member.name
38566+
&& getSymbolAtLocation(member.name)
38567+
|| getSymbolAtLocation(member);
38568+
if (!declaredProp) {
38569+
return MemberOverrideStatus.Ok;
38570+
}
38571+
38572+
return checkMemberForOverrideModifier(
38573+
node,
38574+
staticType,
38575+
baseStaticType,
38576+
baseWithThis,
38577+
type,
38578+
typeWithThis,
38579+
hasOverrideModifier(member),
38580+
hasAbstractModifier(member),
38581+
isStatic(member),
38582+
memberIsParameterProperty,
38583+
symbolName(declaredProp),
38584+
reportErrors ? member : undefined,
38585+
);
38586+
}
3854538587

38546-
const baseClassName = typeToString(baseWithThis);
38547-
if (prop && !baseProp && hasOverride) {
38548-
const suggestion = getSuggestedSymbolForNonexistentClassMember(symbolName(declaredProp), baseType);
38588+
/**
38589+
* Checks a class member declaration for either a missing or an invalid `override` modifier.
38590+
* Note: this function can be used for speculative checking,
38591+
* i.e. checking a member that does not yet exist in the program.
38592+
* An example of that would be to call this function in a completions scenario,
38593+
* when offering a method declaration as completion.
38594+
* @param errorNode The node where we should report an error, or undefined if we should not report errors.
38595+
*/
38596+
function checkMemberForOverrideModifier(
38597+
node: ClassLikeDeclaration,
38598+
staticType: ObjectType,
38599+
baseStaticType: Type,
38600+
baseWithThis: Type | undefined,
38601+
type: InterfaceType,
38602+
typeWithThis: Type,
38603+
memberHasOverrideModifier: boolean,
38604+
memberHasAbstractModifier: boolean,
38605+
memberIsStatic: boolean,
38606+
memberIsParameterProperty: boolean,
38607+
memberName: string,
38608+
errorNode?: Node,
38609+
): MemberOverrideStatus {
38610+
const isJs = isInJSFile(node);
38611+
const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
38612+
if (baseWithThis && (memberHasOverrideModifier || compilerOptions.noImplicitOverride)) {
38613+
const memberEscapedName = escapeLeadingUnderscores(memberName);
38614+
const thisType = memberIsStatic ? staticType : typeWithThis;
38615+
const baseType = memberIsStatic ? baseStaticType : baseWithThis;
38616+
const prop = getPropertyOfType(thisType, memberEscapedName);
38617+
const baseProp = getPropertyOfType(baseType, memberEscapedName);
38618+
38619+
const baseClassName = typeToString(baseWithThis);
38620+
if (prop && !baseProp && memberHasOverrideModifier) {
38621+
if (errorNode) {
38622+
const suggestion = getSuggestedSymbolForNonexistentClassMember(memberName, baseType); // Again, using symbol name: note that's different from `symbol.escapedName`
3854938623
suggestion ?
3855038624
error(
38551-
member,
38625+
errorNode,
3855238626
isJs ?
3855338627
Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_it_is_not_declared_in_the_base_class_0_Did_you_mean_1 :
3855438628
Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0_Did_you_mean_1,
3855538629
baseClassName,
3855638630
symbolToString(suggestion)) :
3855738631
error(
38558-
member,
38632+
errorNode,
3855938633
isJs ?
3856038634
Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_it_is_not_declared_in_the_base_class_0 :
3856138635
Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0,
3856238636
baseClassName);
3856338637
}
38564-
else if (prop && baseProp?.declarations && compilerOptions.noImplicitOverride && !nodeInAmbientContext) {
38565-
const baseHasAbstract = some(baseProp.declarations, hasAbstractModifier);
38566-
if (hasOverride) {
38567-
return;
38568-
}
38638+
return MemberOverrideStatus.HasInvalidOverride;
38639+
}
38640+
else if (prop && baseProp?.declarations && compilerOptions.noImplicitOverride && !nodeInAmbientContext) {
38641+
const baseHasAbstract = some(baseProp.declarations, hasAbstractModifier);
38642+
if (memberHasOverrideModifier) {
38643+
return MemberOverrideStatus.Ok;
38644+
}
3856938645

38570-
if (!baseHasAbstract) {
38646+
if (!baseHasAbstract) {
38647+
if (errorNode) {
3857138648
const diag = memberIsParameterProperty ?
3857238649
isJs ?
3857338650
Diagnostics.This_parameter_property_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0 :
3857438651
Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0 :
3857538652
isJs ?
3857638653
Diagnostics.This_member_must_have_a_JSDoc_comment_with_an_override_tag_because_it_overrides_a_member_in_the_base_class_0 :
3857738654
Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0;
38578-
error(member, diag, baseClassName);
38655+
error(errorNode, diag, baseClassName);
3857938656
}
38580-
else if (hasAbstractModifier(member) && baseHasAbstract) {
38581-
error(member, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0, baseClassName);
38657+
return MemberOverrideStatus.NeedsOverride;
38658+
}
38659+
else if (memberHasAbstractModifier && baseHasAbstract) {
38660+
if (errorNode) {
38661+
error(errorNode, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0, baseClassName);
3858238662
}
38663+
return MemberOverrideStatus.NeedsOverride;
3858338664
}
3858438665
}
38585-
else if (hasOverride) {
38666+
}
38667+
else if (memberHasOverrideModifier) {
38668+
if (errorNode) {
3858638669
const className = typeToString(type);
3858738670
error(
38588-
member,
38671+
errorNode,
3858938672
isJs ?
3859038673
Diagnostics.This_member_cannot_have_a_JSDoc_comment_with_an_override_tag_because_its_containing_class_0_does_not_extend_another_class :
3859138674
Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class,
3859238675
className);
3859338676
}
38677+
return MemberOverrideStatus.HasInvalidOverride;
3859438678
}
38679+
38680+
return MemberOverrideStatus.Ok;
3859538681
}
3859638682

3859738683
function issueMemberSpecificError(node: ClassLikeDeclaration, typeWithThis: Type, baseWithThis: Type, broadDiag: DiagnosticMessage) {
@@ -38638,6 +38724,48 @@ namespace ts {
3863838724
}
3863938725
}
3864038726

38727+
/**
38728+
* Checks a member declaration node to see if has a missing or invalid `override` modifier.
38729+
* @param node Class-like node where the member is declared.
38730+
* @param member Member declaration node.
38731+
* Note: `member` can be a synthetic node without a parent.
38732+
*/
38733+
function getMemberOverrideModifierStatus(node: ClassLikeDeclaration, member: ClassElement): MemberOverrideStatus {
38734+
if (!member.name) {
38735+
return MemberOverrideStatus.Ok;
38736+
}
38737+
38738+
const symbol = getSymbolOfNode(node);
38739+
const type = getDeclaredTypeOfSymbol(symbol) as InterfaceType;
38740+
const typeWithThis = getTypeWithThisArgument(type);
38741+
const staticType = getTypeOfSymbol(symbol) as ObjectType;
38742+
38743+
const baseTypeNode = getEffectiveBaseTypeNode(node);
38744+
const baseTypes = baseTypeNode && getBaseTypes(type);
38745+
const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;
38746+
const baseStaticType = getBaseConstructorTypeOfClass(type);
38747+
38748+
const memberHasOverrideModifier = member.parent
38749+
? hasOverrideModifier(member)
38750+
: hasSyntacticModifier(member, ModifierFlags.Override);
38751+
38752+
const memberName = unescapeLeadingUnderscores(getTextOfPropertyName(member.name));
38753+
38754+
return checkMemberForOverrideModifier(
38755+
node,
38756+
staticType,
38757+
baseStaticType,
38758+
baseWithThis,
38759+
type,
38760+
typeWithThis,
38761+
memberHasOverrideModifier,
38762+
hasAbstractModifier(member),
38763+
isStatic(member),
38764+
/* memberIsParameterProperty */ false,
38765+
memberName,
38766+
);
38767+
}
38768+
3864138769
function getTargetSymbol(s: Symbol) {
3864238770
// if symbol is instantiated its flags are not copied from the 'target'
3864338771
// so we'll need to get back original 'target' symbol to work with correct set of flags

src/compiler/debug.ts

+4
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,10 @@ namespace ts {
351351
return formatEnum(kind, (ts as any).SyntaxKind, /*isFlags*/ false);
352352
}
353353

354+
export function formatSnippetKind(kind: SnippetKind | undefined): string {
355+
return formatEnum(kind, (ts as any).SnippetKind, /*isFlags*/ false);
356+
}
357+
354358
export function formatNodeFlags(flags: NodeFlags | undefined): string {
355359
return formatEnum(flags, (ts as any).NodeFlags, /*isFlags*/ true);
356360
}

src/compiler/emitter.ts

+43-1
Original file line numberDiff line numberDiff line change
@@ -1283,7 +1283,13 @@ namespace ts {
12831283
currentParenthesizerRule = undefined;
12841284
}
12851285

1286-
function pipelineEmitWithHintWorker(hint: EmitHint, node: Node): void {
1286+
function pipelineEmitWithHintWorker(hint: EmitHint, node: Node, allowSnippets = true): void {
1287+
if (allowSnippets) {
1288+
const snippet = getSnippetElement(node);
1289+
if (snippet) {
1290+
return emitSnippetNode(hint, node, snippet);
1291+
}
1292+
}
12871293
if (hint === EmitHint.SourceFile) return emitSourceFile(cast(node, isSourceFile));
12881294
if (hint === EmitHint.IdentifierName) return emitIdentifier(cast(node, isIdentifier));
12891295
if (hint === EmitHint.JsxAttributeValue) return emitLiteral(cast(node, isStringLiteral), /*jsxAttributeEscape*/ true);
@@ -1924,6 +1930,32 @@ namespace ts {
19241930
}
19251931
}
19261932

1933+
//
1934+
// Snippet Elements
1935+
//
1936+
1937+
function emitSnippetNode(hint: EmitHint, node: Node, snippet: SnippetElement) {
1938+
switch (snippet.kind) {
1939+
case SnippetKind.Placeholder:
1940+
emitPlaceholder(hint, node, snippet);
1941+
break;
1942+
case SnippetKind.TabStop:
1943+
emitTabStop(snippet);
1944+
break;
1945+
}
1946+
}
1947+
1948+
function emitPlaceholder(hint: EmitHint, node: Node, snippet: Placeholder) {
1949+
nonEscapingWrite(`\$\{${snippet.order}:`); // `${2:`
1950+
pipelineEmitWithHintWorker(hint, node, /*allowSnippets*/ false); // `...`
1951+
nonEscapingWrite(`\}`); // `}`
1952+
// `${2:...}`
1953+
}
1954+
1955+
function emitTabStop(snippet: TabStop) {
1956+
nonEscapingWrite(`\$${snippet.order}`);
1957+
}
1958+
19271959
//
19281960
// Identifiers
19291961
//
@@ -4457,6 +4489,16 @@ namespace ts {
44574489
writer.writeProperty(s);
44584490
}
44594491

4492+
function nonEscapingWrite(s: string) {
4493+
// This should be defined in a snippet-escaping text writer.
4494+
if (writer.nonEscapingWrite) {
4495+
writer.nonEscapingWrite(s);
4496+
}
4497+
else {
4498+
writer.write(s);
4499+
}
4500+
}
4501+
44604502
function writeLine(count = 1) {
44614503
for (let i = 0; i < count; i++) {
44624504
writer.writeLine(i > 0);

src/compiler/factory/emitNode.ts

+18
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,24 @@ namespace ts {
256256
}
257257
}
258258

259+
/**
260+
* Gets the SnippetElement of a node.
261+
*/
262+
/* @internal */
263+
export function getSnippetElement(node: Node): SnippetElement | undefined {
264+
return node.emitNode?.snippetElement;
265+
}
266+
267+
/**
268+
* Sets the SnippetElement of a node.
269+
*/
270+
/* @internal */
271+
export function setSnippetElement<T extends Node>(node: T, snippet: SnippetElement): T {
272+
const emitNode = getOrCreateEmitNode(node);
273+
emitNode.snippetElement = snippet;
274+
return node;
275+
}
276+
259277
/* @internal */
260278
export function ignoreSourceNewlines<T extends Node>(node: T): T {
261279
getOrCreateEmitNode(node).flags |= EmitFlags.IgnoreSourceNewlines;

0 commit comments

Comments
 (0)