feat(plugin-doc-coverage): add doc-coverage-plugin to analyze documentation in ts/js projects #896
Conversation
Code PushUp😟 Code PushUp report has regressed – compared target commit 4777a83 with source commit 4777a83. 🏷️ Categories
👎 1 group regressed, 👎 5 audits regressed, 12 audits changed without impacting score🗃️ Groups
16 other groups are unchanged. 🛡️ Audits
571 other audits are unchanged. |
…anced and some code improved
…ls. Solid version working
…te unit test for runner file
|
@aramirezj thank you for putting this plugin together, we did discuss it on refinement today and we are exited to adopt it as one of the plugins that should live in this monorepo (there are plugins that we purposely do not maintain here as they do not meet the JS/TS tech stack). We are also looking forward to setup automated docs generation, so it makes even more sense to start tracking it with plugin. There are some things we need to figure out before we merge, mainly related to testing. We are thinking if there should already be a simple e2e tests similar to for the ESLint e2e exaple, is it something you'd also like to contribute? |
I am glad to hear that! And sure! I will work into that ^^ |
| nodesCount: 1, | ||
| issues: [ | ||
| { | ||
| file: expect.stringContaining('classes-coverage'), |
There was a problem hiding this comment.
| file: expect.stringContaining('classes-coverage'), | |
| file: expect.stringContaining('classes-coverage.ts'), |
To make sure it is at the end of the file, and not a different file with same path segment.
This is applicable to multiple places.
There was a problem hiding this comment.
I'd even say it could be stringEndingWith('classes-coverage.ts')
There was a problem hiding this comment.
I have added expect.stringMatching(/properties-coverage\.ts$/) that it should do the trick
There was a problem hiding this comment.
@aramirezj with the latest merge we have OS agnostic path helpers in the codebase:
https://github.com/code-pushup/cli/blob/main/testing/test-setup/src/lib/extend/path.matcher.unit.test.ts
Let's use them.
There was a problem hiding this comment.
Done!, If any of you want to resolve the conversation and approve it ^^ @BioPhoton @vmasek
| @@ -0,0 +1,42 @@ | |||
| { | |||
| "name": "@code-pushup/doc-coverage-plugin", | |||
There was a problem hiding this comment.
I think the plugin name is little bit verbose and too similar to existing coverage-plugin. Naming it @code-pushup/jsdocs-plugin would be better, IMHO. What do you think?
(BTW @nx/workspace:move should help with the renamings.)
| slug: 'doc-coverage-cat', | ||
| title: 'Documentation coverage', |
There was a problem hiding this comment.
We don't use a -cat suffix for any other category slugs, it's implicit from context.
Also, it's best for category names to be as short as possible, since they are very high-level. How about just Documentation/docs?
| slug: 'doc-coverage-cat', | |
| title: 'Documentation coverage', | |
| slug: 'docs', | |
| title: 'Documentation', |
| await docCoverageCoreConfig({ | ||
| sourceGlob: [ | ||
| 'packages/**/src/**/*.ts', | ||
| '!packages/**/node_modules', | ||
| '!packages/**/{mocks,mock}', | ||
| '!**/*.{spec,test}.ts', | ||
| '!**/implementation/**', | ||
| '!**/internal/**', | ||
| ], | ||
| }), |
There was a problem hiding this comment.
The ESLint plugin has a similar parameter called patterns, would be nice to have consistent naming.
| await docCoverageCoreConfig({ | |
| sourceGlob: [ | |
| 'packages/**/src/**/*.ts', | |
| '!packages/**/node_modules', | |
| '!packages/**/{mocks,mock}', | |
| '!**/*.{spec,test}.ts', | |
| '!**/implementation/**', | |
| '!**/internal/**', | |
| ], | |
| }), | |
| await docCoverageCoreConfig({ | |
| patterns: [ | |
| 'packages/**/src/**/*.ts', | |
| '!packages/**/node_modules', | |
| '!packages/**/{mocks,mock}', | |
| '!**/*.{spec,test}.ts', | |
| '!**/implementation/**', | |
| '!**/internal/**', | |
| ], | |
| }), |
Since I'm guessing only these globs are needed for most usages, the plugin parameter could also enable a shorthand (again, similar to ESLint plugin):
| await docCoverageCoreConfig({ | |
| sourceGlob: [ | |
| 'packages/**/src/**/*.ts', | |
| '!packages/**/node_modules', | |
| '!packages/**/{mocks,mock}', | |
| '!**/*.{spec,test}.ts', | |
| '!**/implementation/**', | |
| '!**/internal/**', | |
| ], | |
| }), | |
| await docCoverageCoreConfig([ | |
| 'packages/**/src/**/*.ts', | |
| '!packages/**/node_modules', | |
| '!packages/**/{mocks,mock}', | |
| '!**/*.{spec,test}.ts', | |
| '!**/implementation/**', | |
| '!**/internal/**', | |
| ), |
|
|
||
| - `value`: The value is the number of undocumented nodes -> 4 | ||
| - `displayValue`: `${value} undocumented ${type}` -> 4 undocumented functions | ||
| - `score`: 0.5 -> total nodes 8 undocumented 4 -> 8/4 |
There was a problem hiding this comment.
| - `score`: 0.5 -> total nodes 8 undocumented 4 -> 8/4 | |
| - `score`: 0.5 -> total nodes 8, undocumented 4 -> 4/8 |
| export const docCoverageCoreConfig = async ( | ||
| config: DocCoveragePluginConfig, | ||
| ): Promise<CoreConfig> => { | ||
| return { | ||
| plugins: [docCoveragePlugin(config)], | ||
| categories: getDocCoverageCategories(config), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Doesn't need to be async:
| export const docCoverageCoreConfig = async ( | |
| config: DocCoveragePluginConfig, | |
| ): Promise<CoreConfig> => { | |
| return { | |
| plugins: [docCoveragePlugin(config)], | |
| categories: getDocCoverageCategories(config), | |
| }; | |
| }; | |
| export const docCoverageCoreConfig = ( | |
| config: DocCoveragePluginConfig, | |
| ): CoreConfig => { | |
| return { | |
| plugins: [docCoveragePlugin(config)], | |
| categories: getDocCoverageCategories(config), | |
| }; | |
| }; |
| } satisfies DocumentationData & { | ||
| coverage: number; | ||
| }); |
There was a problem hiding this comment.
The DocumentationData & { coverage: number } type is used a lot. Maybe worth creating a type alias for it?
| ): AuditOutputs { | ||
| return Object.entries(coverageResult) | ||
| .filter(([type]) => { | ||
| const auditSlug = `${type}-coverage`; |
There was a problem hiding this comment.
Although the transformation is simple, I think it would be worth extracting to some coverageTypeToAuditSlug function. Then the slug pattern can be changed in one place if needed.
| }, | ||
| ], | ||
| }, | ||
| } as unknown as DocumentationCoverageReport; |
There was a problem hiding this comment.
Not a fan of as unknown as T, since it loses all type-safety and autocomplete. In this case it's also unnecessary:
| } as unknown as DocumentationCoverageReport; | |
| } as DocumentationCoverageReport; |
| export function calculateCoverage(result: DocumentationReport) { | ||
| return Object.fromEntries( | ||
| Object.entries(result).map(([key, value]) => { | ||
| const type = key as CoverageType; | ||
| return [ | ||
| type, | ||
| { | ||
| coverage: | ||
| value.nodesCount === 0 | ||
| ? 100 | ||
| : Number( | ||
| ((1 - value.issues.length / value.nodesCount) * 100).toFixed( | ||
| 2, | ||
| ), | ||
| ), | ||
| issues: value.issues, | ||
| nodesCount: value.nodesCount, | ||
| }, | ||
| ]; | ||
| }), | ||
| ) as DocumentationCoverageReport; | ||
| } |
There was a problem hiding this comment.
The "better Object.entries/Object.fromEntries" can help here also:
| export function calculateCoverage(result: DocumentationReport) { | |
| return Object.fromEntries( | |
| Object.entries(result).map(([key, value]) => { | |
| const type = key as CoverageType; | |
| return [ | |
| type, | |
| { | |
| coverage: | |
| value.nodesCount === 0 | |
| ? 100 | |
| : Number( | |
| ((1 - value.issues.length / value.nodesCount) * 100).toFixed( | |
| 2, | |
| ), | |
| ), | |
| issues: value.issues, | |
| nodesCount: value.nodesCount, | |
| }, | |
| ]; | |
| }), | |
| ) as DocumentationCoverageReport; | |
| } | |
| export function calculateCoverage( | |
| result: DocumentationReport, | |
| ): DocumentationReport { | |
| return objectFromEntries( | |
| objectToEntries(result).map(([key, value]) => [ | |
| key, | |
| { | |
| coverage: | |
| value.nodesCount === 0 | |
| ? 100 | |
| : Number( | |
| ((1 - value.issues.length / value.nodesCount) * 100).toFixed(2), | |
| ), | |
| issues: value.issues, | |
| nodesCount: value.nodesCount, | |
| }, | |
| ]), | |
| ); | |
| } |
| export function getCoverageTypeFromKind(kind: SyntaxKind): CoverageType { | ||
| switch (kind) { | ||
| case SyntaxKind.ClassDeclaration: | ||
| return 'classes'; | ||
| case SyntaxKind.MethodDeclaration: | ||
| return 'methods'; | ||
| case SyntaxKind.FunctionDeclaration: | ||
| return 'functions'; | ||
| case SyntaxKind.InterfaceDeclaration: | ||
| return 'interfaces'; | ||
| case SyntaxKind.EnumDeclaration: | ||
| return 'enums'; | ||
| case SyntaxKind.VariableStatement: | ||
| case SyntaxKind.VariableDeclaration: | ||
| return 'variables'; | ||
| case SyntaxKind.PropertyDeclaration: | ||
| return 'properties'; | ||
| case SyntaxKind.TypeAliasDeclaration: | ||
| return 'types'; | ||
| default: | ||
| throw new Error(`Unsupported syntax kind: ${kind}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
This mapping duplicates the SyntaxKindToStringLiteral type. It would be better to have a single source of truth for this mapping.
// shared constant acts as source of truth
export const SYNTAX_COVERAGE_MAP = {
[SyntaxKind.ClassDeclaration]: 'classes',
[SyntaxKind.MethodDeclaration]: 'methods',
[SyntaxKind.FunctionDeclaration]: 'functions',
[SyntaxKind.InterfaceDeclaration]: 'interfaces',
[SyntaxKind.EnumDeclaration]: 'enums',
[SyntaxKind.VariableDeclaration]: 'variables',
[SyntaxKind.PropertyDeclaration]: 'properties',
[SyntaxKind.TypeAliasDeclaration]: 'types',
} as const;
// `typeof` gets you the mapped type
export type CoverageType =
(typeof SYNTAX_COVERAGE_MAP)[keyof typeof SYNTAX_COVERAGE_MAP];
// can be reused for runtime conversion also
export function getCoverageTypeFromKind(kind: SyntaxKind): CoverageType {
const map = new Map<SyntaxKind, CoverageType>(
objectToEntries(SYNTAX_COVERAGE_MAP),
);
const type = map.get(kind);
if (!type) {
throw new Error(`Unsupported syntax kind: ${kind}`);
}
return type;
}
This PR includes:
Closes #908
Report Summary
Overview of Audit Groups, Audit Counts, and Diagnostic Issue Types.
classes-coverage,methods-coverage,functions-coverage,interfaces-coverage,variables-coverage,properties-coverage,types-coverage,enums-coverage