diff --git a/cspell.json b/cspell.json index 7c24122..430ec98 100644 --- a/cspell.json +++ b/cspell.json @@ -1,5 +1,12 @@ { "version": "0.2", - "words": ["ahmadnassri", "codecov", "Eriksson", "paleite"], - "ignoreWords": ["pinst", "Wercker"] + "words": ["Unstaged"], + "ignoreWords": [ + "ahmadnassri", + "codecov", + "Eriksson", + "paleite", + "pinst", + "Wercker" + ] } diff --git a/package.json b/package.json index 7091207..0d330c8 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,9 @@ "test": "jest --coverage", "typecheck": "tsc --project tsconfig.json --noEmit" }, + "resolutions": { + "cosmiconfig": "^8.1.3" + }, "devDependencies": { "@paleite/eslint-config": "^1.0.9", "@paleite/eslint-config-base": "^1.0.9", @@ -46,11 +49,13 @@ "@paleite/jest-config": "^1.0.9", "@paleite/prettier-config": "^1.0.9", "@paleite/tsconfig-node16": "^1.0.9", + "@types/debug": "^4.1.7", "@types/eslint": "^8.4.10", "@types/jest": "^29.2.6", "@types/node": "^18.11.18", "@typescript-eslint/eslint-plugin": "^5.48.2", "@typescript-eslint/parser": "^5.48.2", + "debug": "^4.3.2", "eslint": "^8.32.0", "eslint-config-prettier": "^8.6.0", "eslint-import-resolver-typescript": "^3.5.3", @@ -73,8 +78,5 @@ }, "engines": { "node": ">=14.0.0" - }, - "resolutions": { - "cosmiconfig": "^8.1.3" } } diff --git a/src/Range.ts b/src/Range.ts index e9cbc66..ede144e 100644 --- a/src/Range.ts +++ b/src/Range.ts @@ -1,3 +1,5 @@ +import { log } from "./logging"; + class Range { private readonly inclusiveLowerBound: Readonly; private readonly exclusiveUpperBound: Readonly; @@ -17,7 +19,13 @@ class Range { } isWithinRange(n: Readonly): boolean { - return this.inclusiveLowerBound <= n && n < this.exclusiveUpperBound; + log( + `Checking if ${n} is within range ${this.inclusiveLowerBound} - ${this.exclusiveUpperBound}` + ); + const result = + this.inclusiveLowerBound <= n && n < this.exclusiveUpperBound; + + return result; } } diff --git a/src/ci.ts b/src/ci.ts index ecefbc0..a1c7727 100644 --- a/src/ci.ts +++ b/src/ci.ts @@ -1,3 +1,5 @@ +import { log } from "./logging"; + type CiProviderCommon = { name: T }; type CiProvider = @@ -74,24 +76,28 @@ const PROVIDERS = { type CiProviderName = keyof typeof PROVIDERS; -const guessProviders = () => - Object.values(PROVIDERS).reduce<{ name: CiProviderName; branch: string }[]>( - (acc, { name, ...cur }) => { - if (!cur.isSupported || cur.diffBranch === undefined) { - return acc; - } +const guessProviders = () => { + log("Guessing CI providers"); + + return Object.values(PROVIDERS).reduce< + { name: CiProviderName; branch: string }[] + >((acc, { name, ...cur }) => { + if (!cur.isSupported || cur.diffBranch === undefined) { + return acc; + } - const branch = process.env[cur.diffBranch] ?? ""; - if (branch === "") { - return acc; - } + const branch = process.env[cur.diffBranch] ?? ""; + if (branch === "") { + return acc; + } - return [...acc, { name, branch }]; - }, - [] - ); + return [...acc, { name, branch }]; + }, []); +}; const guessBranch = (): string | undefined => { + log("Guessing branch"); + if ((process.env.ESLINT_PLUGIN_COMMIT ?? "").length > 0) { throw Error("ESLINT_PLUGIN_COMMIT already set"); } diff --git a/src/git.test.ts b/src/git.test.ts index c2172d1..d58e0af 100644 --- a/src/git.test.ts +++ b/src/git.test.ts @@ -51,7 +51,11 @@ describe("getDiffForFile", () => { mockedChildProcess.execFileSync.mockReturnValueOnce(Buffer.from(hunks)); process.env.ESLINT_PLUGIN_DIFF_COMMIT = "1234567"; - const diffFromFile = getDiffForFile("./mockfile.js", true); + const diffFromFile = getDiffForFile( + process.env.ESLINT_PLUGIN_DIFF_COMMIT, + "./mockfile.js", + true + ); const expectedCommand = "git"; const expectedArgs = @@ -93,7 +97,7 @@ describe("getDiffFileList", () => { Buffer.from(diffFileList) ); expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(0); - const fileListA = getDiffFileList(); + const fileListA = getDiffFileList("HEAD"); expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1); expect(fileListA).toEqual( diff --git a/src/git.ts b/src/git.ts index 9201779..4a2db96 100644 --- a/src/git.ts +++ b/src/git.ts @@ -1,11 +1,17 @@ import * as child_process from "child_process"; import { resolve } from "path"; +import { log } from "./logging"; import { Range } from "./Range"; const COMMAND = "git"; const OPTIONS = { maxBuffer: 1024 * 1024 * 100 }; -const getDiffForFile = (filePath: string, staged = false): string => { +const getDiffForFile = ( + commit: string, + filePath: string, + staged = false +): string => { + log("Getting diff for file", filePath); const args = [ "diff", "--diff-algorithm=histogram", @@ -15,7 +21,7 @@ const getDiffForFile = (filePath: string, staged = false): string => { "--relative", staged && "--staged", "--unified=0", - process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD", + commit, "--", resolve(filePath), ].reduce( @@ -26,7 +32,9 @@ const getDiffForFile = (filePath: string, staged = false): string => { return child_process.execFileSync(COMMAND, args, OPTIONS).toString(); }; -const getDiffFileList = (staged = false): string[] => { +const getDiffFileList = (commit: string, staged = false): string[] => { + log(`Getting list of files for ${staged ? "staged files" : "changed files"}`); + const args = [ "diff", "--diff-algorithm=histogram", @@ -36,22 +44,28 @@ const getDiffFileList = (staged = false): string[] => { "--no-ext-diff", "--relative", staged && "--staged", - process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD", + commit, "--", ].reduce( (acc, cur) => (typeof cur === "string" ? [...acc, cur] : acc), [] ); - return child_process + const diffFileList = child_process .execFileSync(COMMAND, args, OPTIONS) .toString() .trim() .split("\n") .map((filePath) => resolve(filePath)); + + log(diffFileList); + + return diffFileList; }; const hasCleanIndex = (filePath: string): boolean => { + log("Checking if file has clean index"); + const args = [ "diff", "--no-ext-diff", @@ -72,6 +86,7 @@ const hasCleanIndex = (filePath: string): boolean => { }; const fetchFromOrigin = (branch: string) => { + log("Fetching from origin", branch); const args = ["fetch", "--quiet", "origin", branch]; child_process.execFileSync(COMMAND, args, OPTIONS); @@ -87,6 +102,7 @@ const getUntrackedFileList = ( } if (untrackedFileListCache === undefined || shouldRefresh) { + log("Getting list of files for untracked files"); const args = ["ls-files", "--exclude-standard", "--others"]; untrackedFileListCache = child_process @@ -95,6 +111,8 @@ const getUntrackedFileList = ( .trim() .split("\n") .map((filePath) => resolve(filePath)); + + log("Untracked files", untrackedFileListCache); } return untrackedFileListCache; @@ -141,8 +159,10 @@ const getRangeForChangedLines = (line: string) => { return hasAddedLines ? new Range(start, end) : null; }; -const getRangesForDiff = (diff: string): Range[] => - diff.split("\n").reduce((ranges, line) => { +const getRangesForDiff = (diff: string): Range[] => { + log("Getting ranges for diff"); + + return diff.split("\n").reduce((ranges, line) => { if (!isHunkHeader(line)) { return ranges; } @@ -154,6 +174,7 @@ const getRangesForDiff = (diff: string): Range[] => return [...ranges, range]; }, []); +}; export { fetchFromOrigin, diff --git a/src/logging.ts b/src/logging.ts new file mode 100644 index 0000000..7b16ef2 --- /dev/null +++ b/src/logging.ts @@ -0,0 +1,5 @@ +import debug from "debug"; + +const log = debug("eslint-plugin-diff"); + +export { log }; diff --git a/src/processors.ts b/src/processors.ts index 753c98a..ef42afc 100644 --- a/src/processors.ts +++ b/src/processors.ts @@ -1,4 +1,5 @@ import type { Linter } from "eslint"; +import type { Range } from "./Range"; import { guessBranch } from "./ci"; import { fetchFromOrigin, @@ -8,11 +9,15 @@ import { getUntrackedFileList, hasCleanIndex, } from "./git"; -import type { Range } from "./Range"; +import { log } from "./logging"; -if (process.env.CI !== undefined) { +const isCiEnvironment = process.env.CI !== undefined; +log("Are we in a CI env?", isCiEnvironment ? "Yes" : "No"); +if (isCiEnvironment) { const branch = process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? guessBranch(); - if (branch !== undefined) { + const hasBranch = branch !== undefined; + log("Is the branch defined?", hasBranch ? "Yes" : "No"); + if (hasBranch) { const branchWithoutOrigin = branch.replace(/^origin\//, ""); const branchWithOrigin = `origin/${branchWithoutOrigin}`; fetchFromOrigin(branchWithoutOrigin); @@ -31,15 +36,28 @@ const getPreProcessor = (diffFileList: string[], staged: boolean) => (text: string, filename: string) => { let untrackedFileList = getUntrackedFileList(staged); - const shouldRefresh = - !diffFileList.includes(filename) && !untrackedFileList.includes(filename); + const includedByDiffList = diffFileList.includes(filename); + const includedByUntrackedList = untrackedFileList.includes(filename); + const shouldRefresh = !includedByDiffList && !includedByUntrackedList; if (shouldRefresh) { + log("Refreshing untracked file list"); untrackedFileList = getUntrackedFileList(staged, true); } + const isInDiffFileList = diffFileList.includes(filename); + const isInUntrackedFileList = untrackedFileList.includes(filename); const shouldBeProcessed = process.env.VSCODE_CLI !== undefined || - diffFileList.includes(filename) || - untrackedFileList.includes(filename); + isInDiffFileList || + isInUntrackedFileList; + + log( + isInDiffFileList + ? "Found changes in" + : isInUntrackedFileList + ? "Found untracked file" + : "Found unchanged file", + filename + ); return shouldBeProcessed ? [text] : []; }; @@ -56,6 +74,8 @@ const getUnstagedChangesError = (filename: string): [Linter.LintMessage] => { // unstaged diff and could cause a conflict, so we return a fatal // error-message instead. + log("File has unstaged changes"); + const fatal = true; const message = `${filename} has unstaged changes. Please stage or remove the changes.`; const severity: Linter.Severity = 2; @@ -71,32 +91,48 @@ const getUnstagedChangesError = (filename: string): [Linter.LintMessage] => { return [fatalError]; }; -const getPostProcessor = - (staged = false) => - ( +const getPostProcessor = (staged = false) => { + log( + "Creating post-processor for", + staged ? "staged files only" : "changed files" + ); + + return ( messages: Linter.LintMessage[][], filename: string ): Linter.LintMessage[] => { + log("Processing messages for", filename); if (messages.length === 0) { + log("Skipping file because it has no messages"); // No need to filter, just return return []; } + const untrackedFileList = getUntrackedFileList(staged); if (untrackedFileList.includes(filename)) { + log("Skipping file because it is untracked"); // We don't need to filter the messages of untracked files because they // would all be kept anyway, so we return them as-is. return messages.flat(); } if (staged && !hasCleanIndex(filename)) { + log("Found a partially staged file"); return getUnstagedChangesError(filename); } - const rangesForDiff = getRangesForDiff(getDiffForFile(filename, staged)); + const rangesForDiff = getRangesForDiff( + getDiffForFile( + process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD", + filename, + staged + ) + ); return messages.flatMap((message) => { const filteredMessage = message.filter(({ fatal, line }) => { if (fatal === true) { + log("Found a fatal error-message"); return true; } @@ -104,20 +140,27 @@ const getPostProcessor = isLineWithinRange(line) ); + log("Is the message for a changed line?", isLineWithinSomeRange); return isLineWithinSomeRange; }); + log("Removed", message.length - filteredMessage.length, "messages"); return filteredMessage; }); }; +}; type ProcessorType = "diff" | "staged" | "ci"; const getProcessors = ( processorType: ProcessorType ): Required => { + log("Creating config for processor type", JSON.stringify(processorType)); const staged = processorType === "staged"; - const diffFileList = getDiffFileList(staged); + const diffFileList = getDiffFileList( + process.env.ESLINT_PLUGIN_DIFF_COMMIT ?? "HEAD", + staged + ); return { preprocess: getPreProcessor(diffFileList, staged), @@ -126,7 +169,7 @@ const getProcessors = ( }; }; -const ci = process.env.CI !== undefined ? getProcessors("ci") : {}; +const ci = isCiEnvironment ? getProcessors("ci") : {}; const diff = getProcessors("diff"); const staged = getProcessors("staged"); @@ -140,18 +183,17 @@ const diffConfig: Linter.BaseConfig = { ], }; -const ciConfig: Linter.BaseConfig = - process.env.CI === undefined - ? {} - : { - plugins: ["diff"], - overrides: [ - { - files: ["*"], - processor: "diff/ci", - }, - ], - }; +const ciConfig: Linter.BaseConfig = isCiEnvironment + ? { + plugins: ["diff"], + overrides: [ + { + files: ["*"], + processor: "diff/ci", + }, + ], + } + : {}; const stagedConfig: Linter.BaseConfig = { plugins: ["diff"], @@ -168,7 +210,7 @@ export { ciConfig, diff, diffConfig, + getUnstagedChangesError, staged, stagedConfig, - getUnstagedChangesError, }; diff --git a/yarn.lock b/yarn.lock index 3063c0c..a8ae0ee 100644 --- a/yarn.lock +++ b/yarn.lock @@ -760,6 +760,13 @@ "@types/node" "*" "@types/responselike" "^1.0.0" +"@types/debug@^4.1.7": + version "4.1.7" + resolved "https://registry.yarnpkg.com/@types/debug/-/debug-4.1.7.tgz#7cc0ea761509124709b8b2d1090d8f6c17aadb82" + integrity sha512-9AonUzyTjXXhEOa0DnqpzZi6VHlqKMswga9EXjpXnnqxwLtdvPPtlO8evrI5D9S6asFRCQ6v+wpiUKbw+vKqyg== + dependencies: + "@types/ms" "*" + "@types/eslint@^7.2.13": version "7.29.0" resolved "https://registry.yarnpkg.com/@types/eslint/-/eslint-7.29.0.tgz#e56ddc8e542815272720bb0b4ccc2aff9c3e1c78" @@ -847,6 +854,11 @@ resolved "https://registry.yarnpkg.com/@types/minimist/-/minimist-1.2.2.tgz#ee771e2ba4b3dc5b372935d549fd9617bf345b8c" integrity sha512-jhuKLIRrhvCPLqwPcx6INqmKeiA5EWrsCOPhrlFSrbrmU4ZMPjj5Ul/oLCMDO98XRUIwVm78xICz4EPCektzeQ== +"@types/ms@*": + version "0.7.31" + resolved "https://registry.yarnpkg.com/@types/ms/-/ms-0.7.31.tgz#31b7ca6407128a3d2bbc27fe2d21b345397f6197" + integrity sha512-iiUgKzV9AuaEkZqkOLDIvlQiL6ltuZd9tGcW3gwpnX8JbuiuhFlEGmmFXEXkN50Cvq7Os88IY2v0dkDqXYWVgA== + "@types/node@*": version "20.1.4" resolved "https://registry.yarnpkg.com/@types/node/-/node-20.1.4.tgz#83f148d2d1f5fe6add4c53358ba00d97fc4cdb71"