diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 3465ef333a0..562a1c6a206 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1515,7 +1515,9 @@ Actual: ${stringify(fullActual)}`); "argumentCount", ]; for (const key in options) { - ts.Debug.assert(ts.contains(allKeys, key)); + if (!ts.contains(allKeys, key)) { + ts.Debug.fail("Unexpected key " + key); + } } } diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 2884f7a7a0c..491c905785e 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -29,9 +29,12 @@ namespace ts.SignatureHelp { return undefined; } - if (shouldCarefullyCheckContext(triggerReason)) { - // In the middle of a string, don't provide signature help unless the user explicitly requested it. - if (isInString(sourceFile, position, startingToken)) { + // Only need to be careful if the user typed a character and signature help wasn't showing. + const shouldCarefullyCheckContext = !!triggerReason && triggerReason.kind === "characterTyped"; + + // Bail out quickly in the middle of a string or comment, don't provide signature help unless the user explicitly requested it. + if (shouldCarefullyCheckContext) { + if (isInString(sourceFile, position, startingToken) || isInComment(sourceFile, position)) { return undefined; } } @@ -41,8 +44,8 @@ namespace ts.SignatureHelp { cancellationToken.throwIfCancellationRequested(); - // Semantic filtering of signature help - const candidateInfo = getCandidateInfo(argumentInfo, typeChecker); + // Extra syntactic and semantic filtering of signature help + const candidateInfo = getCandidateInfo(argumentInfo, typeChecker, sourceFile, startingToken, shouldCarefullyCheckContext); cancellationToken.throwIfCancellationRequested(); if (!candidateInfo) { @@ -57,24 +60,57 @@ namespace ts.SignatureHelp { return typeChecker.runWithCancellationToken(cancellationToken, typeChecker => createSignatureHelpItems(candidateInfo.candidates, candidateInfo.resolvedSignature, argumentInfo, sourceFile, typeChecker)); } - function shouldCarefullyCheckContext(reason: SignatureHelpTriggerReason | undefined) { - // Only need to be careful if the user typed a character and signature help wasn't showing. - return !!reason && reason.kind === "characterTyped"; - } + function getCandidateInfo( + argumentInfo: ArgumentListInfo, checker: TypeChecker, sourceFile: SourceFile, startingToken: Node, onlyUseSyntacticOwners: boolean): + { readonly candidates: ReadonlyArray, readonly resolvedSignature: Signature } | undefined { - function getCandidateInfo(argumentInfo: ArgumentListInfo, checker: TypeChecker): { readonly candidates: ReadonlyArray, readonly resolvedSignature: Signature } | undefined { const { invocation } = argumentInfo; if (invocation.kind === InvocationKind.Call) { + if (onlyUseSyntacticOwners) { + if (isCallOrNewExpression(invocation.node)) { + const invocationChildren = invocation.node.getChildren(sourceFile); + switch (startingToken.kind) { + case SyntaxKind.OpenParenToken: + if (!contains(invocationChildren, startingToken)) { + return undefined; + } + break; + case SyntaxKind.CommaToken: + const containingList = findContainingList(startingToken); + if (!containingList || !contains(invocationChildren, findContainingList(startingToken))) { + return undefined; + } + break; + case SyntaxKind.LessThanToken: + if (!lessThanFollowsCalledExpression(startingToken, sourceFile, invocation.node.expression)) { + return undefined; + } + break; + default: + return undefined; + } + } + else { + return undefined; + } + } + const candidates: Signature[] = []; const resolvedSignature = checker.getResolvedSignature(invocation.node, candidates, argumentInfo.argumentCount)!; // TODO: GH#18217 return candidates.length === 0 ? undefined : { candidates, resolvedSignature }; } - else { + else if (invocation.kind === InvocationKind.TypeArgs) { + if (onlyUseSyntacticOwners && !lessThanFollowsCalledExpression(startingToken, sourceFile, invocation.called)) { + return undefined; + } const type = checker.getTypeAtLocation(invocation.called)!; // TODO: GH#18217 const signatures = isNewExpression(invocation.called.parent) ? type.getConstructSignatures() : type.getCallSignatures(); const candidates = signatures.filter(candidate => !!candidate.typeParameters && candidate.typeParameters.length >= argumentInfo.argumentCount); return candidates.length === 0 ? undefined : { candidates, resolvedSignature: first(candidates) }; } + else { + Debug.assertNever(invocation); + } } function createJavaScriptSignatureHelpItems(argumentInfo: ArgumentListInfo, program: Program, cancellationToken: CancellationToken): SignatureHelpItems | undefined { @@ -107,6 +143,14 @@ namespace ts.SignatureHelp { } } + function lessThanFollowsCalledExpression(startingToken: Node, sourceFile: SourceFile, calledExpression: Expression) { + const precedingToken = Debug.assertDefined( + findPrecedingToken(startingToken.getFullStart(), sourceFile, startingToken.parent, /*excludeJsdoc*/ true) + ); + + return rangeContainsRange(calledExpression, precedingToken); + } + export interface ArgumentInfoForCompletions { readonly invocation: CallLikeExpression; readonly argumentIndex: number; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 81cb387b73d..ddad4ca52a6 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -554,7 +554,6 @@ declare namespace FourSlashInterface { overloadsCount?: number; docComment?: string; text?: string; - name?: string; parameterName?: string; parameterSpan?: string; parameterDocComment?: string; diff --git a/tests/cases/fourslash/signatureHelpFilteredTriggerCharacters01.ts b/tests/cases/fourslash/signatureHelpFilteredTriggerCharacters01.ts deleted file mode 100644 index ed97ef020d0..00000000000 --- a/tests/cases/fourslash/signatureHelpFilteredTriggerCharacters01.ts +++ /dev/null @@ -1,23 +0,0 @@ -/// - -////function foo(x: T): T { -//// throw null; -////} -//// -////foo("/**/") - -goTo.marker(); -for (const triggerCharacter of ["<", "(", ","]) { - edit.insert(triggerCharacter); - verify.noSignatureHelpForTriggerReason({ - kind: "characterTyped", - triggerCharacter, - }); - verify.signatureHelpPresentForTriggerReason({ - kind: "retrigger", - triggerCharacter, - }); - edit.backspace(); -} -verify.signatureHelpPresentForTriggerReason(/*triggerReason*/ undefined); -verify.signatureHelpPresentForTriggerReason({ kind: "invoked" }); \ No newline at end of file diff --git a/tests/cases/fourslash/signatureHelpFilteredTriggers01.ts b/tests/cases/fourslash/signatureHelpFilteredTriggers01.ts new file mode 100644 index 00000000000..ebf5ba10f2d --- /dev/null +++ b/tests/cases/fourslash/signatureHelpFilteredTriggers01.ts @@ -0,0 +1,31 @@ +/// + +////function foo(x: T): T { +//// throw null; +////} +//// +////foo("/*1*/"); +////foo('/*2*/'); +////foo(` ${100}/*3*/`); +////foo(/* /*4*/ */); +////foo( +//// ///*5*/ +////); + +for (const marker of test.markers()) { + goTo.marker(marker); + for (const triggerCharacter of ["<", "(", ","]) { + edit.insert(triggerCharacter); + verify.noSignatureHelpForTriggerReason({ + kind: "characterTyped", + triggerCharacter, + }); + verify.signatureHelpPresentForTriggerReason({ + kind: "retrigger", + triggerCharacter, + }); + edit.backspace(); + } + verify.signatureHelpPresentForTriggerReason(/*triggerReason*/ undefined); + verify.signatureHelpPresentForTriggerReason({ kind: "invoked" }); +} diff --git a/tests/cases/fourslash/signatureHelpFilteredTriggers02.ts b/tests/cases/fourslash/signatureHelpFilteredTriggers02.ts new file mode 100644 index 00000000000..e193f919e05 --- /dev/null +++ b/tests/cases/fourslash/signatureHelpFilteredTriggers02.ts @@ -0,0 +1,36 @@ +/// + +////function foo(x: T): T { +//// throw null; +////} +//// +////foo(/*1*/""); +////foo(` ${100/*2*/}`); +////foo(/*3*/); +////foo(100 /*4*/) +////foo([/*5*/]) +////foo({ hello: "hello"/*6*/}) + +const charMap = { + 1: "(", + 2: ",", + 3: "(", + 4: "<", + 5: ",", + 6: ",", +} + +for (const markerName of Object.keys(charMap)) { + const triggerCharacter = charMap[markerName]; + goTo.marker(markerName); + edit.insert(triggerCharacter); + verify.noSignatureHelpForTriggerReason({ + kind: "characterTyped", + triggerCharacter, + }); + verify.signatureHelpPresentForTriggerReason({ + kind: "retrigger", + triggerCharacter, + }); + edit.backspace(triggerCharacter.length); +} diff --git a/tests/cases/fourslash/signatureHelpFilteredTriggers03.ts b/tests/cases/fourslash/signatureHelpFilteredTriggers03.ts new file mode 100644 index 00000000000..7b170823610 --- /dev/null +++ b/tests/cases/fourslash/signatureHelpFilteredTriggers03.ts @@ -0,0 +1,23 @@ +/// + +////declare class ViewJayEss { +//// constructor(obj: object); +////} +////new ViewJayEss({ +//// methods: { +//// sayHello/**/ +//// } +////}); + +goTo.marker(); +edit.insert("("); +verify.noSignatureHelpForTriggerReason({ + kind: "characterTyped", + triggerCharacter: "(", +}); + +edit.insert(") {},"); +verify.noSignatureHelpForTriggerReason({ + kind: "characterTyped", + triggerCharacter: ",", +}); diff --git a/tests/cases/fourslash/signatureHelpWithTriggers01.ts b/tests/cases/fourslash/signatureHelpWithTriggers01.ts new file mode 100644 index 00000000000..ae73b5fe000 --- /dev/null +++ b/tests/cases/fourslash/signatureHelpWithTriggers01.ts @@ -0,0 +1,31 @@ +/// + +////declare function foo(x: T, y: T): T; +//// +////foo/*1*//*2*/; +////foo(/*3*/100/*4*/); +////foo/*5*//*6*/(); + +const charMap = { + 1: "(", + 2: "<", + 3: ",", + 4: ",", + 5: "(", + 6: "<", +} + +for (const markerName of Object.keys(charMap)) { + const triggerCharacter = charMap[markerName]; + goTo.marker(markerName); + edit.insert(triggerCharacter); + verify.signatureHelpPresentForTriggerReason({ + kind: "characterTyped", + triggerCharacter, + }); + verify.signatureHelpPresentForTriggerReason({ + kind: "retrigger", + triggerCharacter, + }); + edit.backspace(triggerCharacter.length); +} diff --git a/tests/cases/fourslash/signatureHelpWithTriggers02.ts b/tests/cases/fourslash/signatureHelpWithTriggers02.ts new file mode 100644 index 00000000000..cb73237f8fd --- /dev/null +++ b/tests/cases/fourslash/signatureHelpWithTriggers02.ts @@ -0,0 +1,38 @@ +/// + +////declare function foo(x: T, y: T): T; +////declare function bar(x: U, y: U): U; +//// +////foo(bar/*1*/) + +goTo.marker("1"); + +edit.insert("("); +verify.signatureHelp({ + text: "bar(x: U, y: U): U", + triggerReason: { + kind: "characterTyped", + triggerCharacter: "(", + } +}); +edit.backspace(); + +edit.insert("<"); +verify.signatureHelp({ + text: "bar(x: U, y: U): U", + triggerReason: { + kind: "characterTyped", + triggerCharacter: "(", + } +}); +edit.backspace(); + +edit.insert(","); +verify.signatureHelp({ + text: "foo(x: (x: U, y: U) => U, y: (x: U, y: U) => U): (x: U, y: U) => U", + triggerReason: { + kind: "characterTyped", + triggerCharacter: "(", + } +}); +edit.backspace(); \ No newline at end of file