From acb8b5258fa645e499831fca43b319b0439c0baf Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Tue, 10 Sep 2024 00:56:58 +0200 Subject: [PATCH] infra: improve error messages for parameter properties (#3082) --- scripts/apidocs/processing/error.ts | 24 ++++++----- scripts/apidocs/processing/jsdocs.ts | 6 ++- scripts/apidocs/processing/parameter.ts | 56 +++++++++++++++---------- scripts/apidocs/utils/value-checks.ts | 56 ++++++++++++++++++------- scripts/env.ts | 3 ++ vitest.config.ts | 5 +-- 6 files changed, 99 insertions(+), 51 deletions(-) create mode 100644 scripts/env.ts diff --git a/scripts/apidocs/processing/error.ts b/scripts/apidocs/processing/error.ts index f171d6b4663..e4117301842 100644 --- a/scripts/apidocs/processing/error.ts +++ b/scripts/apidocs/processing/error.ts @@ -1,4 +1,5 @@ import { FakerError } from '../../../src/errors/faker-error'; +import { CI_PREFLIGHT } from '../../env'; import type { SourceableNode } from './source'; import { getSourcePath } from './source'; @@ -6,14 +7,22 @@ export class FakerApiDocsProcessingError extends FakerError { constructor(options: { type: string; name: string; - source: string | SourceableNode; + source: SourceableNode; cause: unknown; }) { const { type, name, source, cause } = options; - const sourceText = - typeof source === 'string' ? source : getSourcePathText(source); + + const mainText = `Failed to process ${type} '${name}'`; const causeText = cause instanceof Error ? cause.message : ''; - super(`Failed to process ${type} ${name} at ${sourceText} : ${causeText}`, { + const { filePath, line, column } = getSourcePath(source); + const sourceText = `${filePath}:${line}:${column}`; + + if (CI_PREFLIGHT) { + const sourceArgs = `file=${filePath},line=${line},col=${column}`; + console.log(`::error ${sourceArgs}::${mainText}: ${causeText}`); + } + + super(`${mainText} at ${sourceText} : ${causeText}`, { cause, }); } @@ -22,7 +31,7 @@ export class FakerApiDocsProcessingError extends FakerError { export function newProcessingError(options: { type: string; name: string; - source: string | SourceableNode; + source: SourceableNode; cause: unknown; }): FakerApiDocsProcessingError { const { cause } = options; @@ -33,8 +42,3 @@ export function newProcessingError(options: { return new FakerApiDocsProcessingError(options); } - -function getSourcePathText(source: SourceableNode): string { - const { filePath, line, column } = getSourcePath(source); - return `${filePath}:${line}:${column}`; -} diff --git a/scripts/apidocs/processing/jsdocs.ts b/scripts/apidocs/processing/jsdocs.ts index 4135e5e621f..0999abca845 100644 --- a/scripts/apidocs/processing/jsdocs.ts +++ b/scripts/apidocs/processing/jsdocs.ts @@ -10,7 +10,11 @@ import { export type JSDocableLikeNode = Pick; export function getJsDocs(node: JSDocableLikeNode): JSDoc { - return exactlyOne(node.getJsDocs(), 'jsdocs'); + return exactlyOne( + node.getJsDocs(), + 'jsdocs', + 'Please ensure that each method signature has JSDocs, and that all properties of option/object parameters are documented with both @param tags and inline JSDocs.' + ); } export function getDeprecated(jsdocs: JSDoc): string | undefined { diff --git a/scripts/apidocs/processing/parameter.ts b/scripts/apidocs/processing/parameter.ts index 7a6b67da896..05f2f1f3fdb 100644 --- a/scripts/apidocs/processing/parameter.ts +++ b/scripts/apidocs/processing/parameter.ts @@ -1,5 +1,6 @@ import type { PropertySignature, + Symbol, Type, TypeParameterDeclaration, } from 'ts-morph'; @@ -174,30 +175,43 @@ function processComplexParameter( return type .getApparentProperties() .flatMap((parameter) => { - const declaration = exactlyOne( - parameter.getDeclarations(), - 'property declaration' - ) as PropertySignature; - const propertyType = declaration.getType(); - const jsdocs = getJsDocs(declaration); - const deprecated = getDeprecated(jsdocs); - - return [ - { - name: `${name}.${parameter.getName()}${getNameSuffix(propertyType)}`, - type: getTypeText(propertyType, { - abbreviate: false, - stripUndefined: true, - }), - default: getDefault(jsdocs), - description: - getDescription(jsdocs) + - (deprecated ? `\n\n**DEPRECATED:** ${deprecated}` : ''), - }, - ]; + try { + return processComplexParameterProperty(name, parameter); + } catch (error) { + throw newProcessingError({ + type: 'property', + name: `${name}.${parameter.getName()}`, + source: parameter.getDeclarations()[0], + cause: error, + }); + } }) .sort((a, b) => a.name.localeCompare(b.name)); } return []; } + +function processComplexParameterProperty(name: string, parameter: Symbol) { + const declaration = exactlyOne( + parameter.getDeclarations(), + 'property declaration' + ) as PropertySignature; + const propertyType = declaration.getType(); + const jsdocs = getJsDocs(declaration); + const deprecated = getDeprecated(jsdocs); + + return [ + { + name: `${name}.${parameter.getName()}${getNameSuffix(propertyType)}`, + type: getTypeText(propertyType, { + abbreviate: false, + stripUndefined: true, + }), + default: getDefault(jsdocs), + description: + getDescription(jsdocs) + + (deprecated ? `\n\n**DEPRECATED:** ${deprecated}` : ''), + }, + ]; +} diff --git a/scripts/apidocs/utils/value-checks.ts b/scripts/apidocs/utils/value-checks.ts index f8578ccc311..2a4ee1a98d1 100644 --- a/scripts/apidocs/utils/value-checks.ts +++ b/scripts/apidocs/utils/value-checks.ts @@ -1,7 +1,11 @@ -export function exactlyOne(input: ReadonlyArray, property: string): T { +export function exactlyOne( + input: ReadonlyArray, + property: string, + extraDescription: string = '' +): T { if (input.length !== 1) { throw new Error( - `Expected exactly one element for ${property}, got ${input.length}` + `Expected exactly one ${property} element, got ${input.length}. ${extraDescription}` ); } @@ -10,11 +14,12 @@ export function exactlyOne(input: ReadonlyArray, property: string): T { export function optionalOne( input: ReadonlyArray, - property: string + property: string, + extraDescription: string = '' ): T | undefined { if (input.length > 1) { throw new Error( - `Expected one optional element for ${property}, got ${input.length}` + `Expected one optional ${property} element, got ${input.length}. ${extraDescription}` ); } @@ -23,10 +28,13 @@ export function optionalOne( export function required( input: T | undefined, - property: string + property: string, + extraDescription: string = '' ): NonNullable { if (input == null) { - throw new Error(`Expected a value for ${property}, got undefined`); + throw new Error( + `Expected a value for ${property}, got undefined. ${extraDescription}` + ); } return input; @@ -34,17 +42,23 @@ export function required( export function allRequired( input: ReadonlyArray, - property: string + property: string, + extraDescription: string = '' ): Array> { - return input.map((v, i) => required(v, `${property}[${i}]`)); + return input.map((v, i) => + required(v, `${property}[${i}]`, extraDescription) + ); } export function atLeastOne( input: ReadonlyArray, - property: string + property: string, + extraDescription: string = '' ): ReadonlyArray { if (input.length === 0) { - throw new Error(`Expected at least one element for ${property}`); + throw new Error( + `Expected at least one ${property} element. ${extraDescription}` + ); } return input; @@ -52,18 +66,28 @@ export function atLeastOne( export function atLeastOneAndAllRequired( input: ReadonlyArray, - property: string + property: string, + extraDescription: string = '' ): ReadonlyArray> { - return atLeastOne(allRequired(input, property), property); + return atLeastOne( + allRequired(input, property, extraDescription), + property, + extraDescription + ); } -export function valueForKey(input: Record, key: string): T { - return required(input[key], key); +export function valueForKey( + input: Record, + key: string, + extraDescription: string = '' +): T { + return required(input[key], key, extraDescription); } export function valuesForKeys( input: Record, - keys: string[] + keys: string[], + extraDescription: string = '' ): T[] { - return keys.map((key) => valueForKey(input, key)); + return keys.map((key) => valueForKey(input, key, extraDescription)); } diff --git a/scripts/env.ts b/scripts/env.ts new file mode 100644 index 00000000000..f73aa5d949c --- /dev/null +++ b/scripts/env.ts @@ -0,0 +1,3 @@ +import { env } from 'node:process'; + +export const CI_PREFLIGHT = env.CI_PREFLIGHT === 'true'; diff --git a/vitest.config.ts b/vitest.config.ts index 1c73cfbb318..ad95ef967cf 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,5 @@ import { defineConfig } from 'vitest/config'; +import { CI_PREFLIGHT } from './scripts/env'; const VITEST_SEQUENCE_SEED = Date.now(); @@ -14,9 +15,7 @@ export default defineConfig({ reporter: ['clover', 'cobertura', 'lcov', 'text'], include: ['src'], }, - reporters: process.env.CI_PREFLIGHT - ? ['basic', 'github-actions'] - : ['basic'], + reporters: CI_PREFLIGHT ? ['basic', 'github-actions'] : ['basic'], sequence: { seed: VITEST_SEQUENCE_SEED, shuffle: true,