WIP Resolve local links via TypeChecker

[marijnh]

This shows a potential way to improve resolution of @link tags via the TypeScript typechecker (see #3113). It's intended as a sketch to get feedback on at this point, I don't expect it merged as-is.

The most obvious issue with this approach is that it introduces another resolution algorithm, because the existing one in this tree works with TypeDoc reflection objects, which aren't available at the time where we have the node and typechecker around to do this, and the one the TypeScript compiler isn't available to us in a usable way. If that's a non-starter, let me know, and I'll try to just do this in a local plugin instead.

Another issue is whether comments/index.ts is the right place for this code. I can put it into a separate file if you prefer.

Please let me know what you think and whether you see any problems with the code.

[marijnh]

[Edit: Never mind, figured it out—StableKeyMap does the magic. I was just looking at the type, not the initializer.]

I'm trying to debug an issue with this where some (but only some) of the ReflectionSymbolId instances I'm creating in this code later, in resolveLinkTag, fail to resolve. Or rather, I'm unsure why others do resolve, since as far as I can see those are just objects created in createSymbolIdImpl, at which point the only reference to them exists in the @link part, and I don't see how they are supposed to land in ProjectReflection.symbolToReflectionIdMap. There is a matching symbol ID in that map when the lookup fails, but it's a different object.

So how are the symbol ID objects created in the comment lexer (the JSDoc @link lexer does it the same way as my added code) supposed to end up registered in symbolToReflectionIdMap?

[marijnh]

Okay, so the actual problem is that the transientId of the reflection symbol IDs doesn't match. The one my resolver creates has NaN, whereas the actual one produced and registered has a number there. I have not been able to figure out why.

[marijnh]

Rebased the patches on the current code, and added 5fa966c, which shows what only resolving the path's root might look like. I think that is a promising direction—it avoids the resolution algorithm duplication, and could probably also replace the JSDoc-based resolution (though I don't know precisely enough what that provides to be certain). It does add another property to InlineTagDisplayPart (and I'm not entirely sure if there's any serialization/deserialization stuff I missed), but otherwise it's super simple and seems to work great.

[marijnh]

@Gerrit0 you may not be receiving notification for this, but I'd like you to take a look. Sorry for the ping.

[Gerrit0]

This is very clever! There are a few issues still, but I agree this seems like a promising path forwards.

I did a bit of experimenting with replacing the ts.JSDoc implementation, and it almost entirely just works! The remaining delta I think is because TS has weird rules about link resolution (a comment on a namespace which links to Foo checks for Foo in the namespace first, but resolveName with the node we pass in today doesn't do that) https://github.com/TypeStrong/typedoc/tree/resolve-via-typechecker-no-jsdoc has what I got to, though I'm out of time for this week, will have to take another look next weekend.

[Gerrit0]

We also need to loop over the parts in comment.blockTags or we'll miss @link tags in @param comments

[Gerrit0]

ts.SymbolFlags.All?

[marijnh]

That somehow returned different results, and is what sent me down the rabbit hole of believing there were multiple symbols for a name in the first place. Maybe I did something stupid, but just swapping to that broke my tests.

[Gerrit0]

Is there a reason to exclude globals here? This introduces a delta between this behavior and the behavior for ts.JSDoc in the case I do something like {@link Map.size} (assuming Map is in the documentation of course)

[marijnh]

My thinking was that this is the place where we resolve local links, and the regular resolution can take care of globals. But probably that's not right, since links like {@link RegExp} would count as local links in the sense of the resolver, because they don't contain a !, yet refer to a global binding.

[Gerrit0]

I think we need to add node: sf to context here, this link resolution doesn't work on // comments for the module today

[marijnh]

Hi Gerrit, thanks for looking into this! Unfortunately, because I ended up moving on to a different approach for my own use case (directly querying the TS data structures, rather than TypeDoc reflections), I won't be continuing work on this patch. If you feel this approach would a be an improvement, you're of course welcome to fix it up to your standards. If not, also feel free to drop it.

[Gerrit0]

Thanks for the heads up!