Skip to content

Commit 2951fe5

Browse files
committed
a
1 parent ef60977 commit 2951fe5

File tree

10 files changed

+237
-13
lines changed

10 files changed

+237
-13
lines changed

.github/workflows/validate.yml

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ jobs:
3535
cache: npm
3636
- name: Install dependencies
3737
run: npm ci
38-
- name: Validate
38+
- name: Lint
3939
run: npm run lint
4040

4141
format:
@@ -52,5 +52,27 @@ jobs:
5252
cache: npm
5353
- name: Install dependencies
5454
run: npm ci
55-
- name: Validate
56-
run: npm run check-format
55+
- name: Check format
56+
# Rewrite the prettier format into the GitHub Actions annotation format
57+
run: |
58+
npm run check-format
59+
60+
type-warnings:
61+
runs-on: ubuntu-latest
62+
steps:
63+
- name: Checkout
64+
uses: actions/checkout@v4
65+
with:
66+
persist-credentials: false
67+
- name: Install Node.js
68+
uses: actions/setup-node@v4
69+
with:
70+
node-version: 22
71+
cache: npm
72+
- name: Install dependencies
73+
run: npm ci
74+
- name: Generate type warnings
75+
run: node development/ci-generate-type-warnings.js
76+
env:
77+
PR_NUMBER: "${{ github.event.number }}"
78+
GH_REPO: "${{ github.repository }}"

development/builder.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -960,12 +960,13 @@ class Builder {
960960
async validate() {
961961
const errors = [];
962962
const build = await this.build();
963-
for (const [fileName, file] of Object.entries(build.files)) {
963+
for (const [outputFileName, file] of Object.entries(build.files)) {
964964
try {
965965
await file.validate();
966966
} catch (e) {
967967
errors.push({
968-
fileName,
968+
outputFileName,
969+
inputFilePath: file.sourcePath,
969970
error: e,
970971
});
971972
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import pathUtil from "node:path";
2+
import ts from "typescript";
3+
import { createAnnotation, getChangedFiles, isCI } from "./ci-interop.js";
4+
5+
/**
6+
* @fileoverview Generates CI annotations for type warnings in files modified by the
7+
* pull request. Standard TypeScript CLI will include all files and its output is
8+
* detected as errors, so not usable for us.
9+
*/
10+
11+
const check = async () => {
12+
const rootDir = pathUtil.join(import.meta.dirname, "..");
13+
14+
const changedFiles = Array.from(await getChangedFiles()).sort();
15+
const changedFilesAbsolute = changedFiles.map(f => pathUtil.join(rootDir, f));
16+
17+
console.log(`${changedFiles.size} changed files:`);
18+
console.log(Array.from(changedFiles).sort().join('\n'));
19+
console.log('');
20+
21+
const tsconfigPath = pathUtil.join(rootDir, "tsconfig.json");
22+
const commandLine = ts.getParsedCommandLineOfConfigFile(
23+
tsconfigPath,
24+
{},
25+
ts.sys
26+
);
27+
28+
const program = ts.createProgram({
29+
rootNames: commandLine.fileNames,
30+
options: commandLine.options,
31+
});
32+
const emitted = program.emit();
33+
34+
const diagnostics = [
35+
...ts.getPreEmitDiagnostics(program),
36+
...emitted.diagnostics,
37+
];
38+
39+
let numWarnings = 0;
40+
41+
for (const diagnostic of diagnostics) {
42+
if (!changedFilesAbsolute.includes(diagnostic.file.fileName)) {
43+
continue;
44+
}
45+
46+
const startPosition = ts.getLineAndCharacterOfPosition(
47+
diagnostic.file,
48+
diagnostic.start
49+
);
50+
const endPosition = ts.getLineAndCharacterOfPosition(
51+
diagnostic.file,
52+
diagnostic.start
53+
);
54+
// Won't be the most readable, but it's probably fine.
55+
const flattened = ts.flattenDiagnosticMessageText(
56+
diagnostic.messageText,
57+
" ",
58+
0
59+
);
60+
61+
numWarnings++;
62+
createAnnotation({
63+
type: "warning",
64+
file: diagnostic.file.fileName,
65+
title: "Type warning - may indicate a bug - ignore if no bug",
66+
onlyIfChanged: true,
67+
message: flattened,
68+
line: startPosition.line + 1,
69+
col: startPosition.character + 1,
70+
endLine: endPosition.line + 1,
71+
endCol: endPosition.character + 1,
72+
});
73+
}
74+
75+
console.log(`Warnings in changed files: ${numWarnings}`);
76+
console.log(`Warnings in all files: ${diagnostics.length}`);
77+
};
78+
79+
if (isCI()) {
80+
check();
81+
} else {
82+
console.error('This script is only intended to be used in CI. For development, use normal TypeScript CLI instead.')
83+
process.exit(1);
84+
}

development/ci-interop.js

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import pathUtil from "node:path";
2+
3+
/**
4+
* @fileoverview Various utilities related to integrating with CI.
5+
*/
6+
7+
// https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md#limitations
8+
export const MAX_ANNOTATIONS_PER_TYPE_PER_STEP = 10;
9+
export const MAX_ANNOTATIONS_PER_JOB = 50;
10+
11+
/**
12+
* @returns {boolean} true if running in CI (GitHub Actions, etc.)
13+
*/
14+
export const isCI = () => !!process.env.CI;
15+
16+
/**
17+
* Note: PR may have changed between when this CI job was queued and when this job runs.
18+
* So, don't use this for security-critical things where accuracy is a must.
19+
* @returns {Promise<Set<string>>} List of paths (relative to root without leading / or ./) changed by this PR.
20+
*/
21+
export const getChangedFiles = async () => {
22+
if (!isCI()) {
23+
return [];
24+
}
25+
26+
const prNumber = +process.env.PR_NUMBER;
27+
if (!prNumber) {
28+
throw new Error('Missing PR_NUMBER');
29+
}
30+
31+
const repo = process.env.GH_REPO;
32+
if (typeof repo !== 'string' || !repo.includes('/')) {
33+
throw new Error('Missing GH_REPO');
34+
}
35+
36+
const diffResponse = await fetch(`https://patch-diff.githubusercontent.com/raw/${repo}/pull/${prNumber}.diff`);
37+
const diffText = await diffResponse.text();
38+
const fileMatches = [...diffText.matchAll(/^(?:---|\+\+\+) [ab]\/(.+)$/gm)]
39+
.map(match => match[1]);
40+
41+
return new Set(fileMatches);
42+
};
43+
44+
/**
45+
* @typedef Annotation
46+
* @property {'notice'|'warning'|'error'} type
47+
* @property {string} file Absolute path to file or relative from repository root
48+
* @property {string} title
49+
* @property {string} message
50+
* @property {number} [line] 1-indexed
51+
* @property {number} [col] 1-indexed
52+
* @property {number} [endLine] 1-indexed
53+
* @property {number} [endCol] 1-indexed
54+
*/
55+
56+
/**
57+
* @param {Annotation} annotation
58+
*/
59+
export const createAnnotation = (annotation) => {
60+
const rootDir = pathUtil.join(import.meta.dirname, "..");
61+
const relativeFileName = pathUtil.relative(rootDir, annotation.file);
62+
63+
// https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands
64+
65+
let output = "";
66+
output += `::${annotation.type} `;
67+
output += `file=${relativeFileName}`;
68+
69+
// Documentation says line number is not required, but in practice that gets interpreted as
70+
// line 0 and ends up not showing up. So, we'll default to line 1.
71+
if (typeof annotation.line === "number") {
72+
output += `,line=${annotation.line}`;
73+
} else {
74+
output += ',line=1';
75+
}
76+
77+
// These are all actually optional.
78+
if (typeof annotation.col === "number") {
79+
output += `,col=${annotation.col}`;
80+
}
81+
if (typeof annotation.endLine === "number") {
82+
output += `,endLine=${annotation.endLine}`;
83+
}
84+
if (typeof annotation.endCol === "number") {
85+
output += `,endCol=${annotation.endCol}`;
86+
}
87+
88+
output += `,title=${annotation.title}::`;
89+
output += annotation.message;
90+
91+
console.log(output);
92+
};

development/colors.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,6 @@ const color = (i) => (enableColor ? i : "");
44
export const RESET = color("\x1b[0m");
55
export const BOLD = color("\x1b[1m");
66
export const RED = color("\x1b[31m");
7+
export const YELLOW = color("\x1b[33m");
78
export const GREEN = color("\x1b[32m");
9+
export const BLUE = color("\x1b[34m");

development/validate.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
import pathUtil from 'node:path';
12
import Builder from "./builder.js";
3+
import { createAnnotation, isCI } from "./ci-interop.js";
24
import * as Colors from "./colors.js";
35

46
const builder = new Builder("production");
57
const errors = await builder.validate();
68

9+
const rootDir = pathUtil.join(import.meta.dirname, '..');
10+
711
if (errors.length === 0) {
812
console.log(
913
`${Colors.GREEN}${Colors.BOLD}Validation checks passed.${Colors.RESET}`
@@ -15,10 +19,23 @@ if (errors.length === 0) {
1519
errors.length === 1 ? "file" : "files"
1620
} failed validation.${Colors.RESET}`
1721
);
22+
1823
console.error("");
19-
for (const { fileName, error } of errors) {
20-
console.error(`${Colors.BOLD}${fileName}${Colors.RESET}: ${error}`);
24+
25+
for (const { outputFileName, inputFilePath, error } of errors) {
26+
const displayName = inputFilePath ? pathUtil.relative(rootDir, inputFilePath) : outputFileName;
27+
console.error(`${Colors.BOLD}${displayName}${Colors.RESET}: ${error}`);
28+
29+
if (isCI() && inputFilePath) {
30+
createAnnotation({
31+
type: "error",
32+
file: inputFilePath,
33+
title: "Validation error",
34+
message: error,
35+
});
36+
}
2137
}
22-
console.error(``);
38+
39+
console.error("");
2340
process.exit(1);
2441
}

0 commit comments

Comments
 (0)