-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Fix JSDoc @type completion inside function argument #62935
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
827dd6a
d0ec6f2
f37e894
75d7ffc
7437af2
31f64de
558b1ef
227390a
12a4254
daa9527
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -226,6 +226,7 @@ import { | |||||
| isParameterPropertyModifier, | ||||||
| isPartOfTypeNode, | ||||||
| isPossiblyTypeArgumentPosition, | ||||||
| isQualifiedName, | ||||||
| isPrivateIdentifier, | ||||||
| isPrivateIdentifierClassElementDeclaration, | ||||||
| isPropertyAccessExpression, | ||||||
|
|
@@ -256,10 +257,12 @@ import { | |||||
| isTypeOnlyImportDeclaration, | ||||||
| isTypeOnlyImportOrExportDeclaration, | ||||||
| isTypeParameterDeclaration, | ||||||
| isTypeReferenceNode, | ||||||
| isValidTypeOnlyAliasUseSite, | ||||||
| isVariableDeclaration, | ||||||
| isVariableLike, | ||||||
| JsDoc, | ||||||
| JSDoc, | ||||||
| JSDocImportTag, | ||||||
| JSDocParameterTag, | ||||||
| JSDocPropertyTag, | ||||||
|
|
@@ -315,6 +318,7 @@ import { | |||||
| ObjectTypeDeclaration, | ||||||
| or, | ||||||
| ParameterDeclaration, | ||||||
| parseIsolatedJSDocComment, | ||||||
| ParenthesizedTypeNode, | ||||||
| positionBelongsToNode, | ||||||
| positionIsASICandidate, | ||||||
|
|
@@ -3321,6 +3325,9 @@ function getCompletionData( | |||||
| let insideJsDocTagTypeExpression = false; | ||||||
| let insideJsDocImportTag = false; | ||||||
| let isInSnippetScope = false; | ||||||
| // For orphaned JSDoc with qualified name (e.g., t. in function foo(/** @type {t.} */) {}) | ||||||
| // we need to track the left identifier text to enable member completions. See #62281. | ||||||
| let orphanedJsDocQualifiedNameLeft: Identifier | undefined; | ||||||
| if (insideComment) { | ||||||
| if (hasDocComment(sourceFile, position)) { | ||||||
| if (sourceFile.text.charCodeAt(position - 1) === CharacterCodes.at) { | ||||||
|
|
@@ -3382,6 +3389,33 @@ function getCompletionData( | |||||
| } | ||||||
| } | ||||||
| } | ||||||
| else { | ||||||
| // Fallback: Handle orphaned JSDoc comments not attached to any AST node. | ||||||
| // For example: function foo(/** @type {t.} */) {} - no parameter name means | ||||||
| // the parser creates 0 parameters and the JSDoc is not attached. See #62281. | ||||||
| const commentText = sourceFile.text.substring(insideComment.pos, insideComment.end); | ||||||
| const parsed = parseIsolatedJSDocComment(commentText); | ||||||
| if (parsed?.jsDoc?.tags) { | ||||||
| const posInComment = position - insideComment.pos; | ||||||
| for (const parsedTag of parsed.jsDoc.tags) { | ||||||
| const typeExpression = tryGetTypeExpressionFromTag(parsedTag); | ||||||
| if (!typeExpression || typeExpression.pos >= posInComment || posInComment > typeExpression.end) { | ||||||
| continue; | ||||||
| } | ||||||
| insideJsDocTagTypeExpression = true; | ||||||
|
|
||||||
| // For member completions after a dot (e.g., t.), find the namespace identifier | ||||||
| if (typeExpression.kind === SyntaxKind.JSDocTypeExpression) { | ||||||
| const typeNode = typeExpression.type; | ||||||
| if (isTypeReferenceNode(typeNode) && isQualifiedName(typeNode.typeName) && posInComment > typeNode.typeName.left.end && isIdentifier(typeNode.typeName.left)) { | ||||||
| const leftText = commentText.substring(typeNode.typeName.left.pos, typeNode.typeName.left.end).trim(); | ||||||
| orphanedJsDocQualifiedNameLeft = findJsDocImportNamespaceIdentifier(sourceFile, leftText); | ||||||
| } | ||||||
| } | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!insideJsDocTagTypeExpression && !insideJsDocImportTag) { | ||||||
| // Proceed if the current position is in jsDoc tag expression; otherwise it is a normal | ||||||
|
|
@@ -3390,6 +3424,22 @@ function getCompletionData( | |||||
| return undefined; | ||||||
| } | ||||||
| } | ||||||
| else { | ||||||
| // Handle inline JSDoc on function parameters. For these, isInComment returns undefined | ||||||
| // but we can still find a JSDocTag ancestor from the currentToken. See #62281. | ||||||
| const tag = getJsDocTagAtPosition(currentToken, position); | ||||||
| if (tag) { | ||||||
| if (isJSDocImportTag(tag)) { | ||||||
| insideJsDocImportTag = true; | ||||||
| } | ||||||
| else { | ||||||
| const typeExpression = tryGetTypeExpressionFromTag(tag); | ||||||
| if (typeExpression && isCurrentlyEditingNode(typeExpression)) { | ||||||
| insideJsDocTagTypeExpression = true; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| start = timestamp(); | ||||||
| // The decision to provide completion depends on the contextToken, which is determined through the previousToken. | ||||||
|
|
@@ -3440,7 +3490,8 @@ function getCompletionData( | |||||
| isNewIdentifierLocation = importStatementCompletionInfo.isNewIdentifierLocation; | ||||||
| } | ||||||
| // Bail out if this is a known invalid completion location | ||||||
| if (!importStatementCompletionInfo.replacementSpan && isCompletionListBlocker(contextToken)) { | ||||||
| // Skip the blocker check if we're inside a JSDoc type expression (including orphaned JSDoc). See #62281. | ||||||
| if (!importStatementCompletionInfo.replacementSpan && !insideJsDocTagTypeExpression && !insideJsDocImportTag && isCompletionListBlocker(contextToken)) { | ||||||
| log("Returning an empty list because completion was requested in an invalid position."); | ||||||
| return keywordFilters | ||||||
| ? keywordCompletionData(keywordFilters, isJsOnlyLocation, computeCommitCharactersAndIsNewIdentifier().isNewIdentifierLocation) | ||||||
|
|
@@ -3580,6 +3631,14 @@ function getCompletionData( | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Handle orphaned JSDoc with qualified name (e.g., function foo(/** @type {t.} */) {}). | ||||||
| // We found the left identifier's AST node earlier; now use it for member completions. | ||||||
| if (orphanedJsDocQualifiedNameLeft) { | ||||||
| isRightOfDot = true; | ||||||
| node = orphanedJsDocQualifiedNameLeft; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Outdated
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null coalescing operator should be used here to avoid creating an empty array when jsDocNodes is undefined. This avoids unnecessary iteration and is more idiomatic. Consider changing jsDocNodes || [] to jsDocNodes ?? [] to only substitute when the value is nullish rather than any falsy value.
| for (const jsDoc of jsDocNodes || []) { | |
| for (const jsDoc of jsDocNodes ?? []) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Outdated
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null coalescing operator should be used here to avoid creating an empty array when jsDoc.tags is undefined. This avoids unnecessary iteration and is more idiomatic. Consider changing jsDoc.tags || [] to jsDoc.tags ?? [] to only substitute when the value is nullish rather than any falsy value.
| for (const tag of jsDoc.tags || []) { | |
| for (const tag of jsDoc.tags ?? []) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| /// <reference path="fourslash.ts" /> | ||
|
|
||
| // Tests based on issue #62281 | ||
| // JSDoc @type completion in function parameter contexts | ||
|
|
||
| // @allowJs: true | ||
| // @checkJs: true | ||
|
|
||
| // @filename: /types.ts | ||
| ////export interface MyType { | ||
| //// name: string; | ||
| //// value: number; | ||
| ////} | ||
| ////export interface OtherType { | ||
| //// id: number; | ||
| ////} | ||
|
|
||
| // @filename: /main.js | ||
| /////** @import * as t from "./types" */ | ||
| //// | ||
| /////** | ||
| //// * @typedef {Object} MyNamespace | ||
| //// * @property {string} name | ||
| //// */ | ||
| //// | ||
| /////** | ||
| //// * @typedef {Object} MyNamespace.NestedType | ||
| //// * @property {number} value | ||
| //// */ | ||
| //// | ||
| /////** @typedef {number} SomeNumber */ | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 1: Regular @type on variable | ||
| //// // ============================================================ | ||
| //// | ||
| /////** @type {t./*case1*/} */ | ||
| ////const x = {}; | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 2: Inline @type with named parameter | ||
| //// // ============================================================ | ||
| //// | ||
| ////function f2(/** @type {t./*case2*/} */ p) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 3: Property name in type literal (correctly NO completions) | ||
| //// // ============================================================ | ||
| //// | ||
| /////** @type { {/*case3*/ageX: number} } */ | ||
| ////var y; | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 4: @import with named argument | ||
| //// // ============================================================ | ||
| //// | ||
| ////function f4(/** @type {t./*case4*/} */arg) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 5: @import with unnamed argument (PARSER LIMITATION) | ||
| //// // The function is parsed with 0 parameters, JSDoc is orphaned. | ||
| //// // ============================================================ | ||
| //// | ||
| ////function f5(/** @type {t./*case5*/} */) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 6: @typedef with unnamed argument (PARSER LIMITATION) | ||
| //// // The function is parsed with 0 parameters, JSDoc is orphaned. | ||
| //// // ============================================================ | ||
| //// | ||
| ////function f6(/** @type {S/*case6*/} */) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 7: @typedef with named argument | ||
| //// // ============================================================ | ||
| //// | ||
| ////function f7(/** @type {S/*case7*/} */arg) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Case 8: @param tag in function JSDoc | ||
| //// // ============================================================ | ||
| //// | ||
| /////** | ||
| //// * @param {t./*case8*/} p | ||
| //// */ | ||
| ////function f8(p) {} | ||
| //// | ||
| //// // ============================================================ | ||
| //// // Additional: @typedef namespace completions | ||
| //// // ============================================================ | ||
| //// | ||
| /////** @param {MyNamespace./*typedefInParam*/} p */ | ||
| ////function f9(p) {} | ||
| //// | ||
| ////function f10(/** @type {MyNamespace./*typedefInline*/} */ p) {} | ||
|
|
||
| // Cases that SHOULD have completions | ||
| verify.completions( | ||
| // Case 1: Regular @type on variable | ||
| { | ||
| marker: "case1", | ||
| exact: [ | ||
| { name: "MyType", kind: "interface", kindModifiers: "export" }, | ||
| { name: "OtherType", kind: "interface", kindModifiers: "export" }, | ||
| ], | ||
| }, | ||
| // Case 2: Inline @type with named parameter | ||
| { | ||
| marker: "case2", | ||
| exact: [ | ||
| { name: "MyType", kind: "interface", kindModifiers: "export" }, | ||
| { name: "OtherType", kind: "interface", kindModifiers: "export" }, | ||
| ], | ||
| }, | ||
| // Case 4: @import with named argument | ||
| { | ||
| marker: "case4", | ||
| exact: [ | ||
| { name: "MyType", kind: "interface", kindModifiers: "export" }, | ||
| { name: "OtherType", kind: "interface", kindModifiers: "export" }, | ||
| ], | ||
| }, | ||
| // Case 7: @typedef with named argument | ||
| { | ||
| marker: "case7", | ||
| includes: [{ name: "SomeNumber", kind: "type" }], | ||
| }, | ||
| // Case 8: @param tag in function JSDoc | ||
| { | ||
| marker: "case8", | ||
| exact: [ | ||
| { name: "MyType", kind: "interface", kindModifiers: "export" }, | ||
| { name: "OtherType", kind: "interface", kindModifiers: "export" }, | ||
| ], | ||
| }, | ||
| // Additional: @typedef namespace completions | ||
| { | ||
| marker: ["typedefInParam", "typedefInline"], | ||
| includes: [{ name: "NestedType", kind: "type" }], | ||
| } | ||
| ); | ||
|
|
||
| // Case 3: Property name in type literal - NO completions | ||
| // (defining a property name, not referencing a type) | ||
| verify.completions({ | ||
| marker: "case3", | ||
| exact: undefined, | ||
| }); | ||
|
|
||
| // Cases 5 & 6: Previously parser limitations, now FIXED with orphaned JSDoc handling. | ||
| verify.completions( | ||
| { | ||
| marker: "case5", | ||
| exact: [ | ||
| { name: "MyType", kind: "interface", kindModifiers: "export" }, | ||
| { name: "OtherType", kind: "interface", kindModifiers: "export" }, | ||
| ], | ||
| }, | ||
| { | ||
| marker: "case6", | ||
| includes: [{ name: "SomeNumber", kind: "type" }], | ||
| } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.trim()call here may cause issues if the identifier text contains leading or trailing whitespace in the parsed AST positions. Since the positions from the parsed JSDoc should already point to the exact identifier text without whitespace, the trim call is unnecessary and could potentially cause the namespace lookup to fail if the positions include any surrounding whitespace. Consider removing the.trim()call to use the exact substring as indicated by the parsed positions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed