JSDoc @link not resolved across modules

[connor4312]

Bug Report

While verifying microsoft/vscode#119357

🔎 Search Terms

• link
• export
• modules

🕗 Version & Regression Information

• I was unable to test this on prior versions because it did not previously exist

⏯ Playground Link

Workbench Repro

💻 Code

// @filename: a.ts
import { Model } from './b';

/**
 * {@link Model}
 */

// @filename: b.ts
export class Model {}

🙁 Actual behavior

@link Model is not clickable

🙂 Expected behavior

I expected to be able to click on @link Model

[connor4312]

Actually, this may be wrong -- the workbench seems like it might be slightly buggy. This works only if I import the referenced type. However, with the compiler flag disallowed unused locals, this produces a warning if I don't use the type elsewhere in the file.

One fix for that would be to mark linked types as "Used". C# does that in their xml docs. Or just link against all exported globals, but this might direct to the wrong types if they share names.

This is the link where I see it in a real program -- which typedoc, when generated, resolves fine https://github.com/connor4312/cosmonaut/blob/18a954f6184173fff999a4616b7cf8554addd966/src/baseModel.ts#L45

[orta]

Cool, I agree that this should probably work, and should probably look through the whole known list of symbols in the current project (that's what I'd expect as a JS user, and I think it matches how the web version of this works).

From a dive into this it looks like #41877 never made changes to the gotoDefinition service meaning that it will only work with identifiers in the current scope, and doesn't reach out to search the whole project for an identifier

A failing fourslash test:

///<reference path="fourslash.ts" />

// @Filename: foo.ts
//// export interface [|/*def1*/Foo|] {
////     foo: string
//// }

// @Filename: importee.ts
//// /** {@link /*otherFile*/[|Foo|] } not explicitly linked */
//// const f = ""

goTo.marker("otherFile");
verify.goToDefinitionIs("def1");

[orta]

This had a bit of chatter about whether expanding the scope to all identifiers is the right call, so I'll wait till @sandersn is back to chat about that

Otherwise, we'll make the un-used import as used if it's in the @link

[sandersn]

After discussing this with @orta, we switched our opinions. Now I think that expanding the scope to all identifiers is the right call and he doesn't. However: it would be difficult to decide on the precise semantics and difficult to implement efficiently, so I don't think it's worthwhile right now.

What should work is import references: import('./b').Model. I don't think that @link even parses this reference right now.

Edit: @mjbvz beat me to it: #43950

[pokey]

I'd lean towards solving this one by treating imports that are referenced in a @link as used variables so that @typescript-eslint/no-unused-vars doesn't complain. This way the developer can just import variables for links like normal, so they only need to know one mechanism for importing variables.

And then for extra credit, to make the experience even more consistent between @link and normal references:

• Report an error for @link references that can't be resolved
• Support a quick-fix to auto-import types / variables referenced from a @link. Ie If I have

  /** {@link Foo} */

  Then Foo has a quick fix to import Foo.
• Support autocompletion for subscripts, eg with cursor after Foo.