Skip to content

support quick info and go to definition on mapped keys #61061

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

support quick info and go to definition on mapped keys #61061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
712ee1b
support quick info and go to definition on mapped keys
Zzzen Jan 28, 2025
a2bcfaa
format and add tests
Zzzen Feb 1, 2025
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
1 change: 1 addition & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14221,6 +14221,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const stripOptional = strictNullChecks && !isOptional && modifiersProp && modifiersProp.flags & SymbolFlags.Optional;
const lateFlag: CheckFlags = modifiersProp ? getIsLateCheckFlag(modifiersProp) : 0;
const prop = createSymbol(SymbolFlags.Property | (isOptional ? SymbolFlags.Optional : 0), propName, lateFlag | CheckFlags.Mapped | (isReadonly ? CheckFlags.Readonly : 0) | (stripOptional ? CheckFlags.StripOptional : 0)) as MappedSymbol;
prop.parent = mappedType.symbol;
prop.links.mappedType = type;
prop.links.nameType = propNameType;
prop.links.keyType = keyType;
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4408,6 +4408,7 @@ namespace Parser {

function parseMappedType() {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
parseExpected(SyntaxKind.OpenBraceToken);
let readonlyToken: ReadonlyKeyword | PlusToken | MinusToken | undefined;
if (token() === SyntaxKind.ReadonlyKeyword || token() === SyntaxKind.PlusToken || token() === SyntaxKind.MinusToken) {
Expand All @@ -4431,7 +4432,10 @@ namespace Parser {
parseSemicolon();
const members = parseList(ParsingContext.TypeMembers, parseTypeMember);
parseExpected(SyntaxKind.CloseBraceToken);
return finishNode(factory.createMappedTypeNode(readonlyToken, typeParameter, nameType, questionToken, type, members), pos);
return withJSDoc(
finishNode(factory.createMappedTypeNode(readonlyToken, typeParameter, nameType, questionToken, type, members), pos),
hasJSDoc,
);
}

function parseTupleElementType() {
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,8 @@ export type HasJSDoc =
| VariableDeclaration
| VariableStatement
| WhileStatement
| WithStatement;
| WithStatement
| MappedTypeNode;

export type HasType =
| SignatureDeclaration
Expand Down Expand Up @@ -2339,7 +2340,7 @@ export interface IndexedAccessTypeNode extends TypeNode {
readonly indexType: TypeNode;
}

export interface MappedTypeNode extends TypeNode, Declaration, LocalsContainer {
export interface MappedTypeNode extends TypeNode, Declaration, LocalsContainer, JSDocContainer {
readonly kind: SyntaxKind.MappedType;
readonly readonlyToken?: ReadonlyKeyword | PlusToken | MinusToken;
readonly typeParameter: TypeParameterDeclaration;
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2699,7 +2699,8 @@ export function getJSDocCommentRanges(node: Node, text: string): CommentRange[]
node.kind === SyntaxKind.ArrowFunction ||
node.kind === SyntaxKind.ParenthesizedExpression ||
node.kind === SyntaxKind.VariableDeclaration ||
node.kind === SyntaxKind.ExportSpecifier) ?
node.kind === SyntaxKind.ExportSpecifier ||
node.kind === SyntaxKind.MappedType) ?
concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
getLeadingCommentRanges(text, node.pos);
// True if the comment starts with '/**' but not if it is '/**/'
Expand Down Expand Up @@ -4538,6 +4539,7 @@ export function canHaveJSDoc(node: Node): node is HasJSDoc {
case SyntaxKind.VariableStatement:
case SyntaxKind.WhileStatement:
case SyntaxKind.WithStatement:
case SyntaxKind.MappedType:
return true;
default:
return false;
Expand Down
5 changes: 4 additions & 1 deletion src/services/goToDefinition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
AssignmentOperatorToken,
CallLikeExpression,
canHaveSymbol,
CheckFlags,
concatenate,
createTextSpan,
createTextSpanFromBounds,
Expand Down Expand Up @@ -74,6 +75,7 @@ import {
isRightSideOfPropertyAccess,
isStaticModifier,
isSwitchStatement,
isTransientSymbol,
isTypeAliasDeclaration,
isTypeReferenceNode,
isVariableDeclaration,
Expand Down Expand Up @@ -595,7 +597,8 @@ function isExpandoDeclaration(node: Declaration): boolean {
}

function getDefinitionFromSymbol(typeChecker: TypeChecker, symbol: Symbol, node: Node, failedAliasResolution?: boolean, declarationFilter?: (d: Declaration) => boolean): DefinitionInfo[] | undefined {
const filteredDeclarations = declarationFilter !== undefined ? filter(symbol.declarations, declarationFilter) : symbol.declarations;
const declarations = isTransientSymbol(symbol) && symbol.links.checkFlags & CheckFlags.Mapped && !symbol.declarations?.length ? symbol.links.syntheticOrigin?.declarations : symbol.declarations;
Copy link
Member

Choose a reason for hiding this comment

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

I find it sort of concerning to reach into the links like this; I can see maybe one or two other cases where we've done symbol.links outside the checker and I can't say I like those either. Is there no other way to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with your concern about reaching into symbol.links. I think there might be a better way to handle this. Maybe we can create a helper function that gets the documentation from the original symbol without directly accessing links. That would make the code more maintainable and less dependent on internal implementation details.

const filteredDeclarations = declarationFilter !== undefined ? filter(declarations, declarationFilter) : declarations;
// If we have a declaration filter, we are looking for specific declaration(s), so we should not return prematurely.
const signatureDefinition = !declarationFilter && (getConstructSignatureDefinition() || getCallSignatureDefinition());
if (signatureDefinition) {
Expand Down
6 changes: 5 additions & 1 deletion src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
canIncludeBindAndCheckDiagnostics,
changeCompilerHostLikeToUseCache,
CharacterCodes,
CheckFlags,
CheckJsDirective,
Classifications,
ClassifiedSpan,
Expand Down Expand Up @@ -721,7 +722,10 @@ class SymbolObject implements Symbol {
if (!this.documentationComment) {
this.documentationComment = emptyArray; // Set temporarily to avoid an infinite loop finding inherited docs

if (!this.declarations && isTransientSymbol(this) && this.links.target && isTransientSymbol(this.links.target) && this.links.target.links.tupleLabelDeclaration) {
if (!this.declarations && isTransientSymbol(this) && this.links.checkFlags & CheckFlags.Mapped && this.parent?.declarations && some(this.parent?.declarations, decl => hasJSDocInheritDocTag(decl))) {
this.documentationComment = getDocumentationComment(this.parent?.declarations.concat(this.links.syntheticOrigin?.declarations || emptyArray), checker);
}
else if (!this.declarations && isTransientSymbol(this) && this.links.target && isTransientSymbol(this.links.target) && this.links.target.links.tupleLabelDeclaration) {
const labelDecl = this.links.target.links.tupleLabelDeclaration;
this.documentationComment = getDocumentationComment([labelDecl], checker);
}
Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4387,7 +4387,8 @@ declare namespace ts {
| VariableDeclaration
| VariableStatement
| WhileStatement
| WithStatement;
| WithStatement
| MappedTypeNode;
type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
Expand Down Expand Up @@ -4806,7 +4807,7 @@ declare namespace ts {
readonly objectType: TypeNode;
readonly indexType: TypeNode;
}
interface MappedTypeNode extends TypeNode, Declaration, LocalsContainer {
interface MappedTypeNode extends TypeNode, Declaration, LocalsContainer, JSDocContainer {
readonly kind: SyntaxKind.MappedType;
readonly readonlyToken?: ReadonlyKeyword | PlusToken | MinusToken;
readonly typeParameter: TypeParameterDeclaration;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/goToDefinition_filteringGenericMappedType.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
{
"kind": "property",
"name": "id",
"containerName": "",
"containerName": "__type",
"isLocal": false,
"isAmbient": false,
"unverified": false,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/goToDefinition_filteringMappedType.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
{
"kind": "property",
"name": "a",
"containerName": "",
"containerName": "filtered",
"isLocal": false,
"isAmbient": false,
"unverified": false,
Expand Down
28 changes: 28 additions & 0 deletions tests/baselines/reference/goToDefinition_mappedType2.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// === goToDefinition ===
// === /tests/cases/fourslash/goToDefinition_mappedType2.ts ===
// interface Foo {
// <|[|property|]: string|>
// }
//
// type JustMapIt<T> = {[P in keyof T]: 0}
// --- (line: 6) skipped ---

// --- (line: 11) skipped ---
//
// {
// let gotoDef!: MapItWithRemap<Foo>
// gotoDef./*GOTO DEF*/mapped_property
// }

// === Details ===
[
{
"kind": "property",
"name": "mapped_property",
"containerName": "__type",
"isLocal": false,
"isAmbient": false,
"unverified": false,
"failedAliasResolution": false
}
]
27 changes: 27 additions & 0 deletions tests/baselines/reference/goToDefinition_mappedType3.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// === goToDefinition ===
// === /tests/cases/fourslash/goToDefinition_mappedType3.ts ===
// interface Source {
// <|[|alpha|]: number;|>
// beta: string;
// }
//
// --- (line: 6) skipped ---

// --- (line: 22) skipped ---
// };
//
// // ❌ In VSCode, "Go to Definition" on `alphaSuffix` does not navigate to `alpha` in `Source`
// obj./*GOTO DEF*/alphaSuffix();

// === Details ===
[
{
"kind": "property",
"name": "alphaSuffix",
"containerName": "__type",
"isLocal": false,
"isAmbient": false,
"unverified": false,
"failedAliasResolution": false
}
]
Loading