From 9189a4cab593e38f5587874dd7bdb99b108a9e6a Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Thu, 19 Sep 2024 14:33:53 -0600 Subject: [PATCH] Improve handling of external link resolvers Noticed when investing adding symbol ID handling in typedoc-plugin-mdn-links as originally noted was missing in #2700. --- CHANGELOG.md | 5 ++ src/lib/converter/comments/discovery.ts | 12 +++++ src/lib/converter/comments/linkResolver.ts | 19 ++++---- src/lib/converter/comments/parser.ts | 4 ++ .../models/reflections/ReflectionSymbolId.ts | 23 ++++++++- src/lib/models/types.ts | 15 ------ src/lib/utils/fs.ts | 12 +++++ src/test/converter2/issues/gh2700.ts | 6 +++ src/test/issues.c2.test.ts | 48 +++++++++++++++++++ 9 files changed, 120 insertions(+), 24 deletions(-) create mode 100644 src/test/converter2/issues/gh2700.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 58b958810..ef6440f44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Unreleased +### Bug Fixes + +- Correctly handle external link resolver link text when referencing an external symbol, #2700. +- Corrected handling of `@link` tags present in comments at the start of source files. + ## v0.26.7 (2024-09-09) ### Features diff --git a/src/lib/converter/comments/discovery.ts b/src/lib/converter/comments/discovery.ts index d06de0157..f5216f553 100644 --- a/src/lib/converter/comments/discovery.ts +++ b/src/lib/converter/comments/discovery.ts @@ -362,6 +362,18 @@ function findJsDocForComment( .getJSDocCommentsAndTags(node) .map((doc) => ts.findAncestor(doc, ts.isJSDoc)) as ts.JSDoc[]; + if (ts.isSourceFile(node)) { + if (node.statements.length) { + jsDocs.push( + ...(ts + .getJSDocCommentsAndTags(node.statements[0]) + .map((doc) => + ts.findAncestor(doc, ts.isJSDoc), + ) as ts.JSDoc[]), + ); + } + } + return jsDocs.find((doc) => doc.pos === ranges[0].pos); } } diff --git a/src/lib/converter/comments/linkResolver.ts b/src/lib/converter/comments/linkResolver.ts index 1ee32f909..04564785b 100644 --- a/src/lib/converter/comments/linkResolver.ts +++ b/src/lib/converter/comments/linkResolver.ts @@ -141,23 +141,26 @@ function resolveLinkTag( defaultDisplayText = part.tsLinkText || (options.preserveLinkText ? part.text : target.name); - } else if (declRef) { + } else { // If we didn't find a target, we might be pointing to a symbol in another project that will be merged in // or some external symbol, so ask external resolvers to try resolution. Don't use regular declaration ref // resolution in case it matches something that would have been merged in later. + if (declRef) { + pos = declRef[1]; + } const externalResolveResult = externalResolver( - declRef[0], + declRef?.[0] ?? part.target.toDeclarationReference(), reflection, part, - part.target instanceof ReflectionSymbolId - ? part.target - : undefined, + part.target, ); - defaultDisplayText = options.preserveLinkText - ? part.text - : part.text.substring(0, pos); + defaultDisplayText = + part.tsLinkText || + (options.preserveLinkText + ? part.text + : part.text.substring(0, pos)); switch (typeof externalResolveResult) { case "string": diff --git a/src/lib/converter/comments/parser.ts b/src/lib/converter/comments/parser.ts index 4aabc53ce..9945338af 100644 --- a/src/lib/converter/comments/parser.ts +++ b/src/lib/converter/comments/parser.ts @@ -770,6 +770,10 @@ function inlineTag( }; if (tagName.tsLinkTarget) { inlineTag.target = tagName.tsLinkTarget; + } + // Separated from tsLinkTarget to avoid storing a useless empty string + // if TS doesn't have an opinion on what the link text should be. + if (tagName.tsLinkText) { inlineTag.tsLinkText = tagName.tsLinkText; } block.push(inlineTag); diff --git a/src/lib/models/reflections/ReflectionSymbolId.ts b/src/lib/models/reflections/ReflectionSymbolId.ts index 904d79016..a41e053f9 100644 --- a/src/lib/models/reflections/ReflectionSymbolId.ts +++ b/src/lib/models/reflections/ReflectionSymbolId.ts @@ -2,10 +2,16 @@ import { existsSync } from "fs"; import { isAbsolute, join, relative, resolve } from "path"; import ts from "typescript"; import type { JSONOutput, Serializer } from "../../serialization/index"; -import { getCommonDirectory, readFile } from "../../utils/fs"; +import { + findPackageForPath, + getCommonDirectory, + readFile, +} from "../../utils/fs"; import { normalizePath } from "../../utils/paths"; import { getQualifiedName } from "../../utils/tsutils"; import { optional, validate } from "../../utils/validation"; +import type { DeclarationReference } from "../../converter/comments/declarationReference"; +import { splitUnquotedString } from "./utils"; /** * See {@link ReflectionSymbolId} @@ -78,6 +84,21 @@ export class ReflectionSymbolId { } } + toDeclarationReference(): DeclarationReference { + return { + resolutionStart: "global", + moduleSource: findPackageForPath(this.fileName), + symbolReference: { + path: splitUnquotedString(this.qualifiedName, ".").map( + (path) => ({ + navigation: ".", + path, + }), + ), + }, + }; + } + toObject(serializer: Serializer) { const sourceFileName = isAbsolute(this.fileName) ? normalizePath( diff --git a/src/lib/models/types.ts b/src/lib/models/types.ts index 69b64be2c..8c3df568f 100644 --- a/src/lib/models/types.ts +++ b/src/lib/models/types.ts @@ -928,21 +928,6 @@ export class ReferenceType extends Type { .fileName.replace(/\\/g, "/"); if (!symbolPath) return ref; - // Attempt to decide package name from path if it contains "node_modules" - let startIndex = symbolPath.lastIndexOf("node_modules/"); - if (startIndex !== -1) { - startIndex += "node_modules/".length; - let stopIndex = symbolPath.indexOf("/", startIndex); - // Scoped package, e.g. `@types/node` - if (symbolPath[startIndex] === "@") { - stopIndex = symbolPath.indexOf("/", stopIndex + 1); - } - const packageName = symbolPath.substring(startIndex, stopIndex); - ref.package = packageName; - return ref; - } - - // Otherwise, look for a "package.json" file in a parent path ref.package = findPackageForPath(symbolPath); return ref; } diff --git a/src/lib/utils/fs.ts b/src/lib/utils/fs.ts index 99c38fba5..9e7ac7980 100644 --- a/src/lib/utils/fs.ts +++ b/src/lib/utils/fs.ts @@ -338,6 +338,18 @@ export function discoverPackageJson(dir: string) { const packageCache = new Map(); export function findPackageForPath(sourcePath: string): string | undefined { + // Attempt to decide package name from path if it contains "node_modules" + let startIndex = sourcePath.lastIndexOf("node_modules/"); + if (startIndex !== -1) { + startIndex += "node_modules/".length; + let stopIndex = sourcePath.indexOf("/", startIndex); + // Scoped package, e.g. `@types/node` + if (sourcePath[startIndex] === "@") { + stopIndex = sourcePath.indexOf("/", stopIndex + 1); + } + return sourcePath.substring(startIndex, stopIndex); + } + const dir = dirname(sourcePath); const cache = packageCache.get(dir); if (cache) { diff --git a/src/test/converter2/issues/gh2700.ts b/src/test/converter2/issues/gh2700.ts new file mode 100644 index 000000000..7950f325e --- /dev/null +++ b/src/test/converter2/issues/gh2700.ts @@ -0,0 +1,6 @@ +/** + * {@link Map.size | size user specified} + * {@link Map.size user specified} + * {@link Map.size} + */ +export const abc = new Map(); diff --git a/src/test/issues.c2.test.ts b/src/test/issues.c2.test.ts index 3c38d2354..b79e0912e 100644 --- a/src/test/issues.c2.test.ts +++ b/src/test/issues.c2.test.ts @@ -16,6 +16,7 @@ import { QueryType, ReferenceReflection, ReflectionKind, + ReflectionSymbolId, ReflectionType, SignatureReflection, UnionType, @@ -1732,4 +1733,51 @@ describe("Issue Tests", () => { [undefined, "2", '"counterclockwise"'], ); }); + + it("#2700a correctly parses links to global properties", () => { + const project = convert(); + app.options.setValue("validation", { + invalidLink: true, + notDocumented: false, + notExported: false, + }); + + app.validate(project); + logger.expectMessage( + 'warn: Failed to resolve link to "Map.size | size user specified" in comment for abc', + ); + logger.expectMessage( + 'warn: Failed to resolve link to "Map.size user specified" in comment for abc', + ); + logger.expectMessage( + 'warn: Failed to resolve link to "Map.size" in comment for abc', + ); + + const abc = query(project, "abc"); + const link = abc.comment?.summary.find((c) => c.kind === "inline-tag"); + ok(link?.target instanceof ReflectionSymbolId); + }); + + it("#2700b respects user specified link text when resolving external links", () => { + const project = convert(); + + const abc = query(project, "abc"); + ok(abc.comment); + + const resolvers = app.converter["_externalSymbolResolvers"].slice(); + app.converter.addUnknownSymbolResolver(() => { + return { + target: "https://typedoc.org", + caption: "resolver caption", + }; + }); + app.converter.resolveLinks(abc.comment, abc); + app.converter["_externalSymbolResolvers"] = resolvers; + + equal(getLinks(abc), [ + { display: "size user specified", target: "https://typedoc.org" }, + { display: "user specified", target: "https://typedoc.org" }, + { display: "resolver caption", target: "https://typedoc.org" }, + ]); + }); });