From 172933c1ada3f10c77bbbb9215824c3c9f097b82 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Wed, 8 Jan 2025 14:39:24 -0800 Subject: [PATCH] improvement(api-markdown-documenter): Check for duplicate document paths in transformation output and log warning for any encountered (#23514) It is possible to configure the system to generate multiple output documents targeting the same file path. To help users detect such cases, logic has been added to check transformation output for any documents with duplicate paths. If any are found, a warning is logged. --- .../api-item-transforms/TransformApiModel.ts | 31 ++++++++------ .../src/api-item-transforms/Utilities.ts | 41 +++++++++++++++++++ .../src/api-item-transforms/index.ts | 1 + .../test/Utilities.test.ts | 35 ++++++++++++++++ .../src/test/EndToEndTests.ts | 15 ++----- 5 files changed, 100 insertions(+), 23 deletions(-) create mode 100644 tools/api-markdown-documenter/src/api-item-transforms/test/Utilities.test.ts diff --git a/tools/api-markdown-documenter/src/api-item-transforms/TransformApiModel.ts b/tools/api-markdown-documenter/src/api-item-transforms/TransformApiModel.ts index 146046ac73c9..d69a1453345a 100644 --- a/tools/api-markdown-documenter/src/api-item-transforms/TransformApiModel.ts +++ b/tools/api-markdown-documenter/src/api-item-transforms/TransformApiModel.ts @@ -15,7 +15,7 @@ import type { DocumentNode, SectionNode } from "../documentation-domain/index.js import { doesItemRequireOwnDocument, shouldItemBeIncluded } from "./ApiItemTransformUtilities.js"; import { apiItemToDocument, apiItemToSections } from "./TransformApiItem.js"; -import { createDocument } from "./Utilities.js"; +import { checkForDuplicateDocumentPaths, createDocument } from "./Utilities.js"; import { type ApiItemTransformationConfiguration, type ApiItemTransformationOptions, @@ -37,10 +37,10 @@ export function transformApiModel(options: ApiItemTransformationOptions): Docume // If a package has multiple entry-points, it's possible for the same API item to appear under more than one // entry-point (i.e., we are traversing a graph, rather than a tree). // To avoid redundant computation, we will keep a ledger of which API items we have transformed. - const documents: Map = new Map(); + const documentsMap: Map = new Map(); // Always render Model document (this is the "root" of the generated documentation suite). - documents.set(apiModel, createDocumentForApiModel(apiModel, config)); + documentsMap.set(apiModel, createDocumentForApiModel(apiModel, config)); const packages = apiModel.packages; @@ -75,42 +75,49 @@ export function transformApiModel(options: ApiItemTransformationOptions): Docume const entryPoint = packageEntryPoints[0]; - documents.set( + documentsMap.set( packageItem, createDocumentForSingleEntryPointPackage(packageItem, entryPoint, config), ); const packageDocumentItems = getDocumentItems(entryPoint, config); for (const apiItem of packageDocumentItems) { - if (!documents.has(apiItem)) { - documents.set(apiItem, apiItemToDocument(apiItem, config)); + if (!documentsMap.has(apiItem)) { + documentsMap.set(apiItem, apiItemToDocument(apiItem, config)); } } } else { // If a package contains multiple entry-points, we will generate a separate document for each. // The package-level document will enumerate the entry-points. - documents.set( + documentsMap.set( packageItem, createDocumentForMultiEntryPointPackage(packageItem, packageEntryPoints, config), ); for (const entryPoint of packageEntryPoints) { - documents.set(entryPoint, createDocumentForApiEntryPoint(entryPoint, config)); + documentsMap.set(entryPoint, createDocumentForApiEntryPoint(entryPoint, config)); const packageDocumentItems = getDocumentItems(entryPoint, config); for (const apiItem of packageDocumentItems) { - if (!documents.has(apiItem)) { - documents.set(apiItem, apiItemToDocument(apiItem, config)); + if (!documentsMap.has(apiItem)) { + documentsMap.set(apiItem, apiItemToDocument(apiItem, config)); } } } } } - logger.success("API Model documents generated!"); + const documents = [...documentsMap.values()]; + + try { + checkForDuplicateDocumentPaths(documents); + } catch (error: unknown) { + logger.warning((error as Error).message); + } - return [...documents.values()]; + logger.success("API Model documents generated!"); + return documents; } /** diff --git a/tools/api-markdown-documenter/src/api-item-transforms/Utilities.ts b/tools/api-markdown-documenter/src/api-item-transforms/Utilities.ts index 015e93020ff2..edc906a01381 100644 --- a/tools/api-markdown-documenter/src/api-item-transforms/Utilities.ts +++ b/tools/api-markdown-documenter/src/api-item-transforms/Utilities.ts @@ -95,3 +95,44 @@ function resolveSymbolicLink( return getLinkForApiItem(resolvedReference, config); } + +/** + * Checks for duplicate {@link DocumentNode.documentPath}s among the provided set of documents. + * @throws If any duplicates are found. + */ +export function checkForDuplicateDocumentPaths(documents: readonly DocumentNode[]): void { + const documentPathMap = new Map(); + for (const document of documents) { + let entries = documentPathMap.get(document.documentPath); + if (entries === undefined) { + entries = []; + documentPathMap.set(document.documentPath, entries); + } + entries.push(document); + } + + const duplicates = [...documentPathMap.entries()].filter( + ([, documentsUnderPath]) => documentsUnderPath.length > 1, + ); + + if (duplicates.length === 0) { + return; + } + + const errorMessageLines = ["Duplicate output paths found among the generated documents:"]; + + for (const [documentPath, documentsUnderPath] of duplicates) { + errorMessageLines.push(`- ${documentPath}`); + for (const document of documentsUnderPath) { + const errorEntry = document.apiItem + ? `${document.apiItem.displayName} (${document.apiItem.kind})` + : "(No corresponding API item)"; + errorMessageLines.push(` - ${errorEntry}`); + } + } + errorMessageLines.push( + "Check your configuration to ensure different API items do not result in the same output path.", + ); + + throw new Error(errorMessageLines.join("\n")); +} diff --git a/tools/api-markdown-documenter/src/api-item-transforms/index.ts b/tools/api-markdown-documenter/src/api-item-transforms/index.ts index e09cef05e057..e41a85f52b11 100644 --- a/tools/api-markdown-documenter/src/api-item-transforms/index.ts +++ b/tools/api-markdown-documenter/src/api-item-transforms/index.ts @@ -43,3 +43,4 @@ export { export { transformTsdocNode } from "./TsdocNodeTransforms.js"; export { apiItemToDocument, apiItemToSections } from "./TransformApiItem.js"; export { transformApiModel } from "./TransformApiModel.js"; +export { checkForDuplicateDocumentPaths } from "./Utilities.js"; diff --git a/tools/api-markdown-documenter/src/api-item-transforms/test/Utilities.test.ts b/tools/api-markdown-documenter/src/api-item-transforms/test/Utilities.test.ts new file mode 100644 index 000000000000..ce3cc375dfd3 --- /dev/null +++ b/tools/api-markdown-documenter/src/api-item-transforms/test/Utilities.test.ts @@ -0,0 +1,35 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { expect } from "chai"; + +import type { DocumentNode } from "../../index.js"; +import { checkForDuplicateDocumentPaths } from "../Utilities.js"; + +describe("ApiItem to Documentation transformation utilities tests", () => { + describe("checkForDuplicateDocumentPaths", () => { + it("Empty list", () => { + expect(() => checkForDuplicateDocumentPaths([])).to.not.throw(); + }); + + it("No duplicates", () => { + const documents: DocumentNode[] = [ + { documentPath: "foo" } as unknown as DocumentNode, + { documentPath: "bar" } as unknown as DocumentNode, + { documentPath: "baz" } as unknown as DocumentNode, + ]; + expect(() => checkForDuplicateDocumentPaths(documents)).to.not.throw(); + }); + + it("Contains duplicates", () => { + const documents: DocumentNode[] = [ + { documentPath: "foo" } as unknown as DocumentNode, + { documentPath: "bar" } as unknown as DocumentNode, + { documentPath: "foo" } as unknown as DocumentNode, + ]; + expect(() => checkForDuplicateDocumentPaths(documents)).to.throw(); + }); + }); +}); diff --git a/tools/api-markdown-documenter/src/test/EndToEndTests.ts b/tools/api-markdown-documenter/src/test/EndToEndTests.ts index ec2c8f93c78f..4fccb6321cfc 100644 --- a/tools/api-markdown-documenter/src/test/EndToEndTests.ts +++ b/tools/api-markdown-documenter/src/test/EndToEndTests.ts @@ -13,8 +13,9 @@ import type { Suite } from "mocha"; import { loadModel } from "../LoadModel.js"; import { - transformApiModel, type ApiItemTransformationOptions, + checkForDuplicateDocumentPaths, + transformApiModel, } from "../api-item-transforms/index.js"; import type { DocumentNode } from "../documentation-domain/index.js"; @@ -155,16 +156,8 @@ export function endToEndTests( it("Ensure no duplicate file paths", () => { const documents = transformApiModel(apiItemTransformConfig); - const pathMap = new Map(); - for (const document of documents) { - if (pathMap.has(document.documentPath)) { - expect.fail( - `Rendering generated multiple documents to be rendered to the same file path.`, - ); - } else { - pathMap.set(document.documentPath, document); - } - } + // Will throw if any duplicates are found. + checkForDuplicateDocumentPaths(documents); }); // Perform actual output snapshot comparison test against checked-in test collateral.