Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/kbn-docs-utils/scripts/update_fixture_comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const categories = [
{ key: 'missingComplexTypeInfo', title: 'missing complex type info' },
{ key: 'isAnyType', title: 'any usage' },
{ key: 'noReferences', title: 'no references' },
{ key: 'unnamedExports', title: 'unnamed exports' },
];

/**
Expand Down Expand Up @@ -64,11 +65,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');
};
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-docs-utils/src/__test_helpers__/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export const createMockPluginStats = (overrides: Partial<ApiStats> = {}): ApiSta
adoptionTrackedAPIs: [],
adoptionTrackedAPIsCount: 0,
adoptionTrackedAPIsUnreferencedCount: 0,
unnamedExports: [],
...overrides,
});

Expand Down Expand Up @@ -108,5 +109,6 @@ export const createMockPluginMetaInfo = (
owner: { name: 'Test Team', githubTeam: 'test-team' },
description: 'A test plugin',
isPlugin: true,
unnamedExports: [],
...overrides,
});
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const createBaseStats = (pluginId: string): AllPluginStats => ({
eslintDisableFileCount: 0,
eslintDisableLineCount: 0,
enzymeImportCount: 0,
unnamedExports: [],
},
});

Expand Down
8 changes: 5 additions & 3 deletions packages/kbn-docs-utils/src/check_package_docs_cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import {
import type { PluginOrPackage, MissingApiItemMap } from './types';
import type { AllPluginStats } from './cli/types';

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, '../../..');

Expand Down Expand Up @@ -62,6 +62,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;
Expand Down Expand Up @@ -89,10 +90,11 @@ export const getValidationResults = (
pluginStats.paramDocMismatches.length > 0 ||
pluginStats.missingComplexTypeInfo.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),
};
});
};
Expand Down
10 changes: 5 additions & 5 deletions packages/kbn-docs-utils/src/cli/parse_cli_flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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`'
);
});

Expand All @@ -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', () => {
Expand Down
21 changes: 14 additions & 7 deletions packages/kbn-docs-utils/src/cli/parse_cli_flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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`'
);
}

Expand All @@ -58,21 +58,26 @@ 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`'
);
}

return expanded;
};

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`'
);
}
};

Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function buildApiMap(
unreferencedDeprecations,
referencedDeprecations,
adoptionTrackedAPIs,
unnamedExports,
} = getPluginApiMap(project, plugins, log, {
collectReferences: options.collectReferences,
pluginFilter: options.pluginFilter,
Expand All @@ -58,5 +59,6 @@ export function buildApiMap(
referencedDeprecations,
unreferencedDeprecations,
adoptionTrackedAPIs,
unnamedExports,
};
}
2 changes: 2 additions & 0 deletions packages/kbn-docs-utils/src/cli/tasks/collect_stats.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('collectStats', () => {
referencedDeprecations: {},
unreferencedDeprecations: {},
adoptionTrackedAPIs: {},
unnamedExports: {},
};

(collectApiStatsForPlugin as jest.Mock).mockReturnValue({
Expand All @@ -85,6 +86,7 @@ describe('collectStats', () => {
adoptionTrackedAPIs: [],
adoptionTrackedAPIsCount: 0,
adoptionTrackedAPIsUnreferencedCount: 0,
unnamedExports: [],
});

(countEslintDisableLines as jest.Mock).mockResolvedValue({
Expand Down
10 changes: 8 additions & 2 deletions packages/kbn-docs-utils/src/cli/tasks/collect_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ export async function collectStats(
options: CliOptions
): Promise<AllPluginStats> {
const { plugins, pathsByPlugin } = setupResult;
const { pluginApiMap, missingApiItems, referencedDeprecations, adoptionTrackedAPIs } =
apiMapResult;
const {
pluginApiMap,
missingApiItems,
referencedDeprecations,
adoptionTrackedAPIs,
unnamedExports,
} = apiMapResult;

const allPluginStats: AllPluginStats = {};

Expand All @@ -65,6 +70,7 @@ export async function collectStats(
missingApiItems,
referencedDeprecations,
adoptionTrackedAPIs,
unnamedExports,
}),
owner: plugin.manifest.owner,
description: plugin.manifest.description,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe('reportMetrics', () => {
referencedDeprecations: {},
unreferencedDeprecations: {},
adoptionTrackedAPIs: {},
unnamedExports: {},
};

allPluginStats = {
Expand All @@ -94,6 +95,7 @@ describe('reportMetrics', () => {
eslintDisableLineCount: 0,
eslintDisableFileCount: 0,
enzymeImportCount: 0,
unnamedExports: [],
},
};
});
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-docs-utils/src/cli/tasks/write_docs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ describe('writeDocs', () => {
referencedDeprecations: {},
unreferencedDeprecations: {},
adoptionTrackedAPIs: {},
unnamedExports: {},
};

allPluginStats = {
Expand All @@ -106,6 +107,7 @@ describe('writeDocs', () => {
eslintDisableLineCount: 0,
eslintDisableFileCount: 0,
enzymeImportCount: 0,
unnamedExports: [],
},
};

Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-docs-utils/src/cli/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
ReferencedDeprecationsByPlugin,
UnreferencedDeprecationsByPlugin,
AdoptionTrackedAPIsByPlugin,
UnnamedExportsByPlugin,
ApiStats,
PluginMetaInfo,
} from '../types';
Expand Down Expand Up @@ -95,6 +96,8 @@ export interface BuildApiMapResult {
unreferencedDeprecations: UnreferencedDeprecationsByPlugin;
/** Adoption-tracked APIs. */
adoptionTrackedAPIs: AdoptionTrackedAPIsByPlugin;
/** Unnamed exports found during API collection. */
unnamedExports: UnnamedExportsByPlugin;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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: [] });
});
});
Loading
Loading