Skip to content

Commit fe2baac

Browse files
DanielRosenwasserDanielRosenwasser
authored
Merge pull request microsoft#25422 from Microsoft/actualSignatureHelpTriggers
Actual signature help trigger filtering
2 parents 10b174a + 9481faa commit fe2baac

9 files changed

Lines changed: 217 additions & 36 deletions

‎src/harness/fourslash.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,9 @@ Actual: ${stringify(fullActual)}`);
15151515
"argumentCount",
15161516
];
15171517
for (const key in options) {
1518-
ts.Debug.assert(ts.contains(allKeys, key));
1518+
if (!ts.contains(allKeys, key)) {
1519+
ts.Debug.fail("Unexpected key " + key);
1520+
}
15191521
}
15201522
}
15211523

‎src/services/signatureHelp.ts‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ namespace ts.SignatureHelp {
2929
return undefined;
3030
}
3131

32-
if (shouldCarefullyCheckContext(triggerReason)) {
33-
// In the middle of a string, don't provide signature help unless the user explicitly requested it.
34-
if (isInString(sourceFile, position, startingToken)) {
32+
// Only need to be careful if the user typed a character and signature help wasn't showing.
33+
const shouldCarefullyCheckContext = !!triggerReason && triggerReason.kind === "characterTyped";
34+
35+
// Bail out quickly in the middle of a string or comment, don't provide signature help unless the user explicitly requested it.
36+
if (shouldCarefullyCheckContext) {
37+
if (isInString(sourceFile, position, startingToken) || isInComment(sourceFile, position)) {
3538
return undefined;
3639
}
3740
}
@@ -41,8 +44,8 @@ namespace ts.SignatureHelp {
4144

4245
cancellationToken.throwIfCancellationRequested();
4346

44-
// Semantic filtering of signature help
45-
const candidateInfo = getCandidateInfo(argumentInfo, typeChecker);
47+
// Extra syntactic and semantic filtering of signature help
48+
const candidateInfo = getCandidateInfo(argumentInfo, typeChecker, sourceFile, startingToken, shouldCarefullyCheckContext);
4649
cancellationToken.throwIfCancellationRequested();
4750

4851
if (!candidateInfo) {
@@ -57,24 +60,57 @@ namespace ts.SignatureHelp {
5760
return typeChecker.runWithCancellationToken(cancellationToken, typeChecker => createSignatureHelpItems(candidateInfo.candidates, candidateInfo.resolvedSignature, argumentInfo, sourceFile, typeChecker));
5861
}
5962

60-
function shouldCarefullyCheckContext(reason: SignatureHelpTriggerReason | undefined) {
61-
// Only need to be careful if the user typed a character and signature help wasn't showing.
62-
return !!reason && reason.kind === "characterTyped";
63-
}
63+
function getCandidateInfo(
64+
argumentInfo: ArgumentListInfo, checker: TypeChecker, sourceFile: SourceFile, startingToken: Node, onlyUseSyntacticOwners: boolean):
65+
{ readonly candidates: ReadonlyArray<Signature>, readonly resolvedSignature: Signature } | undefined {
6466

65-
function getCandidateInfo(argumentInfo: ArgumentListInfo, checker: TypeChecker): { readonly candidates: ReadonlyArray<Signature>, readonly resolvedSignature: Signature } | undefined {
6667
const { invocation } = argumentInfo;
6768
if (invocation.kind === InvocationKind.Call) {
69+
if (onlyUseSyntacticOwners) {
70+
if (isCallOrNewExpression(invocation.node)) {
71+
const invocationChildren = invocation.node.getChildren(sourceFile);
72+
switch (startingToken.kind) {
73+
case SyntaxKind.OpenParenToken:
74+
if (!contains(invocationChildren, startingToken)) {
75+
return undefined;
76+
}
77+
break;
78+
case SyntaxKind.CommaToken:
79+
const containingList = findContainingList(startingToken);
80+
if (!containingList || !contains(invocationChildren, findContainingList(startingToken))) {
81+
return undefined;
82+
}
83+
break;
84+
case SyntaxKind.LessThanToken:
85+
if (!lessThanFollowsCalledExpression(startingToken, sourceFile, invocation.node.expression)) {
86+
return undefined;
87+
}
88+
break;
89+
default:
90+
return undefined;
91+
}
92+
}
93+
else {
94+
return undefined;
95+
}
96+
}
97+
6898
const candidates: Signature[] = [];
6999
const resolvedSignature = checker.getResolvedSignature(invocation.node, candidates, argumentInfo.argumentCount)!; // TODO: GH#18217
70100
return candidates.length === 0 ? undefined : { candidates, resolvedSignature };
71101
}
72-
else {
102+
else if (invocation.kind === InvocationKind.TypeArgs) {
103+
if (onlyUseSyntacticOwners && !lessThanFollowsCalledExpression(startingToken, sourceFile, invocation.called)) {
104+
return undefined;
105+
}
73106
const type = checker.getTypeAtLocation(invocation.called)!; // TODO: GH#18217
74107
const signatures = isNewExpression(invocation.called.parent) ? type.getConstructSignatures() : type.getCallSignatures();
75108
const candidates = signatures.filter(candidate => !!candidate.typeParameters && candidate.typeParameters.length >= argumentInfo.argumentCount);
76109
return candidates.length === 0 ? undefined : { candidates, resolvedSignature: first(candidates) };
77110
}
111+
else {
112+
Debug.assertNever(invocation);
113+
}
78114
}
79115

80116
function createJavaScriptSignatureHelpItems(argumentInfo: ArgumentListInfo, program: Program, cancellationToken: CancellationToken): SignatureHelpItems | undefined {
@@ -107,6 +143,14 @@ namespace ts.SignatureHelp {
107143
}
108144
}
109145

146+
function lessThanFollowsCalledExpression(startingToken: Node, sourceFile: SourceFile, calledExpression: Expression) {
147+
const precedingToken = Debug.assertDefined(
148+
findPrecedingToken(startingToken.getFullStart(), sourceFile, startingToken.parent, /*excludeJsdoc*/ true)
149+
);
150+
151+
return rangeContainsRange(calledExpression, precedingToken);
152+
}
153+
110154
export interface ArgumentInfoForCompletions {
111155
readonly invocation: CallLikeExpression;
112156
readonly argumentIndex: number;

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,6 @@ declare namespace FourSlashInterface {
554554
overloadsCount?: number;
555555
docComment?: string;
556556
text?: string;
557-
name?: string;
558557
parameterName?: string;
559558
parameterSpan?: string;
560559
parameterDocComment?: string;

‎tests/cases/fourslash/signatureHelpFilteredTriggerCharacters01.ts‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎tests/cases/fourslash/signatureHelpFilteredTriggers01.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////function foo<T>(x: T): T {
4+
//// throw null;
5+
////}
6+
////
7+
////foo("/*1*/");
8+
////foo('/*2*/');
9+
////foo(` ${100}/*3*/`);
10+
////foo(/* /*4*/ */);
11+
////foo(
12+
//// ///*5*/
13+
////);
14+
15+
for (const marker of test.markers()) {
16+
goTo.marker(marker);
17+
for (const triggerCharacter of ["<", "(", ","]) {
18+
edit.insert(triggerCharacter);
19+
verify.noSignatureHelpForTriggerReason({
20+
kind: "characterTyped",
21+
triggerCharacter,
22+
});
23+
verify.signatureHelpPresentForTriggerReason({
24+
kind: "retrigger",
25+
triggerCharacter,
26+
});
27+
edit.backspace();
28+
}
29+
verify.signatureHelpPresentForTriggerReason(/*triggerReason*/ undefined);
30+
verify.signatureHelpPresentForTriggerReason({ kind: "invoked" });
31+
}

‎tests/cases/fourslash/signatureHelpFilteredTriggers02.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////function foo<T>(x: T): T {
4+
//// throw null;
5+
////}
6+
////
7+
////foo(/*1*/"");
8+
////foo(` ${100/*2*/}`);
9+
////foo(/*3*/);
10+
////foo(100 /*4*/)
11+
////foo([/*5*/])
12+
////foo({ hello: "hello"/*6*/})
13+
14+
const charMap = {
15+
1: "(",
16+
2: ",",
17+
3: "(",
18+
4: "<",
19+
5: ",",
20+
6: ",",
21+
}
22+
23+
for (const markerName of Object.keys(charMap)) {
24+
const triggerCharacter = charMap[markerName];
25+
goTo.marker(markerName);
26+
edit.insert(triggerCharacter);
27+
verify.noSignatureHelpForTriggerReason({
28+
kind: "characterTyped",
29+
triggerCharacter,
30+
});
31+
verify.signatureHelpPresentForTriggerReason({
32+
kind: "retrigger",
33+
triggerCharacter,
34+
});
35+
edit.backspace(triggerCharacter.length);
36+
}

‎tests/cases/fourslash/signatureHelpFilteredTriggers03.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////declare class ViewJayEss {
4+
//// constructor(obj: object);
5+
////}
6+
////new ViewJayEss({
7+
//// methods: {
8+
//// sayHello/**/
9+
//// }
10+
////});
11+
12+
goTo.marker();
13+
edit.insert("(");
14+
verify.noSignatureHelpForTriggerReason({
15+
kind: "characterTyped",
16+
triggerCharacter: "(",
17+
});
18+
19+
edit.insert(") {},");
20+
verify.noSignatureHelpForTriggerReason({
21+
kind: "characterTyped",
22+
triggerCharacter: ",",
23+
});

‎tests/cases/fourslash/signatureHelpWithTriggers01.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////declare function foo<T>(x: T, y: T): T;
4+
////
5+
////foo/*1*//*2*/;
6+
////foo(/*3*/100/*4*/);
7+
////foo/*5*//*6*/();
8+
9+
const charMap = {
10+
1: "(",
11+
2: "<",
12+
3: ",",
13+
4: ",",
14+
5: "(",
15+
6: "<",
16+
}
17+
18+
for (const markerName of Object.keys(charMap)) {
19+
const triggerCharacter = charMap[markerName];
20+
goTo.marker(markerName);
21+
edit.insert(triggerCharacter);
22+
verify.signatureHelpPresentForTriggerReason({
23+
kind: "characterTyped",
24+
triggerCharacter,
25+
});
26+
verify.signatureHelpPresentForTriggerReason({
27+
kind: "retrigger",
28+
triggerCharacter,
29+
});
30+
edit.backspace(triggerCharacter.length);
31+
}

‎tests/cases/fourslash/signatureHelpWithTriggers02.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////declare function foo<T>(x: T, y: T): T;
4+
////declare function bar<U>(x: U, y: U): U;
5+
////
6+
////foo(bar/*1*/)
7+
8+
goTo.marker("1");
9+
10+
edit.insert("(");
11+
verify.signatureHelp({
12+
text: "bar<U>(x: U, y: U): U",
13+
triggerReason: {
14+
kind: "characterTyped",
15+
triggerCharacter: "(",
16+
}
17+
});
18+
edit.backspace();
19+
20+
edit.insert("<");
21+
verify.signatureHelp({
22+
text: "bar<U>(x: U, y: U): U",
23+
triggerReason: {
24+
kind: "characterTyped",
25+
triggerCharacter: "(",
26+
}
27+
});
28+
edit.backspace();
29+
30+
edit.insert(",");
31+
verify.signatureHelp({
32+
text: "foo(x: <U>(x: U, y: U) => U, y: <U>(x: U, y: U) => U): <U>(x: U, y: U) => U",
33+
triggerReason: {
34+
kind: "characterTyped",
35+
triggerCharacter: "(",
36+
}
37+
});
38+
edit.backspace();

0 commit comments

Comments
 (0)