Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add inlay hints support #42089

Merged
merged 57 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
cd99389
Add signature arguments label support
Kingwl Dec 23, 2020
ab3c937
Support rest parameters and destruction
Kingwl Dec 24, 2020
d91b2f7
make lint
Kingwl Dec 24, 2020
8388add
Fix tuple rest parameters
Kingwl Dec 24, 2020
1058fdd
Adjust name styles
Kingwl Dec 24, 2020
1517b1f
Rename to inline hints
Kingwl Dec 24, 2020
fd9b09f
Partition inline hints
Kingwl Dec 25, 2020
23afce2
Adjust range pred
Kingwl Dec 25, 2020
ee6527e
Add function expression like hints
Kingwl Dec 25, 2020
679b58d
Support configure inline hints
Kingwl Dec 25, 2020
446bee4
Display hints in single line
Kingwl Dec 25, 2020
d2fbd1e
Add test suits and tests
Kingwl Dec 27, 2020
c4abb87
Add range tests
Kingwl Dec 28, 2020
a9e007a
Support more hints
Kingwl Dec 29, 2020
df62a11
Add more options
Kingwl Dec 30, 2020
fce2619
Fix logical
Kingwl Dec 30, 2020
679f066
Add more cases
Kingwl Dec 30, 2020
eb4b4ad
Support call chains
Kingwl Dec 30, 2020
9297051
Rename options
Kingwl Dec 30, 2020
467b5cd
Match lastest protocol
Kingwl Jan 13, 2021
e5ca31b
Update protocol changes
Kingwl Jan 14, 2021
37a7089
Support context value and hover message
Kingwl Jan 16, 2021
2ccfc98
Revert "Support context value and hover message"
Kingwl Jan 21, 2021
0e5f223
Revert "Update protocol changes"
Kingwl Jan 21, 2021
7197d0d
Add hover message
Kingwl Jan 21, 2021
e785943
Accept baseline
Kingwl Jan 21, 2021
637c7f8
Update src/services/inlineHints.ts
Kingwl Feb 4, 2021
b3c3e7e
Update src/services/inlineHints.ts
Kingwl Feb 4, 2021
7948cec
Merge branch 'master' into signature_arguments_labels_support
Kingwl Feb 4, 2021
a374417
Cache across the program
Kingwl Feb 4, 2021
7ac2a35
Merge branch 'master' into signature_arguments_labels_support
Kingwl May 7, 2021
5767d7e
Fix possible undefined
Kingwl May 7, 2021
d7d72d6
Update protocol changes
Kingwl Jun 3, 2021
9306d72
Fix missing property
Kingwl Jun 3, 2021
5cab46f
Merge branch 'main' into signature_arguments_labels_support
Kingwl Jun 3, 2021
2750c6b
Make lint happy
Kingwl Jun 4, 2021
0090962
Avoid call chain hints
Kingwl Jun 10, 2021
f771afc
I'm bad
Kingwl Jun 10, 2021
67dcbb0
Add whitespace before type
Kingwl Jun 10, 2021
957756c
Add more tests
Kingwl Jun 10, 2021
db4135b
Should care about jsdoc
Kingwl Jun 10, 2021
b74af7c
Support complex rest parameter
Kingwl Jun 10, 2021
388cc6d
Avoid module symbol hints
Kingwl Jun 14, 2021
f5695a1
Care about leading comments
Kingwl Jun 14, 2021
6336803
Fix CR issues
Kingwl Jun 15, 2021
09cdf37
Avoid changes
Kingwl Jun 15, 2021
71bae5e
Simplify comments contains
Kingwl Jun 15, 2021
cba4dc3
Fix CR issues
Kingwl Jun 18, 2021
0e8cdb6
Accept baseline
Kingwl Jun 18, 2021
cc06dbc
Merge branch 'main' into signature_arguments_labels_support
Kingwl Jun 18, 2021
e8fef30
Check parameter name before create regex
Kingwl Jun 18, 2021
d8dc8f1
Rename option
Kingwl Jun 24, 2021
52c9d5c
Avoid makers
Kingwl Jun 24, 2021
7ce7a44
Skip parens for argument
Kingwl Jun 24, 2021
24e1a4a
Fix CR issues
Kingwl Jun 24, 2021
6b279e7
Fix enums
Kingwl Jun 25, 2021
8e9a8d8
Accept baseline
Kingwl Jun 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ namespace ts {
return node ? getTypeFromTypeNode(node) : errorType;
},
getParameterType: getTypeAtPosition,
getParameterIdentifierNameAtPosition,
getPromisedTypeOfPromise,
getAwaitedType: type => getAwaitedType(type),
getReturnTypeOfSignature,
Expand Down Expand Up @@ -30336,6 +30337,39 @@ namespace ts {
return restParameter.escapedName;
}

function getParameterIdentifierNameAtPosition(signature: Signature, pos: number): [__String, boolean] | undefined {
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
const paramCount = signature.parameters.length - (signatureHasRestParameter(signature) ? 1 : 0);
if (pos < paramCount) {
const param = signature.parameters[pos];
return isParameterDeclarationWithIdentifierName(param) ? [param.escapedName, false] : undefined;
}

const restParameter = signature.parameters[paramCount] || unknownSymbol;
if (!isParameterDeclarationWithIdentifierName(restParameter)) {
return undefined;
}

const restType = getTypeOfSymbol(restParameter);
if (isTupleType(restType)) {
const associatedNames = ((restType as TypeReference).target as TupleType).labeledElementDeclarations;
const index = pos - paramCount;
const associatedName = associatedNames?.[index];
const isRestTupleElement = !!associatedName?.dotDotDotToken;
return associatedName ? [
getTupleElementLabel(associatedName),
isRestTupleElement
] : undefined;
}

if (pos === paramCount) {
return [restParameter.escapedName, true];
}
return undefined;
}

function isParameterDeclarationWithIdentifierName(symbol: Symbol) {
return symbol.valueDeclaration && isParameter(symbol.valueDeclaration) && isIdentifier(symbol.valueDeclaration.name);
}
function isValidDeclarationForTupleLabel(d: Declaration): d is NamedTupleMember | (ParameterDeclaration & { name: Identifier }) {
return d.kind === SyntaxKind.NamedTupleMember || (isParameter(d) && d.name && isIdentifier(d.name));
}
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4100,6 +4100,7 @@ namespace ts {
* Returns `any` if the index is not valid.
*/
/* @internal */ getParameterType(signature: Signature, parameterIndex: number): Type;
/* @internal */ getParameterIdentifierNameAtPosition(signature: Signature, parameterIndex: number): [__String, boolean] | undefined;
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
getNullableType(type: Type, flags: TypeFlags): Type;
getNonNullableType(type: Type): Type;
/* @internal */ getNonOptionalType(type: Type): Type;
Expand Down Expand Up @@ -8462,6 +8463,14 @@ namespace ts {
readonly providePrefixAndSuffixTextForRename?: boolean;
readonly includePackageJsonAutoImports?: "auto" | "on" | "off";
readonly provideRefactorNotApplicableReason?: boolean;
readonly includeInlayParameterNameHints?: boolean;
readonly includeInlayNonLiteralParameterNameHints?: boolean;
readonly includeInlayDuplicatedParameterNameHints?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are “duplicated” parameter name hints? Do we need all three of these options? What happens if I have includeInlayParameterNameHints disabled but the other two enabled? (Sorry if these questions have already been addressed.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review.

What are “duplicated” parameter name hints?

The argument is an identifier and it's same as the parameter name.
I'm not sure how to accurately describe this behavior. Any suggestions about the naming?

function foo (a: number) {}
var a = 1
foo(a) 

Do we need all three of these options?

here's some context. I'm ok to remove them.

disabled but the other two enabled?

There's no hints will be provided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like the format that Cyrus showed where it was “suppress hints when ...” because it makes it more clear that those options won’t do anything if includeInlayParameterNameHints is disabled. Also, does VS Code let us show a nested/hierarchical checkbox list like that screenshot? I don’t think so, but that would be really nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suppress hints when ...

I guess that's the description what UI shows. Not about the implementations.
Another problem is VSCode would not open new feature's options by default.
If the option is suppress something, the default behavior is not suppress something, that is include/provide something. (If I'm correct).

I don’t think so, but that would be really nice.

Sadly it's not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

  • includeInlay [xxxxx] HintsWhen [yyyyy]
  • include [xxxxx]InlayHintsWhen [yyyyy]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my ideal scenario would be

includeInlayParameterNameHints?: "none" | "literals" | "all";
suppressInlayParameterNameHintsWhenArgumentMatchesName?: boolean;

and the latter can default to true both in VS Code and in TS Server. But I’m ok with swapping it to includeInlayParameterNameHintsWhenArgumentMatchesName in the options, and maybe trying to use “suppress” language in the VS Code UI and invert the value sent. I just really don’t want the options UI to allow for a bunch of checkboxes that sound like they should enable something but actually do nothing because they’re really sub-options of includeInlayParameterNameHints.

readonly includeInlayFunctionParameterTypeHints?: boolean;
readonly includeInlayVariableTypeHints?: boolean;
readonly includeInlayPropertyDeclarationTypeHints?: boolean;
readonly includeInlayFunctionLikeReturnTypeHints?: boolean;
readonly includeInlayEnumMemberValueHints?: boolean;
}

/** Represents a bigint literal value without requiring bigint support */
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/utilitiesPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,11 @@ namespace ts {
return node && isFunctionLikeDeclarationKind(node.kind);
}

/* @internal */
export function isBooleanLiteral(node: Node): node is BooleanLiteral {
return node.kind === SyntaxKind.TrueKeyword || node.kind === SyntaxKind.FalseKeyword;
}

function isFunctionLikeDeclarationKind(kind: SyntaxKind): boolean {
switch (kind) {
case SyntaxKind.FunctionDeclaration:
Expand Down
16 changes: 16 additions & 0 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,22 @@ namespace ts.server {

applyCodeActionCommand = notImplemented;

provideInlayHints(file: string, span: TextSpan): InlayHint[] {
const { start, length } = span;
const args: protocol.ProvideInlayHintsRequestArgs = { file, start, length };

const request = this.processRequest<protocol.ProvideInlayHintsRequest>(CommandNames.ProvideInlayHints, args);
const response = this.processResponse<protocol.ProvideInlayHintsResponse>(request);

return response.body!.map(item => ({ // TODO: GH#18217
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need the TODO comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems yes.

return response.body!.map(({ fixName, description, changes, commands, fixId, fixAllDescription }) => // TODO: GH#18217

At line 641.

text: item.text,
position: this.lineOffsetToPosition(file, item.position),
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
kind: item.kind as InlayHintKind | undefined,
whitespaceBefore: item.whitespaceBefore,
whitespaceAfter: item.whitespaceAfter
}));
}

private createFileLocationOrRangeRequestArgs(positionOrRange: number | TextRange, fileName: string): protocol.FileLocationOrRangeRequestArgs {
return typeof positionOrRange === "number"
? this.createFileLocationRequestArgs(fileName, positionOrRange)
Expand Down
17 changes: 17 additions & 0 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

namespace FourSlash {
import ArrayOrSingle = FourSlashInterface.ArrayOrSingle;

Expand Down Expand Up @@ -836,6 +837,22 @@ namespace FourSlash {
});
}

public verifyInlayHints(expected: readonly FourSlashInterface.VerifyInlayHintsOptions[], span: ts.TextSpan = { start: 0, length: this.activeFile.content.length }, preference?: ts.InlayHintsOptions) {
const hints = this.languageService.provideInlayHints(this.activeFile.fileName, span, preference);
assert.equal(hints.length, expected.length, "Number of hints");

const sortHints = (a: ts.InlayHint, b: ts.InlayHint) => {
return a.position - b.position;
};
ts.zipWith(hints.sort(sortHints), [...expected].sort(sortHints), (actual, expected) => {
assert.equal(actual.text, expected.text, "Text");
assert.deepEqual(actual.position, expected.position, "Position");
assert.equal(actual.kind, expected.kind, "Kind");
assert.equal(actual.whitespaceBefore, expected.whitespaceBefore, "whitespaceBefore");
assert.equal(actual.whitespaceAfter, expected.whitespaceAfter, "whitespaceAfter");
});
}

public verifyCompletions(options: FourSlashInterface.VerifyCompletionsOptions) {
if (options.marker === undefined) {
this.verifyCompletionsWorker(options);
Expand Down
12 changes: 12 additions & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ namespace FourSlashInterface {
}
}

public getInlayHints(expected: readonly VerifyInlayHintsOptions[], span: ts.TextSpan, preference?: ts.InlayHintsOptions) {
this.state.verifyInlayHints(expected, span, preference);
}

public quickInfoIs(expectedText: string, expectedDocumentation?: string) {
this.state.verifyQuickInfoString(expectedText, expectedDocumentation);
}
Expand Down Expand Up @@ -1663,6 +1667,14 @@ namespace FourSlashInterface {
readonly containerKind?: ts.ScriptElementKind;
}

export interface VerifyInlayHintsOptions {
text: string;
position: number;
kind?: ts.InlayHintKind;
whitespaceBefore?: boolean;
whitespaceAfter?: boolean;
}

export type ArrayOrSingle<T> = T | readonly T[];

export interface VerifyCompletionListContainsOptions extends ts.UserPreferences {
Expand Down
3 changes: 3 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@ namespace Harness.LanguageService {
provideCallHierarchyOutgoingCalls(fileName: string, position: number) {
return unwrapJSONCallResult(this.shim.provideCallHierarchyOutgoingCalls(fileName, position));
}
provideInlayHints(fileName: string, span: ts.TextSpan, preference: ts.InlayHintsOptions) {
return unwrapJSONCallResult(this.shim.provideInlayHints(fileName, span, preference));
}
getEmitOutput(fileName: string): ts.EmitOutput {
return unwrapJSONCallResult(this.shim.getEmitOutput(fileName));
}
Expand Down
35 changes: 35 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ namespace ts.server.protocol {
PrepareCallHierarchy = "prepareCallHierarchy",
ProvideCallHierarchyIncomingCalls = "provideCallHierarchyIncomingCalls",
ProvideCallHierarchyOutgoingCalls = "provideCallHierarchyOutgoingCalls",
ProvideInlayHints = "provideInlayHints"

// NOTE: If updating this, be sure to also update `allCommandNames` in `testRunner/unittests/tsserver/session.ts`.
}
Expand Down Expand Up @@ -2549,6 +2550,40 @@ namespace ts.server.protocol {
body?: SignatureHelpItems;
}

export const enum InlayHintKind {
Other = 0,
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
Type = 1,
Parameter = 2,
}

export interface ProvideInlayHintsRequestArgs extends FileRequestArgs {
/**
* Start position of the span.
*/
start: number;
/**
* Length of the span.
*/
length: number;
}

export interface ProvideInlayHintsRequest extends Request {
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
command: CommandTypes.ProvideInlayHints;
arguments: ProvideInlayHintsRequestArgs;
}

export interface InlayHintItem {
text: string;
position: Location;
kind?: InlayHintKind;
whitespaceBefore?: boolean;
whitespaceAfter?: boolean;
}

export interface ProvideInlayHintsResponse extends Response {
body?: InlayHintItem[];
}

/**
* Synchronous request for semantic diagnostics of one file.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,13 @@ namespace ts.server {
return location;
}

textSpanToProtoTextSpan(range: TextSpan): protocol.TextSpan {
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
return {
start: this.positionToLineOffset(range.start),
end: this.positionToLineOffset(range.start + range.length)
};
}

public isJavaScript() {
return this.scriptKind === ScriptKind.JS || this.scriptKind === ScriptKind.JSX;
}
Expand Down
17 changes: 17 additions & 0 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,20 @@ namespace ts.server {
});
}

private provideInlayHints(args: protocol.ProvideInlayHintsRequestArgs) {
const { file, languageService } = this.getFileAndLanguageServiceForSyntacticOperation(args);
const scriptInfo = this.projectService.getScriptInfoForNormalizedPath(file)!;
const hints = languageService.provideInlayHints(file, args, this.getPreferences(file));

return hints.map(hint => ({
text: hint.text,
position: scriptInfo.positionToLineOffset(hint.position),
Kingwl marked this conversation as resolved.
Show resolved Hide resolved
kind: hint.kind,
whitespaceBefore: hint.whitespaceBefore,
whitespaceAfter: hint.whitespaceAfter
}));
}

private setCompilerOptionsForInferredProjects(args: protocol.SetCompilerOptionsForInferredProjectsArgs): void {
this.projectService.setCompilerOptionsForInferredProjects(args.options, args.projectRootPath);
}
Expand Down Expand Up @@ -2962,6 +2976,9 @@ namespace ts.server {
[CommandNames.UncommentSelectionFull]: (request: protocol.UncommentSelectionRequest) => {
return this.requiredResponse(this.uncommentSelection(request.arguments, /*simplifiedResult*/ false));
},
[CommandNames.ProvideInlayHints]: (request: protocol.ProvideInlayHintsRequest) => {
return this.requiredResponse(this.provideInlayHints(request.arguments));
}
}));

public addProtocolHandler(command: string, handler: (request: protocol.Request) => HandlerResponse) {
Expand Down
Loading