From 305b8b6cc732e2aaa0a773e12c869009092b152c Mon Sep 17 00:00:00 2001 From: Aaron Moat <2937187+AaronMoat@users.noreply.github.com> Date: Sun, 7 Jan 2024 22:10:35 +1100 Subject: [PATCH] Include internal lints in annotations --- src/cli/__snapshots__/lint.int.test.ts.snap | 42 +++++++----- src/cli/configure/upgrade/index.test.ts | 7 ++ src/cli/configure/upgrade/index.ts | 25 ++++++-- src/cli/lint/annotate/buildkite/index.ts | 8 ++- src/cli/lint/annotate/buildkite/internal.ts | 16 +++++ src/cli/lint/annotate/github/index.test.ts | 15 +++++ src/cli/lint/annotate/github/index.ts | 6 +- src/cli/lint/annotate/github/internal.ts | 14 ++++ src/cli/lint/annotate/index.ts | 12 +++- src/cli/lint/external.ts | 30 +-------- src/cli/lint/index.ts | 50 +++++++++++++-- src/cli/lint/internal.test.ts | 1 + src/cli/lint/internal.ts | 64 +++++++------------ .../lint/internalLints/deleteFiles.test.ts | 6 ++ src/cli/lint/internalLints/deleteFiles.ts | 7 +- .../internalLints/noSkubaTemplateJs.test.ts | 6 ++ .../lint/internalLints/noSkubaTemplateJs.ts | 14 +++- .../internalLints/refreshIgnoreFiles.test.ts | 15 +++++ .../lint/internalLints/refreshIgnoreFiles.ts | 19 +++++- 19 files changed, 253 insertions(+), 104 deletions(-) create mode 100644 src/cli/lint/annotate/buildkite/internal.ts create mode 100644 src/cli/lint/annotate/github/internal.ts diff --git a/src/cli/__snapshots__/lint.int.test.ts.snap b/src/cli/__snapshots__/lint.int.test.ts.snap index e078ccc9d..6affc0f02 100644 --- a/src/cli/__snapshots__/lint.int.test.ts.snap +++ b/src/cli/__snapshots__/lint.int.test.ts.snap @@ -24,20 +24,17 @@ Prettier │ d.js Prettier │ package.json tsc │ TSFILE: /lib/tsconfig.tsbuildinfo tsc │ tsc --noEmit exited with code 0 - -ESLint, Prettier found issues that require triage. skuba │ Processed skuba lints in s. +ESLint, Prettier found issues that require triage. + +Try running yarn skuba format to fix them. " `; exports[`fixable 2`] = ` [ " -Options: { - context: 'skuba-lint-external', - scopeContextToStep: true, - style: 'error' -} +Options: { context: 'skuba-lint', scopeContextToStep: true, style: 'error' } \`skuba lint\` found issues that require triage: @@ -76,12 +73,28 @@ Prettier │ Processed 6 files in s. tsc │ TSFILE: /lib/tsconfig.tsbuildinfo tsc │ tsc --noEmit exited with code 0 skuba │ skuba has patches to apply. Run yarn skuba format to run them. skuba-patches -skuba │ Some skuba lints failed. Try running yarn skuba format to fix them. skuba-lint skuba │ Processed skuba lints in s. +skuba found issues that require triage. + +Try running yarn skuba format to fix them. " `; -exports[`needs patches 2`] = `[]`; +exports[`needs patches 2`] = ` +[ + " +Options: { context: 'skuba-lint', scopeContextToStep: true, style: 'error' } + +\`skuba lint\` found issues that require triage: + +**skuba** + +\`\`\`term +/package.json skuba has patches to apply. Run yarn skuba format to run them. +\`\`\` +", +] +`; exports[`ok --debug 1`] = ` " @@ -186,20 +199,17 @@ Prettier │ a/a/a.ts tsc │ d.js(1,1): error TS1128: Declaration or statement expected. tsc │ TSFILE: /lib/tsconfig.tsbuildinfo tsc │ tsc --noEmit exited with code 2 - -ESLint, Prettier, tsc found issues that require triage. skuba │ Processed skuba lints in s. +ESLint, Prettier, tsc found issues that require triage. + +Try running yarn skuba format to fix them. " `; exports[`unfixable 2`] = ` [ " -Options: { - context: 'skuba-lint-external', - scopeContextToStep: true, - style: 'error' -} +Options: { context: 'skuba-lint', scopeContextToStep: true, style: 'error' } \`skuba lint\` found issues that require triage: diff --git a/src/cli/configure/upgrade/index.test.ts b/src/cli/configure/upgrade/index.test.ts index b5274aef8..aae125057 100644 --- a/src/cli/configure/upgrade/index.test.ts +++ b/src/cli/configure/upgrade/index.test.ts @@ -236,6 +236,13 @@ describe('upgradeSkuba in lint mode', () => { await expect(upgradeSkuba('lint', log)).resolves.toEqual({ ok: false, fixable: true, + annotations: [ + { + message: + 'skuba has patches to apply. Run pnpm exec skuba format to run them.', + path: '/package.json', + }, + ], }); }); diff --git a/src/cli/configure/upgrade/index.ts b/src/cli/configure/upgrade/index.ts index 6576430b7..a7aab337d 100644 --- a/src/cli/configure/upgrade/index.ts +++ b/src/cli/configure/upgrade/index.ts @@ -8,6 +8,7 @@ import { getConsumerManifest } from '../../../utils/manifest'; import { detectPackageManager } from '../../../utils/packageManager'; import { getSkubaVersion } from '../../../utils/version'; import type { SkubaPackageJson } from '../../init/writePackageJson'; +import type { InternalLintResult } from '../../lint/internal'; import { formatPackage } from '../processing/package'; const getPatches = async (manifestVersion: string): Promise => { @@ -38,7 +39,10 @@ const resolvePatch = async (patch: string): Promise => { throw new Error(`Could not resolve patch ${patch}`); }; -export const upgradeSkuba = async (mode: 'lint' | 'format', logger: Logger) => { +export const upgradeSkuba = async ( + mode: 'lint' | 'format', + logger: Logger, +): Promise => { const [currentVersion, manifest] = await Promise.all([ getSkubaVersion(), getConsumerManifest(), @@ -68,6 +72,9 @@ export const upgradeSkuba = async (mode: 'lint' | 'format', logger: Logger) => { } if (mode === 'lint') { + // TODO: Do we want to declare patches as optional? + // TODO: Should we return ok: true if all patches are no-ops? + const packageManager = await detectPackageManager(); logger.warn( @@ -78,10 +85,18 @@ export const upgradeSkuba = async (mode: 'lint' | 'format', logger: Logger) => { )} to run them. ${logger.dim('skuba-patches')}`, ); - // TODO: Do we want to declare patches as optional? - // TODO: Should we return ok: true if all patches are no-ops? - - return { ok: false, fixable: true }; + return { + ok: false, + fixable: true, + annotations: [ + { + // package.json as likely skuba version has changed + // TODO: location the "skuba": {} config in the package.json and annotate on the version property + path: manifest.path, + message: `skuba has patches to apply. Run ${packageManager.exec} skuba format to run them.`, + }, + ], + }; } logger.plain('Updating skuba...'); diff --git a/src/cli/lint/annotate/buildkite/index.ts b/src/cli/lint/annotate/buildkite/index.ts index 44a39ec37..decd819c4 100644 --- a/src/cli/lint/annotate/buildkite/index.ts +++ b/src/cli/lint/annotate/buildkite/index.ts @@ -2,30 +2,34 @@ import * as Buildkite from '../../../../api/buildkite'; import type { ESLintOutput } from '../../../adapter/eslint'; import type { PrettierOutput } from '../../../adapter/prettier'; import type { StreamInterceptor } from '../../../lint/external'; +import type { InternalLintResult } from '../../internal'; import { createEslintAnnotations } from './eslint'; +import { createInternalAnnotations } from './internal'; import { createPrettierAnnotations } from './prettier'; import { createTscAnnotations } from './tsc'; export const createBuildkiteAnnotations = async ( + internal: InternalLintResult, eslint: ESLintOutput, prettier: PrettierOutput, tscOk: boolean, tscOutputStream: StreamInterceptor, ): Promise => { - if (eslint.ok && prettier.ok && tscOk) { + if (internal.ok && eslint.ok && prettier.ok && tscOk) { return; } const buildkiteOutput = [ '`skuba lint` found issues that require triage:', + ...createInternalAnnotations(internal), ...createEslintAnnotations(eslint), ...createPrettierAnnotations(prettier), ...createTscAnnotations(tscOk, tscOutputStream), ].join('\n\n'); await Buildkite.annotate(buildkiteOutput, { - context: 'skuba-lint-external', + context: 'skuba-lint', scopeContextToStep: true, style: 'error', }); diff --git a/src/cli/lint/annotate/buildkite/internal.ts b/src/cli/lint/annotate/buildkite/internal.ts new file mode 100644 index 000000000..9000f7950 --- /dev/null +++ b/src/cli/lint/annotate/buildkite/internal.ts @@ -0,0 +1,16 @@ +import * as Buildkite from '../../../../api/buildkite'; +import type { InternalLintResult } from '../../internal'; + +export const createInternalAnnotations = ( + internal: InternalLintResult, +): string[] => + !internal.ok && internal.annotations?.length + ? [ + '**skuba**', + Buildkite.md.terminal( + internal.annotations + .map(({ message, path }) => `${path} ${message}`) + .join('\n'), + ), + ] + : []; diff --git a/src/cli/lint/annotate/github/index.test.ts b/src/cli/lint/annotate/github/index.test.ts index 0a36dd9c1..a526d860a 100644 --- a/src/cli/lint/annotate/github/index.test.ts +++ b/src/cli/lint/annotate/github/index.test.ts @@ -2,6 +2,7 @@ import * as GitHub from '../../../../api/github'; import type { ESLintOutput } from '../../../adapter/eslint'; import type { PrettierOutput } from '../../../adapter/prettier'; import type { StreamInterceptor } from '../../../lint/external'; +import type { InternalLintResult } from '../../internal'; import { createEslintAnnotations } from './eslint'; import { createPrettierAnnotations } from './prettier'; @@ -52,6 +53,11 @@ const prettierOutput: PrettierOutput = { }, }; +const internalOutput: InternalLintResult = { + ok: false, + fixable: false, +}; + const tscOk = false; const mockOutput = jest.fn(); @@ -124,6 +130,7 @@ it('should return immediately if the required environment variables are not set' delete process.env.GITHUB_ACTIONS; await createGitHubAnnotations( + internalOutput, eslintOutput, prettierOutput, tscOk, @@ -135,6 +142,7 @@ it('should return immediately if the required environment variables are not set' it('should call createEslintAnnotations with the ESLint output', async () => { await createGitHubAnnotations( + internalOutput, eslintOutput, prettierOutput, tscOk, @@ -146,6 +154,7 @@ it('should call createEslintAnnotations with the ESLint output', async () => { it('should call createPrettierAnnotations with the Prettier output', async () => { await createGitHubAnnotations( + internalOutput, eslintOutput, prettierOutput, tscOk, @@ -157,6 +166,7 @@ it('should call createPrettierAnnotations with the Prettier output', async () => it('should call createTscAnnotations with tscOk and tscOutputStream', async () => { await createGitHubAnnotations( + internalOutput, eslintOutput, prettierOutput, tscOk, @@ -174,6 +184,7 @@ it('should combine all the annotations into an array for the check run', async ( ]; await createGitHubAnnotations( + internalOutput, eslintOutput, prettierOutput, tscOk, @@ -191,6 +202,7 @@ it('should combine all the annotations into an array for the check run', async ( it('should set the conclusion to failure if any output is not ok', async () => { await createGitHubAnnotations( + { ...internalOutput, ok: true }, { ...eslintOutput, ok: false }, { ...prettierOutput, ok: true }, true, @@ -208,6 +220,7 @@ it('should set the conclusion to failure if any output is not ok', async () => { it('should set the conclusion to success if all outputs are ok', async () => { await createGitHubAnnotations( + { ...internalOutput, ok: true }, { ...eslintOutput, ok: true }, { ...prettierOutput, ok: true }, true, @@ -227,6 +240,7 @@ it('should report that skuba lint failed if the output is not ok', async () => { const expectedSummary = '`skuba lint` found issues that require triage.'; await createGitHubAnnotations( + internalOutput, { ...eslintOutput, ok: false }, { ...prettierOutput, ok: false }, false, @@ -246,6 +260,7 @@ it('should set the summary to `Lint passed` if all outputs are ok', async () => const expectedSummary = '`skuba lint` passed.'; await createGitHubAnnotations( + { ...internalOutput, ok: true }, { ...eslintOutput, ok: true }, { ...prettierOutput, ok: true }, true, diff --git a/src/cli/lint/annotate/github/index.ts b/src/cli/lint/annotate/github/index.ts index afdd48ba7..1adaf2e9a 100644 --- a/src/cli/lint/annotate/github/index.ts +++ b/src/cli/lint/annotate/github/index.ts @@ -6,12 +6,15 @@ import { import type { ESLintOutput } from '../../../adapter/eslint'; import type { PrettierOutput } from '../../../adapter/prettier'; import type { StreamInterceptor } from '../../../lint/external'; +import type { InternalLintResult } from '../../internal'; import { createEslintAnnotations } from './eslint'; +import { createInternalAnnotations } from './internal'; import { createPrettierAnnotations } from './prettier'; import { createTscAnnotations } from './tsc'; export const createGitHubAnnotations = async ( + internal: InternalLintResult, eslint: ESLintOutput, prettier: PrettierOutput, tscOk: boolean, @@ -22,12 +25,13 @@ export const createGitHubAnnotations = async ( } const annotations: GitHub.Annotation[] = [ + ...createInternalAnnotations(internal), ...createEslintAnnotations(eslint), ...createPrettierAnnotations(prettier), ...createTscAnnotations(tscOk, tscOutputStream), ]; - const isOk = eslint.ok && prettier.ok && tscOk; + const isOk = eslint.ok && prettier.ok && internal.ok && tscOk; const summary = isOk ? '`skuba lint` passed.' diff --git a/src/cli/lint/annotate/github/internal.ts b/src/cli/lint/annotate/github/internal.ts new file mode 100644 index 000000000..9893fdea2 --- /dev/null +++ b/src/cli/lint/annotate/github/internal.ts @@ -0,0 +1,14 @@ +import type * as GitHub from '../../../../api/github'; +import type { InternalLintResult } from '../../internal'; + +export const createInternalAnnotations = ( + internal: InternalLintResult, +): GitHub.Annotation[] => + (internal.annotations ?? []).map((annotation) => ({ + annotation_level: 'failure', + start_line: annotation.start_line ?? 1, + end_line: annotation.end_line ?? annotation.start_line ?? 1, + path: annotation.path, + message: annotation.message, + title: 'skuba lint', + })); diff --git a/src/cli/lint/annotate/index.ts b/src/cli/lint/annotate/index.ts index b150d85b3..59a7eeb95 100644 --- a/src/cli/lint/annotate/index.ts +++ b/src/cli/lint/annotate/index.ts @@ -1,18 +1,26 @@ import type { ESLintOutput } from '../../../cli/adapter/eslint'; import type { PrettierOutput } from '../../../cli/adapter/prettier'; import type { StreamInterceptor } from '../external'; +import type { InternalLintResult } from '../internal'; import { createBuildkiteAnnotations } from './buildkite'; import { createGitHubAnnotations } from './github'; export const createAnnotations = async ( + internal: InternalLintResult, eslint: ESLintOutput, prettier: PrettierOutput, tscOk: boolean, tscOutputStream: StreamInterceptor, ): Promise => { await Promise.all([ - createGitHubAnnotations(eslint, prettier, tscOk, tscOutputStream), - createBuildkiteAnnotations(eslint, prettier, tscOk, tscOutputStream), + createGitHubAnnotations(internal, eslint, prettier, tscOk, tscOutputStream), + createBuildkiteAnnotations( + internal, + eslint, + prettier, + tscOk, + tscOutputStream, + ), ]); }; diff --git a/src/cli/lint/external.ts b/src/cli/lint/external.ts index 03354868f..6e4d6f74e 100644 --- a/src/cli/lint/external.ts +++ b/src/cli/lint/external.ts @@ -1,10 +1,5 @@ import stream from 'stream'; -import { inspect } from 'util'; -import { log } from '../../utils/logging'; -import { throwOnTimeout } from '../../utils/wait'; - -import { createAnnotations } from './annotate'; import { runESLintInCurrentThread, runESLintInWorkerThread } from './eslint'; import { runPrettierInCurrentThread, @@ -85,31 +80,10 @@ export const externalLint = async (input: Input) => { const { eslint, prettier, tscOk } = await lint({ ...input, tscOutputStream }); - try { - await throwOnTimeout( - createAnnotations(eslint, prettier, tscOk, tscOutputStream), - { s: 30 }, - ); - } catch (err) { - log.warn('Failed to annotate lint results.'); - log.subtle(inspect(err)); - } - - if (!eslint.ok || !prettier.ok || !tscOk) { - const tools = [ - ...(eslint.ok ? [] : ['ESLint']), - ...(prettier.ok ? [] : ['Prettier']), - ...(tscOk ? [] : ['tsc']), - ]; - - log.newline(); - log.err(`${tools.join(', ')} found issues that require triage.`); - - process.exitCode = 1; - } - return { eslint, prettier, + tscOk, + tscOutputStream, }; }; diff --git a/src/cli/lint/index.ts b/src/cli/lint/index.ts index 1db48060c..346b7b2c3 100644 --- a/src/cli/lint/index.ts +++ b/src/cli/lint/index.ts @@ -1,7 +1,12 @@ import type { Writable } from 'stream'; +import { inspect } from 'util'; import { hasDebugFlag, hasSerialFlag } from '../../utils/args'; +import { log } from '../../utils/logging'; +import { detectPackageManager } from '../../utils/packageManager'; +import { throwOnTimeout } from '../../utils/wait'; +import { createAnnotations } from './annotate'; import { autofix } from './autofix'; import { externalLint } from './external'; import { internalLint } from './internal'; @@ -9,23 +14,58 @@ import type { Input } from './types'; export const lint = async ( args = process.argv.slice(2), - tscOutputStream: Writable | undefined = undefined, + tscWriteable: Writable | undefined = undefined, workerThreads = true, ) => { const opts: Input = { debug: hasDebugFlag(args), serial: hasSerialFlag(args), - tscOutputStream, + tscOutputStream: tscWriteable, workerThreads, }; - const external = await externalLint(opts); + const { eslint, prettier, tscOk, tscOutputStream } = await externalLint(opts); const internal = await internalLint('lint', opts); + try { + await throwOnTimeout( + createAnnotations(internal, eslint, prettier, tscOk, tscOutputStream), + { s: 30 }, + ); + } catch (err) { + log.warn('Failed to annotate lint results.'); + log.subtle(inspect(err)); + } + + if (!internal.ok || !eslint.ok || !prettier.ok || !tscOk) { + process.exitCode = 1; + + const tools = [ + ...(internal.ok ? [] : ['skuba']), + ...(eslint.ok ? [] : ['ESLint']), + ...(prettier.ok ? [] : ['Prettier']), + ...(tscOk ? [] : ['tsc']), + ]; + + log.err(`${tools.join(', ')} found issues that require triage.`); + + if (internal.fixable || eslint.fixable || !prettier.ok) { + const packageManager = await detectPackageManager(); + log.newline(); + log.warn( + `Try running ${log.bold( + packageManager.exec, + 'skuba', + 'format', + )} to fix them.`, + ); + } + } + await autofix({ debug: opts.debug, - eslint: external.eslint.fixable, - prettier: !external.prettier.ok, + eslint: eslint.fixable, + prettier: !prettier.ok, internal: internal.fixable, }); }; diff --git a/src/cli/lint/internal.test.ts b/src/cli/lint/internal.test.ts index 59b986757..05d85b5d0 100644 --- a/src/cli/lint/internal.test.ts +++ b/src/cli/lint/internal.test.ts @@ -15,5 +15,6 @@ describe('internalLint', () => { expect(internalLint('lint')).resolves.toEqual({ ok: true, fixable: false, + annotations: [], })); }); diff --git a/src/cli/lint/internal.ts b/src/cli/lint/internal.ts index 5724d506e..f13bec3a8 100644 --- a/src/cli/lint/internal.ts +++ b/src/cli/lint/internal.ts @@ -3,7 +3,6 @@ import { inspect } from 'util'; import chalk from 'chalk'; import { type Logger, createLogger } from '../../utils/logging'; -import { detectPackageManager } from '../../utils/packageManager'; import { upgradeSkuba } from '../configure/upgrade'; import { deleteFilesLint } from './internalLints/deleteFiles'; @@ -11,15 +10,23 @@ import { noSkubaTemplateJs } from './internalLints/noSkubaTemplateJs'; import { tryRefreshIgnoreFiles } from './internalLints/refreshIgnoreFiles'; import type { Input } from './types'; -const lints = [ - deleteFilesLint, - noSkubaTemplateJs, - tryRefreshIgnoreFiles, - upgradeSkuba, -]; +export type InternalLintResult = { + ok: boolean; + fixable: boolean; + annotations?: Array<{ + start_line?: number; + end_line?: number; + path: string; + message: string; + }>; +}; + +const lints: Array< + (mode: 'format' | 'lint', logger: Logger) => Promise +> = [deleteFilesLint, noSkubaTemplateJs, tryRefreshIgnoreFiles, upgradeSkuba]; const lintSerially = async (mode: 'format' | 'lint', logger: Logger) => { - const results = []; + const results: InternalLintResult[] = []; for (const lint of lints) { results.push(await lint(mode, logger)); } @@ -34,7 +41,10 @@ const selectLintFunction = (input?: Input) => { return isSerial ? lintSerially : lintConcurrently; }; -export const internalLint = async (mode: 'format' | 'lint', input?: Input) => { +export const internalLint = async ( + mode: 'format' | 'lint', + input?: Input, +): Promise => { const start = process.hrtime.bigint(); const logger = createLogger( input?.debug ?? false, @@ -44,7 +54,7 @@ export const internalLint = async (mode: 'format' | 'lint', input?: Input) => { try { const lint = selectLintFunction(input); const results = await lint(mode, logger); - const result = await processResults(results, logger); + const result = combineResults(results); const end = process.hrtime.bigint(); logger.plain(`Processed skuba lints in ${logger.timing(start, end)}.`); return result; @@ -54,42 +64,16 @@ export const internalLint = async (mode: 'format' | 'lint', input?: Input) => { process.exitCode = 1; - return { ok: false, fixable: false }; + return { ok: false, fixable: false, annotations: [] }; } }; -const processResults = async ( - results: Array<{ ok: boolean; fixable: boolean }>, - logger: Logger, -) => { - const result = results.reduce( +const combineResults = (results: InternalLintResult[]): InternalLintResult => + results.reduce( (cur, next) => ({ ok: cur.ok && next.ok, fixable: cur.fixable || next.fixable, + annotations: [...(cur.annotations ?? []), ...(next.annotations ?? [])], }), { ok: true, fixable: false }, ); - - if (!result.ok) { - process.exitCode = 1; - - if (result.fixable) { - const packageManager = await detectPackageManager(); - logger.err( - `Some skuba lints failed. Try running ${logger.bold( - packageManager.exec, - 'skuba', - 'format', - )} to fix them. ${logger.dim('skuba-lint')}`, - ); - } else { - logger.err( - `Some skuba lints failed which require triage. ${logger.dim( - 'skuba-lint', - )}`, - ); - } - } - - return result; -}; diff --git a/src/cli/lint/internalLints/deleteFiles.test.ts b/src/cli/lint/internalLints/deleteFiles.test.ts index dc8a2052a..533199fd1 100644 --- a/src/cli/lint/internalLints/deleteFiles.test.ts +++ b/src/cli/lint/internalLints/deleteFiles.test.ts @@ -50,6 +50,12 @@ describe('deleteFilesLint', () => { await expect(deleteFilesLint('lint', log)).resolves.toEqual({ ok: false, fixable: true, + annotations: [ + { + message: 'This file should be deleted.', + path: 'Dockerfile-incunabulum', + }, + ], }); expect(stdout()).toBe( diff --git a/src/cli/lint/internalLints/deleteFiles.ts b/src/cli/lint/internalLints/deleteFiles.ts index e59513231..c46011ffe 100644 --- a/src/cli/lint/internalLints/deleteFiles.ts +++ b/src/cli/lint/internalLints/deleteFiles.ts @@ -5,6 +5,7 @@ import { pathExists, rm } from 'fs-extra'; import type { Logger } from '../../../utils/logging'; import { detectPackageManager } from '../../../utils/packageManager'; +import type { InternalLintResult } from '../internal'; const AUTOFIX_DELETE_FILES = [ // Try to delete this SEEK-Jobs/gutenberg automation file that may have been @@ -15,7 +16,7 @@ const AUTOFIX_DELETE_FILES = [ export const deleteFilesLint = async ( mode: 'format' | 'lint', logger: Logger, -) => { +): Promise => { const dir = process.cwd(); const toDelete = ( @@ -71,6 +72,10 @@ export const deleteFilesLint = async ( return { ok: false, fixable: true, + annotations: toDelete.map((filename) => ({ + path: filename, + message: 'This file should be deleted.', + })), }; } diff --git a/src/cli/lint/internalLints/noSkubaTemplateJs.test.ts b/src/cli/lint/internalLints/noSkubaTemplateJs.test.ts index 7ed4e281d..a35c6466b 100644 --- a/src/cli/lint/internalLints/noSkubaTemplateJs.test.ts +++ b/src/cli/lint/internalLints/noSkubaTemplateJs.test.ts @@ -49,6 +49,12 @@ describe('noSkubaTemplateJs', () => { await expect(noSkubaTemplateJs(mode, log)).resolves.toEqual({ ok: false, fixable: false, + annotations: [ + { + message: 'Template is incomplete; run pnpm exec skuba configure.', + path: '/Users/amoat/Programming/seek-oss/skuba/skuba.template.js', + }, + ], }); expect(stdout()).toBe( diff --git a/src/cli/lint/internalLints/noSkubaTemplateJs.ts b/src/cli/lint/internalLints/noSkubaTemplateJs.ts index bfe48a05a..853ddd347 100644 --- a/src/cli/lint/internalLints/noSkubaTemplateJs.ts +++ b/src/cli/lint/internalLints/noSkubaTemplateJs.ts @@ -5,11 +5,12 @@ import { pathExists } from 'fs-extra'; import type { Logger } from '../../../utils/logging'; import { getConsumerManifest } from '../../../utils/manifest'; import { detectPackageManager } from '../../../utils/packageManager'; +import type { InternalLintResult } from '../internal'; export const noSkubaTemplateJs = async ( _mode: 'format' | 'lint', logger: Logger, -) => { +): Promise => { const [manifest, packageManager] = await Promise.all([ getConsumerManifest(), detectPackageManager(), @@ -34,7 +35,16 @@ export const noSkubaTemplateJs = async ( )}. ${logger.dim('no-skuba-template-js')}`, ); - return { ok: false, fixable: false }; + return { + ok: false, + fixable: false, + annotations: [ + { + path: templateConfigPath, + message: `Template is incomplete; run ${packageManager.exec} skuba configure.`, + }, + ], + }; } return { ok: true, fixable: false }; diff --git a/src/cli/lint/internalLints/refreshIgnoreFiles.test.ts b/src/cli/lint/internalLints/refreshIgnoreFiles.test.ts index 5f1789859..c9937a5a8 100644 --- a/src/cli/lint/internalLints/refreshIgnoreFiles.test.ts +++ b/src/cli/lint/internalLints/refreshIgnoreFiles.test.ts @@ -58,6 +58,7 @@ describe('refreshIgnoreFiles', () => { await expect(refreshIgnoreFiles('lint', log)).resolves.toEqual({ ok: true, fixable: false, + annotations: [], }); expect(stdout()).toBe(''); @@ -78,6 +79,18 @@ describe('refreshIgnoreFiles', () => { await expect(refreshIgnoreFiles('lint', log)).resolves.toEqual({ ok: false, fixable: true, + annotations: [ + { + message: + 'The .eslintignore file is out of date. Run `pnpm exec skuba format` to update it.', + path: '.eslintignore', + }, + { + message: + 'The .gitignore file is out of date. Run `pnpm exec skuba format` to update it.', + path: '.gitignore', + }, + ], }); expect(`\n${stdout()}`).toBe(` @@ -103,6 +116,7 @@ The .gitignore file is out of date. Run pnpm exec skuba format to update it. ref await expect(refreshIgnoreFiles('format', log)).resolves.toEqual({ ok: true, fixable: false, + annotations: [], }); expect(stdout()).toBe(''); @@ -123,6 +137,7 @@ The .gitignore file is out of date. Run pnpm exec skuba format to update it. ref await expect(refreshIgnoreFiles('format', log)).resolves.toEqual({ ok: true, fixable: false, + annotations: [], }); expect(stdout()).toBe( diff --git a/src/cli/lint/internalLints/refreshIgnoreFiles.ts b/src/cli/lint/internalLints/refreshIgnoreFiles.ts index 64890da3a..1171d54a8 100644 --- a/src/cli/lint/internalLints/refreshIgnoreFiles.ts +++ b/src/cli/lint/internalLints/refreshIgnoreFiles.ts @@ -9,6 +9,7 @@ import { readBaseTemplateFile } from '../../../utils/template'; import { getDestinationManifest } from '../../configure/analysis/package'; import { createDestinationFileReader } from '../../configure/analysis/project'; import { mergeWithIgnoreFile } from '../../configure/processing/ignoreFile'; +import type { InternalLintResult } from '../internal'; const REFRESHABLE_IGNORE_FILES = [ '.eslintignore', @@ -19,7 +20,7 @@ const REFRESHABLE_IGNORE_FILES = [ export const refreshIgnoreFiles = async ( mode: 'format' | 'lint', logger: Logger, -) => { +): Promise => { // TODO: check current state of .gitignore // If it contains !.npmrc, break // If it contains .npmrc, we can either @@ -70,6 +71,8 @@ export const refreshIgnoreFiles = async ( 'skuba', 'format', )} to update it. ${logger.dim('refresh-ignore-files')}`, + filename, + annotationMessage: `The ${filename} file is out of date. Run \`${packageManager.exec} skuba format\` to update it.`, }; } @@ -92,13 +95,24 @@ export const refreshIgnoreFiles = async ( return { ok: !anyNeedChanging, fixable: anyNeedChanging, + annotations: results.flatMap( + ({ needsChange, filename, annotationMessage }) => + needsChange && annotationMessage + ? [ + { + path: filename, + message: annotationMessage, + }, + ] + : [], + ), }; }; export const tryRefreshIgnoreFiles = async ( mode: 'format' | 'lint', logger: Logger, -) => { +): Promise => { try { return await refreshIgnoreFiles(mode, logger); } catch (err) { @@ -108,6 +122,7 @@ export const tryRefreshIgnoreFiles = async ( return { ok: false, fixable: false, + annotations: [], }; } };