diff --git a/sphinx_js/js/call_typedoc.ts b/sphinx_js/js/call_typedoc.ts index 43d2aa29..cc20bdcc 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,13 @@ 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; } + const config = await loadConfig(app.options.getValue("sphinxJsConfig")); + const symbolToType = redirectPrivateTypes(app); const project = await app.convert(); if (!project) { @@ -73,12 +75,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..5b61478c --- /dev/null +++ b/sphinx_js/js/redirectPrivateAliases.ts @@ -0,0 +1,151 @@ +/** + * 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"; + +// Map from the Symbol that is the target of the broken reference to the type +// reflection that it should be replaced by. Depending on whether the reference +// type holds a symbolId or a reflection, we use fileName:position or +// fileName:symbolName as the key (respectively). We could always use the +// symbolName but the position is more specific. +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; +} + +/** + * @param app The typedoc app + * @returns The type reference redirect table to be used in renderType.ts + */ +export function redirectPrivateTypes(app: Application): ReadonlySymbolToType { + const referencedSymbols = 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); + } + }, + ); + + /** + * Get the set of ts.symbols referenced from a ModuleReflection or + * ProjectReflection if there is only one file. + */ + function getReferencedSymbols(owningModule: Reflection): Set { + let set = referencedSymbols.get(owningModule); + if (set) { + return set; + } + set = new Set(); + referencedSymbols.set(owningModule, set); + return set; + } + + function discoverMissingExports( + owningModule: Reflection, + context: Context, + ): ts.Symbol[] { + // An export is missing if it was referenced and is not contained in the + // documented + const referenced = getReferencedSymbols(owningModule); + return Array.from(referenced).filter((s) => { + const refl = context.project.getReflectionFromSymbol(s); + return !refl || refl.flags.isPrivate; + }); + } + + const origCreateSymbolReference = ReferenceType.createSymbolReference; + ReferenceType.createSymbolReference = function (symbol, context, name) { + const owningModule = getOwningModule(context); + getReferencedSymbols(owningModule).add(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); + + const missing = discoverMissingExports(mod, context); + 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..18b99f3b --- /dev/null +++ b/sphinx_js/js/typedocPlugin.ts @@ -0,0 +1,16 @@ +/** + * Typedoc plugin which adds --sphinxJsConfig option + */ + +// TODO: we don't seem to resolve imports correctly in this file, but it works +// to do a dynamic import. Figure out why. + +export async function load(app: any) { + // @ts-ignore + const typedoc = await import("typedoc"); + app.options.addDeclaration({ + name: "sphinxJsConfig", + help: "[typedoc-plugin-sphinx-js]: the sphinx-js config", + type: typedoc.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_paths.py b/tests/test_paths.py index 9996f8d2..86b5f719 100644 --- a/tests/test_paths.py +++ b/tests/test_paths.py @@ -94,7 +94,7 @@ def test_global_install(tmp_path_factory, monkeypatch): tmpdir2 = tmp_path_factory.mktemp("blah") monkeypatch.setenv("npm_config_prefix", str(tmpdir)) monkeypatch.setenv("PATH", str(tmpdir / "bin"), prepend=":") - subprocess.run(["npm", "i", "-g", "typedoc"]) + subprocess.run(["npm", "i", "-g", "typedoc", "typescript"]) typedoc = search_node_modules("typedoc", "typedoc/bin/typedoc", str(tmpdir2)) monkeypatch.setenv("TYPEDOC_NODE_MODULES", str(Path(typedoc).parents[3])) dir = Path(__file__).parents[1].resolve() / "sphinx_js/js" @@ -111,6 +111,7 @@ def test_global_install(tmp_path_factory, monkeypatch): capture_output=True, encoding="utf8", ) + print(res.stdout) print(res.stderr) res.check_returncode() assert "TypeDoc 0.25" in res.stdout 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; }"