diff --git a/packages/kbn-docs-utils/PLAN.md b/packages/kbn-docs-utils/PLAN.md new file mode 100644 index 0000000000000..bb897351317a6 --- /dev/null +++ b/packages/kbn-docs-utils/PLAN.md @@ -0,0 +1,243 @@ +# Improve kbn-docs-utils validation and refactoring + +## Overview + +This plan focuses on improving code validation in `kbn-docs-utils` to ensure documented code meets JSDoc standards (per https://jsdoc.app/tags-param#parameters-with-properties) before documentation is built. The goal is to inform developers of issues early, not just generate documentation. + +## Current State Analysis + +The package currently: +- Builds API documentation from TypeScript using `ts-morph` +- Collects stats via `--stats` flag (any, comments, exports) +- Has integration tests with fixtures demonstrating various scenarios +- Validates missing comments, `any` types, and missing exports +- Handles destructured parameters but may not properly validate nested property JSDoc + +Key files: +- [packages/kbn-docs-utils/src/build_api_docs_cli.ts](packages/kbn-docs-utils/src/build_api_docs_cli.ts) - Main CLI combining build and stats +- [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) - Stats collection logic +- [packages/kbn-docs-utils/src/build_api_declarations/build_parameter_decs.ts](packages/kbn-docs-utils/src/build_api_declarations/build_parameter_decs.ts) - Parameter extraction +- [packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.ts](packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.ts) - JSDoc parsing + +## Phase 0: Baseline Documentation and Understanding ✅ + +### Phase 0.1: Document Current Behavior +- [x] Create `CURRENT_BEHAVIOR.md` documenting: + - How `collectApiStatsForPlugin` works + - What validation rules exist (missing comments, `any` types, missing exports) + - How destructured parameters are currently handled + - Known gaps (destructured params, property tags, etc.) +- [x] Document the flow: `build_api_docs_cli.ts` → `get_plugin_api_map` → `collectApiStatsForPlugin` → stats output + +### Phase 0.2: Map Integration Test Fixtures to Validation Expectations +- [x] Add inline comments to [packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/fns.ts](packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/fns.ts): + - Mark `crazyFunction` lines showing missing property-level JSDoc (should use `@param obj.hi`, `@param { fn1, fn2 }.fn1`, etc. per JSDoc spec) + - Document which lines would be flagged as out-of-norm +- [x] Add comments to other fixture files: + - [packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/classes.ts](packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/classes.ts) - Missing comments, `any` usage + - [packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/index.ts](packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/index.ts) - `any` type usage + - [packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/plugin.ts](packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/plugin.ts) - Inline object parameters + +### Phase 0.3: Run Existing Tests and Document Coverage +- [x] Run all unit tests: `mcp_Kibana_Dev_run_unit_tests` with package `@kbn/docs-utils` +- [x] Document current test coverage gaps +- [x] Identify which validation scenarios are not covered by tests + +## Phase 1: Expand Test Coverage + +### Phase 1.1: Unit Tests for Stats Collection +- [x] Create `src/stats.test.ts`: + - Test `collectApiStatsForPlugin` with various scenarios + - Test missing comment detection (including edge cases) + - Test `any` type detection + - Test missing exports detection + - Test destructured parameter handling + +### Phase 1.2: Unit Tests for Parameter Extraction +- [x] Expand `src/build_api_declarations/buid_api_declaration.test.ts`: + - Test destructured parameter extraction + - Test JSDoc property tag parsing (`@param obj.prop`) + - Test nested destructured parameters + - Test parameter comment matching + +### Phase 1.3: Unit Tests for JSDoc Utilities +- [x] Create `src/build_api_declarations/js_doc_utils.test.ts`: + - Test `getJSDocParamComment` with dot notation (`obj.prop`) + - Test property-level parameter tags + - Test nested property access patterns + +### Phase 1.4: Integration Test Enhancements +- [x] Add test cases to [packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts](packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts): + - Test that destructured params with proper JSDoc (`@param obj.prop`) are validated correctly + - Test that missing property-level comments are detected + - Test validation output format + +## Phase 2: Refactor CLI into Modular Tasks + +### Phase 2.1: Extract CLI Tasks +- [x] Create `src/cli/tasks/` directory structure +- [x] Extract tasks from `build_api_docs_cli.ts`: + - `setup_project.ts` - Project initialization, plugin discovery, path resolution + - `build_api_map.ts` - Building plugin API map + - `collect_stats.ts` - Stats collection logic + - `write_docs.ts` - Documentation writing + - `report_metrics.ts` - CI metrics reporting +- [x] Create `src/cli/types.ts` for shared CLI types +- [x] Create `src/cli/parse_cli_flags.ts` for flag parsing logic + +### Phase 2.2: Document and Test CLI Tasks +- [x] Add JSDoc comments to each task module +- [x] Create unit tests for each task: + - `src/cli/tasks/setup_project.test.ts` + - `src/cli/tasks/build_api_map.test.ts` + - `src/cli/tasks/collect_stats.test.ts` + - `src/cli/tasks/write_docs.test.ts` + - `src/cli/tasks/report_metrics.test.ts` +- [x] Create `src/cli/parse_cli_flags.test.ts` + +### Phase 2.3: Refactor Main CLI +- [x] Update [packages/kbn-docs-utils/src/build_api_docs_cli.ts](packages/kbn-docs-utils/src/build_api_docs_cli.ts) to use extracted tasks +- [x] Maintain backward compatibility +- [x] Ensure all existing functionality works (requires integration testing) + +## Phase 3: Split Stats into Separate CLI + +### Phase 3.1: Create Check Package Docs CLI +- [x] Create `src/cli/check_package_docs_cli.ts`: + - Accept `--plugin` or `--package` flag (single or multiple), treating them as aliases + - Accept validation flags: repeatable `--check` (any|comments|exports|all) + - Output validation results (pass/fail per plugin) + - Exit with non-zero code if validation fails +- [x] Create `src/cli/run_check_package_docs_cli.ts` wrapper +- [x] Export from [packages/kbn-docs-utils/index.ts](packages/kbn-docs-utils/index.ts) + +### Phase 3.2: Update Build CLI +- [x] Keep `--stats` flag on `build_api_docs_cli.ts`, emit a deprecation warning, and route invocation to the new check CLI +- [x] Update help text and documentation to note deprecation and alias behavior +- [x] Ensure build-only mode works correctly + +### Phase 3.3: Update Scripts and Documentation +- [x] Update [packages/kbn-docs-utils/src/README.md](packages/kbn-docs-utils/src/README.md) with new CLI usage +- [x] Document when to use `check_package_docs_cli` vs `build_api_docs_cli` +- [x] Update any scripts that use `--stats` flag + +## Phase 4: Fix Bugs and Fill Gaps + +### Phase 4.0: Refactor and Improve Tooling +- [x] Add `IssuesByPlugin` interface to [packages/kbn-docs-utils/src/types.ts](packages/kbn-docs-utils/src/types.ts) for extensible options +- [x] Refactor `collectApiStatsForPlugin` in [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) to use `IssuesByPlugin` +- [x] Add `hasCommentIssues` utility to [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) +- [x] Update [packages/kbn-docs-utils/src/README.md](packages/kbn-docs-utils/src/README.md) with comprehensive documentation +- [x] Add [packages/kbn-docs-utils/scripts/update_fixture_comments.js](packages/kbn-docs-utils/scripts/update_fixture_comments.js) for fixture maintenance + +### Phase 4.1: Fix Destructured Parameter Validation +- [x] Update `getJSDocParamComment` in [packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.ts](packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.ts): + - Support dot notation (`obj.prop`) for property-level tags + - Handle nested property access (`obj.nested.prop`) + - Match JSDoc spec: https://jsdoc.app/tags-param#parameters-with-properties +- [x] Update `buildApiDecsForParameters` in [packages/kbn-docs-utils/src/build_api_declarations/build_parameter_decs.ts](packages/kbn-docs-utils/src/build_api_declarations/build_parameter_decs.ts): + - For destructured params, check for property-level JSDoc tags + - Only flag missing comments if parent param AND property-level tags are missing + - Handle nested destructuring patterns + +### Phase 4.2: ReactElement Signature Skipped Test +- [x] Unskip and fix `Test ReactElement signature` in [packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts](packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts) +- [x] Implement or adjust handling for ReactElement signatures to satisfy the test + +### Phase 4.3: Improve Missing Comments Detection +- [x] Update `collectStatsForApi` in [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts): + - For destructured parameters, check if parent has comment OR properties have individual comments + - Don't flag as missing if property-level JSDoc exists + - Add validation for required vs. optional parameter documentation + +### Phase 4.4: Add Checks for @returns Tags +- [x] Add `missingReturns` field to `ApiStats` in [packages/kbn-docs-utils/src/types.ts](packages/kbn-docs-utils/src/types.ts) +- [x] Add `trackMissingReturns` function in [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) +- [x] Extend `hasCommentIssues` to include `missingReturns` +- [x] Update mocks and fixture "Expected issues" blocks + +### Phase 4.5: Add Param Doc Mismatch Checks +- [x] Add `paramDocMismatches` field to `ApiStats` in [packages/kbn-docs-utils/src/types.ts](packages/kbn-docs-utils/src/types.ts) +- [x] Add `trackParamDocMismatches` function in [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) +- [x] Extend `hasCommentIssues` to include `paramDocMismatches` +- [x] Update mocks and fixture "Expected issues" blocks + +### Phase 4.6: Add Complex Type Info Checks +- [x] Add `missingComplexTypeInfo` field to `ApiStats` in [packages/kbn-docs-utils/src/types.ts](packages/kbn-docs-utils/src/types.ts) +- [x] Add `trackMissingComplexTypeInfo` function in [packages/kbn-docs-utils/src/stats.ts](packages/kbn-docs-utils/src/stats.ts) +- [x] Extend `hasCommentIssues` to include `missingComplexTypeInfo` +- [x] Update mocks and fixture "Expected issues" blocks + +### Phase 4.7: Improve Multiple Call Signature Validation +- [x] Handle interfaces with multiple call signatures +- [x] Extract parameter documentation from first overload signature +- [x] Add test cases for overloaded functions + +### Phase 4.8: Unnamed Exports Validator +- [x] Add `UnnamedExport` type to [packages/kbn-docs-utils/src/types.ts](packages/kbn-docs-utils/src/types.ts) +- [x] Extend `IssuesByPlugin` with optional `unnamedExports` field +- [x] Update `getDeclarationNodesForPluginScope` in [packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts](packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts) to detect unnamed exports +- [x] Update `getPluginApi` in [packages/kbn-docs-utils/src/get_plugin_api.ts](packages/kbn-docs-utils/src/get_plugin_api.ts) to propagate unnamed exports +- [x] Add `--check unnamed` CLI flag +- [x] Add tests for unnamed export detection + +## Phase 5: CLI Output Improvements +- [x] Improve `getLink` in [packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts](packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts) to use line numbers when available +- [x] Add `printIssueTable` helper for consistent output formatting +- [x] Add `printMissingExportsTable` helper for missing exports output +- [x] Refactor existing console output to use helper functions + +## Phase 6: Flat Stats Output and MCP Auto-Fix Tooling + +### Phase 6.1: Emit Flat Stats File +- [x] Add `--write` CLI flag to `check_package_docs` to emit validation stats as a flat JSON file +- [x] Write stats to each plugin's `target/api_docs/stats.json` (follows Kibana convention for build artifacts) +- [x] Include line-anchored GitHub URLs when line numbers are present +- [x] Stats include counts and detailed entries for: missing comments, any types, no references, missing returns, param doc mismatches, missing complex type info, and missing exports + +### Phase 6.2: MCP Tools for Documentation Checking and Fixing +- [x] Create `check_package_docs` MCP tool for quick validation checks +- [x] Create `fix_package_docs` MCP tool for detailed issue reporting with code snippets +- [x] Both tools registered in `kbn-mcp-dev-server` +- [x] Add usage docs and basic tests for the MCP tools + +## Phase 7: Improve Performance of Single Package Builds and Validation + +### Phase 7.1: Optimize TypeScript Project Loading +- [x] Update `getTsProject` in [packages/kbn-docs-utils/src/cli/tasks/setup_project.ts](packages/kbn-docs-utils/src/cli/tasks/setup_project.ts): + - For single-plugin builds, only load source files from the target plugin directory + - Skip `resolveSourceFileDependencies()` for single-plugin builds; rely on lazy resolution +- [x] Add `allPlugins` to `SetupProjectResult` for cross-reference resolution + +### Phase 7.2: Optimize Documentation Writing +- [x] Update `writeDocs` in [packages/kbn-docs-utils/src/cli/tasks/write_docs.ts](packages/kbn-docs-utils/src/cli/tasks/write_docs.ts): + - Skip aggregate docs (plugin directory, deprecation summaries) for filtered builds + - Move deprecation doc writing outside the per-plugin loop + +### Phase 7.3: Handle Cross-References in Single-Package Builds +- [x] Update `removeBrokenLinksFromApi` in [packages/kbn-docs-utils/src/utils.ts](packages/kbn-docs-utils/src/utils.ts): + - When validating cross-package links, keep references as-is if the target plugin isn't loaded + +### Phase 7.4: Update Tests +- [x] Update [packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts](packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts) with single-plugin tests +- [x] Update other affected test files + +## Phase 8: APM Metrics for New Validation Fields +- [x] Add APM metrics for `missingReturns` count +- [x] Add APM metrics for `paramDocMismatches` count +- [x] Add APM metrics for `missingComplexTypeInfo` count +- [x] Update `passesAllChecks` logic to include new fields +- [x] Add CLI output for new validation fields under `comments` option + +## Implementation Notes + +- Each phase can be tackled independently by an agent +- Phases should be completed in order (0 → 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8) +- Tests should be written before implementing fixes (TDD approach for Phase 4) +- Maintain backward compatibility throughout +- Use existing patterns and conventions from the codebase + +## Related Documentation + +- [CURRENT_BEHAVIOR.md](./CURRENT_BEHAVIOR.md) - Current system behavior and flow + diff --git a/packages/kbn-docs-utils/scripts/update_fixture_comments.js b/packages/kbn-docs-utils/scripts/update_fixture_comments.js index aadb3c1aafd61..dd4def29af904 100644 --- a/packages/kbn-docs-utils/scripts/update_fixture_comments.js +++ b/packages/kbn-docs-utils/scripts/update_fixture_comments.js @@ -15,6 +15,7 @@ const categories = [ { key: 'paramDocMismatches', title: 'param doc mismatches' }, { key: 'isAnyType', title: 'any usage' }, { key: 'noReferences', title: 'no references' }, + { key: 'unnamedExports', title: 'unnamed exports' }, ]; /** @@ -63,11 +64,14 @@ const formatCategory = (title, entries) => { const sorted = [...entries].sort((a, b) => { const lineA = a.lineNumber ?? Number.MAX_SAFE_INTEGER; const lineB = b.lineNumber ?? Number.MAX_SAFE_INTEGER; - return lineA === lineB ? a.label.localeCompare(b.label) : lineA - lineB; + const labelA = a.label || a.textSnippet || ''; + const labelB = b.label || b.textSnippet || ''; + return lineA === lineB ? labelA.localeCompare(labelB) : lineA - lineB; }); sorted.forEach((entry) => { const lineInfo = entry.lineNumber != null ? `line ${entry.lineNumber}` : 'unknown line'; - lines.push(`// ${lineInfo} - ${entry.label}`); + const label = entry.label || entry.textSnippet || '(unnamed)'; + lines.push(`// ${lineInfo} - ${label}`); }); return lines.join('\n'); }; diff --git a/packages/kbn-docs-utils/src/__test_helpers__/mocks.ts b/packages/kbn-docs-utils/src/__test_helpers__/mocks.ts index c989b1f6ac0ce..6ae9ebe668b30 100644 --- a/packages/kbn-docs-utils/src/__test_helpers__/mocks.ts +++ b/packages/kbn-docs-utils/src/__test_helpers__/mocks.ts @@ -72,6 +72,7 @@ export const createMockPluginStats = (overrides: Partial = {}): ApiSta adoptionTrackedAPIs: [], adoptionTrackedAPIsCount: 0, adoptionTrackedAPIsUnreferencedCount: 0, + unnamedExports: [], ...overrides, }); @@ -106,5 +107,6 @@ export const createMockPluginMetaInfo = ( owner: { name: 'Test Team', githubTeam: 'test-team' }, description: 'A test plugin', isPlugin: true, + unnamedExports: [], ...overrides, }); diff --git a/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts b/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts index 8bf6dc0de0876..c3f52463bece8 100644 --- a/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts +++ b/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.test.ts @@ -23,6 +23,7 @@ import { getSignature, normalizeVerboseSignatures } from './get_signature'; import { buildBasicApiDeclaration } from './build_basic_api_declaration'; import { buildVariableDec } from './build_variable_dec'; import { buildCallSignatureDec } from './build_call_signature_dec'; +import { buildMultipleCallSignaturesDec } from './build_multiple_call_signatures_dec'; import { getReferences } from './get_references'; const log = new ToolingLog({ @@ -48,7 +49,13 @@ beforeAll(() => { plugins = [getKibanaPlatformPlugin('pluginA')]; - nodes = getDeclarationNodesForPluginScope(project, plugins[0], ApiScope.CLIENT, log); + const { nodes: decNodes } = getDeclarationNodesForPluginScope( + project, + plugins[0], + ApiScope.CLIENT, + log + ); + nodes = decNodes; }); it('Test number primitive doc def', () => { @@ -494,7 +501,7 @@ describe('buildBasicApiDeclaration edge cases', () => { }); describe('buildVariableDec edge cases', () => { - it('logs warning when variable has multiple call signatures', () => { + it('handles variable with multiple call signatures (function overloads)', () => { const project = new Project({ useInMemoryFileSystem: true, }); @@ -515,7 +522,7 @@ describe('buildVariableDec edge cases', () => { const warnSpy = jest.spyOn(log, 'warning').mockImplementation(); - buildVariableDec(varDecl!, { + const result = buildVariableDec(varDecl!, { name: 'test', id: 'test-id', currentPluginId: 'test-plugin', @@ -525,10 +532,14 @@ describe('buildVariableDec edge cases', () => { scope: ApiScope.CLIENT, }); - expect(warnSpy).toHaveBeenCalledWith( + // Should NOT log a warning - we now handle multiple signatures. + expect(warnSpy).not.toHaveBeenCalledWith( expect.stringContaining('Not handling more than one call signature') ); + // Should return a FunctionKind declaration. + expect(result.type).toBe(TypeKind.FunctionKind); + warnSpy.mockRestore(); }); }); @@ -588,6 +599,130 @@ describe('buildCallSignatureDec edge cases', () => { }); }); +describe('buildMultipleCallSignaturesDec', () => { + it('extracts parameters from the first signature for overloaded functions', () => { + const project = new Project({ + useInMemoryFileSystem: true, + }); + + const sourceFile = project.createSourceFile( + 'test.ts', + ` + /** + * An overloaded function type. + * @param x A number or string parameter. + * @returns The processed value. + */ + type OverloadedFn = { + (x: number): number; + (x: string): string; + (x: number, y: number): number; + }; + const overloaded: OverloadedFn = ((x: any) => x) as any; + ` + ); + + const varDecl = sourceFile.getVariableDeclaration('overloaded'); + expect(varDecl).toBeDefined(); + + const callSignatures = varDecl!.getType().getCallSignatures(); + expect(callSignatures.length).toBe(3); + + const result = buildMultipleCallSignaturesDec(varDecl!, callSignatures, { + name: 'overloaded', + id: 'test-id', + currentPluginId: 'test-plugin', + plugins: [], + log, + captureReferences: false, + scope: ApiScope.CLIENT, + }); + + expect(result.type).toBe(TypeKind.FunctionKind); + expect(result.label).toBe('overloaded'); + // Children are extracted from the first signature (x: number): number + expect(result.children).toBeDefined(); + expect(result.children!.length).toBe(1); + expect(result.children![0].label).toBe('x'); + }); + + it('includes returnComment from JSDoc', () => { + const project = new Project({ + useInMemoryFileSystem: true, + }); + + const sourceFile = project.createSourceFile( + 'test.ts', + ` + /** + * Function with return comment. + * @returns The result value. + */ + type FnWithReturn = { + (): string; + (x: number): number; + }; + const fn: FnWithReturn = (() => '') as any; + ` + ); + + const varDecl = sourceFile.getVariableDeclaration('fn'); + expect(varDecl).toBeDefined(); + + const callSignatures = varDecl!.getType().getCallSignatures(); + + const result = buildMultipleCallSignaturesDec(varDecl!, callSignatures, { + name: 'fn', + id: 'test-id', + currentPluginId: 'test-plugin', + plugins: [], + log, + captureReferences: false, + scope: ApiScope.CLIENT, + }); + + expect(result.returnComment).toBeDefined(); + }); + + it('handles parameters with multiple declarations by using the first', () => { + const project = new Project({ + useInMemoryFileSystem: true, + }); + + const sourceFile = project.createSourceFile( + 'test.ts', + ` + type MultiDeclFn = { + (x: string): void; + (x: number): void; + }; + const fn: MultiDeclFn = () => {}; + ` + ); + + const varDecl = sourceFile.getVariableDeclaration('fn'); + expect(varDecl).toBeDefined(); + + const callSignatures = varDecl!.getType().getCallSignatures(); + + const result = buildMultipleCallSignaturesDec(varDecl!, callSignatures, { + name: 'fn', + id: 'test-id', + currentPluginId: 'test-plugin', + plugins: [], + log, + captureReferences: false, + scope: ApiScope.CLIENT, + }); + + expect(result.type).toBe(TypeKind.FunctionKind); + expect(result.children).toBeDefined(); + // First signature has one parameter 'x'. + expect(result.children!.length).toBe(1); + expect(result.children![0].label).toBe('x'); + }); +}); + describe('getTypeKind edge cases', () => { it('handles boolean literal types', () => { const project = new Project({ diff --git a/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.ts b/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.ts index 28e2a85f2f959..6b26061e35316 100644 --- a/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.ts +++ b/packages/kbn-docs-utils/src/build_api_declarations/build_api_declaration.ts @@ -21,6 +21,7 @@ import { buildInterfaceDec } from './build_interface_dec'; import { buildBasicApiDeclaration } from './build_basic_api_declaration'; import { buildFunctionTypeDec } from './build_function_type_dec'; import { buildCallSignatureDec } from './build_call_signature_dec'; +import { buildMultipleCallSignaturesDec } from './build_multiple_call_signatures_dec'; import type { BuildApiDecOpts } from './types'; import { buildApiId } from './utils'; @@ -68,15 +69,18 @@ export function buildApiDeclaration(node: Node, opts: BuildApiDecOpts): ApiDecla Node.isFunctionDeclaration(node) || Node.isMethodDeclaration(node) || Node.isConstructSignatureDeclaration(node) || - Node.isConstructorDeclaration(node) + Node.isConstructorDeclaration(node) || + Node.isCallSignatureDeclaration(node) ) { return buildFunctionDec(node, { ...opts, - // Use "Constructor" if applicable, instead of the default "Unnamed" + // Use appropriate name based on node type name: Node.isConstructSignatureDeclaration(node) ? 'new' : Node.isConstructorDeclaration(node) ? 'Constructor' + : Node.isCallSignatureDeclaration(node) + ? 'Unnamed' : node.getName() || 'Unnamed', }); } else if ( @@ -97,12 +101,11 @@ export function buildApiDeclaration(node: Node, opts: BuildApiDecOpts): ApiDecla // */ // export type AFn = (t: string) => void; // - if (node.getType().getCallSignatures().length > 0) { - if (node.getType().getCallSignatures().length > 1) { - opts.log.warning(`Not handling more than one call signature for node ${opts.name}`); - } else { - return buildCallSignatureDec(node, node.getType().getCallSignatures()[0], opts); - } + const callSignatures = node.getType().getCallSignatures(); + if (callSignatures.length === 1) { + return buildCallSignatureDec(node, callSignatures[0], opts); + } else if (callSignatures.length > 1) { + return buildMultipleCallSignaturesDec(node, callSignatures, opts); } return buildBasicApiDeclaration(node, opts); diff --git a/packages/kbn-docs-utils/src/build_api_declarations/build_function_dec.ts b/packages/kbn-docs-utils/src/build_api_declarations/build_function_dec.ts index a114bb6a39865..f920893598d4b 100644 --- a/packages/kbn-docs-utils/src/build_api_declarations/build_function_dec.ts +++ b/packages/kbn-docs-utils/src/build_api_declarations/build_function_dec.ts @@ -13,6 +13,7 @@ import type { ConstructorDeclaration, MethodSignature, ConstructSignatureDeclaration, + CallSignatureDeclaration, } from 'ts-morph'; import { buildApiDecsForParameters } from './build_parameter_decs'; @@ -31,7 +32,8 @@ export function buildFunctionDec( | FunctionDeclaration | MethodDeclaration | ConstructorDeclaration - | MethodSignature, + | MethodSignature + | CallSignatureDeclaration, opts: BuildApiDecOpts ): ApiDeclaration { const fn = { diff --git a/packages/kbn-docs-utils/src/build_api_declarations/build_multiple_call_signatures_dec.ts b/packages/kbn-docs-utils/src/build_api_declarations/build_multiple_call_signatures_dec.ts new file mode 100644 index 0000000000000..3717c966b9ee0 --- /dev/null +++ b/packages/kbn-docs-utils/src/build_api_declarations/build_multiple_call_signatures_dec.ts @@ -0,0 +1,67 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { Node, Signature } from 'ts-morph'; +import type { ApiDeclaration } from '../types'; +import { TypeKind } from '../types'; +import { buildApiDeclaration } from './build_api_declaration'; +import { buildBasicApiDeclaration } from './build_basic_api_declaration'; +import { getJSDocParamComment, getJSDocReturnTagComment } from './js_doc_utils'; +import type { BuildApiDecOpts } from './types'; +import { buildApiId, getOptsForChildWithName } from './utils'; + +/** + * Builds an {@link ApiDeclaration} for a node with multiple call signatures (function overloads). + * Parameters are extracted from the first signature to provide structured documentation, + * while all overload signatures are included in the type information. + * + * @param node The ts-morph node with multiple call signatures. + * @param signatures Array of all call signatures for the node. + * @param opts Build options including logging and plugin context. + * @returns An ApiDeclaration representing the overloaded function/type. + */ +export const buildMultipleCallSignaturesDec = ( + node: Node, + signatures: Signature[], + opts: BuildApiDecOpts +): ApiDeclaration => { + // Use the first signature to extract parameter children for documentation. + // This is a reasonable default since overloads typically share common parameters. + const primarySignature = signatures[0]; + + const children = primarySignature.getParameters().reduce((kids, p, index) => { + const declarations = p.getDeclarations(); + if (declarations.length === 1) { + kids.push({ + ...buildApiDeclaration(declarations[0], { + ...getOptsForChildWithName(p.getName(), opts), + id: buildApiId(`$${index + 1}`, opts.id), + }), + description: getJSDocParamComment(node, p.getName()), + }); + } else if (declarations.length > 1) { + // Use the first declaration when multiple exist (common with overloads). + kids.push({ + ...buildApiDeclaration(declarations[0], { + ...getOptsForChildWithName(p.getName(), opts), + id: buildApiId(`$${index + 1}`, opts.id), + }), + description: getJSDocParamComment(node, p.getName()), + }); + } + return kids; + }, [] as ApiDeclaration[]); + + return { + ...buildBasicApiDeclaration(node, opts), + type: TypeKind.FunctionKind, + returnComment: getJSDocReturnTagComment(node), + children, + }; +}; diff --git a/packages/kbn-docs-utils/src/build_api_declarations/build_variable_dec.ts b/packages/kbn-docs-utils/src/build_api_declarations/build_variable_dec.ts index 64ef44ca272b1..08f5029eadbdc 100644 --- a/packages/kbn-docs-utils/src/build_api_declarations/build_variable_dec.ts +++ b/packages/kbn-docs-utils/src/build_api_declarations/build_variable_dec.ts @@ -22,6 +22,7 @@ import { getArrowFunctionDec } from './build_arrow_fn_dec'; import { buildApiDeclaration } from './build_api_declaration'; import { buildBasicApiDeclaration } from './build_basic_api_declaration'; import { buildCallSignatureDec } from './build_call_signature_dec'; +import { buildMultipleCallSignaturesDec } from './build_multiple_call_signatures_dec'; import type { BuildApiDecOpts } from './types'; import { getOptsForChild } from './utils'; @@ -60,12 +61,11 @@ export function buildVariableDec( } // Without this the test "Property on interface pointing to generic function type exported with link" will fail. - if (node.getType().getCallSignatures().length > 0) { - if (node.getType().getCallSignatures().length > 1) { - opts.log.warning(`Not handling more than one call signature for node ${node.getName()}`); - } else { - return buildCallSignatureDec(node, node.getType().getCallSignatures()[0], opts); - } + const callSignatures = node.getType().getCallSignatures(); + if (callSignatures.length === 1) { + return buildCallSignatureDec(node, callSignatures[0], opts); + } else if (callSignatures.length > 1) { + return buildMultipleCallSignaturesDec(node, callSignatures, opts); } return buildBasicApiDeclaration(node, opts); diff --git a/packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.test.ts b/packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.test.ts index 30afada75bf0a..d60913ad48e49 100644 --- a/packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.test.ts +++ b/packages/kbn-docs-utils/src/build_api_declarations/js_doc_utils.test.ts @@ -152,6 +152,29 @@ describe('getJSDocParamComment', () => { const commentUpper = getJSDocParamComment(node!, 'A'); expect(commentUpper.length).toBe(0); // Case-sensitive, so 'A' won't match 'a' }); + + it('handles malformed @param with type but no name', () => { + const testProject = new Project({ + useInMemoryFileSystem: true, + }); + + const testSourceFile = testProject.createSourceFile( + 'test.ts', + ` + /** + * @param {Object} + */ + function malformedParam(obj: object) {} + ` + ); + + const func = testSourceFile.getFunction('malformedParam'); + expect(func).toBeDefined(); + + // Should not throw, and should return empty array since param name is missing in JSDoc. + const comment = getJSDocParamComment(func!, 'obj'); + expect(comment).toEqual([]); + }); }); describe('getJSDocReturnTagComment', () => { diff --git a/packages/kbn-docs-utils/src/build_api_docs_cli.test.ts b/packages/kbn-docs-utils/src/build_api_docs_cli.test.ts index 7dd4f139ac9a3..7b2b92c66d047 100644 --- a/packages/kbn-docs-utils/src/build_api_docs_cli.test.ts +++ b/packages/kbn-docs-utils/src/build_api_docs_cli.test.ts @@ -82,11 +82,13 @@ describe('build_api_docs_cli', () => { }); it('runs build flow when stats are not provided', async () => { + const mockPlugins = [ + { id: 'p1', manifest: { owner: { name: 'team' }, serviceFolders: [] }, isPlugin: true }, + ]; const setupResult = { project: {}, - plugins: [ - { id: 'p1', manifest: { owner: { name: 'team' }, serviceFolders: [] }, isPlugin: true }, - ], + plugins: mockPlugins, + allPlugins: mockPlugins, }; const apiMapResult = { pluginApiMap: {}, @@ -107,6 +109,7 @@ describe('build_api_docs_cli', () => { expect(buildApiMap).toHaveBeenCalledWith( setupResult.project, setupResult.plugins, + setupResult.allPlugins, log, mockTx, { stats: undefined, collectReferences: false } diff --git a/packages/kbn-docs-utils/src/build_api_docs_cli.ts b/packages/kbn-docs-utils/src/build_api_docs_cli.ts index 5f3a42709ba45..491c34d15eacc 100644 --- a/packages/kbn-docs-utils/src/build_api_docs_cli.ts +++ b/packages/kbn-docs-utils/src/build_api_docs_cli.ts @@ -44,6 +44,9 @@ async function endTransactionWithFailure(transaction: Transaction | null) { } } +/** + * Runs the build API docs CLI, generating API documentation for Kibana plugins and packages. + */ export function runBuildApiDocsCli() { startApm(); run( @@ -85,6 +88,7 @@ export function runBuildApiDocsCli() { const apiMapResult = buildApiMap( setupResult.project, setupResult.plugins, + setupResult.allPlugins, log, transaction, options diff --git a/packages/kbn-docs-utils/src/check_package_docs_cli.test.ts b/packages/kbn-docs-utils/src/check_package_docs_cli.test.ts index 9c75b87c0252c..235eb6da26ba5 100644 --- a/packages/kbn-docs-utils/src/check_package_docs_cli.test.ts +++ b/packages/kbn-docs-utils/src/check_package_docs_cli.test.ts @@ -56,6 +56,7 @@ const createBaseStats = (pluginId: string): AllPluginStats => ({ eslintDisableFileCount: 0, eslintDisableLineCount: 0, enzymeImportCount: 0, + unnamedExports: [], }, }); diff --git a/packages/kbn-docs-utils/src/check_package_docs_cli.ts b/packages/kbn-docs-utils/src/check_package_docs_cli.ts index ace0049377208..a26683c1c1426 100644 --- a/packages/kbn-docs-utils/src/check_package_docs_cli.ts +++ b/packages/kbn-docs-utils/src/check_package_docs_cli.ts @@ -9,7 +9,7 @@ import Path from 'path'; -import apm from 'elastic-apm-node'; +import apm, { type Transaction } from 'elastic-apm-node'; import type { ToolingLog } from '@kbn/tooling-log'; import { run } from '@kbn/dev-cli-runner'; import { REPO_ROOT } from '@kbn/repo-info'; @@ -27,10 +27,11 @@ import { } from './cli'; import type { PluginOrPackage, MissingApiItemMap } from './types'; import type { AllPluginStats } from './cli/types'; +import { writeFlatStatsFiles } from './cli/tasks/flat_stats'; -type ValidationCheck = 'any' | 'comments' | 'exports'; +type ValidationCheck = 'any' | 'comments' | 'exports' | 'unnamed'; -const DEFAULT_VALIDATION_CHECKS: ValidationCheck[] = ['any', 'comments', 'exports']; +const DEFAULT_VALIDATION_CHECKS: ValidationCheck[] = ['any', 'comments', 'exports', 'unnamed']; const rootDir = Path.join(__dirname, '../../..'); @@ -62,6 +63,7 @@ export const getValidationResults = ( const shouldCheckAny = checks.includes('any'); const shouldCheckComments = checks.includes('comments'); const shouldCheckExports = checks.includes('exports'); + const shouldCheckUnnamed = checks.includes('unnamed'); const hasPluginFilter = pluginFilter && pluginFilter.length > 0; const hasPackageFilter = packageFilter && packageFilter.length > 0; @@ -87,10 +89,11 @@ export const getValidationResults = ( shouldCheckComments && (pluginStats.missingComments.length > 0 || pluginStats.paramDocMismatches.length > 0); const hasExportIssues = shouldCheckExports && missingExports > 0; + const hasUnnamedIssues = shouldCheckUnnamed && pluginStats.unnamedExports.length > 0; return { pluginId: plugin.id, - passed: !(hasAnyIssues || hasCommentIssues || hasExportIssues), + passed: !(hasAnyIssues || hasCommentIssues || hasExportIssues || hasUnnamedIssues), }; }); }; @@ -118,6 +121,7 @@ export const runCheckPackageDocs = async (log: ToolingLog, flags: CliFlags) => { const apiMapResult = buildApiMap( setupResult.project, setupResult.plugins, + setupResult.allPlugins, log, transaction, optionsWithChecks @@ -131,7 +135,11 @@ export const runCheckPackageDocs = async (log: ToolingLog, flags: CliFlags) => { optionsWithChecks ); - reportMetrics(setupResult, apiMapResult, allPluginStats, log, transaction, { + if (optionsWithChecks.writeStats) { + writeFlatStatsFiles(setupResult.plugins, apiMapResult, allPluginStats); + } + + reportMetrics(setupResult, apiMapResult, allPluginStats, log, transaction as Transaction, { ...optionsWithChecks, stats: checks, }); @@ -166,6 +174,9 @@ export const runCheckPackageDocs = async (log: ToolingLog, flags: CliFlags) => { } }; +/** + * Runs the check package docs CLI, validating API documentation for Kibana plugins and packages. + */ export const runCheckPackageDocsCli = () => { run( async ({ log, flags }) => { @@ -177,11 +188,13 @@ export const runCheckPackageDocsCli = () => { }, flags: { string: ['plugin', 'package', 'check'], + boolean: ['write'], help: ` --plugin Optionally, run for only a specific plugin by its plugin ID (plugin.id in kibana.jsonc). --package Optionally, run for only a specific package by its package ID (id in kibana.jsonc, e.g., @kbn/core). --check Optional. Specify validation checks: any, comments, exports, or all (default). Can be provided multiple times to combine checks. + --write Write stats to a flat JSON file in each plugin's target/api_docs/ directory. `, }, } diff --git a/packages/kbn-docs-utils/src/cli/parse_cli_flags.test.ts b/packages/kbn-docs-utils/src/cli/parse_cli_flags.test.ts index 71336e9ca1e8a..f1bb0b6dc5c0b 100644 --- a/packages/kbn-docs-utils/src/cli/parse_cli_flags.test.ts +++ b/packages/kbn-docs-utils/src/cli/parse_cli_flags.test.ts @@ -85,7 +85,7 @@ describe('parseCliFlags', () => { const result = parseCliFlags(flags); - expect(result.stats).toEqual(['comments', 'any', 'exports']); + expect(result.stats).toEqual(['comments', 'any', 'exports', 'unnamed']); }); it('dedupes overlapping stats and check flags', () => { @@ -113,7 +113,7 @@ describe('parseCliFlags', () => { }; expect(() => parseCliFlags(flags)).toThrow( - 'expected --stats must only contain `any`, `comments` and/or `exports`' + 'expected --stats must only contain `any`, `comments`, `exports`, and/or `unnamed`' ); }); @@ -123,18 +123,18 @@ describe('parseCliFlags', () => { }; expect(() => parseCliFlags(flags)).toThrow( - 'expected --stats must only contain `any`, `comments` and/or `exports`' + 'expected --stats must only contain `any`, `comments`, `exports`, and/or `unnamed`' ); }); it('accepts valid stats values', () => { const flags: CliFlags = { - stats: ['any', 'comments', 'exports'], + stats: ['any', 'comments', 'exports', 'unnamed'], }; const result = parseCliFlags(flags); - expect(result.stats).toEqual(['any', 'comments', 'exports']); + expect(result.stats).toEqual(['any', 'comments', 'exports', 'unnamed']); }); it('handles references flag correctly', () => { diff --git a/packages/kbn-docs-utils/src/cli/parse_cli_flags.ts b/packages/kbn-docs-utils/src/cli/parse_cli_flags.ts index 771113736fd42..3bf8005893296 100644 --- a/packages/kbn-docs-utils/src/cli/parse_cli_flags.ts +++ b/packages/kbn-docs-utils/src/cli/parse_cli_flags.ts @@ -33,7 +33,7 @@ const normalizeStringList = (value: unknown | string[], flagName: string) => { const dedupe = (values: string[] | undefined) => values && values.length > 0 ? Array.from(new Set(values)) : undefined; -const VALID_CHECKS = ['any', 'comments', 'exports', 'all'] as const; +const VALID_CHECKS = ['any', 'comments', 'exports', 'unnamed', 'all'] as const; const normalizeCheckFlagValues = (check: unknown | string[]) => { if (!check) { @@ -46,7 +46,7 @@ const normalizeCheckFlagValues = (check: unknown | string[]) => { if (!isStringArray(check)) { throw createFlagError( - 'expected --check must only contain `any`, `comments`, `exports`, or `all`' + 'expected --check must only contain `any`, `comments`, `exports`, `unnamed`, or `all`' ); } @@ -58,12 +58,14 @@ const expandChecks = (checks: string[] | undefined) => { return undefined; } - const expanded = checks.flatMap((c) => (c === 'all' ? ['any', 'comments', 'exports'] : [c])); + const expanded = checks.flatMap((c) => + c === 'all' ? ['any', 'comments', 'exports', 'unnamed'] : [c] + ); const invalid = expanded.find((c) => !VALID_CHECKS.includes(c as (typeof VALID_CHECKS)[number])); if (invalid) { throw createFlagError( - 'expected --check must only contain `any`, `comments`, `exports`, or `all`' + 'expected --check must only contain `any`, `comments`, `exports`, `unnamed`, or `all`' ); } @@ -71,8 +73,11 @@ const expandChecks = (checks: string[] | undefined) => { }; const validateStats = (stats: string[] | undefined) => { - if (stats && stats.find((s) => s !== 'any' && s !== 'comments' && s !== 'exports')) { - throw createFlagError('expected --stats must only contain `any`, `comments` and/or `exports`'); + const validValues = ['any', 'comments', 'exports', 'unnamed']; + if (stats && stats.find((s) => !validValues.includes(s))) { + throw createFlagError( + 'expected --stats must only contain `any`, `comments`, `exports`, and/or `unnamed`' + ); } }; @@ -86,7 +91,9 @@ const normalizeStats = (value: unknown | string[]) => { } if (!isStringArray(value)) { - throw createFlagError('expected --stats must only contain `any`, `comments` and/or `exports`'); + throw createFlagError( + 'expected --stats must only contain `any`, `comments`, `exports`, and/or `unnamed`' + ); } return value; @@ -101,6 +108,7 @@ const normalizeStats = (value: unknown | string[]) => { */ export function parseCliFlags(flags: CliFlags): CliOptions { const collectReferences = flags.references === true; + const writeStats = flags.write === true; const pluginFilter = dedupe(normalizeStringList(flags.plugin, 'plugin')); const packageFilter = dedupe(normalizeStringList(flags.package, 'package')); const rawStats = normalizeStats(flags.stats); @@ -115,5 +123,6 @@ export function parseCliFlags(flags: CliFlags): CliOptions { stats, pluginFilter, packageFilter, + writeStats, }; } diff --git a/packages/kbn-docs-utils/src/cli/tasks/build_api_map.test.ts b/packages/kbn-docs-utils/src/cli/tasks/build_api_map.test.ts index 495960ad9e59f..3c2da2fdfe245 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/build_api_map.test.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/build_api_map.test.ts @@ -67,9 +67,10 @@ describe('buildApiMap', () => { pluginFilter: ['test-plugin'], }; - buildApiMap(project, plugins, log, transaction, options); + const allPlugins = [...plugins, { id: 'other-plugin' }]; + buildApiMap(project, plugins, allPlugins, log, transaction, options); - expect(getPluginApiMap).toHaveBeenCalledWith(project, plugins, log, { + expect(getPluginApiMap).toHaveBeenCalledWith(project, plugins, allPlugins, log, { collectReferences: true, pluginFilter: ['test-plugin'], }); @@ -80,7 +81,7 @@ describe('buildApiMap', () => { collectReferences: false, }; - const result = buildApiMap(project, plugins, log, transaction, options); + const result = buildApiMap(project, plugins, plugins, log, transaction, options); expect(result).toBeDefined(); expect(result.pluginApiMap).toBeDefined(); @@ -95,7 +96,7 @@ describe('buildApiMap', () => { collectReferences: false, }; - buildApiMap(project, plugins, log, transaction, options); + buildApiMap(project, plugins, plugins, log, transaction, options); expect(transaction.startSpan).toHaveBeenCalledWith('build_api_docs.getPluginApiMap', 'setup'); }); diff --git a/packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts b/packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts index 8fa5444ba1533..d8b60f5ea9574 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts @@ -25,6 +25,7 @@ import type { BuildApiMapResult, CliOptions } from '../types'; * * @param project - TypeScript project instance. * @param plugins - List of plugins and packages to analyze. + * @param allPlugins - All plugins/packages for cross-reference resolution. * @param log - Tooling log instance. * @param transaction - APM transaction for tracking. * @param options - CLI options including collectReferences and pluginFilter. @@ -33,6 +34,7 @@ import type { BuildApiMapResult, CliOptions } from '../types'; export function buildApiMap( project: Project, plugins: PluginOrPackage[], + allPlugins: PluginOrPackage[], log: ToolingLog, transaction: Transaction, options: CliOptions @@ -45,7 +47,8 @@ export function buildApiMap( unreferencedDeprecations, referencedDeprecations, adoptionTrackedAPIs, - } = getPluginApiMap(project, plugins, log, { + unnamedExports, + } = getPluginApiMap(project, plugins, allPlugins, log, { collectReferences: options.collectReferences, pluginFilter: options.pluginFilter, }); @@ -58,5 +61,6 @@ export function buildApiMap( referencedDeprecations, unreferencedDeprecations, adoptionTrackedAPIs, + unnamedExports, }; } diff --git a/packages/kbn-docs-utils/src/cli/tasks/collect_stats.test.ts b/packages/kbn-docs-utils/src/cli/tasks/collect_stats.test.ts index 60020dc6527c9..825cd072d8994 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/collect_stats.test.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/collect_stats.test.ts @@ -55,6 +55,7 @@ describe('collectStats', () => { plugins: [mockPlugin], pathsByPlugin: new Map([[mockPlugin, ['src/plugins/test/public/index.ts']]]), project: {} as any, + allPlugins: [mockPlugin], }; apiMapResult = { @@ -70,6 +71,7 @@ describe('collectStats', () => { referencedDeprecations: {}, unreferencedDeprecations: {}, adoptionTrackedAPIs: {}, + unnamedExports: {}, }; (collectApiStatsForPlugin as jest.Mock).mockReturnValue({ @@ -84,6 +86,7 @@ describe('collectStats', () => { adoptionTrackedAPIs: [], adoptionTrackedAPIsCount: 0, adoptionTrackedAPIsUnreferencedCount: 0, + unnamedExports: [], }); (countEslintDisableLines as jest.Mock).mockResolvedValue({ diff --git a/packages/kbn-docs-utils/src/cli/tasks/collect_stats.ts b/packages/kbn-docs-utils/src/cli/tasks/collect_stats.ts index d42ebb80461cf..497c609ecf7ef 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/collect_stats.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/collect_stats.ts @@ -38,8 +38,13 @@ export async function collectStats( options: CliOptions ): Promise { const { plugins, pathsByPlugin } = setupResult; - const { pluginApiMap, missingApiItems, referencedDeprecations, adoptionTrackedAPIs } = - apiMapResult; + const { + pluginApiMap, + missingApiItems, + referencedDeprecations, + adoptionTrackedAPIs, + unnamedExports, + } = apiMapResult; const allPluginStats: AllPluginStats = {}; @@ -65,6 +70,7 @@ export async function collectStats( missingApiItems, referencedDeprecations, adoptionTrackedAPIs, + unnamedExports, }), owner: plugin.manifest.owner, description: plugin.manifest.description, diff --git a/packages/kbn-docs-utils/src/cli/tasks/flat_stats.ts b/packages/kbn-docs-utils/src/cli/tasks/flat_stats.ts new file mode 100644 index 0000000000000..9efd090230f6b --- /dev/null +++ b/packages/kbn-docs-utils/src/cli/tasks/flat_stats.ts @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import fs from 'fs'; +import Path from 'path'; + +import type { ApiDeclaration, MissingApiItemMap, PluginOrPackage } from '../../types'; +import type { AllPluginStats, BuildApiMapResult } from '../types'; + +const getLink = (declaration: ApiDeclaration): string => { + const base = `https://github.com/elastic/kibana/blob/main/${declaration.path}`; + if (declaration.lineNumber) { + return `${base}#L${declaration.lineNumber}`; + } + return `https://github.com/elastic/kibana/tree/main/${ + declaration.path + }#:~:text=${encodeURIComponent(declaration.label)}`; +}; + +const mapStat = (dec: ApiDeclaration) => ({ + id: dec.id, + label: dec.label, + path: dec.path, + type: dec.type, + lineNumber: dec.lineNumber, + columnNumber: dec.columnNumber, + link: getLink(dec), +}); + +export const buildFlatStatsForPlugin = ( + pluginId: string, + pluginStats: AllPluginStats[string], + missingApiItems: MissingApiItemMap +) => { + const missingExportsCount = missingApiItems[pluginId] + ? Object.keys(missingApiItems[pluginId]).length + : 0; + const missingExportsList = missingApiItems[pluginId] + ? Object.keys(missingApiItems[pluginId]).map((source) => ({ + source, + references: missingApiItems[pluginId][source], + })) + : []; + + return { + counts: { + apiCount: pluginStats.apiCount, + missingExports: missingExportsCount, + missingComments: pluginStats.missingComments.length, + isAnyType: pluginStats.isAnyType.length, + noReferences: pluginStats.noReferences.length, + paramDocMismatches: pluginStats.paramDocMismatches.length, + }, + missingComments: pluginStats.missingComments.map(mapStat), + isAnyType: pluginStats.isAnyType.map(mapStat), + noReferences: pluginStats.noReferences.map(mapStat), + paramDocMismatches: pluginStats.paramDocMismatches.map(mapStat), + missingExports: missingExportsList, + }; +}; + +export const writeFlatStatsFiles = ( + plugins: PluginOrPackage[], + apiMapResult: BuildApiMapResult, + allPluginStats: AllPluginStats +) => { + for (const plugin of plugins) { + const stats = allPluginStats[plugin.id]; + if (!stats) continue; + const flat = buildFlatStatsForPlugin(plugin.id, stats, apiMapResult.missingApiItems); + const pluginTargetDir = Path.resolve(plugin.directory, 'target', 'api_docs'); + fs.mkdirSync(pluginTargetDir, { recursive: true }); + const target = Path.join(pluginTargetDir, 'stats.json'); + fs.writeFileSync(target, JSON.stringify(flat, null, 2)); + } +}; diff --git a/packages/kbn-docs-utils/src/cli/tasks/report_metrics.test.ts b/packages/kbn-docs-utils/src/cli/tasks/report_metrics.test.ts index 80449fd6d990d..6bbdfa484e5ef 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/report_metrics.test.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/report_metrics.test.ts @@ -64,6 +64,7 @@ describe('reportMetrics', () => { plugins: [mockPlugin], pathsByPlugin: new Map(), project: {} as any, + allPlugins: [mockPlugin], }; apiMapResult = { @@ -72,6 +73,7 @@ describe('reportMetrics', () => { referencedDeprecations: {}, unreferencedDeprecations: {}, adoptionTrackedAPIs: {}, + unnamedExports: {}, }; allPluginStats = { @@ -93,6 +95,7 @@ describe('reportMetrics', () => { eslintDisableLineCount: 0, eslintDisableFileCount: 0, enzymeImportCount: 0, + unnamedExports: [], }, }; }); diff --git a/packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts b/packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts index fc3d467712439..c0ca3a700a890 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts @@ -16,10 +16,17 @@ import type { AllPluginStats, BuildApiMapResult, CliOptions, SetupProjectResult /** * Generates a link to the GitHub source for an API declaration. * + * TODO: clintandrewhall - allow `base` to be overridden in the instance of a CI build + * associated with a PR. + * * @param declaration - API declaration to generate link for. * @returns GitHub link to the source code. */ function getLink(declaration: ApiDeclaration): string { + const base = `https://github.com/elastic/kibana/blob/main/${declaration.path}`; + if (declaration.lineNumber) { + return `${base}#L${declaration.lineNumber}`; + } return `https://github.com/elastic/kibana/tree/main/${ declaration.path }#:~:text=${encodeURIComponent(declaration.label)}`; @@ -53,6 +60,40 @@ export function reportMetrics( const { missingApiItems, referencedDeprecations } = apiMapResult; const reporter = CiStatsReporter.fromEnv(log); + const printIssueTable = ( + title: string, + rows: Array<{ id: string; link: string; line?: number }> + ) => { + const count = rows.length; + if (count === 0) { + log.info(`${title}: none`); + return; + } + log.info(`${title} (${count})`); + // eslint-disable-next-line no-console + console.table(rows); + }; + + const printMissingExportsTable = ( + title: string, + entries: Array<{ source: string; references: string }> + ) => { + const header = title.toUpperCase(); + const count = entries.length; + if (count === 0) { + log.info(`${header}: none`); + return; + } + log.info(`${header} (${count})`); + // eslint-disable-next-line no-console + console.table( + entries.map(({ source, references }) => ({ + 'Not exported source': source, + references, + })) + ); + }; + for (const plugin of plugins) { // Note that the filtering is done here (per-plugin), rather than earlier in the pipeline. // This keeps the metrics task aligned with how other docs tasks process plugins and ensures @@ -89,6 +130,12 @@ export function reportMetrics( group: 'API count missing comments', value: pluginStats.missingComments.length, }, + { + id, + meta: { pluginTeam }, + group: 'API count with param doc mismatches', + value: pluginStats.paramDocMismatches.length, + }, { id, meta: { pluginTeam }, @@ -181,17 +228,16 @@ export function reportMetrics( const passesAllChecks = pluginStats.isAnyType.length === 0 && pluginStats.missingComments.length === 0 && + pluginStats.paramDocMismatches.length === 0 && pluginStats.deprecatedAPIsReferencedCount === 0 && (!missingApiItems[id] || Object.keys(missingApiItems[id]).length === 0); log.info(`--- Plugin '${id}' ${passesAllChecks ? 'passes all checks ----' : '----'}`); if (!passesAllChecks) { - log.info(`${pluginStats.isAnyType.length} API items with ANY`); - if (options.stats.includes('any')) { - // eslint-disable-next-line no-console - console.table( + printIssueTable( + 'API items with ANY', pluginStats.isAnyType.map((d) => ({ id: d.id, link: getLink(d), @@ -199,27 +245,30 @@ export function reportMetrics( ); } - log.info(`${pluginStats.missingComments.length} API items missing comments`); if (options.stats.includes('comments')) { - // eslint-disable-next-line no-console - console.table( + printIssueTable( + 'API items missing comments', pluginStats.missingComments.map((d) => ({ id: d.id, link: getLink(d), })) ); + printIssueTable( + 'API items with param doc mismatches', + pluginStats.paramDocMismatches.map((d) => ({ + id: d.id, + link: getLink(d), + })) + ); } if (missingApiItems[id]) { - log.info(`${Object.keys(missingApiItems[id]).length} referenced API items not exported`); if (options.stats.includes('exports')) { - // eslint-disable-next-line no-console - console.table( - Object.keys(missingApiItems[id]).map((key) => ({ - 'Not exported source': key, - references: missingApiItems[id][key].join(', '), - })) - ); + const exportsTable = Object.keys(missingApiItems[id]).map((key) => ({ + source: key, + references: missingApiItems[id][key].join(', '), + })); + printMissingExportsTable('Referenced API items not exported', exportsTable); } } } diff --git a/packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts b/packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts index 4009496c575b9..42a555809fe2e 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts @@ -11,6 +11,7 @@ import Path from 'path'; import { ToolingLog } from '@kbn/tooling-log'; import { setupProject } from './setup_project'; import type { CliContext, CliOptions } from '../types'; +import type { FindPluginsOptions } from '../../find_plugins'; // Mock dependencies - order matters: mock get_all_doc_file_ids first to prevent globby from loading jest.mock('../../mdx/get_all_doc_file_ids', () => ({ @@ -112,6 +113,7 @@ describe('setupProject', () => { const result = await setupProject(context, options); expect(result.plugins).toBeDefined(); + expect(result.allPlugins).toBeDefined(); expect(result.pathsByPlugin).toBeDefined(); expect(result.project).toBeDefined(); }); @@ -134,9 +136,21 @@ describe('setupProject', () => { const Fs = jest.requireMock('fs'); Fs.existsSync.mockReturnValue(true); - (findPlugins as jest.Mock).mockReturnValue([ - { id: 'test-plugin', isPlugin: true, directory: '/tmp/test' }, - ]); + const mockPlugin = { + id: 'test-plugin', + directory: '/mock/repo/root/src/plugins/test', + isPlugin: true, + manifest: { id: 'test-plugin', owner: { name: 'test-team' }, serviceFolders: [] }, + manifestPath: '/mock/repo/root/src/plugins/test/kibana.json', + }; + + // Return all plugins for allPlugins, filtered for filteredPlugins + (findPlugins as jest.Mock).mockImplementation((options?: FindPluginsOptions) => { + if (options?.pluginFilter) { + return [mockPlugin]; + } + return [mockPlugin, { ...mockPlugin, id: 'other-plugin' }]; + }); const options: CliOptions = { collectReferences: false, @@ -173,4 +187,76 @@ describe('setupProject', () => { "expected --package '@kbn/nonexistent-package' was not found" ); }); + + it('scopes TypeScript project to single plugin directory when pluginFilter has one plugin', async () => { + const { Project } = jest.requireMock('ts-morph'); + const mockProject = Project(); + + jest.clearAllMocks(); + + const mockPlugin = { + id: 'single-plugin', + directory: '/mock/repo/root/src/plugins/single', + isPlugin: true, + manifest: { id: 'single-plugin', owner: { name: 'test-team' }, serviceFolders: [] }, + manifestPath: '/mock/repo/root/src/plugins/single/kibana.json', + }; + + // Return all plugins for allPlugins, filtered for plugins + (findPlugins as jest.Mock).mockImplementation((options?: FindPluginsOptions) => { + if (options?.pluginFilter) { + return [mockPlugin]; + } + return [mockPlugin, { ...mockPlugin, id: 'other-plugin' }]; + }); + + const options: CliOptions = { + collectReferences: false, + pluginFilter: ['single-plugin'], + }; + + await setupProject(context, options); + + // Should use the single plugin's tsconfig + expect(Project).toHaveBeenCalledWith({ + tsConfigFilePath: '/mock/repo/root/src/plugins/single/tsconfig.json', + skipAddingFilesFromTsConfig: true, + }); + + // Should only add files from the single plugin directory + expect(mockProject.addSourceFilesAtPaths).toHaveBeenCalledWith([ + '/mock/repo/root/src/plugins/single/**/*.ts', + '!**/*.d.ts', + ]); + + // Should NOT call resolveSourceFileDependencies for single-plugin builds + expect(mockProject.resolveSourceFileDependencies).not.toHaveBeenCalled(); + }); + + it('loads full codebase and resolves dependencies when no pluginFilter', async () => { + const { Project } = jest.requireMock('ts-morph'); + const mockProject = Project(); + + jest.clearAllMocks(); + + (findPlugins as jest.Mock).mockReturnValue([]); + + const options: CliOptions = { + collectReferences: false, + }; + + await setupProject(context, options); + + // Should use the repo root tsconfig + expect(Project).toHaveBeenCalledWith({ + tsConfigFilePath: '/mock/repo/root/tsconfig.json', + skipAddingFilesFromTsConfig: true, + }); + + // Should add files from all directories + expect(mockProject.addSourceFilesAtPaths).toHaveBeenCalledTimes(8); + + // Should call resolveSourceFileDependencies for full builds + expect(mockProject.resolveSourceFileDependencies).toHaveBeenCalled(); + }); }); diff --git a/packages/kbn-docs-utils/src/cli/tasks/setup_project.ts b/packages/kbn-docs-utils/src/cli/tasks/setup_project.ts index f8c828945a037..5c5368dc5e3bf 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/setup_project.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/setup_project.ts @@ -36,6 +36,7 @@ function getTsProject(repoPath: string, overridePath?: string): Project { }); if (!overridePath) { + // Full build: load all source files and resolve dependencies upfront project.addSourceFilesAtPaths([`${repoPath}/x-pack/plugins/**/*.ts`, '!**/*.d.ts']); project.addSourceFilesAtPaths([`${repoPath}/x-pack/packages/**/*.ts`, '!**/*.d.ts']); project.addSourceFilesAtPaths([`${repoPath}/x-pack/platform/**/*.ts`, '!**/*.d.ts']); @@ -44,10 +45,17 @@ function getTsProject(repoPath: string, overridePath?: string): Project { project.addSourceFilesAtPaths([`${repoPath}/src/platform/**/*.ts`, '!**/*.d.ts']); project.addSourceFilesAtPaths([`${repoPath}/src/core/packages/**/*.ts`, '!**/*.d.ts']); project.addSourceFilesAtPaths([`${repoPath}/packages/**/*.ts`, '!**/*.d.ts']); + project.resolveSourceFileDependencies(); } else { + // Single-plugin build: only load files from the target plugin directory. + // We intentionally skip resolveSourceFileDependencies() here because: + // 1. ts-morph resolves dependencies lazily when accessed (e.g., via getType()). + // 2. This significantly reduces memory usage and startup time for single-plugin builds. + // 3. Cross-package type references still resolve correctly via the tsconfig paths. + // Trade-off: First access to external types may be slightly slower, but overall + // build time is reduced since we don't load the entire codebase into memory. project.addSourceFilesAtPaths([`${overridePath}/**/*.ts`, '!**/*.d.ts']); } - project.resolveSourceFileDependencies(); return project; } @@ -84,11 +92,14 @@ export async function setupProject( spanInitialDocIds?.end(); const spanPlugins = transaction.startSpan('build_api_docs.findPlugins', 'setup'); - const plugins = hasAnyFilter ? findPlugins({ pluginFilter, packageFilter }) : findPlugins(); + // Always find all plugins for cross-reference resolution. + const allPlugins = findPlugins(); + // Find filtered plugins if a filter is provided. + const filteredPlugins = hasAnyFilter ? findPlugins({ pluginFilter, packageFilter }) : allPlugins; // Validate that all requested plugins were found. if (hasPluginFilter && pluginFilter) { - const foundPluginIds = plugins.filter((p) => p.isPlugin).map((p) => p.id); + const foundPluginIds = filteredPlugins.filter((p) => p.isPlugin).map((p) => p.id); const missingPlugins = pluginFilter.filter((id) => !foundPluginIds.includes(id)); if (missingPlugins.length > 0) { throw createFlagError(`expected --plugin '${missingPlugins.join(', ')}' was not found`); @@ -97,12 +108,15 @@ export async function setupProject( // Validate that all requested packages were found. if (hasPackageFilter && packageFilter) { - const foundPackageIds = plugins.filter((p) => !p.isPlugin).map((p) => p.id); + const foundPackageIds = filteredPlugins.filter((p) => !p.isPlugin).map((p) => p.id); const missingPackages = packageFilter.filter((id) => !foundPackageIds.includes(id)); if (missingPackages.length > 0) { throw createFlagError(`expected --package '${missingPackages.join(', ')}' was not found`); } } + + // Use filtered plugins for iteration, all plugins for reference resolution + const plugins = hasAnyFilter ? filteredPlugins : allPlugins; spanPlugins?.end(); const spanPathsByPackage = transaction.startSpan('build_api_docs.getPathsByPackage', 'setup'); @@ -112,7 +126,7 @@ export async function setupProject( const spanProject = transaction.startSpan('build_api_docs.getTsProject', 'setup'); // Optimize: when building a single plugin/package, scope the TS project to just that directory const singlePluginDirectory = - hasAnyFilter && plugins.length === 1 ? plugins[0].directory : undefined; + hasAnyFilter && filteredPlugins.length === 1 ? filteredPlugins[0].directory : undefined; const project = getTsProject(REPO_ROOT, singlePluginDirectory); spanProject?.end(); @@ -130,6 +144,7 @@ export async function setupProject( return { plugins, + allPlugins, pathsByPlugin, project, initialDocIds, diff --git a/packages/kbn-docs-utils/src/cli/tasks/write_docs.test.ts b/packages/kbn-docs-utils/src/cli/tasks/write_docs.test.ts index 415515b21cec8..9d843e1720da3 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/write_docs.test.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/write_docs.test.ts @@ -69,6 +69,7 @@ describe('writeDocs', () => { pathsByPlugin: new Map(), project: {} as any, initialDocIds: ['doc1', 'doc2'], + allPlugins: [mockPlugin], }; apiMapResult = { @@ -84,6 +85,7 @@ describe('writeDocs', () => { referencedDeprecations: {}, unreferencedDeprecations: {}, adoptionTrackedAPIs: {}, + unnamedExports: {}, }; allPluginStats = { @@ -105,6 +107,7 @@ describe('writeDocs', () => { eslintDisableLineCount: 0, eslintDisableFileCount: 0, enzymeImportCount: 0, + unnamedExports: [], }, }; diff --git a/packages/kbn-docs-utils/src/cli/tasks/write_docs.ts b/packages/kbn-docs-utils/src/cli/tasks/write_docs.ts index 0a6e6388d7c7d..9971ec78a2024 100644 --- a/packages/kbn-docs-utils/src/cli/tasks/write_docs.ts +++ b/packages/kbn-docs-utils/src/cli/tasks/write_docs.ts @@ -45,20 +45,50 @@ export async function writeDocs( ): Promise { const { log, transaction, outputFolder } = context; const { initialDocIds } = setupResult; - const { plugins } = setupResult; + const { plugins, allPlugins } = setupResult; const { pluginApiMap, referencedDeprecations, unreferencedDeprecations } = apiMapResult; - if (!options.stats) { + // Only write aggregate docs (plugin directory, deprecation summaries) for full builds. + // Filtered builds skip these because: + // 1. Aggregate docs require complete data from all plugins to be accurate. + // 2. Writing partial deprecation data would be misleading (e.g., "deprecation by team"). + // 3. Single-plugin builds focus on validating that plugin's docs only. + if (!options.stats && !options.pluginFilter) { const spanWritePluginDirectoryDoc = transaction.startSpan( 'build_api_docs.writePluginDirectoryDoc', 'write' ); - await writePluginDirectoryDoc(outputFolder, pluginApiMap, allPluginStats, log); - spanWritePluginDirectoryDoc?.end(); + + const spanWriteDeprecationDocByPlugin = transaction.startSpan( + 'build_api_docs.writeDeprecationDocByPlugin', + 'write' + ); + await writeDeprecationDocByPlugin(outputFolder, referencedDeprecations, log); + spanWriteDeprecationDocByPlugin?.end(); + + const spanWriteDeprecationDueByTeam = transaction.startSpan( + 'build_api_docs.writeDeprecationDueByTeam', + 'write' + ); + await writeDeprecationDueByTeam(outputFolder, referencedDeprecations, allPlugins, log); + spanWriteDeprecationDueByTeam?.end(); + + const spanWriteDeprecationDocByApi = transaction.startSpan( + 'build_api_docs.writeDeprecationDocByApi', + 'write' + ); + await writeDeprecationDocByApi( + outputFolder, + referencedDeprecations, + unreferencedDeprecations, + log + ); + spanWriteDeprecationDocByApi?.end(); } + // Write individual plugin docs for (const plugin of plugins) { // Note that the filtering is done in this task, and not during plugin discovery, because the entire // public plugin API has to be parsed in order to correctly determine reference links, and ensure that @@ -86,38 +116,6 @@ export async function writeDocs( } else { log.info(`Plugin ${pluginApi.id} has no public API.`); } - - const spanWriteDeprecationDocByPlugin = transaction.startSpan( - 'build_api_docs.writeDeprecationDocByPlugin', - 'write' - ); - - await writeDeprecationDocByPlugin(outputFolder, referencedDeprecations, log); - - spanWriteDeprecationDocByPlugin?.end(); - - const spanWriteDeprecationDueByTeam = transaction.startSpan( - 'build_api_docs.writeDeprecationDueByTeam', - 'write' - ); - - await writeDeprecationDueByTeam(outputFolder, referencedDeprecations, plugins, log); - - spanWriteDeprecationDueByTeam?.end(); - - const spanWriteDeprecationDocByApi = transaction.startSpan( - 'build_api_docs.writeDeprecationDocByApi', - 'write' - ); - - await writeDeprecationDocByApi( - outputFolder, - referencedDeprecations, - unreferencedDeprecations, - log - ); - - spanWriteDeprecationDocByApi?.end(); } } diff --git a/packages/kbn-docs-utils/src/cli/types.ts b/packages/kbn-docs-utils/src/cli/types.ts index 2e0f01c32f6fa..8f43d1bdba302 100644 --- a/packages/kbn-docs-utils/src/cli/types.ts +++ b/packages/kbn-docs-utils/src/cli/types.ts @@ -17,6 +17,7 @@ import type { ReferencedDeprecationsByPlugin, UnreferencedDeprecationsByPlugin, AdoptionTrackedAPIsByPlugin, + UnnamedExportsByPlugin, ApiStats, PluginMetaInfo, } from '../types'; @@ -37,6 +38,8 @@ export interface CliFlags { plugin?: string | string[]; /** Package filter: single package ID or array of package IDs (id from kibana.jsonc). */ package?: string | string[]; + /** Whether to write stats to a flat JSON file. */ + write?: boolean; } /** @@ -51,6 +54,8 @@ export interface CliOptions { pluginFilter?: string[]; /** Package filter IDs (id from kibana.jsonc, e.g., @kbn/package-name). */ packageFilter?: string[]; + /** Whether to write stats to a flat JSON file. */ + writeStats?: boolean; } /** @@ -71,8 +76,10 @@ export interface CliContext { * Result from setup_project task. */ export interface SetupProjectResult { - /** Discovered plugins and packages. */ + /** Plugins/packages to analyze (may be filtered via --plugin/--package). */ plugins: PluginOrPackage[]; + /** All discovered plugins/packages (for cross-reference resolution). */ + allPlugins: PluginOrPackage[]; /** File paths grouped by package. */ pathsByPlugin: Map; /** TypeScript project instance. */ @@ -95,6 +102,8 @@ export interface BuildApiMapResult { unreferencedDeprecations: UnreferencedDeprecationsByPlugin; /** Adoption-tracked APIs. */ adoptionTrackedAPIs: AdoptionTrackedAPIsByPlugin; + /** Unnamed exports found during API collection. */ + unnamedExports: UnnamedExportsByPlugin; } /** diff --git a/packages/kbn-docs-utils/src/find_plugins.ts b/packages/kbn-docs-utils/src/find_plugins.ts index b206e7800370c..d8e522f6d56c6 100644 --- a/packages/kbn-docs-utils/src/find_plugins.ts +++ b/packages/kbn-docs-utils/src/find_plugins.ts @@ -124,6 +124,12 @@ export function findPlugins(options?: FindPluginsOptions): PluginOrPackage[] { return result; } +/** + * Finds all plugins owned by a specific team. + * + * @param team - The GitHub team identifier (e.g., `@elastic/kibana-core`). + * @returns Array of plugins owned by the specified team. + */ export function findTeamPlugins(team: string): PluginOrPackage[] { const packages = getPackages(REPO_ROOT); const plugins = packages.filter( diff --git a/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.test.ts b/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.test.ts index 44564608ba89f..382ef0407abb9 100644 --- a/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.test.ts +++ b/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.test.ts @@ -38,9 +38,9 @@ describe('getDeclarationNodesForPluginScope', () => { }, }; - const nodes = getDeclarationNodesForPluginScope(project, plugin, ApiScope.CLIENT, log); + const result = getDeclarationNodesForPluginScope(project, plugin, ApiScope.CLIENT, log); - expect(nodes).toEqual([]); + expect(result).toEqual({ nodes: [], unnamedExports: [] }); }); it('handles files that do not exist gracefully', () => { @@ -61,9 +61,9 @@ describe('getDeclarationNodesForPluginScope', () => { }, }; - const nodes = getDeclarationNodesForPluginScope(project, plugin, ApiScope.CLIENT, log); + const result = getDeclarationNodesForPluginScope(project, plugin, ApiScope.CLIENT, log); - // Should return empty array when file doesn't exist - expect(nodes).toEqual([]); + // Should return empty result when file doesn't exist + expect(result).toEqual({ nodes: [], unnamedExports: [] }); }); }); diff --git a/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts b/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts index cb306c4bec2e5..741033066e651 100644 --- a/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts +++ b/packages/kbn-docs-utils/src/get_declaration_nodes_for_plugin.ts @@ -10,9 +10,17 @@ import Path from 'path'; import type { ToolingLog } from '@kbn/tooling-log'; import type { Project, SourceFile, Node } from 'ts-morph'; -import type { ApiScope, PluginOrPackage } from './types'; +import { type ApiScope, type PluginOrPackage, type UnnamedExport } from './types'; import { isNamedNode, getSourceFileMatching } from './tsmorph_utils'; +/** + * Result of collecting declaration nodes, including any unnamed exports found. + */ +export interface DeclarationNodesResult { + nodes: Node[]; + unnamedExports: UnnamedExport[]; +} + /** * Determines which file in the project to grab nodes from, depending on the plugin and scope, then returns those nodes. * @@ -21,17 +29,20 @@ import { isNamedNode, getSourceFileMatching } from './tsmorph_utils'; * @param scope - The "scope" of the API we want to extract: public, server or common. * @param log - logging utility. * - * @return Every publically exported Node from the given plugin and scope (public, server, common). + * @return Every publicly exported Node from the given plugin and scope (public, server, common), + * along with any unnamed exports that were encountered. */ export function getDeclarationNodesForPluginScope( project: Project, plugin: PluginOrPackage, scope: ApiScope, log: ToolingLog -): Node[] { +): DeclarationNodesResult { // Packages specify the intended scope in the package.json, while plugins specify the scope // using folder structure. - if (!plugin.isPlugin && scope !== plugin.scope) return []; + if (!plugin.isPlugin && scope !== plugin.scope) { + return { nodes: [], unnamedExports: [] }; + } const path = plugin.isPlugin ? Path.join(`${plugin.directory}`, scope.toString(), 'index.ts') @@ -39,20 +50,30 @@ export function getDeclarationNodesForPluginScope( const file = getSourceFileMatching(project, path); if (file) { - return getExportedFileDeclarations(file, log); + return getExportedFileDeclarations(file, plugin.id, scope, log); } else { log.debug(`No file found: ${path}`); - return []; + return { nodes: [], unnamedExports: [] }; } } /** + * Extracts exported declaration nodes from a source file. * - * @param source the file we want to extract exported declaration nodes from. - * @param log + * @param source - The file we want to extract exported declaration nodes from. + * @param pluginId - The plugin or package ID for tracking unnamed exports. + * @param scope - The API scope (client, server, common). + * @param log - Logging utility. + * @returns The extracted nodes and any unnamed exports encountered. */ -function getExportedFileDeclarations(source: SourceFile, log: ToolingLog): Node[] { +function getExportedFileDeclarations( + source: SourceFile, + pluginId: string, + scope: ApiScope, + log: ToolingLog +): DeclarationNodesResult { const nodes: Node[] = []; + const unnamedExports: UnnamedExport[] = []; const exported = source.getExportedDeclarations(); // Filter out the exported declarations that exist only for the plugin system itself. @@ -70,11 +91,29 @@ function getExportedFileDeclarations(source: SourceFile, log: ToolingLog): Node[ if (name && name !== '') { nodes.push(ed); } else { - log.warning(`API with missing name encountered, text is ` + ed.getText().substring(0, 50)); + const filePath = source.getFilePath(); + const lineNumber = ed.getStartLineNumber(); + const textSnippet = ed.getText().substring(0, 100).replace(/\n/g, ' '); + unnamedExports.push({ + pluginId, + scope, + path: filePath, + lineNumber, + textSnippet, + }); + log.debug( + `Unnamed export in ${pluginId} at ${filePath}:${lineNumber}: ${textSnippet.substring( + 0, + 50 + )}` + ); } }); }); log.debug(`Collected ${nodes.length} exports from file ${source.getFilePath()}`); - return nodes; + if (unnamedExports.length > 0) { + log.debug(`Found ${unnamedExports.length} unnamed exports in ${source.getFilePath()}`); + } + return { nodes, unnamedExports }; } diff --git a/packages/kbn-docs-utils/src/get_plugin_api.ts b/packages/kbn-docs-utils/src/get_plugin_api.ts index 04eac17794580..07003e5ba3eb0 100644 --- a/packages/kbn-docs-utils/src/get_plugin_api.ts +++ b/packages/kbn-docs-utils/src/get_plugin_api.ts @@ -10,13 +10,28 @@ import Path from 'path'; import type { Node, Project } from 'ts-morph'; import type { ToolingLog } from '@kbn/tooling-log'; -import type { PluginOrPackage } from './types'; +import type { PluginOrPackage, UnnamedExport } from './types'; import { ApiScope, Lifecycle } from './types'; import type { ApiDeclaration, PluginApi } from './types'; import { buildApiDeclarationTopNode } from './build_api_declarations/build_api_declaration'; import { getDeclarationNodesForPluginScope } from './get_declaration_nodes_for_plugin'; import { getSourceFileMatching } from './tsmorph_utils'; +/** + * Warnings encountered during plugin API collection. + */ +export interface PluginApiWarnings { + unnamedExports: UnnamedExport[]; +} + +/** + * Result of collecting plugin API, including any warnings found. + */ +export interface PluginApiResult { + pluginApi: PluginApi; + warnings: PluginApiWarnings; +} + /** * Collects all the information necessary to generate this plugins mdx api file(s). */ @@ -26,23 +41,60 @@ export function getPluginApi( plugins: PluginOrPackage[], log: ToolingLog, captureReferences: boolean -): PluginApi { - const client = getDeclarations(project, plugin, ApiScope.CLIENT, plugins, log, captureReferences); - const server = getDeclarations(project, plugin, ApiScope.SERVER, plugins, log, captureReferences); - const common = getDeclarations(project, plugin, ApiScope.COMMON, plugins, log, captureReferences); +): PluginApiResult { + const clientResult = getDeclarations( + project, + plugin, + ApiScope.CLIENT, + plugins, + log, + captureReferences + ); + const serverResult = getDeclarations( + project, + plugin, + ApiScope.SERVER, + plugins, + log, + captureReferences + ); + const commonResult = getDeclarations( + project, + plugin, + ApiScope.COMMON, + plugins, + log, + captureReferences + ); + + const unnamedExports = [ + ...clientResult.unnamedExports, + ...serverResult.unnamedExports, + ...commonResult.unnamedExports, + ]; + return { - id: plugin.id, - client, - server, - common, - serviceFolders: plugin.manifest.serviceFolders, + pluginApi: { + id: plugin.id, + client: clientResult.declarations, + server: serverResult.declarations, + common: commonResult.declarations, + serviceFolders: plugin.manifest.serviceFolders, + }, + warnings: { + unnamedExports, + }, }; } +interface DeclarationResult { + declarations: ApiDeclaration[]; + unnamedExports: UnnamedExport[]; +} + /** - * - * @returns All exported ApiDeclarations for the given plugin and scope (client, server, common), broken into - * groups of typescript kinds (functions, classes, interfaces, etc). + * Returns all exported ApiDeclarations for the given plugin and scope (client, server, common), + * along with any unnamed exports that were encountered. */ function getDeclarations( project: Project, @@ -51,8 +103,8 @@ function getDeclarations( plugins: PluginOrPackage[], log: ToolingLog, captureReferences: boolean -): ApiDeclaration[] { - const nodes = getDeclarationNodesForPluginScope(project, plugin, scope, log); +): DeclarationResult { + const { nodes, unnamedExports } = getDeclarationNodesForPluginScope(project, plugin, scope, log); const contractTypes = getContractTypes(project, plugin, scope); @@ -78,7 +130,7 @@ function getDeclarations( }, []); // We have all the ApiDeclarations, now lets group them by typescript kinds. - return declarations; + return { declarations, unnamedExports }; } /** diff --git a/packages/kbn-docs-utils/src/get_plugin_api_map.test.ts b/packages/kbn-docs-utils/src/get_plugin_api_map.test.ts index c2e037944ddf1..f22f83522a0df 100644 --- a/packages/kbn-docs-utils/src/get_plugin_api_map.test.ts +++ b/packages/kbn-docs-utils/src/get_plugin_api_map.test.ts @@ -39,7 +39,7 @@ describe('getPluginApiMap', () => { ); const plugins = [pluginA, pluginB]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: false, }); @@ -54,7 +54,7 @@ describe('getPluginApiMap', () => { const pluginA = getKibanaPlatformPlugin('pluginA'); const plugins = [pluginA]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: false, }); @@ -67,7 +67,7 @@ describe('getPluginApiMap', () => { const pluginA = getKibanaPlatformPlugin('pluginA'); const plugins = [pluginA]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: true, }); @@ -79,7 +79,7 @@ describe('getPluginApiMap', () => { const pluginA = getKibanaPlatformPlugin('pluginA'); const plugins = [pluginA]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: false, }); @@ -96,7 +96,7 @@ describe('getPluginApiMap', () => { ); const plugins = [pluginA, pluginB]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: false, pluginFilter: ['pluginA'], }); @@ -111,7 +111,7 @@ describe('getPluginApiMap', () => { const pluginA = getKibanaPlatformPlugin('pluginA'); const plugins = [pluginA]; - const result = getPluginApiMap(project, plugins, log, { + const result = getPluginApiMap(project, plugins, plugins, log, { collectReferences: true, }); @@ -124,7 +124,7 @@ describe('getPluginApiMap', () => { }); it('handles empty plugin list', () => { - const result = getPluginApiMap(project, [], log, { + const result = getPluginApiMap(project, [], [], log, { collectReferences: false, }); diff --git a/packages/kbn-docs-utils/src/get_plugin_api_map.ts b/packages/kbn-docs-utils/src/get_plugin_api_map.ts index 0a760bdd7c46d..4e6a4a7870591 100644 --- a/packages/kbn-docs-utils/src/get_plugin_api_map.ts +++ b/packages/kbn-docs-utils/src/get_plugin_api_map.ts @@ -18,6 +18,7 @@ import type { PluginOrPackage, ReferencedDeprecationsByPlugin, UnreferencedDeprecationsByPlugin, + UnnamedExportsByPlugin, } from './types'; import { removeBrokenLinks } from './utils'; import type { AdoptionTrackedAPIStats } from './types'; @@ -25,6 +26,7 @@ import type { AdoptionTrackedAPIStats } from './types'; export function getPluginApiMap( project: Project, plugins: PluginOrPackage[], + allPlugins: PluginOrPackage[], log: ToolingLog, { collectReferences, pluginFilter }: { collectReferences: boolean; pluginFilter?: string[] } ): { @@ -33,13 +35,28 @@ export function getPluginApiMap( referencedDeprecations: ReferencedDeprecationsByPlugin; unreferencedDeprecations: UnreferencedDeprecationsByPlugin; adoptionTrackedAPIs: AdoptionTrackedAPIsByPlugin; + unnamedExports: UnnamedExportsByPlugin; } { log.debug('Building plugin API map, getting missing comments, and collecting deprecations...'); const pluginApiMap: { [key: string]: PluginApi } = {}; + const unnamedExports: UnnamedExportsByPlugin = {}; + plugins.forEach((plugin) => { const captureReferences = collectReferences && (!pluginFilter || pluginFilter.indexOf(plugin.id) >= 0); - pluginApiMap[plugin.id] = getPluginApi(project, plugin, plugins, log, captureReferences); + + // Pass allPlugins for cross-reference resolution (links to other packages) + const { pluginApi, warnings } = getPluginApi( + project, + plugin, + allPlugins, + log, + captureReferences + ); + pluginApiMap[plugin.id] = pluginApi; + if (warnings.unnamedExports.length > 0) { + unnamedExports[plugin.id] = warnings.unnamedExports; + } }); // Mapping of plugin id to the missing source API id to all the plugin API items that referenced this item. @@ -61,6 +78,7 @@ export function getPluginApiMap( referencedDeprecations, unreferencedDeprecations, adoptionTrackedAPIs, + unnamedExports, }; } diff --git a/packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts b/packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts index e396784906664..b5854eb840067 100644 --- a/packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts +++ b/packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts @@ -64,8 +64,47 @@ export interface MyProps { export type AReactElementFn = () => ReactElement; +/** + * A function type with multiple call signatures (overloads). + * This demonstrates handling of overloaded function types. + * + * @param input The input value to process. + * @returns The processed result. + */ +export interface OverloadedFunction { + /** + * Parse a string and return a number. + * @param input A string to parse. + * @returns The parsed number. + */ + (input: string): number; + /** + * Double a number. + * @param input A number to double. + * @returns The doubled value. + */ + // Intentionally separate overloads to test docs tooling handling of overloaded function types. + // eslint-disable-next-line @typescript-eslint/unified-signatures + (input: number): number; + /** + * Parse an array of strings. + * @param input An array of strings to parse. + * @returns An array of parsed numbers. + */ + (input: string[]): number[]; +} + +/** + * A variable typed with the overloaded function. + */ +export const overloadedFn: OverloadedFunction = ((input: string | number | string[]) => { + if (typeof input === 'string') return parseInt(input, 10); + if (typeof input === 'number') return input * 2; + return input.map((s) => parseInt(s, 10)); +}) as OverloadedFunction; + // Expected issues: -// missing comments (14): +// missing comments (15): // line 19 - TypeWithGeneric // line 21 - ImAType // line 28 - t @@ -80,11 +119,13 @@ export type AReactElementFn = () => ReactElement; // line 61 - foo // line 62 - bar // line 65 - AReactElementFn -// param doc mismatches (3): +// line 80 - input +// param doc mismatches (4): // line 30 - FnTypeWithGeneric // line 54 - foo // line 62 - bar -// no references (21): +// line 100 - overloadedFn +// no references (30): // line 14 - StringOrUndefinedType // line 19 - TypeWithGeneric // line 21 - ImAType @@ -106,3 +147,12 @@ export type AReactElementFn = () => ReactElement; // line 61 - foo // line 62 - bar // line 65 - AReactElementFn +// line 67 - OverloadedFunction +// line 75 - Unnamed +// line 80 - input +// line 80 - input +// line 81 - Unnamed +// line 88 - input +// line 89 - Unnamed +// line 94 - input +// line 100 - overloadedFn diff --git a/packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts b/packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts index 6b436988f1a3e..052a8d8d65c4d 100644 --- a/packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts +++ b/packages/kbn-docs-utils/src/integration_tests/api_doc_suite.test.ts @@ -119,8 +119,13 @@ beforeAll(async () => { pluginA.manifest.serviceFolders = ['foo']; const plugins: PluginOrPackage[] = [pluginA, pluginB]; - const { pluginApiMap, missingApiItems, referencedDeprecations, adoptionTrackedAPIs } = - getPluginApiMap(project, plugins, log, { collectReferences: false }); + const { + pluginApiMap, + missingApiItems, + referencedDeprecations, + adoptionTrackedAPIs, + unnamedExports, + } = getPluginApiMap(project, plugins, plugins, log, { collectReferences: false }); doc = pluginApiMap.pluginA; @@ -128,11 +133,13 @@ beforeAll(async () => { missingApiItems, referencedDeprecations, adoptionTrackedAPIs, + unnamedExports, }); pluginBStats = collectApiStatsForPlugin(pluginApiMap.pluginB, { missingApiItems, referencedDeprecations, adoptionTrackedAPIs, + unnamedExports, }); mdxOutputFolder = Path.resolve(__dirname, 'snapshots'); @@ -155,11 +162,21 @@ beforeAll(async () => { paramDocMismatches: pluginAStats.paramDocMismatches.length, isAnyType: pluginAStats.isAnyType.length, noReferences: pluginAStats.noReferences.length, + unnamedExports: pluginAStats.unnamedExports.length, }, missingComments: pluginAStats.missingComments.map(mapStat), paramDocMismatches: pluginAStats.paramDocMismatches.map(mapStat), isAnyType: pluginAStats.isAnyType.map(mapStat), noReferences: pluginAStats.noReferences.map(mapStat), + unnamedExports: pluginAStats.unnamedExports.map( + ({ pluginId, scope, path, lineNumber, textSnippet }) => ({ + pluginId, + scope, + path, + lineNumber, + textSnippet, + }) + ), }; fs.writeFileSync( Path.resolve(mdxOutputFolder, 'plugin_a.stats.json'), @@ -691,9 +708,8 @@ describe('validation and stats', () => { expect(missingComment).toBeDefined(); }); - it('validates missingComments includes destructured parameter children', () => { + it('does not flag destructured parameter children when documented', () => { // crazyFunction has destructured params with nested properties - // Current behavior: nested properties without comments are flagged const fn = doc.client.find((c) => c.label === 'crazyFunction'); expect(fn).toBeDefined(); @@ -703,34 +719,22 @@ describe('validation and stats', () => { const hiProp = objParam!.children?.find((c) => c.label === 'hi'); expect(hiProp).toBeDefined(); - // Current behavior: property without comment is flagged - // Note: This is a false positive that will be fixed in Phase 4.2 - // Verify the property structure exists and check if it's in missingComments expect(hiProp!.description).toBeDefined(); - const hasDescription = hiProp!.description!.length > 0; + expect(hiProp!.description!.length).toBeGreaterThan(0); const missingComment = pluginAStats.missingComments.find((d) => d.id === hiProp!.id); - - // If property has no description, it should be in missingComments - // If it has a description, it should not be in missingComments - if (!hasDescription) { - expect(missingComment).toBeDefined(); - } else { - expect(missingComment).toBeUndefined(); - } + expect(missingComment).toBeUndefined(); }); - it.skip('does not flag destructured params when `@param obj` exists', () => { + it('does not flag destructured params when `@param obj` exists', () => { const fn = doc.client.find((c) => c.label === 'crazyFunction'); expect(fn).toBeDefined(); const objParam = fn!.children?.find((c) => c.label === 'obj'); expect(objParam).toBeDefined(); expect(objParam!.description).toBeDefined(); - // Expected once fixed: the @param obj comment is captured. expect(objParam!.description!.length).toBeGreaterThan(0); const missingComment = pluginAStats.missingComments.find((d) => d.id === objParam!.id); - // Expected once fixed: the destructured param should not be flagged as missing. expect(missingComment).toBeUndefined(); }); @@ -979,4 +983,51 @@ describe('validation and stats', () => { expect(true).toBe(true); }); }); + + describe('multiple call signatures (function overloads)', () => { + it('handles OverloadedFunction interface with call signatures as children', () => { + const overloadedFnType = doc.client.find((c) => c.label === 'OverloadedFunction'); + expect(overloadedFnType).toBeDefined(); + expect(overloadedFnType!.type).toBe(TypeKind.InterfaceKind); + // Interface signature is undefined (self-referential); call signatures appear as children. + expect(overloadedFnType!.signature).toBeUndefined(); + expect(overloadedFnType!.children).toBeDefined(); + expect(overloadedFnType!.children!.length).toBe(3); // Three overload signatures. + }); + + it('handles overloadedFn variable with multiple call signatures', () => { + const overloadedFn = doc.client.find((c) => c.label === 'overloadedFn'); + expect(overloadedFn).toBeDefined(); + // Should be typed as FunctionKind since it has call signatures. + expect(overloadedFn!.type).toBe(TypeKind.FunctionKind); + // Should have children (parameters from the first signature). + expect(overloadedFn!.children).toBeDefined(); + expect(overloadedFn!.children!.length).toBeGreaterThan(0); + // The first parameter should be 'input'. + expect(overloadedFn!.children![0].label).toBe('input'); + }); + + it('extracts parameter documentation from the first overload signature', () => { + const overloadedFn = doc.client.find((c) => c.label === 'overloadedFn'); + expect(overloadedFn).toBeDefined(); + expect(overloadedFn!.children).toBeDefined(); + + const inputParam = overloadedFn!.children!.find((c) => c.label === 'input'); + expect(inputParam).toBeDefined(); + // The description should come from the JSDoc on the variable or first signature. + // Note: JSDoc on individual overload signatures inside a type literal + // is typically not extracted by ts-morph in the same way as top-level JSDoc. + }); + + it('links to OverloadedFunction interface in signature field', () => { + const overloadedFn = doc.client.find((c) => c.label === 'overloadedFn'); + expect(overloadedFn).toBeDefined(); + expect(overloadedFn!.signature).toBeDefined(); + // The signature contains a reference link to the OverloadedFunction interface. + const sigText = overloadedFn! + .signature!.map((s) => (typeof s === 'string' ? s : s.text)) + .join(''); + expect(sigText).toContain('OverloadedFunction'); + }); + }); }); diff --git a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.devdocs.json b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.devdocs.json index 068dbbdeddff4..f91d6d61f0cd2 100644 --- a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.devdocs.json +++ b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.devdocs.json @@ -775,6 +775,47 @@ "something!" ], "initialIsOpen": false + }, + { + "parentPluginId": "pluginA", + "id": "def-public.overloadedFn", + "type": "Function", + "tags": [], + "label": "overloadedFn", + "description": [ + "\nA variable typed with the overloaded function." + ], + "signature": [ + { + "pluginId": "pluginA", + "scope": "public", + "docId": "kibPluginAPluginApi", + "section": "def-public.OverloadedFunction", + "text": "OverloadedFunction" + } + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 100, + "columnNumber": 14, + "deprecated": false, + "trackAdoption": false, + "returnComment": [], + "children": [ + { + "parentPluginId": "pluginA", + "id": "def-public.overloadedFn.$1", + "type": "string", + "tags": [], + "label": "input", + "description": [], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 80, + "columnNumber": 4, + "deprecated": false, + "trackAdoption": false + } + ], + "initialIsOpen": false } ], "interfaces": [ @@ -1364,6 +1405,150 @@ ], "initialIsOpen": false }, + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction", + "type": "Interface", + "tags": [], + "label": "OverloadedFunction", + "description": [ + "\nA function type with multiple call signatures (overloads).\nThis demonstrates handling of overloaded function types.\n" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 67, + "columnNumber": 1, + "deprecated": false, + "trackAdoption": false, + "children": [ + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed", + "type": "Function", + "tags": [], + "label": "Unnamed", + "description": [ + "\nParse a string and return a number." + ], + "signature": [ + "any" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 75, + "columnNumber": 3, + "deprecated": false, + "trackAdoption": false, + "children": [ + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed.$1", + "type": "string", + "tags": [], + "label": "input", + "description": [ + "A string to parse." + ], + "signature": [ + "string" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 80, + "columnNumber": 4, + "deprecated": false, + "trackAdoption": false, + "isRequired": true + } + ], + "returnComment": [ + "The parsed number." + ] + }, + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed", + "type": "Function", + "tags": [], + "label": "Unnamed", + "description": [ + "\nDouble a number." + ], + "signature": [ + "any" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 81, + "columnNumber": 3, + "deprecated": false, + "trackAdoption": false, + "children": [ + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed.$1", + "type": "number", + "tags": [], + "label": "input", + "description": [ + "A number to double." + ], + "signature": [ + "number" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 88, + "columnNumber": 4, + "deprecated": false, + "trackAdoption": false, + "isRequired": true + } + ], + "returnComment": [ + "The doubled value." + ] + }, + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed", + "type": "Function", + "tags": [], + "label": "Unnamed", + "description": [ + "\nParse an array of strings." + ], + "signature": [ + "any" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 89, + "columnNumber": 3, + "deprecated": false, + "trackAdoption": false, + "children": [ + { + "parentPluginId": "pluginA", + "id": "def-public.OverloadedFunction.Unnamed.$1", + "type": "Array", + "tags": [], + "label": "input", + "description": [ + "An array of strings to parse." + ], + "signature": [ + "string[]" + ], + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "lineNumber": 94, + "columnNumber": 4, + "deprecated": false, + "trackAdoption": false, + "isRequired": true + } + ], + "returnComment": [ + "An array of parsed numbers." + ] + } + ], + "initialIsOpen": false + }, { "parentPluginId": "pluginA", "id": "def-public.SearchSpec", diff --git a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.mdx b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.mdx index b482cd94fb6e0..abc95a2cad2e8 100644 --- a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.mdx +++ b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.mdx @@ -21,7 +21,7 @@ Contact Kibana Core for questions regarding this plugin. | Public API count | Any count | Items lacking comments | Missing exports | |-------------------|-----------|------------------------|-----------------| -| 136 | 1 | 64 | 2 | +| 145 | 1 | 65 | 2 | ## Client diff --git a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.stats.json b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.stats.json index 3ab7a37d4f29f..13a8c881f16e2 100644 --- a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.stats.json +++ b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a.stats.json @@ -1,11 +1,12 @@ { "counts": { - "apiCount": 136, + "apiCount": 145, "missingExports": 2, - "missingComments": 64, - "paramDocMismatches": 13, + "missingComments": 65, + "paramDocMismatches": 14, "isAnyType": 1, - "noReferences": 135 + "noReferences": 144, + "unnamedExports": 0 }, "missingComments": [ { @@ -496,6 +497,14 @@ "lineNumber": 65, "columnNumber": 1 }, + { + "id": "def-public.overloadedFn.$1", + "label": "input", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "string", + "lineNumber": 80, + "columnNumber": 4 + }, { "id": "def-common.commonFoo", "label": "commonFoo", @@ -625,6 +634,14 @@ "type": "Function", "lineNumber": 62, "columnNumber": 3 + }, + { + "id": "def-public.overloadedFn", + "label": "overloadedFn", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Function", + "lineNumber": 100, + "columnNumber": 14 } ], "isAnyType": [ @@ -1694,6 +1711,78 @@ "lineNumber": 65, "columnNumber": 1 }, + { + "id": "def-public.OverloadedFunction.Unnamed.$1", + "label": "input", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "string", + "lineNumber": 80, + "columnNumber": 4 + }, + { + "id": "def-public.OverloadedFunction.Unnamed", + "label": "Unnamed", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Function", + "lineNumber": 75, + "columnNumber": 3 + }, + { + "id": "def-public.OverloadedFunction.Unnamed.$1", + "label": "input", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "number", + "lineNumber": 88, + "columnNumber": 4 + }, + { + "id": "def-public.OverloadedFunction.Unnamed", + "label": "Unnamed", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Function", + "lineNumber": 81, + "columnNumber": 3 + }, + { + "id": "def-public.OverloadedFunction.Unnamed.$1", + "label": "input", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Array", + "lineNumber": 94, + "columnNumber": 4 + }, + { + "id": "def-public.OverloadedFunction.Unnamed", + "label": "Unnamed", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Function", + "lineNumber": 89, + "columnNumber": 3 + }, + { + "id": "def-public.OverloadedFunction", + "label": "OverloadedFunction", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Interface", + "lineNumber": 67, + "columnNumber": 1 + }, + { + "id": "def-public.overloadedFn.$1", + "label": "input", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "string", + "lineNumber": 80, + "columnNumber": 4 + }, + { + "id": "def-public.overloadedFn", + "label": "overloadedFn", + "path": "packages/kbn-docs-utils/src/integration_tests/__fixtures__/src/plugin_a/public/types.ts", + "type": "Function", + "lineNumber": 100, + "columnNumber": 14 + }, { "id": "def-common.commonFoo", "label": "commonFoo", @@ -1718,5 +1807,6 @@ "lineNumber": 12, "columnNumber": 1 } - ] + ], + "unnamedExports": [] } diff --git a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a_foo.mdx b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a_foo.mdx index f94bd3e30c522..31197d82ebaee 100644 --- a/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a_foo.mdx +++ b/packages/kbn-docs-utils/src/integration_tests/snapshots/plugin_a_foo.mdx @@ -21,7 +21,7 @@ Contact Kibana Core for questions regarding this plugin. | Public API count | Any count | Items lacking comments | Missing exports | |-------------------|-----------|------------------------|-----------------| -| 136 | 1 | 64 | 2 | +| 145 | 1 | 65 | 2 | ## Client diff --git a/packages/kbn-docs-utils/src/mdx/split_apis_by_folder.test.ts b/packages/kbn-docs-utils/src/mdx/split_apis_by_folder.test.ts index ccb81a5797c21..d76d21b961466 100644 --- a/packages/kbn-docs-utils/src/mdx/split_apis_by_folder.test.ts +++ b/packages/kbn-docs-utils/src/mdx/split_apis_by_folder.test.ts @@ -38,11 +38,12 @@ beforeAll(() => { pluginA.manifest.serviceFolders = ['foo']; const plugins: PluginOrPackage[] = [pluginA]; - doc = getPluginApi(project, plugins[0], plugins, log, false); + const { pluginApi } = getPluginApi(project, plugins[0], plugins, log, false); + doc = pluginApi; }); test('foo service has all exports', () => { - expect(doc?.client.length).toBe(38); + expect(doc?.client.length).toBe(40); const split = splitApisByFolder(doc); expect(split.length).toBe(2); @@ -51,5 +52,5 @@ test('foo service has all exports', () => { expect(fooDoc?.common.length).toBe(1); expect(fooDoc?.client.length).toBe(2); - expect(mainDoc?.client.length).toBe(36); + expect(mainDoc?.client.length).toBe(38); }); diff --git a/packages/kbn-docs-utils/src/mdx/write_plugin_split_by_folder.test.ts b/packages/kbn-docs-utils/src/mdx/write_plugin_split_by_folder.test.ts index bbbdb94e920b4..862b0fc120387 100644 --- a/packages/kbn-docs-utils/src/mdx/write_plugin_split_by_folder.test.ts +++ b/packages/kbn-docs-utils/src/mdx/write_plugin_split_by_folder.test.ts @@ -56,7 +56,7 @@ export interface Zed = { zed: string }` }, ]; - const doc = getPluginApi(project, plugins[0], plugins, log, false); + const { pluginApi: doc } = getPluginApi(project, plugins[0], plugins, log, false); const docs = splitApisByFolder(doc); // The api at the main level, and one on a service level. diff --git a/packages/kbn-docs-utils/src/stats.ts b/packages/kbn-docs-utils/src/stats.ts index 13b1731c51782..f05473a9867bd 100644 --- a/packages/kbn-docs-utils/src/stats.ts +++ b/packages/kbn-docs-utils/src/stats.ts @@ -20,7 +20,7 @@ import { * Collects API stats for a single plugin. */ export function collectApiStatsForPlugin(doc: PluginApi, issues: IssuesByPlugin): ApiStats { - const { missingApiItems, referencedDeprecations, adoptionTrackedAPIs } = issues; + const { missingApiItems, referencedDeprecations, adoptionTrackedAPIs, unnamedExports } = issues; const stats: ApiStats = { missingComments: [], @@ -34,6 +34,7 @@ export function collectApiStatsForPlugin(doc: PluginApi, issues: IssuesByPlugin) adoptionTrackedAPIsUnreferencedCount: 0, apiCount: countApiForPlugin(doc), missingExports: Object.values(missingApiItems[doc.id] ?? {}).length, + unnamedExports: unnamedExports?.[doc.id] || [], }; Object.values(doc.client).forEach((def) => { collectStatsForApi(def, stats, doc); @@ -66,7 +67,13 @@ function collectAdoptionTrackedAPIStats( } function collectStatsForApi(doc: ApiDeclaration, stats: ApiStats, pluginApi: PluginApi): void { - const missingComment = doc.description === undefined || doc.description.length === 0; + const hasDescription = doc.description !== undefined && doc.description.length > 0; + const childHasDescription = + doc.children?.some( + (child) => child.description !== undefined && child.description.length > 0 + ) ?? false; + const isParameterNode = doc.id.includes('.$'); // parameters and destructured parameter nodes carry .$ in their id + const missingComment = !hasDescription && !(isParameterNode && childHasDescription); // Ignore all stats coming from third party libraries, we can't fix that! if (doc.path.includes('node_modules')) return; diff --git a/packages/kbn-docs-utils/src/types.ts b/packages/kbn-docs-utils/src/types.ts index 0721feb54dc16..d0f572bf47e1f 100644 --- a/packages/kbn-docs-utils/src/types.ts +++ b/packages/kbn-docs-utils/src/types.ts @@ -307,6 +307,30 @@ export interface ApiStats { * Number of adoption-tracked APIs that are still not referenced. */ adoptionTrackedAPIsUnreferencedCount: number; + /** + * Unnamed exports found in this plugin (e.g., JSDoc comments above non-declarations). + */ + unnamedExports: UnnamedExport[]; +} + +/** + * Represents an exported declaration that has no identifiable name. + * This typically occurs when a JSDoc-style comment appears above a line + * that isn't a proper declaration. + */ +export interface UnnamedExport { + pluginId: string; + scope: ApiScope; + path: string; + lineNumber: number; + textSnippet: string; +} + +/** + * A mapping of plugin id to a list of unnamed exports found in that plugin. + */ +export interface UnnamedExportsByPlugin { + [pluginId: string]: UnnamedExport[]; } /** @@ -317,6 +341,7 @@ export interface IssuesByPlugin { missingApiItems: MissingApiItemMap; referencedDeprecations: ReferencedDeprecationsByPlugin; adoptionTrackedAPIs: AdoptionTrackedAPIsByPlugin; + unnamedExports?: UnnamedExportsByPlugin; } export type PluginMetaInfo = ApiStats & { diff --git a/packages/kbn-docs-utils/src/utils.test.ts b/packages/kbn-docs-utils/src/utils.test.ts index 706e8cdaf8959..2f3dfdce9c5a6 100644 --- a/packages/kbn-docs-utils/src/utils.test.ts +++ b/packages/kbn-docs-utils/src/utils.test.ts @@ -96,7 +96,7 @@ it('test removeBrokenLinks', () => { const pluginApiMap: { [key: string]: PluginApi } = {}; plugins.map((plugin) => { - pluginApiMap[plugin.id] = getPluginApi(project, plugin, plugins, log, false); + pluginApiMap[plugin.id] = getPluginApi(project, plugin, plugins, log, false).pluginApi; }); const missingApiItems: { [key: string]: { [key: string]: string[] } } = {}; diff --git a/packages/kbn-docs-utils/src/utils.ts b/packages/kbn-docs-utils/src/utils.ts index c80c6371c0c6d..f8031be84c961 100644 --- a/packages/kbn-docs-utils/src/utils.ts +++ b/packages/kbn-docs-utils/src/utils.ts @@ -211,7 +211,16 @@ function removeBrokenLinksFromApi( if (api.signature) { api.signature = api.signature.map((sig) => { if (typeof sig !== 'string') { - if (!apiItemExists(sig.text, sig.scope, pluginApiMap[sig.pluginId])) { + const referencedPluginApi = pluginApiMap[sig.pluginId]; + + // If the referenced plugin isn't in our plugin map (e.g., single-package build), + // keep the link as-is since we can't verify if it exists. + if (!referencedPluginApi) { + return sig; + } + + // Plugin is in the map - check if the specific API item exists. + if (!apiItemExists(sig.text, sig.scope, referencedPluginApi)) { if (missingApiItems[sig.pluginId] === undefined) { missingApiItems[sig.pluginId] = {}; } @@ -223,6 +232,7 @@ function removeBrokenLinksFromApi( missingApiItems[sig.pluginId][sourceId].push(`${pluginId}-${api.id}`); missingCnt++; + // Return plain text for broken links. return sig.text; } return sig; diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/README.md b/src/platform/packages/shared/kbn-mcp-dev-server/README.md index 76a90376aa287..584df8f767f72 100644 --- a/src/platform/packages/shared/kbn-mcp-dev-server/README.md +++ b/src/platform/packages/shared/kbn-mcp-dev-server/README.md @@ -145,6 +145,103 @@ By following these steps, you can successfully add and register a new tool to th The following tools are available in the MCP Dev Server. +## Documentation Validation Tools + +### `check_package_docs` + +Quickly check a Kibana plugin or package for documentation issues. Returns pass/fail status and issue counts. Use this for initial assessment before deciding to fix issues. + +**Parameters:** + +| Name | Type | Required | Description | +|------|------|----------|-------------| +| `package` | string | Yes* | The plugin or package ID (e.g., `@kbn/docs-utils`). | +| `file` | string | Yes* | A file path; the owning package will be inferred. | + +*One of `package` or `file` is required. + +**Example response:** + +```json +{ + "package": "@kbn/some-plugin", + "directory": "/path/to/plugin", + "passed": false, + "totalIssues": 7, + "actionable": 5, + "pending": 2, + "counts": { + "apiCount": 20, + "missingComments": 3, + "missingReturns": 1, + "paramDocMismatches": 1, + "missingComplexTypeInfo": 0, + "isAnyType": 0, + "missingExports": 2 + } +} +``` + +> **Note:** `passed` is based on `actionable` issues only. `pending` issues (like `missingExports`) require human input or changes in other packages. + +### `fix_package_docs` + +Get detailed documentation issues for a Kibana plugin, package, or file. Returns issues grouped by file with source context and fix templates. Use this after `check_package_docs` identifies problems. + +**Parameters:** + +| Name | Type | Required | Description | +|------|------|----------|-------------| +| `package` | string | Yes* | The plugin or package ID. | +| `file` | string | Yes* | A file path; the owning package will be inferred. | +| `issueTypes` | array | No | Filter to specific issue types (see below). | + +*One of `package` or `file` is required. + +**Issue Types:** + +| Type | Default | Description | +|------|---------|-------------| +| `missingComments` | ✅ | APIs without JSDoc comments. | +| `missingReturns` | ✅ | Functions missing `@returns` tags. | +| `paramDocMismatches` | ✅ | Parameter documentation mismatches. | +| `missingComplexTypeInfo` | ✅ | Complex types lacking documentation. | +| `isAnyType` | ✅ | APIs using `any` type. | +| `missingExports` | ❌ | Types referenced but not exported. Opt-in only; often requires changes in consuming packages. | + +**Example response:** + +```json +{ + "package": "@kbn/some-plugin", + "totalIssues": 2, + "issuesByFile": [ + { + "file": "src/index.ts", + "issues": [ + { + "issueType": "missingReturns", + "id": "def-public.myFunction", + "label": "myFunction", + "file": "src/index.ts", + "line": 42, + "link": "https://github.com/elastic/kibana/blob/main/src/index.ts#L42", + "type": "Function", + "sourceSnippet": "39| /**\n40| * Does something.\n41| */\n42| export const myFunction = () => {\n43| return 'hello';\n44| };", + "template": "@returns {TYPE}" + } + ] + } + ] +} +``` + +**Workflow:** + +1. Run `check_package_docs` to get a quick summary. +2. If issues exist, run `fix_package_docs` to get detailed context. +3. Use the `sourceSnippet` and `template` to add missing documentation. + # Semantic Code Search For semantic code search, please use the [semantic-code-search-mcp-server](https://github.com/elastic/semantic-code-search-mcp-server). This server provides a suite of tools for exploring and understanding the Kibana codebase. diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/src/server/index.ts b/src/platform/packages/shared/kbn-mcp-dev-server/src/server/index.ts index a239c89f01836..e9c3b4e1ea534 100644 --- a/src/platform/packages/shared/kbn-mcp-dev-server/src/server/index.ts +++ b/src/platform/packages/shared/kbn-mcp-dev-server/src/server/index.ts @@ -19,6 +19,8 @@ import { runUnitTestsTool } from '../tools/run_unit_tests'; import { runCiChecksTool } from '../tools/run_ci_checks'; import { searchByCodeownerTool } from '../tools/search_by_codeowner'; import { findDependencyReferencesTool } from '../tools/find_dependency_references'; +import { checkPackageDocsTool } from '../tools/check_package_docs'; +import { fixPackageDocsTool } from '../tools/fix_package_docs'; run(async () => { const server = new McpServer({ name: 'mcp-dev-server', version: '1.0.0' }); @@ -30,6 +32,8 @@ run(async () => { addTool(server, runCiChecksTool); addTool(server, searchByCodeownerTool); addTool(server, findDependencyReferencesTool); + addTool(server, checkPackageDocsTool); + addTool(server, fixPackageDocsTool); const transport = new StdioServerTransport(); await server.connect(transport); diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.test.ts b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.test.ts new file mode 100644 index 0000000000000..7aad702d7f537 --- /dev/null +++ b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.test.ts @@ -0,0 +1,307 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import fs from 'fs'; + +import { checkPackageDocsTool } from './check_package_docs'; +import { parseToolResultJsonContent } from './test_utils'; + +jest.mock('@kbn/repo-packages', () => ({ + getPackages: jest.fn(() => [ + { + id: '@kbn/test-pkg', + directory: '/repo/packages/test-pkg', + manifest: { id: '@kbn/test-pkg' }, + isPlugin: (): boolean => false, + }, + { + id: '@kbn/dashboard-plugin', + directory: '/repo/src/platform/plugins/shared/dashboard', + manifest: { id: '@kbn/dashboard-plugin', plugin: { id: 'dashboard' } }, + isPlugin: (): boolean => true, + }, + { + id: '@kbn/dashboard-markdown', + directory: '/repo/src/platform/plugins/shared/dashboard_markdown', + manifest: { id: '@kbn/dashboard-markdown', plugin: { id: 'dashboardMarkdown' } }, + isPlugin: (): boolean => true, + }, + ]), +})); + +jest.mock('@kbn/repo-info', () => ({ + REPO_ROOT: '/repo', +})); + +jest.mock('execa', () => jest.fn().mockResolvedValue({ exitCode: 0 })); + +jest.mock('fs', () => ({ + existsSync: jest.fn(), + readFileSync: jest.fn(), +})); + +const mockFs = fs as jest.Mocked; + +const createMockStats = (overrides = {}) => ({ + counts: { + apiCount: 5, + missingExports: 0, + missingComments: 0, + isAnyType: 0, + noReferences: 0, + missingReturns: 0, + paramDocMismatches: 0, + missingComplexTypeInfo: 0, + }, + missingComments: [], + isAnyType: [], + missingReturns: [], + paramDocMismatches: [], + missingComplexTypeInfo: [], + ...overrides, +}); + +describe('checkPackageDocsTool', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('target parameter', () => { + it('returns error when package is not found', async () => { + const result = await checkPackageDocsTool.handler({ target: '@kbn/unknown' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain("'@kbn/unknown' not found"); + }); + + it('returns error with suggestions when package is not found but similar ones exist', async () => { + // Use a partial match that doesn't exactly match any plugin ID. + const result = await checkPackageDocsTool.handler({ target: 'dashb' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain("'dashb' not found"); + expect(parsed.error).toContain('Did you mean'); + expect(parsed.error).toContain('dashboard (@kbn/dashboard-plugin)'); + }); + + it('finds plugin by plugin ID (e.g., dashboard)', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + // Using the plugin ID directly should work. + const result = await checkPackageDocsTool.handler({ target: 'dashboard' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/dashboard-plugin'); + expect(parsed.passed).toBe(true); + }); + + it('finds package by manifest ID (auto-detects scoped package names)', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + // Scoped package names starting with @ should be auto-detected as IDs, not files. + const result = await checkPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/test-pkg'); + }); + }); + + describe('type parameter', () => { + it('treats target as file when type is "file"', async () => { + const result = await checkPackageDocsTool.handler({ + target: '/some/random/file.ts', + type: 'file', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Could not find a package'); + }); + + it('auto-detects file type when target contains "/"', async () => { + const result = await checkPackageDocsTool.handler({ + target: '/some/random/file.ts', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Could not find a package'); + }); + + it('treats target as plugin/package when type is "plugin"', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await checkPackageDocsTool.handler({ + target: 'dashboard', + type: 'plugin', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/dashboard-plugin'); + }); + + it('treats target as package when type is "package"', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await checkPackageDocsTool.handler({ + target: '@kbn/test-pkg', + type: 'package', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/test-pkg'); + }); + }); + + describe('stats file handling', () => { + it('returns error when stats file does not exist after CLI run', async () => { + mockFs.existsSync.mockReturnValue(false); + + const result = await checkPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Stats file not found'); + }); + }); + + describe('pass/fail status', () => { + it('returns pass/fail status with counts for a clean package', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await checkPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.passed).toBe(true); + expect(parsed.totalIssues).toBe(0); + expect(parsed.counts.apiCount).toBe(5); + }); + + it('returns pass=false when issues exist', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue( + JSON.stringify( + createMockStats({ + counts: { + apiCount: 10, + missingExports: 1, + missingComments: 3, + isAnyType: 2, + noReferences: 0, + missingReturns: 1, + paramDocMismatches: 0, + missingComplexTypeInfo: 0, + }, + missingComments: [{ path: 'src/a.ts' }, { path: 'src/b.ts' }, { path: 'src/c.ts' }], + isAnyType: [{ path: 'src/a.ts' }, { path: 'src/b.ts' }], + missingReturns: [{ path: 'src/a.ts' }], + }) + ) + ); + + const result = await checkPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.passed).toBe(false); + expect(parsed.totalIssues).toBe(7); // 3 + 2 + 1 + 1 (pending) + expect(parsed.actionable).toBe(6); // 3 + 2 + 1 + expect(parsed.pending).toBe(1); + expect(parsed.counts.missingComments).toBe(3); + expect(parsed.counts.isAnyType).toBe(2); + expect(parsed.counts.missingReturns).toBe(1); + expect(parsed.counts.missingExports).toBe(1); + }); + + it('returns passed=true when only pending issues exist', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue( + JSON.stringify( + createMockStats({ + counts: { + apiCount: 5, + missingExports: 3, + missingComments: 0, + isAnyType: 0, + noReferences: 0, + missingReturns: 0, + paramDocMismatches: 0, + missingComplexTypeInfo: 0, + }, + }) + ) + ); + + const result = await checkPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + // Package passes because missingExports are pending (need human input). + expect(parsed.passed).toBe(true); + expect(parsed.totalIssues).toBe(3); // includes pending + expect(parsed.actionable).toBe(0); + expect(parsed.pending).toBe(3); + }); + }); + + describe('file filtering', () => { + it('filters issues to specific file when target is a file path', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue( + JSON.stringify( + createMockStats({ + counts: { + apiCount: 10, + missingExports: 0, + missingComments: 3, + isAnyType: 1, + noReferences: 0, + missingReturns: 1, + paramDocMismatches: 0, + missingComplexTypeInfo: 0, + }, + missingComments: [ + { path: 'packages/test-pkg/src/index.ts' }, + { path: 'packages/test-pkg/src/other.ts' }, + { path: 'packages/test-pkg/src/index.ts' }, + ], + isAnyType: [{ path: 'packages/test-pkg/src/index.ts' }], + missingReturns: [{ path: 'packages/test-pkg/src/other.ts' }], + }) + ) + ); + + const result = await checkPackageDocsTool.handler({ + target: 'packages/test-pkg/src/index.ts', + type: 'file', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.file).toBe('packages/test-pkg/src/index.ts'); + // Should only count issues from index.ts (2 missingComments + 1 isAnyType). + expect(parsed.totalIssues).toBe(3); + expect(parsed.counts.missingComments).toBe(2); + expect(parsed.counts.isAnyType).toBe(1); + expect(parsed.counts.missingReturns).toBe(0); + }); + }); + + describe('tool metadata', () => { + it('has correct tool metadata', () => { + expect(checkPackageDocsTool.name).toBe('check_package_docs'); + expect(checkPackageDocsTool.description).toContain('Check'); + expect(checkPackageDocsTool.description).toContain('documentation issues'); + }); + }); +}); diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.ts b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.ts new file mode 100644 index 0000000000000..1a7cffeaa6c15 --- /dev/null +++ b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/check_package_docs.ts @@ -0,0 +1,362 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import fs from 'fs'; +import Path from 'path'; + +import { z } from '@kbn/zod'; +import execa from 'execa'; +import { getPackages } from '@kbn/repo-packages'; +import { REPO_ROOT } from '@kbn/repo-info'; + +import type { ToolDefinition } from '../types'; + +interface FlatStats { + counts: { + apiCount: number; + missingExports: number; + missingComments: number; + isAnyType: number; + noReferences: number; + missingReturns: number; + paramDocMismatches: number; + missingComplexTypeInfo: number; + }; + missingComments: Array<{ path: string }>; + isAnyType: Array<{ path: string }>; + missingReturns: Array<{ path: string }>; + paramDocMismatches: Array<{ path: string }>; + missingComplexTypeInfo: Array<{ path: string }>; +} + +interface PackageInfo { + pkg: ReturnType[number]; + /** The CLI flag to use: '--plugin' for plugins, '--package' for packages. */ + cliFlag: '--plugin' | '--package'; + /** The ID to pass to the CLI (plugin.id for plugins, manifest.id for packages). */ + cliId: string; +} + +type FindPackageResult = + | { + found: true; + info: PackageInfo; + } + | { + found: false; + suggestions: string[]; + }; + +/** + * Finds a package by ID and returns the appropriate CLI flag and ID. + * + * For plugins, returns the plugin.id to use with --plugin. + * For packages, returns the manifest.id to use with --package. + */ +const findPackage = (packageId: string): FindPackageResult => { + const packages = getPackages(REPO_ROOT); + const normalizedId = packageId.toLowerCase(); + + // First, try to find by package manifest ID (e.g., @kbn/dashboard-plugin). + const byManifestId = packages.find((pkg) => pkg.manifest.id === packageId); + if (byManifestId) { + if (byManifestId.isPlugin()) { + return { + found: true, + info: { + pkg: byManifestId, + cliFlag: '--plugin', + cliId: byManifestId.manifest.plugin.id, + }, + }; + } + return { + found: true, + info: { + pkg: byManifestId, + cliFlag: '--package', + cliId: byManifestId.manifest.id, + }, + }; + } + + // Second, try to find by plugin ID (e.g., dashboard). + const byPluginId = packages.find((pkg) => pkg.isPlugin() && pkg.manifest.plugin.id === packageId); + if (byPluginId && byPluginId.isPlugin()) { + return { + found: true, + info: { + pkg: byPluginId, + cliFlag: '--plugin', + cliId: byPluginId.manifest.plugin.id, + }, + }; + } + + // Not found - collect suggestions for helpful error message. + const suggestions: string[] = []; + + // Find packages/plugins that contain the search term. + for (const pkg of packages) { + const manifestId = pkg.manifest.id.toLowerCase(); + if (pkg.isPlugin()) { + const pluginId = pkg.manifest.plugin.id.toLowerCase(); + if (pluginId.includes(normalizedId) || manifestId.includes(normalizedId)) { + suggestions.push(`${pkg.manifest.plugin.id} (${pkg.manifest.id})`); + } + } else if (manifestId.includes(normalizedId)) { + suggestions.push(pkg.manifest.id); + } + } + + return { found: false, suggestions: suggestions.slice(0, 5) }; +}; + +/** + * Finds the package that contains a given file path. + */ +const findPackageForFile = (filePath: string): PackageInfo | undefined => { + const packages = getPackages(REPO_ROOT); + const absolutePath = Path.resolve(REPO_ROOT, filePath); + const pkg = packages.find((p) => absolutePath.startsWith(p.directory)); + + if (!pkg) return undefined; + + if (pkg.isPlugin()) { + return { + pkg, + cliFlag: '--plugin', + cliId: pkg.manifest.plugin.id, + }; + } + return { + pkg, + cliFlag: '--package', + cliId: pkg.manifest.id, + }; +}; + +/** + * Runs the check_package_docs CLI to generate fresh stats. + */ +const generateStats = async ( + cliFlag: '--plugin' | '--package', + cliId: string +): Promise<{ success: boolean; error?: string }> => { + try { + await execa('node', ['scripts/check_package_docs', cliFlag, cliId, '--write'], { + cwd: REPO_ROOT, + timeout: 120000, + }); + return { success: true }; + } catch (err: unknown) { + // Exit code 1 means validation failed (issues found) - this is expected. + if ( + err && + typeof err === 'object' && + 'exitCode' in err && + (err as { exitCode: number }).exitCode === 1 + ) { + return { success: true }; + } + + // Extract meaningful error information from execa error. + if (err && typeof err === 'object') { + const execaErr = err as { stderr?: string; stdout?: string; message?: string }; + // Prefer stderr as it usually contains the actual error. + if (execaErr.stderr && execaErr.stderr.trim()) { + return { success: false, error: execaErr.stderr.trim() }; + } + if (execaErr.message) { + return { success: false, error: execaErr.message }; + } + } + + const error = err instanceof Error ? err.message : String(err); + return { success: false, error }; + } +}; + +/** + * Detects the target type based on the target string. + * Scoped package names (starting with @) are treated as IDs. + * Paths containing '/' are treated as files; otherwise treated as plugin/package IDs. + */ +const detectTargetType = (target: string): 'file' | 'id' => { + // Scoped package names like @kbn/dashboard-plugin are IDs, not files. + if (target.startsWith('@')) { + return 'id'; + } + // Paths containing '/' are files. + if (target.includes('/')) { + return 'file'; + } + return 'id'; +}; + +const checkPackageDocsInputSchema = z.object({ + target: z + .string() + .describe( + 'The plugin ID (e.g., "dashboard"), manifest ID (e.g., "@kbn/dashboard-plugin"), or file path to check.' + ), + type: z + .enum(['plugin', 'package', 'file']) + .optional() + .describe( + 'How to interpret the target. If omitted, auto-detected: paths containing "/" are treated as files; otherwise tries plugin ID first, then manifest ID.' + ), +}); + +const checkPackageDocs = async (input: z.infer) => { + const { target } = input; + + // Determine effective type: use explicit type or auto-detect. + const effectiveType = input.type ?? (detectTargetType(target) === 'file' ? 'file' : 'plugin'); + + let pkgInfo: PackageInfo | undefined; + let filePath: string | undefined; + + if (effectiveType === 'file') { + filePath = target; + pkgInfo = findPackageForFile(target); + if (!pkgInfo) { + return { + error: `Could not find a package containing file '${target}'.`, + }; + } + } else { + // For 'plugin' type: try plugin ID first, then manifest ID (current findPackage behavior). + // For 'package' type: try manifest ID only. + const result = findPackage(target); + if (!result.found) { + let errorMsg = `Plugin or package '${target}' not found.`; + if (result.suggestions.length > 0) { + errorMsg += ` Did you mean: ${result.suggestions.join(', ')}?`; + } + return { error: errorMsg }; + } + pkgInfo = result.info; + } + + const { pkg, cliFlag, cliId } = pkgInfo; + + // Generate fresh stats. + const genResult = await generateStats(cliFlag, cliId); + if (!genResult.success) { + return { + error: `Failed to generate stats: ${genResult.error}`, + }; + } + + const statsPath = Path.resolve(pkg.directory, 'target', 'api_docs', 'stats.json'); + + if (!fs.existsSync(statsPath)) { + return { + error: `Stats file not found at ${statsPath}. This may indicate an issue with the package.`, + }; + } + + let stats: FlatStats; + try { + stats = JSON.parse(fs.readFileSync(statsPath, 'utf-8')); + } catch { + return { + error: `Failed to parse stats file at ${statsPath}.`, + }; + } + + // If filtering by file, count issues only for that file + if (filePath) { + const normalizedPath = filePath.startsWith('/') ? filePath : Path.resolve(REPO_ROOT, filePath); + const relativePath = Path.relative(REPO_ROOT, normalizedPath); + + const filterByFile = (arr: T[]) => + arr.filter((item) => item.path === relativePath || item.path === filePath); + + const fileCounts = { + missingComments: filterByFile(stats.missingComments).length, + missingReturns: filterByFile(stats.missingReturns).length, + paramDocMismatches: filterByFile(stats.paramDocMismatches).length, + missingComplexTypeInfo: filterByFile(stats.missingComplexTypeInfo).length, + isAnyType: filterByFile(stats.isAnyType).length, + }; + + const totalIssues = Object.values(fileCounts).reduce((sum, count) => sum + count, 0); + const filePassed = totalIssues === 0; + + return { + package: pkg.id, + file: filePath, + passed: filePassed, + totalIssues, + counts: fileCounts, + // Provide a hint for agents when there are issues to fix. + ...(filePassed + ? {} + : { + hint: `Use the fix_package_docs tool with target "${filePath}" to get detailed issues with source context and fix templates.`, + }), + }; + } + + // Actionable issues are documentation problems that can be fixed within this package. + // Pending issues require human input or changes in other packages. + const actionable = + stats.counts.missingComments + + stats.counts.missingReturns + + stats.counts.paramDocMismatches + + stats.counts.missingComplexTypeInfo + + stats.counts.isAnyType; + + const pending = stats.counts.missingExports; + const passed = actionable === 0; + + return { + package: pkg.id, + directory: pkg.directory, + passed, + totalIssues: actionable + pending, + actionable, + pending, + counts: { + apiCount: stats.counts.apiCount, + missingComments: stats.counts.missingComments, + missingReturns: stats.counts.missingReturns, + paramDocMismatches: stats.counts.paramDocMismatches, + missingComplexTypeInfo: stats.counts.missingComplexTypeInfo, + isAnyType: stats.counts.isAnyType, + missingExports: stats.counts.missingExports, + }, + // Provide a hint for agents when there are issues to fix. + ...(passed + ? {} + : { + hint: `Use the fix_package_docs tool with target "${target}" to get detailed issues with source context and fix templates.`, + }), + }; +}; + +export const checkPackageDocsTool: ToolDefinition = { + name: 'check_package_docs', + description: + 'Check a Kibana plugin or package for documentation issues. Returns pass/fail status and issue counts. Use this for quick validation before deciding to fix issues.', + inputSchema: checkPackageDocsInputSchema, + handler: async (input) => { + const result = await checkPackageDocs(input); + return { + content: [ + { + type: 'text', + text: JSON.stringify(result, null, 2), + }, + ], + }; + }, +}; diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.test.ts b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.test.ts new file mode 100644 index 0000000000000..b0bd3226b3e95 --- /dev/null +++ b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.test.ts @@ -0,0 +1,302 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import fs from 'fs'; + +import { fixPackageDocsTool } from './fix_package_docs'; +import { parseToolResultJsonContent } from './test_utils'; + +jest.mock('@kbn/repo-packages', () => ({ + getPackages: jest.fn(() => [ + { + id: '@kbn/test-pkg', + directory: '/repo/packages/test-pkg', + manifest: { id: '@kbn/test-pkg' }, + isPlugin: (): boolean => false, + }, + { + id: '@kbn/dashboard-plugin', + directory: '/repo/src/platform/plugins/shared/dashboard', + manifest: { id: '@kbn/dashboard-plugin', plugin: { id: 'dashboard' } }, + isPlugin: (): boolean => true, + }, + ]), +})); + +jest.mock('@kbn/repo-info', () => ({ + REPO_ROOT: '/repo', +})); + +jest.mock('execa', () => jest.fn().mockResolvedValue({ exitCode: 0 })); + +jest.mock('fs', () => ({ + existsSync: jest.fn(), + readFileSync: jest.fn(), +})); + +const mockFs = fs as jest.Mocked; + +const createMockStats = (overrides = {}) => ({ + counts: { + apiCount: 10, + missingExports: 0, + missingComments: 2, + isAnyType: 1, + noReferences: 0, + missingReturns: 1, + paramDocMismatches: 1, + missingComplexTypeInfo: 0, + }, + missingComments: [ + { + id: 'def-public.fn1', + label: 'fn1', + path: 'packages/test-pkg/src/index.ts', + type: 'Function', + lineNumber: 10, + columnNumber: 1, + link: 'https://github.com/elastic/kibana/blob/main/packages/test-pkg/src/index.ts#L10', + }, + { + id: 'def-public.fn2', + label: 'fn2', + path: 'packages/test-pkg/src/utils.ts', + type: 'Function', + lineNumber: 5, + columnNumber: 1, + link: 'https://github.com/elastic/kibana/blob/main/packages/test-pkg/src/utils.ts#L5', + }, + ], + isAnyType: [ + { + id: 'def-public.badFn', + label: 'badFn', + path: 'packages/test-pkg/src/index.ts', + type: 'Function', + lineNumber: 20, + columnNumber: 1, + link: 'https://github.com/elastic/kibana/blob/main/packages/test-pkg/src/index.ts#L20', + }, + ], + noReferences: [], + missingReturns: [ + { + id: 'def-public.fn1', + label: 'fn1', + path: 'packages/test-pkg/src/index.ts', + type: 'Function', + lineNumber: 10, + columnNumber: 1, + link: 'https://github.com/elastic/kibana/blob/main/packages/test-pkg/src/index.ts#L10', + }, + ], + paramDocMismatches: [ + { + id: 'def-public.fn3', + label: 'fn3', + path: 'packages/test-pkg/src/index.ts', + type: 'Function', + lineNumber: 30, + columnNumber: 1, + link: 'https://github.com/elastic/kibana/blob/main/packages/test-pkg/src/index.ts#L30', + }, + ], + missingComplexTypeInfo: [], + missingExports: [], + ...overrides, +}); + +describe('fixPackageDocsTool', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('target parameter', () => { + it('returns error when package is not found', async () => { + const result = await fixPackageDocsTool.handler({ target: '@kbn/unknown' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain("'@kbn/unknown' not found"); + }); + + it('finds package by manifest ID', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/test-pkg'); + }); + + it('finds plugin by plugin ID', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ target: 'dashboard' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toBeUndefined(); + expect(parsed.package).toBe('@kbn/dashboard-plugin'); + }); + }); + + describe('type parameter', () => { + it('treats target as file when type is "file"', async () => { + const result = await fixPackageDocsTool.handler({ + target: '/some/random/file.ts', + type: 'file', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Could not find a package'); + }); + + it('auto-detects file type when target contains "/"', async () => { + const result = await fixPackageDocsTool.handler({ + target: '/some/random/file.ts', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Could not find a package'); + }); + }); + + describe('stats file handling', () => { + it('returns error when stats file does not exist', async () => { + mockFs.existsSync.mockReturnValue(false); + + const result = await fixPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.error).toContain('Stats file not found'); + }); + }); + + describe('issues grouping', () => { + it('returns issues grouped by file', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ target: '@kbn/test-pkg' }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.package).toBe('@kbn/test-pkg'); + expect(parsed.totalIssues).toBe(5); // 2 + 1 + 1 + 1 + expect(parsed.issuesByFile).toHaveLength(2); // index.ts and utils.ts + + const indexFile = parsed.issuesByFile.find((g: { file: string }) => + g.file.includes('index.ts') + ); + expect(indexFile).toBeDefined(); + expect(indexFile.issues.length).toBeGreaterThan(0); + }); + }); + + describe('issue type filtering', () => { + it('filters by issue type when specified', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ + target: '@kbn/test-pkg', + issueTypes: ['missingReturns'], + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.totalIssues).toBe(1); + const allIssueTypes = parsed.issuesByFile.flatMap((g: { issues: { issueType: string }[] }) => + g.issues.map((i) => i.issueType) + ); + expect(allIssueTypes).toEqual(['missingReturns']); + }); + + it('includes templates for actionable issues', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ + target: '@kbn/test-pkg', + issueTypes: ['missingReturns'], + }); + const parsed = parseToolResultJsonContent(result); + + const issue = parsed.issuesByFile[0].issues[0]; + expect(issue.template).toBe('@returns {TYPE}'); + }); + + it('includes missingExports when requested and counts them in totalIssues', async () => { + const statsWithExports = createMockStats({ + missingExports: [ + { source: 'SomeType', references: ['file1.ts', 'file2.ts'] }, + { source: 'OtherType', references: ['file3.ts'] }, + ], + }); + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(statsWithExports)); + + const result = await fixPackageDocsTool.handler({ + target: '@kbn/test-pkg', + issueTypes: ['missingExports'], + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.missingExports).toHaveLength(2); + expect(parsed.missingExports[0].source).toBe('SomeType'); + // totalIssues should include missingExports when explicitly requested. + expect(parsed.totalIssues).toBe(2); + }); + }); + + describe('file filtering', () => { + it('filters issues by file path using relative path', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ + target: 'packages/test-pkg/src/index.ts', + type: 'file', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.package).toBe('@kbn/test-pkg'); + expect(parsed.file).toBe('packages/test-pkg/src/index.ts'); + // Should only include issues from index.ts (fn1 missingComments, badFn isAnyType, fn1 missingReturns, fn3 paramDocMismatches). + expect(parsed.totalIssues).toBe(4); + expect(parsed.issuesByFile).toHaveLength(1); + expect(parsed.issuesByFile[0].file).toBe('packages/test-pkg/src/index.ts'); + }); + + it('filters issues by file path using absolute path', async () => { + mockFs.existsSync.mockReturnValue(true); + mockFs.readFileSync.mockReturnValue(JSON.stringify(createMockStats())); + + const result = await fixPackageDocsTool.handler({ + target: '/repo/packages/test-pkg/src/utils.ts', + type: 'file', + }); + const parsed = parseToolResultJsonContent(result); + + expect(parsed.package).toBe('@kbn/test-pkg'); + // Should only include issues from utils.ts (fn2 missingComments). + expect(parsed.totalIssues).toBe(1); + expect(parsed.issuesByFile).toHaveLength(1); + expect(parsed.issuesByFile[0].file).toBe('packages/test-pkg/src/utils.ts'); + }); + }); + + describe('tool metadata', () => { + it('has correct tool metadata', () => { + expect(fixPackageDocsTool.name).toBe('fix_package_docs'); + expect(fixPackageDocsTool.description).toContain('documentation issues'); + expect(fixPackageDocsTool.description).toContain('source context'); + }); + }); +}); diff --git a/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.ts b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.ts new file mode 100644 index 0000000000000..e9d3b4992bfde --- /dev/null +++ b/src/platform/packages/shared/kbn-mcp-dev-server/src/tools/fix_package_docs.ts @@ -0,0 +1,478 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import fs from 'fs'; +import Path from 'path'; + +import { z } from '@kbn/zod'; +import execa from 'execa'; +import { getPackages } from '@kbn/repo-packages'; +import { REPO_ROOT } from '@kbn/repo-info'; + +import type { ToolDefinition } from '../types'; + +interface StatEntry { + id: string; + label: string; + path: string; + type: string; + lineNumber?: number; + columnNumber?: number; + link: string; +} + +interface MissingExportEntry { + source: string; + references: string[]; +} + +interface FlatStats { + counts: { + apiCount: number; + missingExports: number; + missingComments: number; + isAnyType: number; + noReferences: number; + missingReturns: number; + paramDocMismatches: number; + missingComplexTypeInfo: number; + }; + missingComments: StatEntry[]; + isAnyType: StatEntry[]; + noReferences: StatEntry[]; + missingReturns: StatEntry[]; + paramDocMismatches: StatEntry[]; + missingComplexTypeInfo: StatEntry[]; + missingExports: MissingExportEntry[]; +} + +interface EnrichedIssue { + issueType: string; + id: string; + label: string; + file: string; + line?: number; + column?: number; + link: string; + type: string; + sourceSnippet?: string; + template?: string; +} + +interface FileGroup { + file: string; + issues: EnrichedIssue[]; +} + +const SNIPPET_CONTEXT_LINES = 3; + +/** + * Reads a few lines of source context around a given line number. + */ +const getSourceSnippet = (filePath: string, lineNumber?: number): string | undefined => { + if (!lineNumber) return undefined; + + const absolutePath = Path.resolve(REPO_ROOT, filePath); + if (!fs.existsSync(absolutePath)) return undefined; + + try { + const content = fs.readFileSync(absolutePath, 'utf-8'); + const lines = content.split('\n'); + const startLine = Math.max(0, lineNumber - SNIPPET_CONTEXT_LINES - 1); + const endLine = Math.min(lines.length, lineNumber + SNIPPET_CONTEXT_LINES); + return lines + .slice(startLine, endLine) + .map((line, idx) => `${startLine + idx + 1}| ${line}`) + .join('\n'); + } catch { + return undefined; + } +}; + +/** + * Generates a mechanical template for the agent to complete. + */ +const getTemplate = (issueType: string, entry: StatEntry): string | undefined => { + switch (issueType) { + case 'missingReturns': + return `@returns {TYPE}`; + case 'missingComments': + return `/** Description for ${entry.label}. */`; + case 'paramDocMismatches': + return `@param {TYPE} paramName -`; + case 'missingComplexTypeInfo': + return `/** Description for ${entry.label}. */`; + default: + return undefined; + } +}; + +/** + * Enriches a stat entry with source snippet and template. + */ +const enrichEntry = (issueType: string, entry: StatEntry): EnrichedIssue => ({ + issueType, + id: entry.id, + label: entry.label, + file: entry.path, + line: entry.lineNumber, + column: entry.columnNumber, + link: entry.link, + type: entry.type, + sourceSnippet: getSourceSnippet(entry.path, entry.lineNumber), + template: getTemplate(issueType, entry), +}); + +/** + * Groups issues by file path. + */ +const groupByFile = (issues: EnrichedIssue[]): FileGroup[] => { + const groups = new Map(); + + for (const issue of issues) { + const existing = groups.get(issue.file) ?? []; + existing.push(issue); + groups.set(issue.file, existing); + } + + return Array.from(groups.entries()) + .map(([file, fileIssues]) => ({ file, issues: fileIssues })) + .sort((a, b) => a.file.localeCompare(b.file)); +}; + +interface PackageInfo { + pkg: ReturnType[number]; + /** The CLI flag to use: '--plugin' for plugins, '--package' for packages. */ + cliFlag: '--plugin' | '--package'; + /** The ID to pass to the CLI (plugin.id for plugins, manifest.id for packages). */ + cliId: string; +} + +type FindPackageResult = + | { + found: true; + info: PackageInfo; + } + | { + found: false; + suggestions: string[]; + }; + +/** + * Finds a package by ID and returns the appropriate CLI flag and ID. + * + * For plugins, returns the plugin.id to use with --plugin. + * For packages, returns the manifest.id to use with --package. + */ +const findPackage = (packageId: string): FindPackageResult => { + const packages = getPackages(REPO_ROOT); + const normalizedId = packageId.toLowerCase(); + + // First, try to find by package manifest ID (e.g., @kbn/dashboard-plugin). + const byManifestId = packages.find((pkg) => pkg.manifest.id === packageId); + if (byManifestId) { + if (byManifestId.isPlugin()) { + return { + found: true, + info: { + pkg: byManifestId, + cliFlag: '--plugin', + cliId: byManifestId.manifest.plugin.id, + }, + }; + } + return { + found: true, + info: { + pkg: byManifestId, + cliFlag: '--package', + cliId: byManifestId.manifest.id, + }, + }; + } + + // Second, try to find by plugin ID (e.g., dashboard). + const byPluginId = packages.find((pkg) => pkg.isPlugin() && pkg.manifest.plugin.id === packageId); + if (byPluginId && byPluginId.isPlugin()) { + return { + found: true, + info: { + pkg: byPluginId, + cliFlag: '--plugin', + cliId: byPluginId.manifest.plugin.id, + }, + }; + } + + // Not found - collect suggestions for helpful error message. + const suggestions: string[] = []; + + // Find packages/plugins that contain the search term. + for (const pkg of packages) { + const manifestId = pkg.manifest.id.toLowerCase(); + if (pkg.isPlugin()) { + const pluginId = pkg.manifest.plugin.id.toLowerCase(); + if (pluginId.includes(normalizedId) || manifestId.includes(normalizedId)) { + suggestions.push(`${pkg.manifest.plugin.id} (${pkg.manifest.id})`); + } + } else if (manifestId.includes(normalizedId)) { + suggestions.push(pkg.manifest.id); + } + } + + return { found: false, suggestions: suggestions.slice(0, 5) }; +}; + +/** + * Finds the package that contains a given file path. + */ +const findPackageForFile = (filePath: string): PackageInfo | undefined => { + const packages = getPackages(REPO_ROOT); + const absolutePath = Path.resolve(REPO_ROOT, filePath); + const pkg = packages.find((p) => absolutePath.startsWith(p.directory)); + + if (!pkg) return undefined; + + if (pkg.isPlugin()) { + return { + pkg, + cliFlag: '--plugin', + cliId: pkg.manifest.plugin.id, + }; + } + return { + pkg, + cliFlag: '--package', + cliId: pkg.manifest.id, + }; +}; + +/** + * Detects the target type based on the target string. + * Scoped package names (starting with @) are treated as IDs. + * Paths containing '/' are treated as files; otherwise treated as plugin/package IDs. + */ +const detectTargetType = (target: string): 'file' | 'id' => { + // Scoped package names like @kbn/dashboard-plugin are IDs, not files. + if (target.startsWith('@')) { + return 'id'; + } + // Paths containing '/' are files. + if (target.includes('/')) { + return 'file'; + } + return 'id'; +}; + +const fixPackageDocsInputSchema = z.object({ + target: z + .string() + .describe( + 'The plugin ID (e.g., "dashboard"), manifest ID (e.g., "@kbn/dashboard-plugin"), or file path to get issues for.' + ), + type: z + .enum(['plugin', 'package', 'file']) + .optional() + .describe( + 'How to interpret the target. If omitted, auto-detected: paths containing "/" are treated as files; otherwise tries plugin ID first, then manifest ID.' + ), + issueTypes: z + .array( + z.enum([ + 'missingComments', + 'missingReturns', + 'paramDocMismatches', + 'missingComplexTypeInfo', + 'isAnyType', + 'missingExports', + ]) + ) + .optional() + .describe( + 'Filter to specific issue types. Defaults to actionable types (missingComments, missingReturns, paramDocMismatches, missingComplexTypeInfo, isAnyType). Use "missingExports" explicitly to include export issues, which are informational and often require changes in consuming packages.' + ), +}); + +/** + * Runs the check_package_docs CLI to generate fresh stats. + */ +const generateStats = async ( + cliFlag: '--plugin' | '--package', + cliId: string +): Promise<{ success: boolean; error?: string }> => { + try { + await execa('node', ['scripts/check_package_docs', cliFlag, cliId, '--write'], { + cwd: REPO_ROOT, + timeout: 120000, + }); + return { success: true }; + } catch (err: unknown) { + // Exit code 1 means validation failed (issues found) - this is expected. + if ( + err && + typeof err === 'object' && + 'exitCode' in err && + (err as { exitCode: number }).exitCode === 1 + ) { + return { success: true }; + } + + // Extract meaningful error information from execa error. + if (err && typeof err === 'object') { + const execaErr = err as { stderr?: string; stdout?: string; message?: string }; + // Prefer stderr as it usually contains the actual error. + if (execaErr.stderr && execaErr.stderr.trim()) { + return { success: false, error: execaErr.stderr.trim() }; + } + if (execaErr.message) { + return { success: false, error: execaErr.message }; + } + } + + const error = err instanceof Error ? err.message : String(err); + return { success: false, error }; + } +}; + +const fixPackageDocs = async (input: z.infer) => { + const { target } = input; + + // Determine effective type: use explicit type or auto-detect. + const effectiveType = input.type ?? (detectTargetType(target) === 'file' ? 'file' : 'plugin'); + + let pkgInfo: PackageInfo | undefined; + let filePath: string | undefined; + + if (effectiveType === 'file') { + filePath = target; + pkgInfo = findPackageForFile(target); + if (!pkgInfo) { + return { + error: `Could not find a package containing file '${target}'.`, + }; + } + } else { + // For 'plugin' type: try plugin ID first, then manifest ID (current findPackage behavior). + // For 'package' type: try manifest ID only. + const result = findPackage(target); + if (!result.found) { + let errorMsg = `Plugin or package '${target}' not found.`; + if (result.suggestions.length > 0) { + errorMsg += ` Did you mean: ${result.suggestions.join(', ')}?`; + } + return { error: errorMsg }; + } + pkgInfo = result.info; + } + + const { pkg, cliFlag, cliId } = pkgInfo; + + // Generate fresh stats by running the CLI. + const genResult = await generateStats(cliFlag, cliId); + if (!genResult.success) { + return { + error: `Failed to generate stats: ${genResult.error}`, + }; + } + + const statsPath = Path.resolve(pkg.directory, 'target', 'api_docs', 'stats.json'); + + if (!fs.existsSync(statsPath)) { + return { + error: `Stats file not found at ${statsPath}. This may indicate an issue with the package.`, + }; + } + + let stats: FlatStats; + try { + stats = JSON.parse(fs.readFileSync(statsPath, 'utf-8')); + } catch { + return { + error: `Failed to parse stats file at ${statsPath}.`, + }; + } + + // Default actionable issue types - these are documentation issues fixable within this package. + // Excludes noReferences (informational) and missingExports (often requires changes in + // consuming packages rather than documentation fixes). + const defaultIssueTypes = [ + 'missingComments', + 'missingReturns', + 'paramDocMismatches', + 'missingComplexTypeInfo', + 'isAnyType', + ] as const; + + const issueTypesToInclude = input.issueTypes ?? defaultIssueTypes; + + // Collect all enriched issues + let allIssues: EnrichedIssue[] = []; + + if (issueTypesToInclude.includes('missingComments')) { + allIssues.push(...stats.missingComments.map((e) => enrichEntry('missingComments', e))); + } + if (issueTypesToInclude.includes('missingReturns')) { + allIssues.push(...stats.missingReturns.map((e) => enrichEntry('missingReturns', e))); + } + if (issueTypesToInclude.includes('paramDocMismatches')) { + allIssues.push(...stats.paramDocMismatches.map((e) => enrichEntry('paramDocMismatches', e))); + } + if (issueTypesToInclude.includes('missingComplexTypeInfo')) { + allIssues.push( + ...stats.missingComplexTypeInfo.map((e) => enrichEntry('missingComplexTypeInfo', e)) + ); + } + if (issueTypesToInclude.includes('isAnyType')) { + allIssues.push(...stats.isAnyType.map((e) => enrichEntry('isAnyType', e))); + } + + // Filter by file if specified + if (filePath) { + const normalizedPath = filePath.startsWith('/') ? filePath : Path.resolve(REPO_ROOT, filePath); + const relativePath = Path.relative(REPO_ROOT, normalizedPath); + allIssues = allIssues.filter((issue) => issue.file === relativePath || issue.file === filePath); + } + + // Group by file + const groupedIssues = groupByFile(allIssues); + + // Handle missing exports separately (different structure). + // missingExports are included in totalIssues when explicitly requested to maintain + // consistency with check_package_docs which includes pending issues in totals. + const includeMissingExports = input.issueTypes?.includes('missingExports') ?? false; + const missingExports = includeMissingExports ? stats.missingExports : []; + const totalIssues = allIssues.length + missingExports.length; + + return { + package: pkg.id, + directory: pkg.directory, + file: filePath, + totalIssues, + issuesByFile: groupedIssues, + missingExports, + }; +}; + +export const fixPackageDocsTool: ToolDefinition = { + name: 'fix_package_docs', + description: + 'Get detailed documentation issues for a Kibana plugin, package, or file. Returns issues grouped by file with source context and fix templates. Use this after check_package_docs identifies problems.', + inputSchema: fixPackageDocsInputSchema, + handler: async (input) => { + const result = await fixPackageDocs(input); + return { + content: [ + { + type: 'text', + text: JSON.stringify(result, null, 2), + }, + ], + }; + }, +};