From 8402bef94711f71273de38925961e4a5f930a60a Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 29 Oct 2023 22:25:28 +0100 Subject: [PATCH 1/5] refactor(models): refine types for better DX and exploration --- .../src/lib/implementation/json-to-gql.ts | 6 ++-- packages/models/src/index.ts | 8 ++++- packages/models/src/lib/category-config.ts | 33 +++++++++++-------- .../models/src/lib/implementation/schemas.ts | 24 ++++++++------ packages/models/src/lib/plugin-config.ts | 11 ++++--- .../test/fixtures/eslint-plugin.mock.ts | 4 +-- .../plugin-eslint/src/lib/runner/transform.ts | 3 +- packages/utils/src/lib/report.spec.ts | 15 ++++++--- packages/utils/src/lib/report.ts | 17 ++++++---- packages/utils/src/lib/scoring.ts | 8 +++-- 10 files changed, 81 insertions(+), 48 deletions(-) diff --git a/packages/core/src/lib/implementation/json-to-gql.ts b/packages/core/src/lib/implementation/json-to-gql.ts index 540213501..1c17c85b9 100644 --- a/packages/core/src/lib/implementation/json-to-gql.ts +++ b/packages/core/src/lib/implementation/json-to-gql.ts @@ -4,7 +4,7 @@ import { IssueSourceType, SaveReportMutationVariables, } from '@code-pushup/portal-client'; -import { Issue, Report } from '@code-pushup/models'; +import { IssueSeverity as CliIssueSeverity, Report } from '@code-pushup/models'; export function jsonToGql(report: Report) { return { @@ -71,7 +71,7 @@ export function jsonToGql(report: Report) { >; } -function transformSeverity(severity: Issue['severity']): IssueSeverity { +function transformSeverity(severity: CliIssueSeverity): IssueSeverity { switch (severity) { case 'info': return IssueSeverity.Info; @@ -79,5 +79,7 @@ function transformSeverity(severity: Issue['severity']): IssueSeverity { return IssueSeverity.Error; case 'warning': return IssueSeverity.Warning; + default: + throw new Error(`Severity ${severity} unknown`); } } diff --git a/packages/models/src/index.ts b/packages/models/src/index.ts index 51b094a14..86d125d36 100644 --- a/packages/models/src/index.ts +++ b/packages/models/src/index.ts @@ -1,4 +1,8 @@ -export { CategoryConfig, categoryConfigSchema } from './lib/category-config'; +export { + CategoryRefSchema, + CategoryConfig, + categoryConfigSchema, +} from './lib/category-config'; export { CoreConfig, coreConfigSchema, @@ -14,10 +18,12 @@ export { } from './lib/persist-config'; export { Audit, + AuditGroupRef, AuditGroup, AuditOutput, AuditOutputs, Issue, + IssueSeverity, PluginConfig, auditGroupSchema, auditOutputsSchema, diff --git a/packages/models/src/lib/category-config.ts b/packages/models/src/lib/category-config.ts index 6ede923e9..88a209d26 100644 --- a/packages/models/src/lib/category-config.ts +++ b/packages/models/src/lib/category-config.ts @@ -13,22 +13,26 @@ type _RefsList = { plugin?: string; }[]; +const categoryRefSchema = weightedRefSchema( + 'Weighted references to audits and/or groups for the category', + 'Slug of an audit or group (depending on `type`)', +).merge( + z.object({ + type: z.enum(['audit', 'group'], { + description: + 'Discriminant for reference kind, affects where `slug` is looked up', + }), + plugin: slugSchema( + 'Plugin slug (plugin should contain referenced audit or group)', + ), + }), +); + +export type CategoryRefSchema = z.infer; + export const categoryConfigSchema = scorableSchema( 'Category with a score calculated from audits and groups from various plugins', - weightedRefSchema( - 'Weighted references to audits and/or groups for the category', - 'Slug of an audit or group (depending on `type`)', - ).merge( - z.object({ - type: z.enum(['audit', 'group'], { - description: - 'Discriminant for reference kind, affects where `slug` is looked up', - }), - plugin: slugSchema( - 'Plugin slug (plugin should contain referenced audit or group)', - ), - }), - ), + categoryRefSchema, getDuplicateRefsInCategoryMetrics, duplicateRefsInCategoryMetricsErrorMsg, ) @@ -60,6 +64,7 @@ export function duplicateRefsInCategoryMetricsErrorMsg(metrics: _RefsList) { duplicateRefs, )}`; } + function getDuplicateRefsInCategoryMetrics(metrics: _RefsList) { return hasDuplicateStrings( metrics.map(({ slug, type, plugin }) => `${type} :: ${plugin} / ${slug}`), diff --git a/packages/models/src/lib/implementation/schemas.ts b/packages/models/src/lib/implementation/schemas.ts index 92fec9b02..18163ed27 100644 --- a/packages/models/src/lib/implementation/schemas.ts +++ b/packages/models/src/lib/implementation/schemas.ts @@ -69,6 +69,10 @@ export function titleSchema(description = 'Descriptive name') { return z.string({ description }).max(256); } +/** + * Used for categories, plugins and audits + * @param options + */ export function metaSchema(options?: { titleDescription?: string; descriptionDescription?: string; @@ -102,16 +106,6 @@ export function filePathSchema(description: string) { .min(1, { message: 'path is invalid' }); } -/** - * Schema for a weight - * @param description - */ -export function weightSchema( - description = 'Coefficient for the given score (use weight 0 if only for display)', -) { - return positiveIntSchema(description); -} - /** * Schema for a positiveInt * @param description @@ -138,6 +132,16 @@ export function packageVersionSchema(options?: { ); } +/** + * Schema for a weight + * @param description + */ +export function weightSchema( + description = 'Coefficient for the given score (use weight 0 if only for display)', +) { + return positiveIntSchema(description); +} + export function weightedRefSchema( description: string, slugDescription: string, diff --git a/packages/models/src/lib/plugin-config.ts b/packages/models/src/lib/plugin-config.ts index e537a7c5e..eeecd8b0b 100644 --- a/packages/models/src/lib/plugin-config.ts +++ b/packages/models/src/lib/plugin-config.ts @@ -70,13 +70,15 @@ export const auditSchema = z export type Audit = z.infer; +const auditGroupRef = weightedRefSchema( + 'Weighted references to audits', + "Reference slug to an audit within this plugin (e.g. 'max-lines')", +); +export type AuditGroupRef = z.infer; export const auditGroupSchema = scorableSchema( 'An audit group aggregates a set of audits into a single score which can be referenced from a category. ' + 'E.g. the group slug "performance" groups audits and can be referenced in a category as "[plugin-slug]#group:[group-slug]")', - weightedRefSchema( - 'Weighted references to audits', - "Reference slug to an audit within this plugin (e.g. 'max-lines')", - ), + auditGroupRef, getDuplicateRefsInGroups, duplicateRefsInGroupsErrorMsg, ).merge( @@ -183,6 +185,7 @@ const sourceFileLocationSchema = z.object( { description: 'Source file location' }, ); +export type IssueSeverity = 'info' | 'warning' | 'error'; export const issueSchema = z.object( { message: z.string({ description: 'Descriptive error message' }).max(128), diff --git a/packages/models/test/fixtures/eslint-plugin.mock.ts b/packages/models/test/fixtures/eslint-plugin.mock.ts index 649f2e2af..c38826b7a 100644 --- a/packages/models/test/fixtures/eslint-plugin.mock.ts +++ b/packages/models/test/fixtures/eslint-plugin.mock.ts @@ -1,7 +1,7 @@ import { join } from 'node:path'; import type { Audit, - CategoryConfig, + CategoryRefSchema, PluginConfig, PluginReport, } from '../../src'; @@ -51,7 +51,7 @@ type ESLintAuditSlug = keyof typeof ESLINT_AUDITS_MAP; export function eslintAuditRef( slug: ESLintAuditSlug, weight = 1, -): CategoryConfig['refs'][number] { +): CategoryRefSchema { return { type: 'audit', plugin: 'eslint', diff --git a/packages/plugin-eslint/src/lib/runner/transform.ts b/packages/plugin-eslint/src/lib/runner/transform.ts index d930e19d7..0e212c4f1 100644 --- a/packages/plugin-eslint/src/lib/runner/transform.ts +++ b/packages/plugin-eslint/src/lib/runner/transform.ts @@ -1,5 +1,6 @@ import type { Linter } from 'eslint'; import type { AuditOutput, Issue } from '@code-pushup/models'; +import { IssueSeverity } from '@code-pushup/models'; import { compareIssueSeverity, countOccurrences, @@ -71,7 +72,7 @@ function convertIssue(issue: LintIssue): Issue { }; } -function convertSeverity(severity: Linter.Severity): Issue['severity'] { +function convertSeverity(severity: Linter.Severity): IssueSeverity { switch (severity) { case 2: return 'error'; diff --git a/packages/utils/src/lib/report.spec.ts b/packages/utils/src/lib/report.spec.ts index 5539bcb57..9a7339d72 100644 --- a/packages/utils/src/lib/report.spec.ts +++ b/packages/utils/src/lib/report.spec.ts @@ -1,5 +1,10 @@ import { describe, expect } from 'vitest'; -import { CategoryConfig, Issue } from '@code-pushup/models'; +import { + CategoryConfig, + CategoryRefSchema, + Issue, + IssueSeverity, +} from '@code-pushup/models'; import { calcDuration, compareIssueSeverity, @@ -98,7 +103,7 @@ describe('formatCount', () => { describe('countWeightedRefs', () => { it('should calc weighted refs only', () => { - const refs: CategoryConfig['refs'] = [ + const refs: CategoryRefSchema[] = [ { slug: 'a1', weight: 0, @@ -119,16 +124,16 @@ describe('countWeightedRefs', () => { describe('compareIssueSeverity', () => { it('should order severities in logically ascending order when used as compareFn with .sort()', () => { expect( - (['error', 'info', 'warning'] satisfies Issue['severity'][]).sort( + (['error', 'info', 'warning'] satisfies IssueSeverity[]).sort( compareIssueSeverity, ), - ).toEqual(['info', 'warning', 'error'] satisfies Issue['severity'][]); + ).toEqual(['info', 'warning', 'error'] satisfies IssueSeverity[]); }); }); describe('sumRefs', () => { it('should sum refs correctly', () => { - const refs: CategoryConfig['refs'] = [ + const refs: CategoryRefSchema[] = [ { slug: 'a1', weight: 0, diff --git a/packages/utils/src/lib/report.ts b/packages/utils/src/lib/report.ts index 9832727fe..9dc7156a0 100644 --- a/packages/utils/src/lib/report.ts +++ b/packages/utils/src/lib/report.ts @@ -1,4 +1,9 @@ -import { CategoryConfig, Issue } from '@code-pushup/models'; +import { + CategoryConfig, + CategoryRefSchema, + Issue, + IssueSeverity, +} from '@code-pushup/models'; import { pluralize } from './utils'; export const FOOTER_PREFIX = 'Made with ❤️ by'; @@ -34,17 +39,17 @@ export function formatCount(count: number, name: string) { return `${count} ${text}`; } -export function countWeightedRefs(refs: CategoryConfig['refs']) { +export function countWeightedRefs(refs: CategoryRefSchema[]) { return refs .filter(({ weight }) => weight > 0) .reduce((sum, { weight }) => sum + weight, 0); } export function compareIssueSeverity( - severity1: Issue['severity'], - severity2: Issue['severity'], + severity1: IssueSeverity, + severity2: IssueSeverity, ): number { - const levels: Record = { + const levels: Record = { info: 0, warning: 1, error: 2, @@ -53,6 +58,6 @@ export function compareIssueSeverity( } // @TODO replace with real scoring logic -export function sumRefs(refs: CategoryConfig['refs']) { +export function sumRefs(refs: CategoryRefSchema[]) { return refs.reduce((sum, { weight }) => sum + weight, 0); } diff --git a/packages/utils/src/lib/scoring.ts b/packages/utils/src/lib/scoring.ts index 4db112a38..68f3dd8bf 100644 --- a/packages/utils/src/lib/scoring.ts +++ b/packages/utils/src/lib/scoring.ts @@ -1,7 +1,9 @@ import { AuditGroup, + AuditGroupRef, AuditReport, CategoryConfig, + CategoryRefSchema, PluginReport, Report, } from '@code-pushup/models'; @@ -20,7 +22,7 @@ export type ScoredReport = Omit & { function groupRefToScore( audits: AuditReport[], -): (ref: AuditGroup['refs'][0]) => number { +): (ref: AuditGroupRef) => number { return ref => { const score = audits.find(audit => audit.slug === ref.slug)?.score; if (score == null) { @@ -35,8 +37,8 @@ function groupRefToScore( function categoryRefToScore( audits: EnrichedAuditReport[], groups: EnrichedScoredAuditGroup[], -): (ref: CategoryConfig['refs'][0]) => number { - return (ref: CategoryConfig['refs'][0]): number => { +): (ref: CategoryRefSchema) => number { + return (ref: CategoryRefSchema): number => { switch (ref.type) { case 'audit': // eslint-disable-next-line no-case-declarations From 8d3b7332da44db0ebb53094ac7e8ba6dd0cf9a07 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 29 Oct 2023 22:31:35 +0100 Subject: [PATCH 2/5] refactor(utils): fix format --- packages/utils/src/lib/report.spec.ts | 7 +------ packages/utils/src/lib/report.ts | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/utils/src/lib/report.spec.ts b/packages/utils/src/lib/report.spec.ts index 9a7339d72..e68cd4b17 100644 --- a/packages/utils/src/lib/report.spec.ts +++ b/packages/utils/src/lib/report.spec.ts @@ -1,10 +1,5 @@ import { describe, expect } from 'vitest'; -import { - CategoryConfig, - CategoryRefSchema, - Issue, - IssueSeverity, -} from '@code-pushup/models'; +import { CategoryRefSchema, IssueSeverity } from '@code-pushup/models'; import { calcDuration, compareIssueSeverity, diff --git a/packages/utils/src/lib/report.ts b/packages/utils/src/lib/report.ts index 9dc7156a0..d0932fac4 100644 --- a/packages/utils/src/lib/report.ts +++ b/packages/utils/src/lib/report.ts @@ -1,9 +1,4 @@ -import { - CategoryConfig, - CategoryRefSchema, - Issue, - IssueSeverity, -} from '@code-pushup/models'; +import { CategoryRefSchema, IssueSeverity } from '@code-pushup/models'; import { pluralize } from './utils'; export const FOOTER_PREFIX = 'Made with ❤️ by'; From c06f3204cfd6c2bb6c9cce5d2bb92a143180f59f Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 30 Oct 2023 14:58:31 +0100 Subject: [PATCH 3/5] refactor(core): adopt typing names to be more clear --- packages/core/src/lib/implementation/json-to-gql.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/lib/implementation/json-to-gql.ts b/packages/core/src/lib/implementation/json-to-gql.ts index 1c17c85b9..b210bc93a 100644 --- a/packages/core/src/lib/implementation/json-to-gql.ts +++ b/packages/core/src/lib/implementation/json-to-gql.ts @@ -1,7 +1,7 @@ import { CategoryConfigRefType, - IssueSeverity, IssueSourceType, + IssueSeverity as PortalIssueSeverity, SaveReportMutationVariables, } from '@code-pushup/portal-client'; import { IssueSeverity as CliIssueSeverity, Report } from '@code-pushup/models'; @@ -71,14 +71,14 @@ export function jsonToGql(report: Report) { >; } -function transformSeverity(severity: CliIssueSeverity): IssueSeverity { +function transformSeverity(severity: CliIssueSeverity): PortalIssueSeverity { switch (severity) { case 'info': - return IssueSeverity.Info; + return PortalIssueSeverity.Info; case 'error': - return IssueSeverity.Error; + return PortalIssueSeverity.Error; case 'warning': - return IssueSeverity.Warning; + return PortalIssueSeverity.Warning; default: throw new Error(`Severity ${severity} unknown`); } From 363cd583eb99c5eb30e11baf019c0b9d0d45b66f Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 30 Oct 2023 15:08:02 +0100 Subject: [PATCH 4/5] refactor(core): adopt typing of severity --- packages/models/src/lib/plugin-config.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/models/src/lib/plugin-config.ts b/packages/models/src/lib/plugin-config.ts index eeecd8b0b..7030f8de0 100644 --- a/packages/models/src/lib/plugin-config.ts +++ b/packages/models/src/lib/plugin-config.ts @@ -185,13 +185,14 @@ const sourceFileLocationSchema = z.object( { description: 'Source file location' }, ); -export type IssueSeverity = 'info' | 'warning' | 'error'; +export const issueSeveritySchema = z.enum(['info', 'warning', 'error'], { + description: 'Severity level', +}); +export type IssueSeverity = z.infer; export const issueSchema = z.object( { message: z.string({ description: 'Descriptive error message' }).max(128), - severity: z.enum(['info', 'warning', 'error'], { - description: 'Severity level', - }), + severity: issueSeveritySchema, source: sourceFileLocationSchema.optional(), }, { description: 'Issue information' }, From ace977b0cda132f24a6c2113c97eda7cb9946903 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 30 Oct 2023 15:10:56 +0100 Subject: [PATCH 5/5] refactor(core): rename types --- packages/core/src/lib/implementation/json-to-gql.ts | 2 -- packages/models/src/index.ts | 2 +- packages/models/src/lib/category-config.ts | 2 +- packages/models/test/fixtures/eslint-plugin.mock.ts | 12 ++---------- packages/utils/src/lib/report.spec.ts | 6 +++--- packages/utils/src/lib/report.ts | 6 +++--- packages/utils/src/lib/scoring.ts | 6 +++--- 7 files changed, 13 insertions(+), 23 deletions(-) diff --git a/packages/core/src/lib/implementation/json-to-gql.ts b/packages/core/src/lib/implementation/json-to-gql.ts index b210bc93a..637d8b986 100644 --- a/packages/core/src/lib/implementation/json-to-gql.ts +++ b/packages/core/src/lib/implementation/json-to-gql.ts @@ -79,7 +79,5 @@ function transformSeverity(severity: CliIssueSeverity): PortalIssueSeverity { return PortalIssueSeverity.Error; case 'warning': return PortalIssueSeverity.Warning; - default: - throw new Error(`Severity ${severity} unknown`); } } diff --git a/packages/models/src/index.ts b/packages/models/src/index.ts index 86d125d36..7477a4021 100644 --- a/packages/models/src/index.ts +++ b/packages/models/src/index.ts @@ -1,5 +1,5 @@ export { - CategoryRefSchema, + CategoryRef, CategoryConfig, categoryConfigSchema, } from './lib/category-config'; diff --git a/packages/models/src/lib/category-config.ts b/packages/models/src/lib/category-config.ts index 88a209d26..cf6d6248a 100644 --- a/packages/models/src/lib/category-config.ts +++ b/packages/models/src/lib/category-config.ts @@ -28,7 +28,7 @@ const categoryRefSchema = weightedRefSchema( }), ); -export type CategoryRefSchema = z.infer; +export type CategoryRef = z.infer; export const categoryConfigSchema = scorableSchema( 'Category with a score calculated from audits and groups from various plugins', diff --git a/packages/models/test/fixtures/eslint-plugin.mock.ts b/packages/models/test/fixtures/eslint-plugin.mock.ts index c38826b7a..ba5b3cedf 100644 --- a/packages/models/test/fixtures/eslint-plugin.mock.ts +++ b/packages/models/test/fixtures/eslint-plugin.mock.ts @@ -1,10 +1,5 @@ import { join } from 'node:path'; -import type { - Audit, - CategoryRefSchema, - PluginConfig, - PluginReport, -} from '../../src'; +import type { Audit, CategoryRef, PluginConfig, PluginReport } from '../../src'; import { echoRunnerConfig } from './echo-runner-config.mock'; import { ESLINT_AUDITS_MAP } from './eslint-audits.mock'; @@ -48,10 +43,7 @@ export function eslintPluginReport(): PluginReport { type ESLintAuditSlug = keyof typeof ESLINT_AUDITS_MAP; -export function eslintAuditRef( - slug: ESLintAuditSlug, - weight = 1, -): CategoryRefSchema { +export function eslintAuditRef(slug: ESLintAuditSlug, weight = 1): CategoryRef { return { type: 'audit', plugin: 'eslint', diff --git a/packages/utils/src/lib/report.spec.ts b/packages/utils/src/lib/report.spec.ts index e68cd4b17..fdd404991 100644 --- a/packages/utils/src/lib/report.spec.ts +++ b/packages/utils/src/lib/report.spec.ts @@ -1,5 +1,5 @@ import { describe, expect } from 'vitest'; -import { CategoryRefSchema, IssueSeverity } from '@code-pushup/models'; +import { CategoryRef, IssueSeverity } from '@code-pushup/models'; import { calcDuration, compareIssueSeverity, @@ -98,7 +98,7 @@ describe('formatCount', () => { describe('countWeightedRefs', () => { it('should calc weighted refs only', () => { - const refs: CategoryRefSchema[] = [ + const refs: CategoryRef[] = [ { slug: 'a1', weight: 0, @@ -128,7 +128,7 @@ describe('compareIssueSeverity', () => { describe('sumRefs', () => { it('should sum refs correctly', () => { - const refs: CategoryRefSchema[] = [ + const refs: CategoryRef[] = [ { slug: 'a1', weight: 0, diff --git a/packages/utils/src/lib/report.ts b/packages/utils/src/lib/report.ts index d0932fac4..c4745ecb0 100644 --- a/packages/utils/src/lib/report.ts +++ b/packages/utils/src/lib/report.ts @@ -1,4 +1,4 @@ -import { CategoryRefSchema, IssueSeverity } from '@code-pushup/models'; +import { CategoryRef, IssueSeverity } from '@code-pushup/models'; import { pluralize } from './utils'; export const FOOTER_PREFIX = 'Made with ❤️ by'; @@ -34,7 +34,7 @@ export function formatCount(count: number, name: string) { return `${count} ${text}`; } -export function countWeightedRefs(refs: CategoryRefSchema[]) { +export function countWeightedRefs(refs: CategoryRef[]) { return refs .filter(({ weight }) => weight > 0) .reduce((sum, { weight }) => sum + weight, 0); @@ -53,6 +53,6 @@ export function compareIssueSeverity( } // @TODO replace with real scoring logic -export function sumRefs(refs: CategoryRefSchema[]) { +export function sumRefs(refs: CategoryRef[]) { return refs.reduce((sum, { weight }) => sum + weight, 0); } diff --git a/packages/utils/src/lib/scoring.ts b/packages/utils/src/lib/scoring.ts index 68f3dd8bf..65441754a 100644 --- a/packages/utils/src/lib/scoring.ts +++ b/packages/utils/src/lib/scoring.ts @@ -3,7 +3,7 @@ import { AuditGroupRef, AuditReport, CategoryConfig, - CategoryRefSchema, + CategoryRef, PluginReport, Report, } from '@code-pushup/models'; @@ -37,8 +37,8 @@ function groupRefToScore( function categoryRefToScore( audits: EnrichedAuditReport[], groups: EnrichedScoredAuditGroup[], -): (ref: CategoryRefSchema) => number { - return (ref: CategoryRefSchema): number => { +): (ref: CategoryRef) => number { + return (ref: CategoryRef): number => { switch (ref.type) { case 'audit': // eslint-disable-next-line no-case-declarations