Skip to content

Commit

Permalink
refactor(api-markdown-documenter): Add first-step functionality for A…
Browse files Browse the repository at this point in the history
…PI Model TSDoc linting (#22150)

Adds function `lintApiModel`, which performs API Model "linting"
validation. For now, it strictly evaluates `{@inheritdoc}` tags to
ensure the target reference is valid within the Model. More features
will be added in subsequent PRs.

Note that the API is not yet-package exported. That will come in a
future PR after the core functionality has landed.

Also updates the `simple-suite-test` test data to include an example of
an `{@inheritdoc}` comment with an invalid target.
  • Loading branch information
Josmithr committed Aug 15, 2024
1 parent 24ad748 commit 56989e4
Show file tree
Hide file tree
Showing 22 changed files with 470 additions and 49 deletions.
187 changes: 187 additions & 0 deletions tools/api-markdown-documenter/src/LintApiModel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

import { fail } from "node:assert";
import {
ApiDocumentedItem,
type ApiItem,
ApiItemContainerMixin,
type ApiModel,
} from "@microsoft/api-extractor-model";
import type { DocInheritDocTag } from "@microsoft/tsdoc";
import { defaultConsoleLogger } from "./Logging.js";
import { resolveSymbolicReference } from "./utilities/index.js";
import type { ConfigurationBase } from "./ConfigurationBase.js";

/**
* {@link lintApiModel} configuration.
*/
export interface LintApiModelConfiguration extends ConfigurationBase {
/**
* The API model to lint.
*/
apiModel: ApiModel;
}

/**
* {@link LintApiModelConfiguration} defaults.
*/
const defaultLintApiModelConfiguration: Required<Omit<LintApiModelConfiguration, "apiModel">> = {
logger: defaultConsoleLogger,
};

// TODO: common TsdocError base (associatedItem, packageName)

/**
* An error resulting from a reference tag (e.g., `link` or `inheritDoc` tags) with an invalid target.
*/
export interface ReferenceError {
/**
* The tag name with the invalid reference.
*/
readonly tagName: string;

/**
* Name of the item that included a reference to an invalid target.
*/
readonly sourceItem: string;

/**
* The name of the package that the {@link ReferenceError.sourceItem} belongs to.
*/
readonly packageName: string;

/**
* The string provided as the reference in a reference tag.
*/
readonly referenceTarget: string;

/**
* Link alias text, if any.
*/
readonly linkText: string | undefined;
}

/**
* Mutable {@link LinterErrors}.
* @remarks Used while walking the API model to accumulate errors, and converted to {@link LinterErrors} to return to the caller.
*/
interface MutableLinterErrors {
readonly referenceErrors: Set<ReferenceError>;
}

/**
* Errors found during linting.
*/
export interface LinterErrors {
/**
* Errors related to reference tags (e.g., `link` or `inheritDoc` tags) with invalid targets.
*/
readonly referenceErrors: ReadonlySet<ReferenceError>;

// TODO: add other error kinds as needed.
}

/**
* Validates the given API model.
*
* @returns The set of errors encountered during linting, if any were found.
* Otherwise, `undefined`.
*/
export async function lintApiModel(
configuration: LintApiModelConfiguration,
): Promise<LinterErrors | undefined> {
const optionsWithDefaults: Required<LintApiModelConfiguration> = {
...defaultLintApiModelConfiguration,
...configuration,
};
const { apiModel, logger } = optionsWithDefaults;

logger.verbose("Linting API model...");

const errors: MutableLinterErrors = {
referenceErrors: new Set<ReferenceError>(),
};
lintApiItem(apiModel, apiModel, optionsWithDefaults, errors);
const anyErrors = errors.referenceErrors.size > 0;

logger.verbose("API model linting completed.");
logger.verbose(`Linting result: ${anyErrors ? "failure" : "success"}.`);

return anyErrors
? {
referenceErrors: errors.referenceErrors,
}
: undefined;
}

/**
* Recursively validates the given API item and all its descendants within the API model.
*/
function lintApiItem(
apiItem: ApiItem,
apiModel: ApiModel,
options: Required<LintApiModelConfiguration>,
errors: MutableLinterErrors,
): void {
// If the item is documented, lint its documentation contents.
if (apiItem instanceof ApiDocumentedItem && apiItem.tsdocComment !== undefined) {
const comment = apiItem.tsdocComment;

// Lint `@inheritDoc` tag, if present
// Note: API-Extractor resolves `@inheritDoc` during model generation, so such tags are never expected to appear in the `tsdocComment` tree (unless malformed).
// Therefore, we need to handle them specially here.
if (comment.inheritDocTag !== undefined) {
// eslint-disable-next-line unicorn/prevent-abbreviations
const inheritDocReferenceError = checkInheritDocTag(
comment.inheritDocTag,
apiItem,
apiModel,
);
if (inheritDocReferenceError !== undefined) {
errors.referenceErrors.add(inheritDocReferenceError);
}
}

// TODO: Check other TSDoc contents
}

// If the item has children, recursively validate them.
if (ApiItemContainerMixin.isBaseClassOf(apiItem)) {
for (const member of apiItem.members) {
lintApiItem(member, apiModel, options, errors);
}
}
}

/**
* Checks the provided API item's `{@inheritDoc}` tag, ensuring that the target reference is valid within the API model.
*/
// eslint-disable-next-line unicorn/prevent-abbreviations
function checkInheritDocTag(
// eslint-disable-next-line unicorn/prevent-abbreviations
inheritDocTag: DocInheritDocTag,
associatedItem: ApiDocumentedItem,
apiModel: ApiModel,
): ReferenceError | undefined {
if (inheritDocTag?.declarationReference === undefined) {
return undefined;
}

try {
resolveSymbolicReference(associatedItem, inheritDocTag.declarationReference, apiModel);
} catch {
return {
tagName: "@inheritDoc",
sourceItem: associatedItem.getScopedNameWithinPackage(),
packageName:
associatedItem.getAssociatedPackage()?.name ?? fail("Package name not found"),
referenceTarget: inheritDocTag.declarationReference.emitAsTsdoc(),
linkText: undefined,
};
}

return undefined;
}
2 changes: 1 addition & 1 deletion tools/api-markdown-documenter/src/LoadModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const defaultLoadModelOptions: Required<Omit<LoadModelOptions, "modelDire
* @param reportsDirectoryPath - Path to the directory containing the API reports.
* @param logger - Optional logger for reporting system events while loading the model.
*
* @throws If {@link LoadModelOptions.modelDirectoryPath} doesn't exist, or if no `.api.json` files are found directly under it.
* @throws If the specified {@link LoadModelOptions.modelDirectoryPath} doesn't exist, or if no `.api.json` files are found directly under it.
*
* @public
*/
Expand Down
6 changes: 3 additions & 3 deletions tools/api-markdown-documenter/src/Logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export interface Logger {
verbose: LoggingFunction;
}

function noop(): void {}

/**
* Default logger, configured to log to the console.
*
Expand All @@ -55,9 +57,7 @@ export const defaultConsoleLogger: Logger = {
warning: logWarningToConsole,
error: logErrorToConsole,
success: logSuccessToConsole,
verbose: () => {
/* no-op */
},
verbose: noop,
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ export interface TsdocNodeTransformOptions extends ConfigurationBase {
* Callback for resolving symbolic links to API items.
*
* @param codeDestination - The referenced target.
* @param contextApiItem -
*
* @returns The appropriate URL target if the reference can be resolved. Otherwise, `undefined`.
* @returns The appropriate URL target if the reference can be resolved.
* Otherwise, `undefined`.
*/
readonly resolveApiReference: (codeDestination: DocDeclarationReference) => Link | undefined;
}
Expand Down
43 changes: 9 additions & 34 deletions tools/api-markdown-documenter/src/api-item-transforms/Utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
* Licensed under the MIT License.
*/

import {
ApiItemKind,
type ApiItem,
type IResolveDeclarationReferenceResult,
type ApiPackage,
} from "@microsoft/api-extractor-model";
import type { ApiItem } from "@microsoft/api-extractor-model";
import { type DocDeclarationReference } from "@microsoft/tsdoc";

import { DocumentNode, type SectionNode } from "../documentation-domain/index.js";
Expand All @@ -21,6 +16,7 @@ import {
import { type TsdocNodeTransformOptions } from "./TsdocNodeTransforms.js";
import { type ApiItemTransformationConfiguration } from "./configuration/index.js";
import { wrapInSection } from "./helpers/index.js";
import { resolveSymbolicReference } from "../utilities/index.js";

/**
* Creates a {@link DocumentNode} representing the provided API item.
Expand Down Expand Up @@ -82,40 +78,19 @@ function resolveSymbolicLink(
): Link | undefined {
const { apiModel, logger } = config;

const resolvedReference: IResolveDeclarationReferenceResult =
apiModel.resolveDeclarationReference(codeDestination, contextApiItem);

if (resolvedReference.resolvedApiItem === undefined) {
logger.warning(
`Unable to resolve reference "${codeDestination.emitAsTsdoc()}" from "${getScopedMemberNameForDiagnostics(
contextApiItem,
)}":`,
resolvedReference.errorMessage,
);

let resolvedReference: ApiItem;
try {
resolvedReference = resolveSymbolicReference(contextApiItem, codeDestination, apiModel);
} catch (error: unknown) {
logger.warning((error as Error).message);
return undefined;
}
const resolvedApiItem = resolvedReference.resolvedApiItem;

// Return undefined if the resolved API item should be excluded based on release tags
if (!shouldItemBeIncluded(resolvedApiItem, config)) {
if (!shouldItemBeIncluded(resolvedReference, config)) {
logger.verbose("Excluding link to item based on release tags");
return undefined;
}

return getLinkForApiItem(resolvedReference.resolvedApiItem, config);
}

/**
* Creates a scoped member specifier for the provided API item, including the name of the package the item belongs to
* if applicable.
*
* Intended for use in diagnostic messaging.
*/
export function getScopedMemberNameForDiagnostics(apiItem: ApiItem): string {
return apiItem.kind === ApiItemKind.Package
? (apiItem as ApiPackage).displayName
: `${
apiItem.getAssociatedPackage()?.displayName ?? "<NO-PACKAGE>"
}#${apiItem.getScopedNameWithinPackage()}`;
return getLinkForApiItem(resolvedReference, config);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import {
} from "@microsoft/api-extractor-model";

import type { SectionNode } from "../../documentation-domain/index.js";
import { ApiModifier, isStatic } from "../../utilities/index.js";
import { ApiModifier, getScopedMemberNameForDiagnostics, isStatic } from "../../utilities/index.js";
import type { ApiItemTransformationConfiguration } from "../configuration/index.js";
import { createChildDetailsSection, createMemberTables } from "../helpers/index.js";
import { filterChildMembers } from "../ApiItemTransformUtilities.js";
import { getScopedMemberNameForDiagnostics } from "../Utilities.js";

/**
* Default documentation transform for `Class` items.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { DocumentationNode, SectionNode } from "../../documentation-domain/
import type { ApiItemTransformationConfiguration } from "../configuration/index.js";
import { createMemberTables, wrapInSection } from "../helpers/index.js";
import { filterChildMembers } from "../ApiItemTransformUtilities.js";
import { getScopedMemberNameForDiagnostics } from "../Utilities.js";
import { getScopedMemberNameForDiagnostics } from "../../utilities/index.js";

/**
* Default documentation transform for `Enum` items.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { SectionNode } from "../../documentation-domain/index.js";
import type { ApiItemTransformationConfiguration } from "../configuration/index.js";
import { createChildDetailsSection, createMemberTables } from "../helpers/index.js";
import { filterChildMembers } from "../ApiItemTransformUtilities.js";
import { getScopedMemberNameForDiagnostics } from "../Utilities.js";
import { getScopedMemberNameForDiagnostics } from "../../utilities/index.js";

/**
* Default documentation transform for `Interface` items.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { ApiModuleLike } from "../../utilities/index.js";
import type { ApiItemTransformationConfiguration } from "../configuration/index.js";
import { createChildDetailsSection, createMemberTables } from "../helpers/index.js";
import { filterItems } from "../ApiItemTransformUtilities.js";
import { getScopedMemberNameForDiagnostics } from "../Utilities.js";
import { getScopedMemberNameForDiagnostics } from "../../utilities/index.js";

/**
* Default documentation transform for module-like API items (packages, namespaces).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function documentationNodeToHtml(
): HastNodes {
if (context.transformations[node.type] === undefined) {
throw new Error(
`Encountered a DocumentationNode with neither a user-provided nor system-default renderer. Type: ${node.type}. Please provide a transformation for this type.`,
`Encountered a DocumentationNode with neither a user-provided nor system-default renderer. Type: "${node.type}". Please provide a transformation for this type.`,
);
}
return context.transformations[node.type](node, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { type Transformations } from "./Transformation.js";
*/
export interface TransformationConfig extends ConfigurationBase {
/**
* User-specified renderers.
* User-specified transformations.
*
* @remarks May override default behaviors or add transformation capabilities for custom {@link DocumentationNode}s.
*/
Expand Down
41 changes: 41 additions & 0 deletions tools/api-markdown-documenter/src/test/LintApiModel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

import * as Path from "node:path";
import { fileURLToPath } from "node:url";

import { expect } from "chai";

import { lintApiModel, type ReferenceError, type LinterErrors } from "../LintApiModel.js";
import { loadModel } from "../LoadModel.js";

const dirname = Path.dirname(fileURLToPath(import.meta.url));
const testModelsDirectoryPath = Path.resolve(dirname, "..", "..", "src", "test", "test-data");

describe("lintApiModel", () => {
// TODO: add case with no errors

it("API Model with invalid links yields the expected errors", async () => {
const modelDirectoryPath = Path.resolve(testModelsDirectoryPath, "simple-suite-test");
const apiModel = await loadModel({ modelDirectoryPath });

const expected: LinterErrors = {
referenceErrors: new Set<ReferenceError>([
// TODO: add other expected errors once they are validated
{
tagName: "@inheritDoc",
sourceItem: "TestInterface.propertyWithBadInheritDocTarget",
packageName: "simple-suite-test",
referenceTarget: "BadInheritDocTarget",
linkText: undefined,
},
]),
};

const result = await lintApiModel({ apiModel });

expect(result).to.deep.equal(expected);
});
});
Loading

0 comments on commit 56989e4

Please sign in to comment.