From 436e39813e2e5f8e7365bd637b0832b330f4057d Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Tue, 8 Oct 2024 11:53:37 -0700 Subject: [PATCH 1/9] Eslint -> ESLint Let's make sure our naming is consistent. This is how it is written on the ESLint docs site. --- action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/action.yml b/action.yml index bbcdb67..f3f1ce8 100644 --- a/action.yml +++ b/action.yml @@ -1,5 +1,5 @@ -name: "Balto - Eslint" -description: "Run eslint on your repo" +name: "Balto - ESLint" +description: "Run ESLint on your repo" runs: using: node20 main: dist/index.js From 436d9d9f8a814fd472d5366b980305b90c72e936 Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Tue, 8 Oct 2024 11:46:18 -0700 Subject: [PATCH 2/9] Prefix automated test commits messages Having the prefix makes it easier to quickly tell which commits were generated. Has been helpful in local dev when generating multiple test commits and needing to squash them down before pushing them up to Github. --- devbox.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/devbox.json b/devbox.json index 572fbf7..f3b7a4d 100644 --- a/devbox.json +++ b/devbox.json @@ -24,14 +24,14 @@ "cp test/snapshots/base/* test/v7", "cp test/snapshots/base/* test/v8", "cp test/snapshots/base/* test/v9", - "git add . && git commit -m 'Undo simulated changes'", + "git add . && git commit -m 'Automated: Undo simulated changes'", "echo \"{ \\\"pull_request\\\": { \\\"base\\\": { \\\"sha\\\": \\\"$(git rev-parse HEAD)\\\" } } }\" > test/pull_request_event_payload.json", - "git add . && git commit -m 'Update test target sha'", + "git add . && git commit -m 'Automated: Update test target sha'", "cp test/snapshots/updates/* test/v6", "cp test/snapshots/updates/* test/v7", "cp test/snapshots/updates/* test/v8", "cp test/snapshots/updates/* test/v9", - "git add . && git commit -m 'Simulate changes'" + "git add . && git commit -m 'Automated: Simulate changes'" ] } } From 8c1391e0e056a18792f356bde24f87771e8ba014 Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Tue, 8 Oct 2024 11:44:56 -0700 Subject: [PATCH 3/9] Include exit code in debug output ESLint will return exit code 2 if something completely breaks during parsing. Let's make sure that's exposed if we're looking for it. --- dist/index.js | 3 ++- src/index.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dist/index.js b/dist/index.js index 45be5d7..c4c19e8 100644 --- a/dist/index.js +++ b/dist/index.js @@ -26363,11 +26363,12 @@ async function run() { changedFiles = await (0, git_utils_1.detectChangedFiles)(compareSha); } core.debug(`Changed files: ${changedFiles}`); - let { stdout: eslintOut } = await (0, exec_1.getExecOutput)("npx eslint --format=json", changedFiles, + let { stdout: eslintOut, exitCode } = await (0, exec_1.getExecOutput)("npx eslint --format=json", changedFiles, // Eslint will return exit code 1 if it finds linting problems, but that is // expected and we don't want to stop execution because of it. { ignoreReturnCode: true }); let eslintJson = JSON.parse(eslintOut); + core.debug(`Eslint exit code: ${exitCode}`); let promises = eslintJson.map((resultObject) => eslint_result_1.EslintResult.for(resultObject, compareSha)); let eslintResults = await Promise.all(promises); core.debug("Eslint results ->"); diff --git a/src/index.ts b/src/index.ts index 8411d50..b13f017 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,7 +34,7 @@ async function run() { core.debug(`Changed files: ${changedFiles}`) - let { stdout: eslintOut } = await getExecOutput( + let { stdout: eslintOut, exitCode } = await getExecOutput( "npx eslint --format=json", changedFiles, // Eslint will return exit code 1 if it finds linting problems, but that is @@ -42,6 +42,7 @@ async function run() { { ignoreReturnCode: true }, ) let eslintJson = JSON.parse(eslintOut) + core.debug(`Eslint exit code: ${exitCode}`) let promises: Array> = eslintJson.map( (resultObject: ResultObject) => EslintResult.for(resultObject, compareSha), From 84ea85862b889c06f9204a069ab8a405a84c7f85 Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Thu, 10 Oct 2024 15:30:07 -0700 Subject: [PATCH 4/9] Update ignored file to reproduce parsing error Because the current file is blank, it was not causing the parsing issues we are seeing with v1.0.0 and non-js files --- test/snapshots/updates/some_file_that_eslint_should_ignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/snapshots/updates/some_file_that_eslint_should_ignore b/test/snapshots/updates/some_file_that_eslint_should_ignore index e69de29..27921aa 100644 --- a/test/snapshots/updates/some_file_that_eslint_should_ignore +++ b/test/snapshots/updates/some_file_that_eslint_should_ignore @@ -0,0 +1,4 @@ +class EslintShouldIgnoreThis + def evenThoughItsWeird (*) + end + end From 54de0dbcb922b6544025d1604d46146fcbdbda81 Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Thu, 10 Oct 2024 15:31:19 -0700 Subject: [PATCH 5/9] Automated: Undo simulated changes --- test/v6/existing.js | 4 +--- test/v6/new.js | 9 --------- test/v6/some_file_that_eslint_should_ignore | 0 test/v7/existing.js | 4 +--- test/v7/new.js | 9 --------- test/v7/some_file_that_eslint_should_ignore | 0 test/v8/existing.js | 4 +--- test/v8/new.js | 9 --------- test/v8/some_file_that_eslint_should_ignore | 0 test/v9/existing.js | 4 +--- test/v9/new.js | 9 --------- test/v9/some_file_that_eslint_should_ignore | 0 12 files changed, 4 insertions(+), 48 deletions(-) delete mode 100644 test/v6/new.js delete mode 100644 test/v6/some_file_that_eslint_should_ignore delete mode 100644 test/v7/new.js delete mode 100644 test/v7/some_file_that_eslint_should_ignore delete mode 100644 test/v8/new.js delete mode 100644 test/v8/some_file_that_eslint_should_ignore delete mode 100644 test/v9/new.js delete mode 100644 test/v9/some_file_that_eslint_should_ignore diff --git a/test/v6/existing.js b/test/v6/existing.js index a2bf2fe..0edb818 100644 --- a/test/v6/existing.js +++ b/test/v6/existing.js @@ -1,5 +1,3 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") - const a = 0 - a = 1 + console.log("hello world") } diff --git a/test/v6/new.js b/test/v6/new.js deleted file mode 100644 index 498f055..0000000 --- a/test/v6/new.js +++ /dev/null @@ -1,9 +0,0 @@ -function addOne(i) { - if (i != NaN) { - return i ++ - } else { - return - } -}; - -const not_camel_case = true diff --git a/test/v6/some_file_that_eslint_should_ignore b/test/v6/some_file_that_eslint_should_ignore deleted file mode 100644 index e69de29..0000000 diff --git a/test/v7/existing.js b/test/v7/existing.js index a2bf2fe..0edb818 100644 --- a/test/v7/existing.js +++ b/test/v7/existing.js @@ -1,5 +1,3 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") - const a = 0 - a = 1 + console.log("hello world") } diff --git a/test/v7/new.js b/test/v7/new.js deleted file mode 100644 index 498f055..0000000 --- a/test/v7/new.js +++ /dev/null @@ -1,9 +0,0 @@ -function addOne(i) { - if (i != NaN) { - return i ++ - } else { - return - } -}; - -const not_camel_case = true diff --git a/test/v7/some_file_that_eslint_should_ignore b/test/v7/some_file_that_eslint_should_ignore deleted file mode 100644 index e69de29..0000000 diff --git a/test/v8/existing.js b/test/v8/existing.js index a2bf2fe..0edb818 100644 --- a/test/v8/existing.js +++ b/test/v8/existing.js @@ -1,5 +1,3 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") - const a = 0 - a = 1 + console.log("hello world") } diff --git a/test/v8/new.js b/test/v8/new.js deleted file mode 100644 index 498f055..0000000 --- a/test/v8/new.js +++ /dev/null @@ -1,9 +0,0 @@ -function addOne(i) { - if (i != NaN) { - return i ++ - } else { - return - } -}; - -const not_camel_case = true diff --git a/test/v8/some_file_that_eslint_should_ignore b/test/v8/some_file_that_eslint_should_ignore deleted file mode 100644 index e69de29..0000000 diff --git a/test/v9/existing.js b/test/v9/existing.js index a2bf2fe..0edb818 100644 --- a/test/v9/existing.js +++ b/test/v9/existing.js @@ -1,5 +1,3 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") - const a = 0 - a = 1 + console.log("hello world") } diff --git a/test/v9/new.js b/test/v9/new.js deleted file mode 100644 index 498f055..0000000 --- a/test/v9/new.js +++ /dev/null @@ -1,9 +0,0 @@ -function addOne(i) { - if (i != NaN) { - return i ++ - } else { - return - } -}; - -const not_camel_case = true diff --git a/test/v9/some_file_that_eslint_should_ignore b/test/v9/some_file_that_eslint_should_ignore deleted file mode 100644 index e69de29..0000000 From 914a37524c87d2c8d98ddba6dd86c9c3ecc1bc8d Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Thu, 10 Oct 2024 15:31:19 -0700 Subject: [PATCH 6/9] Automated: Update test target sha --- test/pull_request_event_payload.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pull_request_event_payload.json b/test/pull_request_event_payload.json index dde140b..904c5b5 100644 --- a/test/pull_request_event_payload.json +++ b/test/pull_request_event_payload.json @@ -1 +1 @@ -{ "pull_request": { "base": { "sha": "c2eaebd173390cb17efa35fce4d4941ccd5b3b5c" } } } +{ "pull_request": { "base": { "sha": "54de0dbcb922b6544025d1604d46146fcbdbda81" } } } From 3affcca70f19fe0874c7fd94c2de94c5961bf53d Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Thu, 10 Oct 2024 15:31:19 -0700 Subject: [PATCH 7/9] Automated: Simulate changes --- test/v6/existing.js | 4 +++- test/v6/new.js | 9 +++++++++ test/v6/some_file_that_eslint_should_ignore | 4 ++++ test/v7/existing.js | 4 +++- test/v7/new.js | 9 +++++++++ test/v7/some_file_that_eslint_should_ignore | 4 ++++ test/v8/existing.js | 4 +++- test/v8/new.js | 9 +++++++++ test/v8/some_file_that_eslint_should_ignore | 4 ++++ test/v9/existing.js | 4 +++- test/v9/new.js | 9 +++++++++ test/v9/some_file_that_eslint_should_ignore | 4 ++++ 12 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 test/v6/new.js create mode 100644 test/v6/some_file_that_eslint_should_ignore create mode 100644 test/v7/new.js create mode 100644 test/v7/some_file_that_eslint_should_ignore create mode 100644 test/v8/new.js create mode 100644 test/v8/some_file_that_eslint_should_ignore create mode 100644 test/v9/new.js create mode 100644 test/v9/some_file_that_eslint_should_ignore diff --git a/test/v6/existing.js b/test/v6/existing.js index 0edb818..a2bf2fe 100644 --- a/test/v6/existing.js +++ b/test/v6/existing.js @@ -1,3 +1,5 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") + console.log("hello world") + const a = 0 + a = 1 } diff --git a/test/v6/new.js b/test/v6/new.js new file mode 100644 index 0000000..498f055 --- /dev/null +++ b/test/v6/new.js @@ -0,0 +1,9 @@ +function addOne(i) { + if (i != NaN) { + return i ++ + } else { + return + } +}; + +const not_camel_case = true diff --git a/test/v6/some_file_that_eslint_should_ignore b/test/v6/some_file_that_eslint_should_ignore new file mode 100644 index 0000000..27921aa --- /dev/null +++ b/test/v6/some_file_that_eslint_should_ignore @@ -0,0 +1,4 @@ +class EslintShouldIgnoreThis + def evenThoughItsWeird (*) + end + end diff --git a/test/v7/existing.js b/test/v7/existing.js index 0edb818..a2bf2fe 100644 --- a/test/v7/existing.js +++ b/test/v7/existing.js @@ -1,3 +1,5 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") + console.log("hello world") + const a = 0 + a = 1 } diff --git a/test/v7/new.js b/test/v7/new.js new file mode 100644 index 0000000..498f055 --- /dev/null +++ b/test/v7/new.js @@ -0,0 +1,9 @@ +function addOne(i) { + if (i != NaN) { + return i ++ + } else { + return + } +}; + +const not_camel_case = true diff --git a/test/v7/some_file_that_eslint_should_ignore b/test/v7/some_file_that_eslint_should_ignore new file mode 100644 index 0000000..27921aa --- /dev/null +++ b/test/v7/some_file_that_eslint_should_ignore @@ -0,0 +1,4 @@ +class EslintShouldIgnoreThis + def evenThoughItsWeird (*) + end + end diff --git a/test/v8/existing.js b/test/v8/existing.js index 0edb818..a2bf2fe 100644 --- a/test/v8/existing.js +++ b/test/v8/existing.js @@ -1,3 +1,5 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") + console.log("hello world") + const a = 0 + a = 1 } diff --git a/test/v8/new.js b/test/v8/new.js new file mode 100644 index 0000000..498f055 --- /dev/null +++ b/test/v8/new.js @@ -0,0 +1,9 @@ +function addOne(i) { + if (i != NaN) { + return i ++ + } else { + return + } +}; + +const not_camel_case = true diff --git a/test/v8/some_file_that_eslint_should_ignore b/test/v8/some_file_that_eslint_should_ignore new file mode 100644 index 0000000..27921aa --- /dev/null +++ b/test/v8/some_file_that_eslint_should_ignore @@ -0,0 +1,4 @@ +class EslintShouldIgnoreThis + def evenThoughItsWeird (*) + end + end diff --git a/test/v9/existing.js b/test/v9/existing.js index 0edb818..a2bf2fe 100644 --- a/test/v9/existing.js +++ b/test/v9/existing.js @@ -1,3 +1,5 @@ function hello(this_should_be_ignored_because_its_an_existing_violation) { - console.log("hello world") + console.log("hello world") + const a = 0 + a = 1 } diff --git a/test/v9/new.js b/test/v9/new.js new file mode 100644 index 0000000..498f055 --- /dev/null +++ b/test/v9/new.js @@ -0,0 +1,9 @@ +function addOne(i) { + if (i != NaN) { + return i ++ + } else { + return + } +}; + +const not_camel_case = true diff --git a/test/v9/some_file_that_eslint_should_ignore b/test/v9/some_file_that_eslint_should_ignore new file mode 100644 index 0000000..27921aa --- /dev/null +++ b/test/v9/some_file_that_eslint_should_ignore @@ -0,0 +1,4 @@ +class EslintShouldIgnoreThis + def evenThoughItsWeird (*) + end + end From 7d1b5fbd2db91144e27a30a0413dcc898246e60d Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Tue, 8 Oct 2024 12:04:05 -0700 Subject: [PATCH 8/9] Use extensions input to filter files passed to ESLint Without this manual filtering, ESLint will attempt to lint files that should be ignored (like ruby files). ESLint v9 has the `files` option in the flat config, and earlier versions have the `--ext` CLI flag. Let's aim for maximum compatibility and just ignore the files that don't end in the extensions we want. --- README.md | 1 + action.yml | 4 ++++ dist/index.js | 10 +++++++++- src/index.ts | 15 ++++++++++++++- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index df16ee4..8c66f6a 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ jobs: | `failure-level` | The lowest annotation level to fail on ("warning or "error"") | no | `"error"` | | `conclusion-level` | Action conclusion ("success" or "failure") if annotations of the failure-level were created. | no | `"success"` | | `working-directory` | Which directory to run the action in | no | `"."` | +| `extensions` | A comma separated list of extensions to run ESLint on. | no | `".js,.ts,.jsx,.tsx,.mjs,.cjs"` | ## Outputs diff --git a/action.yml b/action.yml index f3f1ce8..e3c797b 100644 --- a/action.yml +++ b/action.yml @@ -19,6 +19,10 @@ inputs: description: Which directory to run the action in required: false default: "." + extensions: + description: A comma separated list of extensions to run ESLint on. + required: false + default: ".js,.ts,.jsx,.tsx,.mjs,.cjs" outputs: warning-count: description: "Number of relevant warnings found" diff --git a/dist/index.js b/dist/index.js index c4c19e8..855c49e 100644 --- a/dist/index.js +++ b/dist/index.js @@ -26363,7 +26363,15 @@ async function run() { changedFiles = await (0, git_utils_1.detectChangedFiles)(compareSha); } core.debug(`Changed files: ${changedFiles}`); - let { stdout: eslintOut, exitCode } = await (0, exec_1.getExecOutput)("npx eslint --format=json", changedFiles, + let extensions = core.getInput("extensions").split(","); + core.debug(`Extensions: ${extensions}`); + let changedFilesMatchingExtensions = changedFiles.filter((file) => extensions.some((ext) => file.endsWith(ext))); + core.debug(`Changed files matching extensions: ${changedFilesMatchingExtensions}`); + // Bail out early if the file list is empty (older ESLint versions will + // complain if the list is empty) + if (changedFilesMatchingExtensions.length === 0) + return; + let { stdout: eslintOut, exitCode } = await (0, exec_1.getExecOutput)("npx eslint --format=json", changedFilesMatchingExtensions, // Eslint will return exit code 1 if it finds linting problems, but that is // expected and we don't want to stop execution because of it. { ignoreReturnCode: true }); diff --git a/src/index.ts b/src/index.ts index b13f017..600e5bc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,9 +34,22 @@ async function run() { core.debug(`Changed files: ${changedFiles}`) + let extensions = core.getInput("extensions").split(",") + core.debug(`Extensions: ${extensions}`) + let changedFilesMatchingExtensions = changedFiles.filter((file) => + extensions.some((ext) => file.endsWith(ext)), + ) + core.debug( + `Changed files matching extensions: ${changedFilesMatchingExtensions}`, + ) + + // Bail out early if the file list is empty (older ESLint versions will + // complain if the list is empty) + if (changedFilesMatchingExtensions.length === 0) return + let { stdout: eslintOut, exitCode } = await getExecOutput( "npx eslint --format=json", - changedFiles, + changedFilesMatchingExtensions, // Eslint will return exit code 1 if it finds linting problems, but that is // expected and we don't want to stop execution because of it. { ignoreReturnCode: true }, From 61837c790b8eb9f91b8800276cb442587e3c57c5 Mon Sep 17 00:00:00 2001 From: Nick Sheck Date: Thu, 10 Oct 2024 15:33:50 -0700 Subject: [PATCH 9/9] More Eslint -> ESLint --- dist/index.js | 10 +++++----- src/eslint_result.ts | 12 ++++++------ src/index.ts | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/dist/index.js b/dist/index.js index 855c49e..74e822d 100644 --- a/dist/index.js +++ b/dist/index.js @@ -26176,7 +26176,7 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.EslintResult = void 0; +exports.ESLintResult = void 0; const core = __importStar(__nccwpck_require__(2186)); const git_utils_1 = __nccwpck_require__(5601); var Severity; @@ -26184,7 +26184,7 @@ var Severity; Severity[Severity["Warning"] = 1] = "Warning"; Severity[Severity["Error"] = 2] = "Error"; })(Severity || (Severity = {})); -class EslintResult { +class ESLintResult { relevantWarningCount = 0; relevantErrorCount = 0; resultObject; @@ -26192,7 +26192,7 @@ class EslintResult { relevantMessages = []; static async for(resultObject, compareSha) { const changeRanges = await (0, git_utils_1.generateChangeRanges)(resultObject.filePath, compareSha); - return new EslintResult(resultObject, changeRanges); + return new ESLintResult(resultObject, changeRanges); } constructor(resultObject, changeRanges) { this.resultObject = resultObject; @@ -26247,7 +26247,7 @@ class EslintResult { return this.resultObject.filePath.replace(absoluteFolderPath, ""); } } -exports.EslintResult = EslintResult; +exports.ESLintResult = ESLintResult; /***/ }), @@ -26377,7 +26377,7 @@ async function run() { { ignoreReturnCode: true }); let eslintJson = JSON.parse(eslintOut); core.debug(`Eslint exit code: ${exitCode}`); - let promises = eslintJson.map((resultObject) => eslint_result_1.EslintResult.for(resultObject, compareSha)); + let promises = eslintJson.map((resultObject) => eslint_result_1.ESLintResult.for(resultObject, compareSha)); let eslintResults = await Promise.all(promises); core.debug("Eslint results ->"); eslintResults.forEach((result) => core.debug(JSON.stringify(result))); diff --git a/src/eslint_result.ts b/src/eslint_result.ts index 281cfb1..62c7b4e 100644 --- a/src/eslint_result.ts +++ b/src/eslint_result.ts @@ -3,10 +3,10 @@ import { ChangeRange, generateChangeRanges } from "./git_utils" export type ResultObject = { filePath: string - messages: EslintMessage[] + messages: ESLintMessage[] } -type EslintMessage = { +type ESLintMessage = { ruleId: string severity: Severity message: string @@ -24,22 +24,22 @@ enum Severity { Error = 2, } -export class EslintResult { +export class ESLintResult { public relevantWarningCount: number = 0 public relevantErrorCount: number = 0 private resultObject: ResultObject private changeRanges: ChangeRange[] - private relevantMessages: EslintMessage[] = [] + private relevantMessages: ESLintMessage[] = [] static async for( resultObject: ResultObject, compareSha: string, - ): Promise { + ): Promise { const changeRanges = await generateChangeRanges( resultObject.filePath, compareSha, ) - return new EslintResult(resultObject, changeRanges) + return new ESLintResult(resultObject, changeRanges) } constructor(resultObject: ResultObject, changeRanges: ChangeRange[]) { diff --git a/src/index.ts b/src/index.ts index 600e5bc..cd80156 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ import * as core from "@actions/core" import { detectChangedFiles, detectChangedFilesInFolder } from "./git_utils" import { getExecOutput } from "@actions/exec" -import { ResultObject, EslintResult } from "./eslint_result" +import { ResultObject, ESLintResult } from "./eslint_result" async function run() { let workingDirectory = core.getInput("working-directory") @@ -57,8 +57,8 @@ async function run() { let eslintJson = JSON.parse(eslintOut) core.debug(`Eslint exit code: ${exitCode}`) - let promises: Array> = eslintJson.map( - (resultObject: ResultObject) => EslintResult.for(resultObject, compareSha), + let promises: Array> = eslintJson.map( + (resultObject: ResultObject) => ESLintResult.for(resultObject, compareSha), ) let eslintResults = await Promise.all(promises)