diff --git a/.github/workflows/lintdiff-test.yaml b/.github/workflows/lintdiff-test.yaml index fa0a09658b4a..3cce648b57b4 100644 --- a/.github/workflows/lintdiff-test.yaml +++ b/.github/workflows/lintdiff-test.yaml @@ -31,4 +31,4 @@ jobs: uses: ./.github/workflows/_reusable-eng-tools-test.yaml with: package: lint-diff - lint: false + lint: true diff --git a/eng/tools/lint-diff/eslint.config.js b/eng/tools/lint-diff/eslint.config.js new file mode 100644 index 000000000000..b73f157aa435 --- /dev/null +++ b/eng/tools/lint-diff/eslint.config.js @@ -0,0 +1,30 @@ +// Keep in sync with .github/shared/eslint.config.js + +import eslint from "@eslint/js"; +import { defineConfig } from "eslint/config"; +import globals from "globals"; +import tseslint from "typescript-eslint"; + +/** @type {import('eslint').Linter.Config[]} */ +export default defineConfig( + eslint.configs.recommended, + tseslint.configs.recommendedTypeChecked, + { + languageOptions: { + // we only run in node, not browser + globals: globals.node, + // required to use tseslint.configs.recommendedTypeChecked + parserOptions: { + projectService: { + allowDefaultProject: ["*.js", "cmd/*.js"], + }, + // ensures the tsconfig path resolves relative to this file + // default is process.cwd() when running eslint, which may be incorrect + tsconfigRootDir: import.meta.dirname, + }, + }, + }, + { + ignores: ["coverage/**", "dist/**"], + }, +); diff --git a/eng/tools/lint-diff/package.json b/eng/tools/lint-diff/package.json index 40e378ee7f43..0d76dcee458f 100644 --- a/eng/tools/lint-diff/package.json +++ b/eng/tools/lint-diff/package.json @@ -8,10 +8,12 @@ }, "scripts": { "build": "tsc --build", + "check": "npm run build && npm run lint && npm run format:check && npm run test:ci", "clean": "rm -rf dist && rm -rf node_modules", "format": "prettier . --ignore-path ../.prettierignore --write", "format:check": "prettier . --ignore-path ../.prettierignore --check", "format:check:ci": "prettier . --ignore-path ../.prettierignore --check --log-level debug", + "lint": "cross-env DEBUG=eslint:eslint eslint", "test": "vitest", "test:ci": "vitest run --coverage --reporter=verbose" }, @@ -30,14 +32,17 @@ "marked": "^17.0.0" }, "devDependencies": { + "@eslint/js": "^9.22.0", "@types/deep-eql": "^4.0.2", "@types/node": "^18.19.31", "@vitest/coverage-v8": "^3.0.2", + "eslint": "^9.22.0", "execa": "^9.5.2", "memfs": "^4.17.0", "prettier": "~3.6.2", "prettier-plugin-organize-imports": "^4.2.0", "typescript": "~5.9.2", + "typescript-eslint": "^8.45.0", "vitest": "^3.2.4" } } diff --git a/eng/tools/lint-diff/src/correlateResults.ts b/eng/tools/lint-diff/src/correlateResults.ts index f69ed1eb5ece..9fa18ee3e30a 100644 --- a/eng/tools/lint-diff/src/correlateResults.ts +++ b/eng/tools/lint-diff/src/correlateResults.ts @@ -40,7 +40,7 @@ export async function correlateRuns( const beforeReadme = new Readme(beforeReadmePath); const defaultTag = await getDefaultTag(beforeReadme); if (!defaultTag) { - throw new Error(`No default tag found for readme ${readme} in before state`); + throw new Error(`No default tag found for readme ${readme.toString()} in before state`); } const beforeDefaultTagCandidates = beforeChecks.filter( (r) => relative(beforePath, r.readme.path) === readmePathRelative && r.tag === defaultTag, @@ -69,7 +69,9 @@ export async function correlateRuns( }); continue; } else if (beforeReadmeCandidate.length > 1) { - throw new Error(`Multiple before candidates found for key ${key} using readme ${readme}`); + throw new Error( + `Multiple before candidates found for key ${key} using readme ${readme.toString()}`, + ); } } @@ -148,7 +150,7 @@ export function getLintDiffViolations(runResult: AutorestRunResult): LintDiffVio continue; } - const result = JSON.parse(line.trim()); + const result = JSON.parse(line.trim()) as { code: string }; if (result.code == undefined) { // Results without a code can be assumed to be fatal errors. Set the code // to "FATAL" @@ -213,7 +215,7 @@ export function isSameSources(a: Source[], b: Source[]) { return true; } -export function arrayIsEqual(a: any[], b: any[]) { +export function arrayIsEqual(a: unknown[], b: unknown[]) { if (a.length !== b.length) { return false; } diff --git a/eng/tools/lint-diff/src/generateReport.ts b/eng/tools/lint-diff/src/generateReport.ts index bb2fab806137..7f79fd868c2c 100644 --- a/eng/tools/lint-diff/src/generateReport.ts +++ b/eng/tools/lint-diff/src/generateReport.ts @@ -148,7 +148,7 @@ export async function generateAutoRestErrorReport( console.error("LintDiff detected AutoRest errors"); outputMarkdown += "**AutoRest errors:**\n\n"; for (const { result, errors } of autoRestErrors) { - console.log(`AutoRest errors for ${result.readme} (${result.tag})`); + console.log(`AutoRest errors for ${result.readme.toString()} (${result.tag})`); const readmePath = relative(result.rootPath, result.readme.path); diff --git a/eng/tools/lint-diff/src/lint-diff.ts b/eng/tools/lint-diff/src/lint-diff.ts index 0f3107a88c8a..f60009839628 100644 --- a/eng/tools/lint-diff/src/lint-diff.ts +++ b/eng/tools/lint-diff/src/lint-diff.ts @@ -1,6 +1,6 @@ import { SpecModelError } from "@azure-tools/specs-shared/spec-model-error"; import { writeFile } from "node:fs/promises"; -import { parseArgs, ParseArgsConfig } from "node:util"; +import { inspect, parseArgs, ParseArgsConfig } from "node:util"; import { correlateRuns } from "./correlateResults.js"; import { generateAutoRestErrorReport, generateLintDiffReport } from "./generateReport.js"; import { getRunList } from "./processChanges.js"; @@ -67,13 +67,13 @@ export async function main() { // TODO: Handle trailing slashes properly if (!beforeArg || !(await pathExists(beforeArg as string))) { validArgs = false; - console.log(`--before must be a valid path. Value passed: ${beforeArg || ""}`); + console.log(`--before must be a valid path. Value passed: ${inspect(beforeArg) || ""}`); } // TODO: Handle trailing slashes properly if (!afterArg || !(await pathExists(afterArg as string))) { validArgs = false; - console.log(`--after must be a valid path. Value passed: ${afterArg || ""}`); + console.log(`--after must be a valid path. Value passed: ${inspect(afterArg) || ""}`); } if (!changedFilesPath || !(await pathExists(changedFilesPath as string))) { @@ -121,7 +121,7 @@ async function runLintDiff( } catch (error) { if (error instanceof SpecModelError) { console.log("\n❌ Error building Spec Model from changed file list:"); - console.log(`${error}`); + console.log(`${inspect(error)}`); process.exitCode = 1; return; diff --git a/eng/tools/lint-diff/src/lintdiff-types.ts b/eng/tools/lint-diff/src/lintdiff-types.ts index 6515cbe3c760..787003e67c09 100644 --- a/eng/tools/lint-diff/src/lintdiff-types.ts +++ b/eng/tools/lint-diff/src/lintdiff-types.ts @@ -14,7 +14,7 @@ export type AutorestRunResult = { export interface AutoRestMessage { level: "information" | "warning" | "error" | "debug" | "verbose" | "fatal"; - code?: any; + code?: unknown; message: string; readme?: string; tag?: string; diff --git a/eng/tools/lint-diff/src/markdown-utils.ts b/eng/tools/lint-diff/src/markdown-utils.ts index 96a729064342..f8d1de1e8348 100644 --- a/eng/tools/lint-diff/src/markdown-utils.ts +++ b/eng/tools/lint-diff/src/markdown-utils.ts @@ -1,6 +1,7 @@ import { Readme } from "@azure-tools/specs-shared/readme"; import { kebabCase } from "change-case"; -import { marked } from "marked"; +import { marked, Tokens } from "marked"; +import { inspect } from "util"; export enum MarkdownType { Arm = "arm", @@ -88,9 +89,9 @@ export async function getRelatedArmRpcFromDoc(ruleName: string): Promise= 1 && - token.text.trim().toLowerCase() === "related arm guideline code" + (token as Tokens.Heading).depth >= 1 && + (token as Tokens.Heading).text.trim().toLowerCase() === "related arm guideline code" ) { // The next token should be a list const next = tokens[i + 1]; - if (next && next.type === "list" && Array.isArray(next.items)) { - for (const item of next.items) { + if (next && next.type === "list") { + for (const item of (next as Tokens.List).items) { // item.text may contain comma-separated codes if (typeof item.text === "string") { rpcRules.push(...item.text.split(",").map((c: string) => c.trim())); diff --git a/eng/tools/lint-diff/src/processChanges.ts b/eng/tools/lint-diff/src/processChanges.ts index ea2400474d46..997d7487f078 100644 --- a/eng/tools/lint-diff/src/processChanges.ts +++ b/eng/tools/lint-diff/src/processChanges.ts @@ -82,7 +82,7 @@ export async function buildState( // Get affected services from changed files // e.g. specification/service1/readme.md -> specification/service1 - const affectedServiceDirectories = await getAffectedServices(existingChangedFiles); + const affectedServiceDirectories = getAffectedServices(existingChangedFiles); // Build service models of affected services const specModels = new Map(); @@ -233,10 +233,10 @@ export async function readFileList(changedFilesPath: string): Promise * @param changedFiles a list of changed files * @returns A list of "services" that are affected by the changed files */ -export async function getAffectedServices(changedFiles: string[]) { +export function getAffectedServices(changedFiles: string[]) { const affectedServices = new Set(); for (const file of changedFiles) { - const service = await getService(file); + const service = getService(file); if (service) { affectedServices.add(service); } diff --git a/eng/tools/lint-diff/src/runChecks.ts b/eng/tools/lint-diff/src/runChecks.ts index ec7cc6807197..6e99ddcfa881 100644 --- a/eng/tools/lint-diff/src/runChecks.ts +++ b/eng/tools/lint-diff/src/runChecks.ts @@ -1,4 +1,4 @@ -import { ExecError, execNpmExec, isExecError } from "@azure-tools/specs-shared/exec"; +import { execNpmExec, isExecError } from "@azure-tools/specs-shared/exec"; import { debugLogger } from "@azure-tools/specs-shared/logger"; import { join } from "path"; @@ -22,14 +22,14 @@ export async function runChecks( for (const [readme, tags] of runList.entries()) { const changedFilePath = join(path, readme); - let openApiType = await getOpenapiType(tags.readme); + const openApiType = await getOpenapiType(tags.readme); // From momentOfTruth.ts:executeAutoRestWithLintDiff // This is a quick workaround for https://github.com/Azure/azure-sdk-tools/issues/6549 // We override the openapi-subtype with the value of openapi-type, // to prevent LintDiff from reading openapi-subtype from the AutoRest config file (README) // and overriding openapi-type with it. - let openApiSubType = openApiType; + const openApiSubType = openApiType; // If the tags array is empty run the loop once but with a null tag const coalescedTags = tags.changedTags?.size ? [...tags.changedTags] : [null]; @@ -77,16 +77,15 @@ export async function runChecks( throw error; } - const execError = error as ExecError; lintDiffResult = { autorestCommand, rootPath: path, readme: tags.readme, tag: tag ? tag : "", openApiType, - error: execError, - stdout: execError.stdout || "", - stderr: execError.stderr || "", + error, + stdout: error.stdout || "", + stderr: error.stderr || "", } as AutorestRunResult; logAutorestExecutionErrors(lintDiffResult); diff --git a/eng/tools/lint-diff/src/util.ts b/eng/tools/lint-diff/src/util.ts index e9c629ca26de..55f0d4678c86 100644 --- a/eng/tools/lint-diff/src/util.ts +++ b/eng/tools/lint-diff/src/util.ts @@ -31,7 +31,9 @@ export async function pathExists(path: string): Promise { /* v8 ignore start */ export async function getDependencyVersion(dependenciesDir: string): Promise { const packageJsonPath = join(dependenciesDir, "package.json"); - const packageJson = JSON.parse(await readFile(packageJsonPath, { encoding: "utf-8" })); + const packageJson = JSON.parse(await readFile(packageJsonPath, { encoding: "utf-8" })) as { + version?: string; + }; const version = packageJson.version; if (!version) { throw new Error(`Version not found in package.json at ${packageJsonPath}`); diff --git a/eng/tools/lint-diff/test/correlateResults.test.ts b/eng/tools/lint-diff/test/correlateResults.test.ts index 7f06982fe783..c46db1a0626d 100644 --- a/eng/tools/lint-diff/test/correlateResults.test.ts +++ b/eng/tools/lint-diff/test/correlateResults.test.ts @@ -297,7 +297,7 @@ describe("isSameSources", () => { }); }); -describe("getLintDiffViolations", async () => { +describe("getLintDiffViolations", () => { function createRunResult(stdout: string, stderr: string = ""): AutorestRunResult { return { rootPath: "string", @@ -359,7 +359,7 @@ describe("getLintDiffViolations", async () => { }); describe("arrayIsEqual", () => { - test("returns true for equal arrays", async () => { + test("returns true for equal arrays", () => { const a = ["a", "b", "c"]; const b = ["a", "b", "c"]; @@ -367,7 +367,7 @@ describe("arrayIsEqual", () => { expect(result).toEqual(true); }); - test("returns false for different arrays", async () => { + test("returns false for different arrays", () => { const a = ["a", "b", "c"]; const b = ["a", "b", "d"]; @@ -375,7 +375,7 @@ describe("arrayIsEqual", () => { expect(result).toEqual(false); }); - test("returns false for different lengths", async () => { + test("returns false for different lengths", () => { const a = ["a", "b", "c"]; const b = ["a", "b"]; @@ -383,7 +383,7 @@ describe("arrayIsEqual", () => { expect(result).toEqual(false); }); - test("returns true for empty arrays", async () => { + test("returns true for empty arrays", () => { const a: string[] = []; const b: string[] = []; @@ -391,7 +391,7 @@ describe("arrayIsEqual", () => { expect(result).toEqual(true); }); - test("returns true for equal arrays with different types", async () => { + test("returns true for equal arrays with different types", () => { const a = ["a", 1, "c"]; const b = ["a", 1, "c"]; diff --git a/eng/tools/lint-diff/test/generateReport.test.ts b/eng/tools/lint-diff/test/generateReport.test.ts index cb0f56a32394..c4330d202af2 100644 --- a/eng/tools/lint-diff/test/generateReport.test.ts +++ b/eng/tools/lint-diff/test/generateReport.test.ts @@ -23,7 +23,7 @@ import { isWindows } from "./test-util.js"; import { vol } from "memfs"; vi.mock("node:fs/promises", async () => { - const memfs = (await vi.importActual("memfs")) as typeof import("memfs"); + const memfs = await vi.importActual("memfs"); return { ...memfs.fs.promises, }; diff --git a/eng/tools/lint-diff/test/processChanges.fs.test.ts b/eng/tools/lint-diff/test/processChanges.fs.test.ts index 67426b4399ef..11709671fb03 100644 --- a/eng/tools/lint-diff/test/processChanges.fs.test.ts +++ b/eng/tools/lint-diff/test/processChanges.fs.test.ts @@ -6,7 +6,7 @@ import { readFileList } from "../src/processChanges.js"; // These tests are in a separate module because fs mocking is difficult to undo vi.mock("node:fs/promises", async () => { - const memfs = (await vi.importActual("memfs")) as typeof import("memfs"); + const memfs = await vi.importActual("memfs"); return { ...memfs.fs.promises, }; diff --git a/eng/tools/lint-diff/test/processChanges.test.ts b/eng/tools/lint-diff/test/processChanges.test.ts index 7224e4978994..7afa2b4230bf 100644 --- a/eng/tools/lint-diff/test/processChanges.test.ts +++ b/eng/tools/lint-diff/test/processChanges.test.ts @@ -14,20 +14,20 @@ import { resolve } from "node:path"; import { isWindows } from "./test-util.js"; describe("getAffectedServices", () => { - test.skipIf(isWindows())("returns single service with multiple files", async () => { + test.skipIf(isWindows())("returns single service with multiple files", () => { const changedFiles = ["specification/service1/file1.json", "specification/service1/file2.json"]; - const affectedServices = await getAffectedServices(changedFiles); + const affectedServices = getAffectedServices(changedFiles); expect(affectedServices).toEqual(new Set(["specification/service1"])); }); - test.skipIf(isWindows())("returns multiple services", async () => { + test.skipIf(isWindows())("returns multiple services", () => { const changedFiles = [ "specification/service1/file1.json", "specification/service1/file2.json", "specification/service2/file1.json", ]; - const affectedServices = await getAffectedServices(changedFiles); + const affectedServices = getAffectedServices(changedFiles); expect(affectedServices).toEqual( new Set(["specification/service1", "specification/service2"]), @@ -36,26 +36,23 @@ describe("getAffectedServices", () => { }); describe("getService", () => { - test.skipIf(isWindows())("returns service name from file path", async () => { + test.skipIf(isWindows())("returns service name from file path", () => { const filePath = "specification/service1/file1.json"; - const serviceName = await getService(filePath); + const serviceName = getService(filePath); expect(serviceName).toEqual("specification/service1"); }); - test.skipIf(isWindows())( - "returns service name from file path with leading separator", - async () => { - const filePath = "/specification/service1/file1.json"; - const serviceName = await getService(filePath); + test.skipIf(isWindows())("returns service name from file path with leading separator", () => { + const filePath = "/specification/service1/file1.json"; + const serviceName = getService(filePath); - expect(serviceName).toEqual("specification/service1"); - }, - ); + expect(serviceName).toEqual("specification/service1"); + }); - test("throws when file path does not contain enough pieces to assemble a service name", async () => { + test("throws when file path does not contain enough pieces to assemble a service name", () => { const filePath = "file1.json"; - await expect(() => getService(filePath)).toThrow("Could not find service for file path"); + expect(() => getService(filePath)).toThrow("Could not find service for file path"); }); }); @@ -139,7 +136,7 @@ describe("reconcileChangedFilesAndTags", () => { describe("getChangedSwaggers", () => { test("returns an empty set if no swaggers are changed", async () => { - expect( + await expect( getChangedSwaggers( "test/fixtures/getChangedSwaggers/before", "test/fixtures/getChangedSwaggers/after", @@ -229,7 +226,7 @@ describe("buildState", () => { "specification/edit-in-place/readme.md", { changedTags: new Set(), - readme: expect.any(Readme), + readme: expect.any(Readme) as unknown, }, ], ]), @@ -242,12 +239,12 @@ describe("buildState", () => { }); test("does not throw if a file is missing", async () => { - expect(() => + await expect( buildState( ["specification/edit-in-place/data-plane/does-not-exist.json"], "test/fixtures/buildState/", ), - ).not.toThrow(); + ).resolves.not.toThrow(); }); test.skipIf(isWindows())("does not include readme files that has no input-file:", async () => { diff --git a/eng/tools/lint-diff/test/runChecks.test.ts b/eng/tools/lint-diff/test/runChecks.test.ts index 5dd16b881dfc..86dcebf07720 100644 --- a/eng/tools/lint-diff/test/runChecks.test.ts +++ b/eng/tools/lint-diff/test/runChecks.test.ts @@ -24,7 +24,7 @@ vi.mock(import("../src/markdown-utils.js"), async (importOriginal) => { }; }); -import { execNpmExec } from "@azure-tools/specs-shared/exec"; +import { ExecError, execNpmExec } from "@azure-tools/specs-shared/exec"; import { Readme } from "@azure-tools/specs-shared/readme"; import { AutorestRunResult, ReadmeAffectedTags } from "../src/lintdiff-types.js"; import { getAutorestErrors, runChecks } from "../src/runChecks.js"; @@ -68,10 +68,10 @@ describe("runChecks", () => { test("error path populates error, stdout, stderr", async () => { // Consturct an error object that will return true when passed to isExecError - const err = new Error(); - (err as any).stdout = "s"; - (err as any).stderr = "e"; - (err as any).code = 1; + const err: ExecError = new Error(); + err.stdout = "s"; + err.stderr = "e"; + err.code = 1; (execNpmExec as Mock).mockRejectedValue(err); const runList = new Map([ @@ -94,7 +94,7 @@ describe("runChecks", () => { const runList = new Map([ ["readme.md", { readme: new Readme(""), changedTags: new Set(["tag1", "tag2"]) }], ]); - expect(runChecks("root", runList)).rejects.toThrow(); + await expect(runChecks("root", runList)).rejects.toThrow(); }); }); @@ -107,7 +107,7 @@ describe("getAutorestErrors", () => { {"level":"error","message":"stack: Error: There are multiple operations defined for \\n 'get: /providers/Microsoft.PortalServices/operations'.\\n\\n You are probably trying to use an input with multiple API versions with an autorest V2 generator, and that will not work. \\n at NewComposer.visitPath (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:4371:23)\\n at NewComposer.visitPaths (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:4357:22)\\n at NewComposer.process (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:4305:26)\\n at NewComposer.runProcess (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:16339:28)\\n at NewComposer.getOutput (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:16259:9)\\n at compose (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:4624:56)\\n at ScheduleNode (/home/cloudtest/.autorest/@autorest_core@3.10.4/node_modules/@autorest/core/dist/src_lib_autorest-core_ts.js:1351:29)"} {"level":"error","message":"Autorest completed with an error. If you think the error message is unclear, or is a bug, please declare an issues at https://github.com/Azure/autorest/issues with the error message you are seeing."}`; - const runResult = { stdout: lines, stderr: "" } as any; + const runResult = { stdout: lines, stderr: "" } as AutorestRunResult; const errors = getAutorestErrors(runResult); expect(errors).toEqual( diff --git a/eng/tools/lint-diff/test/util.test.ts b/eng/tools/lint-diff/test/util.test.ts index 8cda2857a072..d0e2273a97ad 100644 --- a/eng/tools/lint-diff/test/util.test.ts +++ b/eng/tools/lint-diff/test/util.test.ts @@ -1,12 +1,11 @@ -import { vol } from "memfs"; +import { fs as memfs, vol } from "memfs"; import { beforeEach } from "node:test"; import { describe, expect, test, vi } from "vitest"; import { isFailure, isWarning, pathExists } from "../src/util.js"; vi.mock("fs/promises", () => { - const memfs = require("memfs"); return { - ...memfs.fs.promises, + ...memfs.promises, }; }); diff --git a/package-lock.json b/package-lock.json index 74eaa4449de3..3800393776b0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -126,14 +126,17 @@ "lint-diff": "cmd/lint-diff.js" }, "devDependencies": { + "@eslint/js": "^9.22.0", "@types/deep-eql": "^4.0.2", "@types/node": "^18.19.31", "@vitest/coverage-v8": "^3.0.2", + "eslint": "^9.22.0", "execa": "^9.5.2", "memfs": "^4.17.0", "prettier": "~3.6.2", "prettier-plugin-organize-imports": "^4.2.0", "typescript": "~5.9.2", + "typescript-eslint": "^8.45.0", "vitest": "^3.2.4" }, "engines": {