diff --git a/.github/workflows/node-test.yml b/.github/workflows/node-test.yml index 9e3e258232b..ac0ffcce097 100644 --- a/.github/workflows/node-test.yml +++ b/.github/workflows/node-test.yml @@ -37,7 +37,7 @@ jobs: - run: npm i -g npm@9.5 - run: npm ci - - run: npm run lint:changed-files + - run: npm run lint:changed-files -- --only-changed-lines - run: npm run lint working-directory: firebase-vscode diff --git a/scripts/lint-changed-files.ts b/scripts/lint-changed-files.ts index 07585272bbb..62760f9b532 100644 --- a/scripts/lint-changed-files.ts +++ b/scripts/lint-changed-files.ts @@ -3,58 +3,272 @@ * working branch and runs the linter on them. */ -import { execSync } from "child_process"; -import { extname, resolve } from "path"; +import { spawn, spawnSync } from "child_process"; +import { extname, relative, resolve } from "path"; +import * as readline from "readline"; + +interface EslintInstance { + lintFiles(files: string[]): Promise; + loadFormatter(name?: string): Promise<{ + format(results: EslintResult[]): string; + }>; +} + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { ESLint } = require("eslint") as { + ESLint: { + new (options?: { fix?: boolean }): EslintInstance; + outputFixes(results: EslintResult[]): Promise; + }; +}; const root = resolve(__dirname, ".."); -const deletedFileRegex = /^D\s.+$/; -const extensionsToCheck = [".js", ".ts"]; +class LintError extends Error { + constructor(public mode: "filtered" | "unfiltered") { + super(`Lint errors found (${mode})`); + this.name = "LintError"; + } +} -/** - * Returns the last element of an array. - * @param arr any array. - * @return the last element of the array. - */ -function last(arr: Array): T { - return arr[arr.length - 1]; +interface EslintMessage { + ruleId: string; + severity: number; + message: string; + line: number; + column: number; } -/** - * Main function of the script. - */ -function main(): void { +interface EslintResult { + filePath: string; + messages: EslintMessage[]; + errorCount: number; + warningCount: number; +} + +function getChangedFiles(cmpBranch: string): { files: string[]; ignored: string[] } { const files: string[] = []; const ignoredFiles: string[] = []; + const extensionsToCheck = [".js", ".ts"]; - const otherArgs = process.argv.slice(2); - - let cmpBranch = "main"; - if (process.env.CI) { - cmpBranch = "origin/main"; + const gitDiff = spawnSync("git", ["diff", "--diff-filter=d", "--name-only", cmpBranch], { + cwd: root, + }); + if (gitDiff.status !== 0) { + throw new Error(`git diff failed: ${gitDiff.stderr.toString()}`); } + const gitOutput = gitDiff.stdout.toString().trim(); - const gitOutput = execSync(`git diff --name-status ${cmpBranch}`, { cwd: root }) - .toString() - .trim(); + if (!gitOutput) { + return { files, ignored: ignoredFiles }; + } for (const line of gitOutput.split("\n")) { - const l = line.trim(); - if (deletedFileRegex.test(l)) { - continue; - } - const entries = l.split(/\s/); - const file = last(entries); + const file = line.trim(); + if (!file) continue; if (extensionsToCheck.includes(extname(file))) { files.push(file); } else { ignoredFiles.push(file); } } + return { files, ignored: ignoredFiles }; +} + +async function getChangedLines( + cmpBranch: string, + files: string[], +): Promise>> { + const args = ["diff", "-U0", cmpBranch]; + if (files.length > 0) { + args.push("--", ...files); + } + + const git = spawn("git", args, { cwd: root }); + const rl = readline.createInterface({ + input: git.stdout, + terminal: false, + }); + + const changedLinesByFile: Record> = {}; + let currentFile = ""; + + for await (const line of rl) { + if (line.startsWith("diff --git")) { + currentFile = ""; + } else if (line.startsWith("+++ b/")) { + currentFile = line.substring(6); + changedLinesByFile[currentFile] = new Set(); + } else if (line.startsWith("@@ ")) { + const match = /^@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,(\d+))?\s+@@/.exec(line); + if (match && currentFile) { + const start = parseInt(match[1], 10); + const length = match[2] ? parseInt(match[2], 10) : 1; + for (let i = 0; i < length; i++) { + changedLinesByFile[currentFile].add(start + i); + } + } + } + } + + return new Promise((resolvePromise, reject) => { + git.on("close", (code) => { + if (code === 0) { + resolvePromise(changedLinesByFile); + } else { + reject(new Error(`git diff failed with code ${code ?? "unknown"}`)); + } + }); + }); +} + +async function runLint( + files: string[], + otherArgs: string[], +): Promise<{ results: EslintResult[]; eslint: EslintInstance }> { + const fix = otherArgs.includes("--fix"); + const eslint = new ESLint({ fix }); + const results = await eslint.lintFiles(files); + + if (fix) { + await ESLint.outputFixes(results); + } + return { results, eslint }; +} + +async function reportStandard( + results: EslintResult[], + eslint: EslintInstance, + quiet: boolean, + maxWarnings: number, +): Promise { + let processedResults = results; + if (quiet) { + processedResults = results + .map((r) => ({ + ...r, + messages: r.messages.filter((m) => m.severity === 2), + errorCount: r.messages.filter((m) => m.severity === 2).length, + warningCount: 0, + })) + .filter((r) => r.messages.length > 0 || r.errorCount > 0); + } + + const formatter = await eslint.loadFormatter("stylish"); + const resultText = formatter.format(processedResults); + console.log(resultText); + + const errorCount = processedResults.reduce( + (acc: number, r: EslintResult) => acc + r.errorCount, + 0, + ); + const warningCount = processedResults.reduce( + (acc: number, r: EslintResult) => acc + r.warningCount, + 0, + ); + + if (errorCount > 0) { + throw new LintError("unfiltered"); + } + + if (maxWarnings >= 0 && warningCount > maxWarnings) { + console.error( + `\nFound ${warningCount} warnings, which exceeds the max-warnings limit of ${maxWarnings}.`, + ); + throw new LintError("unfiltered"); + } +} + +async function reportFiltered( + results: EslintResult[], + eslint: EslintInstance, + changedLinesByFile: Record>, + quiet: boolean, + maxWarnings: number, +): Promise { + const filteredResults: EslintResult[] = results + .map((r) => { + const relPath = relative(root, r.filePath); + const changedLines = changedLinesByFile[relPath] || new Set(); + const messages = r.messages.filter((msg) => { + const lineMatch = changedLines.has(msg.line); + const quietMatch = !quiet || msg.severity === 2; + return lineMatch && quietMatch; + }); + return { + ...r, + messages, + errorCount: messages.filter((m) => m.severity === 2).length, + warningCount: messages.filter((m) => m.severity === 1).length, + }; + }) + .filter((r) => r.messages.length > 0); + + const formatter = await eslint.loadFormatter("stylish"); + const resultText = formatter.format(filteredResults); + if (resultText) { + console.log(resultText); + } + + const errorCount = filteredResults.reduce((acc, r) => acc + r.errorCount, 0); + const warningCount = filteredResults.reduce((acc, r) => acc + r.warningCount, 0); + + if (errorCount > 0) { + throw new LintError("filtered"); + } + + if (maxWarnings >= 0 && warningCount > maxWarnings) { + console.error( + `\nFound ${warningCount} warnings on changed lines, which exceeds the max-warnings limit of ${maxWarnings}.`, + ); + throw new LintError("filtered"); + } + + if (filteredResults.length > 0) { + console.log(`\nNo errors found on changed lines (found ${warningCount} warnings).`); + } else { + console.log("\nClean on changed lines."); + } +} + +/** + * Main function of the script. + */ +async function main(): Promise { + const args = process.argv.slice(2); + const onlyChangedLines = args.includes("--only-changed-lines"); + + let quiet = false; + let maxWarnings = -1; + const otherArgs: string[] = []; + + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (arg === "--only-changed-lines") { + continue; + } else if (arg === "--quiet") { + quiet = true; + } else if (arg === "--max-warnings") { + const nextArg = args[i + 1]; + if (nextArg && /^\d+$/.test(nextArg)) { + maxWarnings = parseInt(nextArg, 10); + i++; + } else { + console.error("Error: --max-warnings requires a numeric value."); + process.exit(1); + } + } else { + otherArgs.push(arg); + } + } + + const cmpBranch = process.env.CI ? "origin/main" : "main"; - if (ignoredFiles.length) { + const { files, ignored } = getChangedFiles(cmpBranch); + + if (ignored.length) { console.log("Ignoring changed files:"); - for (const f of ignoredFiles) { + for (const f of ignored) { console.log(` - ${f}`); } console.log(); @@ -65,16 +279,20 @@ function main(): void { return; } - try { - execSync(`eslint ${otherArgs.join(" ")} ${files.join(" ")}`, { - cwd: root, - stdio: ["pipe", process.stdout, process.stderr], - }); - } catch (e: any) { - console.error("eslint failed, see errors above."); - console.error(); - process.exit(e.status); + const { results, eslint } = await runLint(files, otherArgs); + + if (onlyChangedLines) { + const changedLines = await getChangedLines(cmpBranch, files); + await reportFiltered(results, eslint, changedLines, quiet, maxWarnings); + } else { + await reportStandard(results, eslint, quiet, maxWarnings); } } -main(); +main().catch((e) => { + if (e instanceof LintError) { + process.exit(1); + } + console.error("Script failed:", e); + process.exit(1); +});