Link tags in line comments are not resolved via TypeScript

[marijnh]

Search terms

@link line comment useTsLinkResolution

Expected Behavior

If I use line comments for my doc comments, I'd expect TypeScript-driven resolution of @link targets to still work.

Actual Behavior

Only the old, built-in resolution is used in this case.

In fact, the machinery needed to do this seems to be entirely absent from lineLexer.ts. Is this intentional, or an oversight? (If you want, I can look into adding it. In that case, I'd like to know why these lexers exist as completely separate code, rather than being unified, since they seem very similar.)

Steps to reproduce the bug

Set commentStyle to "line", and use a @link that requires TypeScript to resolve (I ran across this trying to reference an import from a sibling package included in the build). TypeDoc will never call getSymbolAtLocation for it, and thus fail to resolve it.

Environment

• TypeDoc version: 0.28.19
• TypeScript version: 6.0.3
• Node.js version: 22.20.0
• OS: Debian Linux

[Gerrit0]

This is unfortunately intentional. It is like this because typescript does not parse line comments for tags, so we can't use TS link resolution here.

[marijnh]

Oh right, I see, you're getting the node you pass to getSymbolAtLocation from the TypeScript parse.

If I were to find some kind of horrible kludge to beat this information out of the TS API anyway (I haven't yet, but I'd try), would you be interested, or do you feel that it wouldn't be worth the trouble?

[marijnh]

It seems possible to do this, if a little awkwardly, though resolveName. I have a sketch of a solution at #3115, would appreciate feedback.

[Gerrit0]

That's far less code than I expected it to be! Unfortunately, we now have 3 different resolution algorithms, which is far from ideal.

Before committing to this route I'd definitely want to try to remove the ts.JSDoc path and convert it to use resolveName. If that can be made to produce the same results (or slightly different but acceptable results due to the meaning handling) then I think I'd be happy to replace the ts.JSDoc path with resolveName

[marijnh]

want to try to remove the ts.JSDoc path

That should be very easy (and maybe even make the change a net negative in lines of code).

One thing I'm uneasy about is the way TypeScript's members and exports properties on symbols can only hold a single symbol per name. It is possible to have, for example, both a type and a value and a namespace exported from a parent namespace under the same name. But where resolveName allows you to pass in flags to resolve these separately, members/exports don't support that. In my project that's not an issue (and the patch is working wonderfully), but there's situations where this could be a problem. We could go walk the syntax tree to try and find all members, but that gets a lot more messy and fragile.

Do you know whether ts.JSDoc supports meaning.label filters? My code passes on trying to resolve those, because at the time where it runs those labels are (I think) not yet available, but if this were to become the main resolution algorithm, it should probably support them.

One possible alternative direction would be to only resolve the top/root identifier, and leave descending from that to TypeDoc's existing resolution algorithm. I couldn't get this to work cleanly because I didn't know how to communicate the half-resolved result from the comment lexer to the resolution code. But now that I think about it both the issues I list above could be avoided if we set things up that way.

[Gerrit0]

It is possible to have, for example, both a type and a value and a namespace exported from a parent namespace under the same name.

Yes, but those only receive a single symbol! Take a look at ts-ast-viewer (click on Foo), the symbol has declarations for both the ModuleDeclaration and the TypeAliasDeclaration.

There's an obnoxious number of edge cases in link resolution linkResolution.ts has several test cases... this becomes scarier the more I think about this. Link resolution has as many different behaviors as there are implementers. TypeScript's was originally roughly based on JSDoc, but they dropped some features and added others. TypeDoc's was based off of TypeScript's, but needed to work on reflections as the type checker doesn't exist anymore (and needs to not be used for memory management issues & link resolution not in code files), TSDoc came up with two incompatible link specification forms, so I tried to make TypeDoc mostly compatible with the one which more closely matched how TS works (and how people seem to intuitively think to write links)

The hardest part about changing any of this is that in an ideal world TypeDoc's link resolution should give you the same link targets as you get when looking at rendered doc comments in your editor. This is why it tries to use ts.JSDoc today - that's what VSCode uses to get link targets.

Do you know whether ts.JSDoc supports meaning.label filters?

It does not, that's something entirely dreamed up by the API Extractor people that got put in the TSDoc standard, then later taken out... The ts.JSDoc version doesn't even support meaning.keyword! However, attempting to use a meaning link generally causes TS to fail to resolve the link, so it falls back on TypeDoc's custom resolution and happens to work today.

The label we could probably figure out, but as it isn't supported at all right now, I'm tempted to say we should completely ignore the meaning today...

One possible alternative direction would be to only resolve the top/root identifier, and leave descending from that to TypeDoc's existing resolution algorithm.

Yeah... that's not going to play well with how things are set up today. Resolution either needs to target a ReflectionSymbolId or fail. I don't think further complicating that already annoyingly tangled code is a good idea.

[marijnh]

Yes, but those only receive a single symbol!

Oh, that's freaky, but convenient. Then I'm a bit confused by why resolveName can be parameterized to return only a certain type of symbol, if there's only going to be a single symbol for a name anyway. That probably does mean that we can drop the three different calls to resolveName in my patch, since they should all return the same symbol.

[marijnh]

Yeah... that's not going to play well with how things are set up today. Resolution either needs to target a ReflectionSymbolId or fail. I don't think further complicating that already annoyingly tangled code is a good idea.

I actually don't feel this would be terribly difficult to set up. Resolution would always happen in the resolution stage again, which clears things up. We'd just need a way to smuggle in a root reflection id—could be a non-public property on the @link part, or a weak map, or a table by source position that's returned from the conversion stage.