Skip to content

Commit

Permalink
Include internal lints in annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
AaronMoat committed Jan 7, 2024
1 parent 0063ea6 commit 305b8b6
Show file tree
Hide file tree
Showing 19 changed files with 253 additions and 104 deletions.
42 changes: 26 additions & 16 deletions src/cli/__snapshots__/lint.int.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,17 @@ Prettier │ d.js
Prettier │ package.json
tsc │ TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc │ tsc --noEmit exited with code 0
ESLint, Prettier found issues that require triage.
skuba │ Processed skuba lints in <random>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:
Expand Down Expand Up @@ -76,12 +73,28 @@ Prettier │ Processed 6 files in <random>s.
tsc │ TSFILE: <random>/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 <random>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
<random>/package.json skuba has patches to apply. Run yarn skuba format to run them.
\`\`\`
",
]
`;
exports[`ok --debug 1`] = `
"
Expand Down Expand Up @@ -186,20 +199,17 @@ Prettier │ a/a/a.ts
tsc │ d.js(1,1): error TS1128: Declaration or statement expected.
tsc │ TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc │ tsc --noEmit exited with code 2
ESLint, Prettier, tsc found issues that require triage.
skuba │ Processed skuba lints in <random>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:
Expand Down
7 changes: 7 additions & 0 deletions src/cli/configure/upgrade/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
});
});

Expand Down
25 changes: 20 additions & 5 deletions src/cli/configure/upgrade/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string[]> => {
Expand Down Expand Up @@ -38,7 +39,10 @@ const resolvePatch = async (patch: string): Promise<Patch> => {
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<InternalLintResult> => {
const [currentVersion, manifest] = await Promise.all([
getSkubaVersion(),
getConsumerManifest(),
Expand Down Expand Up @@ -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(
Expand All @@ -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...');
Expand Down
8 changes: 6 additions & 2 deletions src/cli/lint/annotate/buildkite/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> => {
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',
});
Expand Down
16 changes: 16 additions & 0 deletions src/cli/lint/annotate/buildkite/internal.ts
Original file line number Diff line number Diff line change
@@ -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'),
),
]
: [];
15 changes: 15 additions & 0 deletions src/cli/lint/annotate/github/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -52,6 +53,11 @@ const prettierOutput: PrettierOutput = {
},
};

const internalOutput: InternalLintResult = {
ok: false,
fixable: false,
};

const tscOk = false;
const mockOutput = jest.fn<string, any>();

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -174,6 +184,7 @@ it('should combine all the annotations into an array for the check run', async (
];

await createGitHubAnnotations(
internalOutput,
eslintOutput,
prettierOutput,
tscOk,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion src/cli/lint/annotate/github/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.'
Expand Down
14 changes: 14 additions & 0 deletions src/cli/lint/annotate/github/internal.ts
Original file line number Diff line number Diff line change
@@ -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',
}));
12 changes: 10 additions & 2 deletions src/cli/lint/annotate/index.ts
Original file line number Diff line number Diff line change
@@ -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<void> => {
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,
),
]);
};
Loading

0 comments on commit 305b8b6

Please sign in to comment.