From ad8a3e57f447f359a73daa0e7de35408b8dc6226 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 1 May 2024 16:57:47 +0200 Subject: [PATCH] Add a typedoc plugin to resolve References to private type aliases If we have a private type alias that appears in a public function, it generates a broken XRef. This replaces these type references with a reflection containing the original contents. It's a bit involved, we had to add a typedoc plugin based on `typedoc-plugin-missing-exports` to create the missing Reflections --- sphinx_js/js/call_typedoc.ts | 41 ++--- sphinx_js/js/convertTopLevel.ts | 12 +- sphinx_js/js/redirectPrivateAliases.ts | 148 ++++++++++++++++++ sphinx_js/js/renderType.ts | 48 +++++- sphinx_js/js/typedocPatches.ts | 7 + sphinx_js/js/typedocPlugin.ts | 12 ++ sphinx_js/typedoc.py | 2 +- tests/test_typedoc_analysis/source/types.ts | 11 ++ .../test_typedoc_analysis.py | 8 + 9 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 sphinx_js/js/redirectPrivateAliases.ts create mode 100644 sphinx_js/js/typedocPatches.ts create mode 100644 sphinx_js/js/typedocPlugin.ts diff --git a/sphinx_js/js/call_typedoc.ts b/sphinx_js/js/call_typedoc.ts index 43d2aa29..e78d568c 100644 --- a/sphinx_js/js/call_typedoc.ts +++ b/sphinx_js/js/call_typedoc.ts @@ -8,6 +8,8 @@ import { import { writeFile } from "fs/promises"; import { Converter } from "./convertTopLevel.ts"; import { SphinxJsConfig } from "./sphinxJsConfig.ts"; +import { fileURLToPath } from "url"; +import { redirectPrivateTypes } from "./redirectPrivateAliases.ts"; const ExitCodes = { Ok: 0, @@ -20,26 +22,25 @@ const ExitCodes = { }; async function bootstrapAppTypedoc0_25(args: string[]): Promise { - return await Application.bootstrapWithPlugins({}, [ - new ArgumentsReader(0, args), - new TypeDocReader(), - new PackageJsonReader(), - new TSConfigReader(), - new ArgumentsReader(300, args), - ]); + return await Application.bootstrapWithPlugins( + { plugin: [fileURLToPath(import.meta.resolve("./typedocPlugin.ts"))] }, + [ + new ArgumentsReader(0, args), + new TypeDocReader(), + new PackageJsonReader(), + new TSConfigReader(), + new ArgumentsReader(300, args), + ], + ); } -async function loadConfig(args: string[]): Promise { - const configIndex = args.indexOf("--sphinx-js-config"); - if (configIndex === -1) { +async function loadConfig( + configPath: string | undefined, +): Promise { + if (!configPath) { return {}; } - if (configIndex === args.length) { - console.error("Expected --sphinx-js-config to have an argument"); - process.exit(1); - } - const [_option, value] = args.splice(configIndex, 2); - const configModule = await import(value); + const configModule = await import(configPath); return configModule.config; } @@ -47,12 +48,14 @@ async function main() { // Most of this stuff is copied from typedoc/src/lib/cli.ts const start = Date.now(); const args = process.argv.slice(2); - const config = await loadConfig(args); let app = await bootstrapAppTypedoc0_25(args); if (app.options.getValue("version")) { console.log(app.toString()); return ExitCodes.Ok; } + app.options.getValue("modifierTags").push("@hidetype"); + const config = await loadConfig(app.options.getValue("sphinxJsConfig")); + const symbolToType = redirectPrivateTypes(app); const project = await app.convert(); if (!project) { @@ -73,12 +76,12 @@ async function main() { return ExitCodes.ValidationError; } - const json = app.options.getValue("json"); const basePath = app.options.getValue("basePath"); - const converter = new Converter(project, basePath, config); + const converter = new Converter(project, basePath, config, symbolToType); converter.computePaths(); const space = app.options.getValue("pretty") ? "\t" : ""; const res = JSON.stringify(converter.convertAll(), null, space); + const json = app.options.getValue("json"); await writeFile(json, res); app.logger.info(`JSON written to ${json}`); app.logger.verbose(`JSON rendering took ${Date.now() - start}ms`); diff --git a/sphinx_js/js/convertTopLevel.ts b/sphinx_js/js/convertTopLevel.ts index 4ad61471..85ef7dc8 100644 --- a/sphinx_js/js/convertTopLevel.ts +++ b/sphinx_js/js/convertTopLevel.ts @@ -31,6 +31,7 @@ import { } from "./ir.ts"; import { sep, relative } from "path"; import { SphinxJsConfig } from "./sphinxJsConfig.ts"; +import { ReadonlySymbolToType } from "./redirectPrivateAliases.ts"; export function parseFilePath(path: string, base_dir: string): string[] { // First we want to know if path is under base_dir. @@ -329,6 +330,7 @@ export class Converter { readonly project: ProjectReflection; readonly basePath: string; readonly config: SphinxJsConfig; + readonly symbolToType: ReadonlySymbolToType; readonly pathMap: Map; readonly filePathMap: Map< @@ -341,17 +343,25 @@ export class Converter { project: ProjectReflection, basePath: string, config: SphinxJsConfig, + symbolToType: ReadonlySymbolToType, ) { this.project = project; this.basePath = basePath; this.config = config; + this.symbolToType = symbolToType; this.pathMap = new Map(); this.filePathMap = new Map(); this.documentationRoots = new Set(); } renderType(type: SomeType, context: TypeContext = TypeContext.none): Type { - return renderType(this.basePath, this.pathMap, type, context); + return renderType( + this.basePath, + this.pathMap, + this.symbolToType, + type, + context, + ); } computePaths() { diff --git a/sphinx_js/js/redirectPrivateAliases.ts b/sphinx_js/js/redirectPrivateAliases.ts new file mode 100644 index 00000000..1f99edc9 --- /dev/null +++ b/sphinx_js/js/redirectPrivateAliases.ts @@ -0,0 +1,148 @@ +/** + * This is very heavily inspired by typedoc-plugin-missing-exports. + * + * The goal isn't to document the missing exports, but rather to remove them + * from the documentation of actually exported stuff. If someone says: + * + * ``` + * type MyPrivateAlias = ... + * + * function f(a: MyPrivateAlias) { + * + * } + * ``` + * + * Then the documentation for f should document the value of MyPrivateAlias. We + * create a ReflectionType for each missing export and stick them in a + * SymbolToType map which we add to the application. In renderType.ts, if we + * have a reference type we check if it's in the SymbolToType map and if so we + * can use the reflection in place of the reference. + * + * More or less unrelatedly, we also add the --sphinxJsConfig option to the + * options parser so we can pass the sphinxJsConfig on the command line. + */ +import { + Application, + Context, + Converter, + DeclarationReflection, + ProjectReflection, + ReferenceType, + Reflection, + ReflectionKind, + SomeType, +} from "typedoc"; +import ts from "typescript"; + +type SymbolToTypeKey = `${string}:${number}` | `${string}:${string}`; +export type SymbolToType = Map; +export type ReadonlySymbolToType = ReadonlyMap; + +const ModuleLike: ReflectionKind = + ReflectionKind.Project | ReflectionKind.Module; + +function getOwningModule(context: Context): Reflection { + let refl = context.scope; + // Go up the reflection hierarchy until we get to a module + while (!refl.kindOf(ModuleLike)) { + refl = refl.parent!; + } + + return refl; +} + +export function redirectPrivateTypes(app: Application): ReadonlySymbolToType { + const referencedSymbols = new Map>(); + const symbolToOwningModule = new Map(); + const knownPrograms = new Map(); + const symbolToType: SymbolToType = new Map<`${string}:${number}`, SomeType>(); + + app.converter.on( + Converter.EVENT_CREATE_DECLARATION, + (context: Context, refl: Reflection) => { + if (refl.kindOf(ModuleLike)) { + knownPrograms.set(refl, context.program); + } + }, + ); + + function discoverMissingExports( + owningModule: Reflection, + context: Context, + program: ts.Program, + ): Set { + // An export is missing if if was referenced and is not contained in the + // documented + const referenced = referencedSymbols.get(program) || new Set(); + const ownedByOther = new Set(); + referencedSymbols.set(program, ownedByOther); + + for (const s of [...referenced]) { + const refl = context.project.getReflectionFromSymbol(s); + if (refl && !refl.flags.isPrivate) { + // Already documented + referenced.delete(s); + } else if (symbolToOwningModule.get(s) !== owningModule) { + referenced.delete(s); + ownedByOther.add(s); + } + } + + return referenced; + } + + const origCreateSymbolReference = ReferenceType.createSymbolReference; + ReferenceType.createSymbolReference = function (symbol, context, name) { + const owningModule = getOwningModule(context); + const set = referencedSymbols.get(context.program); + symbolToOwningModule.set(symbol, owningModule); + if (set) { + set.add(symbol); + } else { + referencedSymbols.set(context.program, new Set([symbol])); + } + return origCreateSymbolReference.call(this, symbol, context, name); + }; + + function onResolveBegin(context: Context) { + const modules: (DeclarationReflection | ProjectReflection)[] = + context.project.getChildrenByKind(ReflectionKind.Module); + if (modules.length === 0) { + // Single entry point, just target the project. + modules.push(context.project); + } + + for (const mod of modules) { + const program = knownPrograms.get(mod); + if (!program) continue; + + // Nasty hack here that will almost certainly break in future TypeDoc versions. + context.setActiveProgram(program); + + let missing = discoverMissingExports(mod, context, program); + for (const name of missing) { + const decl = name.declarations![0]; + if (decl.getSourceFile().fileName.includes("node_modules")) { + continue; + } + // TODO: maybe handle things other than TypeAliases? + if (ts.isTypeAliasDeclaration(decl)) { + const sf = decl.getSourceFile(); + const fileName = sf.fileName; + const pos = decl.pos; + const converted = context.converter.convertType(context, decl.type); + // Depending on whether we have a symbolId or a reflection in + // renderType we'll use different keys to look this up. + symbolToType.set(`${fileName}:${pos}`, converted); + // Ideally we should be able to key on position rather than file and + // name when the reflection is present but I couldn't figure out how. + symbolToType.set(`${fileName}:${decl.name.getText()}`, converted); + } + } + context.setActiveProgram(void 0); + } + } + + app.converter.on(Converter.EVENT_RESOLVE_BEGIN, onResolveBegin); + return symbolToType; +} diff --git a/sphinx_js/js/renderType.ts b/sphinx_js/js/renderType.ts index 74b418f0..179e192d 100644 --- a/sphinx_js/js/renderType.ts +++ b/sphinx_js/js/renderType.ts @@ -34,6 +34,7 @@ import { intrinsicType, } from "./ir.ts"; import { parseFilePath } from "./convertTopLevel.ts"; +import { ReadonlySymbolToType } from "./redirectPrivateAliases.ts"; /** * Render types into a list of strings and XRefs. @@ -46,16 +47,23 @@ import { parseFilePath } from "./convertTopLevel.ts"; class TypeRenderer implements TypeVisitor { private readonly basePath: string; // For resolving XRefs. - private readonly reflToPath: Map< + private readonly reflToPath: ReadonlyMap< DeclarationReflection | SignatureReflection, string[] >; + private readonly symbolToType: ReadonlySymbolToType; + constructor( basePath: string, - reflToPath: Map, + reflToPath: ReadonlyMap< + DeclarationReflection | SignatureReflection, + string[] + >, + symbolToType: ReadonlySymbolToType, ) { this.basePath = basePath; this.reflToPath = reflToPath; + this.symbolToType = symbolToType; } /** @@ -153,6 +161,24 @@ class TypeRenderer implements TypeVisitor { return this.addTypeParams(type, [type.name]); } if (type.reflection) { + const refl = type.reflection; + if (refl.flags.isPrivate && refl.isDeclaration()) { + // If it's private, we don't really want to emit an XRef to it. In the + // typedocPlugin.ts we tried to calculate Reflections for these, so now + // we try to look it up. I couldn't get the line+column numbers to match + // up so in this case we index on file name and reference name. + + // Another place where we incorrectly handle merged declarations + const src = refl.sources![0]; + const newTarget = this.symbolToType.get( + `${src.fullFileName}:${refl.name}`, + ); + if (newTarget) { + // TODO: this doesn't handle parentheses correctly. + return newTarget.visit(this); + } + } + const path = this.reflToPath.get( type.reflection as DeclarationReflection, ); @@ -171,6 +197,16 @@ class TypeRenderer implements TypeVisitor { if (!type.symbolId) { throw new Error("This should not happen"); } + // See if this refers to a private type. In that case we should inline the + // type reflection rather than referring to the non-exported name. + const newTarget = this.symbolToType.get( + `${type.symbolId.fileName}:${type.symbolId.pos}`, + ); + if (newTarget) { + // TODO: this doesn't handle parentheses correctly. + return newTarget.visit(this); + } + const path = parseFilePath(type.symbolId.fileName, this.basePath); let res: TypeXRefExternal | TypeXRefInternal; if (path.includes("node_modules/")) { @@ -323,10 +359,14 @@ class TypeRenderer implements TypeVisitor { export function renderType( basePath: string, - reflToPath: Map, + reflToPath: ReadonlyMap< + DeclarationReflection | SignatureReflection, + string[] + >, + symbolToType: ReadonlySymbolToType, type: SomeType, context: TypeContext = TypeContext.none, ): Type { - const renderer = new TypeRenderer(basePath, reflToPath); + const renderer = new TypeRenderer(basePath, reflToPath, symbolToType); return renderer.render(type, context); } diff --git a/sphinx_js/js/typedocPatches.ts b/sphinx_js/js/typedocPatches.ts new file mode 100644 index 00000000..bd02cd4c --- /dev/null +++ b/sphinx_js/js/typedocPatches.ts @@ -0,0 +1,7 @@ +/** Declare some extra stuff we monkeypatch on to typedoc */ + +declare module "typedoc" { + export interface TypeDocOptionMap { + sphinxJsConfig: string; + } +} diff --git a/sphinx_js/js/typedocPlugin.ts b/sphinx_js/js/typedocPlugin.ts new file mode 100644 index 00000000..28045bc6 --- /dev/null +++ b/sphinx_js/js/typedocPlugin.ts @@ -0,0 +1,12 @@ +/** + * Typedoc plugin which adds --sphinxJsConfig option + */ +import { Application, ParameterType } from "typedoc"; + +export function load(app: Application) { + app.options.addDeclaration({ + name: "sphinxJsConfig", + help: "[typedoc-plugin-sphinx-js]: the sphinx-js config", + type: ParameterType.String, + }); +} diff --git a/sphinx_js/typedoc.py b/sphinx_js/typedoc.py index 01ea597f..e955b2e8 100644 --- a/sphinx_js/typedoc.py +++ b/sphinx_js/typedoc.py @@ -72,7 +72,7 @@ def typedoc_output( command.add("--import", str(dir / "registerImportHook.mjs")) command.add(str(dir / "call_typedoc.ts")) if ts_sphinx_js_config: - command.add("--sphinx-js-config", ts_sphinx_js_config) + command.add("--sphinxJsConfig", ts_sphinx_js_config) command.add("--entryPointStrategy", "expand") if typedoc_config_path: diff --git a/tests/test_typedoc_analysis/source/types.ts b/tests/test_typedoc_analysis/source/types.ts index 64442e03..3130e8b2 100644 --- a/tests/test_typedoc_analysis/source/types.ts +++ b/tests/test_typedoc_analysis/source/types.ts @@ -203,3 +203,14 @@ export function namedTupleArg(namedTuple: [key: string, value: any]) {} export let queryType: typeof A; export let typeOperatorType: keyof A; + +type PrivateTypeAlias1 = { a: number; b: string }; + +// Should expand the private type alias +export let typeIsPrivateTypeAlias1: PrivateTypeAlias1; + +/** @private */ +export type PrivateTypeAlias2 = { a: number; b: string }; + +// Should expand the private type alias +export let typeIsPrivateTypeAlias2: PrivateTypeAlias2; diff --git a/tests/test_typedoc_analysis/test_typedoc_analysis.py b/tests/test_typedoc_analysis/test_typedoc_analysis.py index c1ff9178..fe49e03e 100644 --- a/tests/test_typedoc_analysis/test_typedoc_analysis.py +++ b/tests/test_typedoc_analysis/test_typedoc_analysis.py @@ -627,3 +627,11 @@ def test_query(self): def test_type_operator(self): obj = self.analyzer.get_object(["typeOperatorType"]) assert join_type(obj.type) == "keyof A" + + def test_private_type_alias1(self): + obj = self.analyzer.get_object(["typeIsPrivateTypeAlias1"]) + assert join_type(obj.type) == "{ a: number; b: string; }" + + def test_private_type_alias2(self): + obj = self.analyzer.get_object(["typeIsPrivateTypeAlias2"]) + assert join_type(obj.type) == "{ a: number; b: string; }"