Skip to content

Commit 558b1ef

Browse files
caveraccaverac
committed
fix(completions): clean up logic for orphaned comments, added tests
1 parent 31f64de commit 558b1ef

2 files changed

Lines changed: 220 additions & 27 deletions

File tree

‎src/services/completions.ts‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3397,28 +3397,22 @@ function getCompletionData(
33973397
const parsed = parseIsolatedJSDocComment(commentText);
33983398
if (parsed?.jsDoc?.tags) {
33993399
const posInComment = position - insideComment.pos;
3400-
for (const parsedTag of parsed.jsDoc.tags) { // TODO: Too slow (?)
3400+
for (const parsedTag of parsed.jsDoc.tags) {
34013401
const typeExpression = tryGetTypeExpressionFromTag(parsedTag);
3402-
if (typeExpression && typeExpression.pos < posInComment && posInComment <= typeExpression.end) {
3403-
insideJsDocTagTypeExpression = true;
3404-
// Check if we're after a dot in a QualifiedName (for member completions)
3405-
if (typeExpression.kind !== SyntaxKind.JSDocTypeExpression) break;
3402+
if (!typeExpression || typeExpression.pos >= posInComment || posInComment > typeExpression.end) {
3403+
continue;
3404+
}
3405+
insideJsDocTagTypeExpression = true;
3406+
3407+
// For member completions after a dot (e.g., t.), find the namespace identifier
3408+
if (typeExpression.kind === SyntaxKind.JSDocTypeExpression) {
34063409
const typeNode = typeExpression.type;
3407-
if (isTypeReferenceNode(typeNode) && isQualifiedName(typeNode.typeName)) {
3408-
const qualifiedName = typeNode.typeName;
3409-
// Position after the dot means we want to complete members of the left side
3410-
const dotPos = qualifiedName.left.end;
3411-
if (posInComment > dotPos && isIdentifier(qualifiedName.left)) {
3412-
const leftText = commentText.substring(qualifiedName.left.pos, qualifiedName.left.end).trim();
3413-
// Find the corresponding JSDocImportTag in the source file
3414-
const found = findJsDocImportNamespaceIdentifier(sourceFile, leftText);
3415-
if (found) {
3416-
orphanedJsDocQualifiedNameLeft = found;
3417-
}
3418-
}
3410+
if (isTypeReferenceNode(typeNode) && isQualifiedName(typeNode.typeName) && posInComment > typeNode.typeName.left.end && isIdentifier(typeNode.typeName.left)) {
3411+
const leftText = commentText.substring(typeNode.typeName.left.pos, typeNode.typeName.left.end).trim();
3412+
orphanedJsDocQualifiedNameLeft = findJsDocImportNamespaceIdentifier(sourceFile, leftText);
34193413
}
3420-
break;
34213414
}
3415+
break;
34223416
}
34233417
}
34243418
}
@@ -3775,16 +3769,13 @@ function getCompletionData(
37753769
*/
37763770
function findJsDocImportNamespaceIdentifier(sf: SourceFile, namespaceName: string): Identifier | undefined {
37773771
for (const statement of sf.statements) {
3778-
// JSDoc comments are attached to statements via the jsDoc property
37793772
const jsDocNodes = (statement as Node & { jsDoc?: JSDoc[] }).jsDoc;
3780-
if (!jsDocNodes) continue;
3781-
for (const jsDoc of jsDocNodes) {
3782-
if (!jsDoc.tags) continue;
3783-
for (const tag of jsDoc.tags) {
3784-
if (isJSDocImportTag(tag) && tag.importClause?.namedBindings) {
3785-
const namedBindings = tag.importClause.namedBindings;
3786-
if (isNamespaceImport(namedBindings) && namedBindings.name.text === namespaceName) {
3787-
return namedBindings.name;
3773+
for (const jsDoc of jsDocNodes || []) {
3774+
for (const tag of jsDoc.tags || []) {
3775+
if (isJSDocImportTag(tag)) {
3776+
const bindings = tag.importClause?.namedBindings;
3777+
if (bindings && isNamespaceImport(bindings) && bindings.name.text === namespaceName) {
3778+
return bindings.name;
37883779
}
37893780
}
37903781
}

‎tests/cases/fourslash/jsdocTypeCompletionOrphanedBranches.ts‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// Comprehensive tests for orphaned JSDoc handling branches
4+
// Tests issue #62281 - all logical branches
5+
6+
// @allowJs: true
7+
// @checkJs: true
8+
9+
// @filename: /types.ts
10+
////export interface MyType { name: string; }
11+
////export interface OtherType { id: number; }
12+
////export namespace Nested {
13+
//// export interface DeepType { value: string; }
14+
////}
15+
16+
// @filename: /main.js
17+
/////** @import * as t from "./types" */
18+
/////** @import { MyType } from "./types" */
19+
/////** @typedef {number} LocalNum */
20+
/////** @typedef {{name: string}} LocalObj */
21+
////
22+
////// ============================================================
23+
////// Branch 1: Valid qualified name after dot - MEMBER completions
24+
////// ============================================================
25+
////function branch1(/** @type {t./*b1*/} */) {}
26+
////
27+
////// ============================================================
28+
////// Branch 2: Simple type reference (no dot) - GLOBAL type completions
29+
////// ============================================================
30+
////function branch2(/** @type {Local/*b2*/} */) {}
31+
////
32+
////// ============================================================
33+
////// Branch 3: Primitive type - type completions
34+
////// ============================================================
35+
////function branch3(/** @type {str/*b3*/} */) {}
36+
////
37+
////// ============================================================
38+
////// Branch 4: Object literal type - cursor at property name
39+
////// (Known limitation: orphaned JSDoc gives type completions here)
40+
////// ============================================================
41+
////function branch4(/** @type {{/*b4*/name: string}} */) {}
42+
////
43+
////// ============================================================
44+
////// Branch 5: Named import (not namespace) with dot
45+
////// MyType is an interface, not a namespace - falls back to type completions
46+
////// ============================================================
47+
////function branch5(/** @type {MyType./*b5*/} */) {}
48+
////
49+
////// ============================================================
50+
////// Branch 6: Position BEFORE the dot - type completions, not member
51+
////// ============================================================
52+
////function branch6(/** @type {t/*b6*/.} */) {}
53+
////
54+
////// ============================================================
55+
////// Branch 7: Nested qualified name (a.b.)
56+
////// Known limitation: only Identifier.X supported, not QualifiedName.X
57+
////// ============================================================
58+
////function branch7(/** @type {t.Nested./*b7*/} */) {}
59+
////
60+
////// ============================================================
61+
////// Branch 8: Empty/whitespace JSDoc - NO completions
62+
////// ============================================================
63+
////function branch8(/** /*b8*/ */) {}
64+
////
65+
////// ============================================================
66+
////// Branch 9: Regular comment (not JSDoc) - NO completions
67+
////// ============================================================
68+
////function branch9(/* regular /*b9*/ comment */) {}
69+
////
70+
////// ============================================================
71+
////// Branch 10: @param tag without type - NO completions
72+
////// ============================================================
73+
////function branch10(/** @param /*b10*/ */) {}
74+
////
75+
////// ============================================================
76+
////// Branch 11: @import not at first statement - should still find it
77+
////// ============================================================
78+
////const dummy = 1;
79+
/////** @import * as t2 from "./types" */
80+
////function branch11(/** @type {t2./*b11*/} */) {}
81+
////
82+
////// ============================================================
83+
////// Branch 12: Multiple orphaned params - each should work
84+
////// ============================================================
85+
////function branch12(/** @type {t./*b12a*/} */, /** @type {Local/*b12b*/} */) {}
86+
////
87+
////// ============================================================
88+
////// Branch 13: Cursor right after opening brace - type completions
89+
////// ============================================================
90+
////function branch13(/** @type {/*b13*/} */) {}
91+
////
92+
////// ============================================================
93+
////// Branch 14: Non-existent namespace - falls back to type completions
94+
////// ============================================================
95+
////function branch14(/** @type {nonexistent./*b14*/} */) {}
96+
97+
// Branch 1: Qualified name with namespace import - MEMBER completions
98+
verify.completions({
99+
marker: "b1",
100+
exact: [
101+
{ name: "MyType", kind: "interface", kindModifiers: "export" },
102+
{ name: "Nested", kind: "module", kindModifiers: "export" },
103+
{ name: "OtherType", kind: "interface", kindModifiers: "export" },
104+
],
105+
});
106+
107+
// Branch 2: Simple type reference - should get matching types
108+
verify.completions({
109+
marker: "b2",
110+
includes: [
111+
{ name: "LocalNum", kind: "type" },
112+
{ name: "LocalObj", kind: "type" },
113+
],
114+
});
115+
116+
// Branch 3: Primitive type prefix - should get type completions
117+
verify.completions({
118+
marker: "b3",
119+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
120+
});
121+
122+
// Branch 4: Object literal property name in orphaned JSDoc
123+
// Known limitation: orphaned JSDoc provides type completions at property name positions
124+
verify.completions({
125+
marker: "b4",
126+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
127+
});
128+
129+
// Branch 5: Named import used with dot - type completions (not member)
130+
// MyType is an interface, not a namespace, so no member completions
131+
verify.completions({
132+
marker: "b5",
133+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
134+
excludes: ["name"], // No member completions from MyType interface
135+
});
136+
137+
// Branch 6: Position before dot - should get type completions, not member
138+
verify.completions({
139+
marker: "b6",
140+
includes: [{ name: "t", kind: "alias" }],
141+
excludes: ["OtherType"], // OtherType is only accessible via t., not directly
142+
});
143+
144+
// Branch 7: Nested qualified name (a.b.) - KNOWN LIMITATION
145+
// Our orphaned JSDoc handler only handles simple Identifier.X patterns
146+
verify.completions({
147+
marker: "b7",
148+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
149+
excludes: ["DeepType"], // Can't resolve nested qualified names
150+
});
151+
152+
// Branch 8: Empty JSDoc - NO completions
153+
verify.completions({
154+
marker: "b8",
155+
exact: undefined,
156+
});
157+
158+
// Branch 9: Regular comment - NO completions
159+
verify.completions({
160+
marker: "b9",
161+
exact: undefined,
162+
});
163+
164+
// Branch 10: @param without type - NO completions
165+
verify.completions({
166+
marker: "b10",
167+
exact: undefined,
168+
});
169+
170+
// Branch 11: @import not first - should still find namespace
171+
verify.completions({
172+
marker: "b11",
173+
exact: [
174+
{ name: "MyType", kind: "interface", kindModifiers: "export" },
175+
{ name: "Nested", kind: "module", kindModifiers: "export" },
176+
{ name: "OtherType", kind: "interface", kindModifiers: "export" },
177+
],
178+
});
179+
180+
// Branch 12a: First param in multi-param - member completions
181+
verify.completions({
182+
marker: "b12a",
183+
includes: [{ name: "MyType", kind: "interface", kindModifiers: "export" }],
184+
});
185+
186+
// Branch 12b: Second param in multi-param - type completions
187+
verify.completions({
188+
marker: "b12b",
189+
includes: [{ name: "LocalNum", kind: "type" }],
190+
});
191+
192+
// Branch 13: Right after opening brace - type completions
193+
verify.completions({
194+
marker: "b13",
195+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
196+
});
197+
198+
// Branch 14: Non-existent namespace - falls back to type completions
199+
verify.completions({
200+
marker: "b14",
201+
includes: [{ name: "string", kind: "keyword", sortText: completion.SortText.GlobalsOrKeywords }],
202+
});

0 commit comments

Comments
 (0)