diff --git a/src/domains/commits/tokens/SquashMarkerToken.tests.ts b/src/domains/commits/tokens/SquashMarkerToken.tests.ts index 568dd0e7..f7d06c09 100644 --- a/src/domains/commits/tokens/SquashMarkerToken.tests.ts +++ b/src/domains/commits/tokens/SquashMarkerToken.tests.ts @@ -22,6 +22,8 @@ describe("in the default configuration", () => { ${"squash the bugs"} | ${["squash the bugs"]} ${"Squash tennis and pumpkins"} | ${["Squash tennis and pumpkins"]} ${"!!!amend!!!this"} | ${["!!!amend!!!this"]} + ${"Done!"} | ${["Done!"]} + ${"let's go!!"} | ${["let's go!!"]} ${"amend!"} | ${[squashMarker("amend!")]} ${" amend!Apply strawberry jam to make the code sweeter"} | ${[squashMarker(" amend!"), "Apply strawberry jam to make the code sweeter"]} ${" fixup! "} | ${[squashMarker(" fixup! ")]} diff --git a/src/domains/rules/NoSquashMarkers.tests.ts b/src/domains/rules/NoSquashMarkers.tests.ts new file mode 100644 index 00000000..eab7d3df --- /dev/null +++ b/src/domains/rules/NoSquashMarkers.tests.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from "vitest" +import { fakeCommit } from "#commits/Commit.fixtures.ts" +import type { Commit } from "#commits/Commit.ts" +import type { RuleKey, RuleOptionsOf } from "#configurations/Configuration.ts" +import type { Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" +import { noSquashMarkers } from "#rules/NoSquashMarkers.ts" +import type { CharacterRange } from "#types/CharacterRange.ts" +import type { Vector } from "#types/Vector.ts" + +const rule: RuleKey = "noSquashMarkers" + +const enabled: RuleOptionsOf = {} +const disabled: RuleOptionsOf = null + +describe.each` + subjectLine | expectedRange + ${"fixup!"} | ${[0, 6]} + ${"amend! Resolve a bug that thought it was a feature"} | ${[0, 6]} + ${"squash! make the program act like a clown"} | ${[0, 7]} + ${" Fixup! Added some extra love to the code"} | ${[1, 7]} + ${"amend!Solved the problem"} | ${[0, 6]} + ${" squash! organise the bookshelf"} | ${[3, 10]} + ${'!fixup Revert "Bugfix"'} | ${[0, 6]} + ${"!amend Wonder if this will work?"} | ${[0, 6]} + ${"!SqUaSh retrieve data from third-party service"} | ${[0, 7]} + ${"fixup!! another attempt"} | ${[0, 7]} + ${"amend!! Update the documentation"} | ${[0, 7]} + ${" squash!!! make changes"} | ${[1, 10]} +`( + "when the subject line of $subjectLine starts with a single squash marker", + (props: { subjectLine: string; expectedRange: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + it("raises a concern about the squash marker", () => { + const actualConcerns = noSquashMarkers([commit], enabled) + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRange }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers([commit], disabled) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRange + ${"squash!amend!"} | ${[0, 13]} + ${"fixup! FIXUP!! Enforce the linting rules"} | ${[0, 14]} + ${" squash! amend! fixup! long polling"} | ${[1, 26]} +`( + "when the subject line of $subjectLine starts with multiple squash markers", + (props: { subjectLine: string; expectedRange: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + it("raises a concern about all squash markers", () => { + const actualConcerns = noSquashMarkers([commit], enabled) + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRange }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers([commit], disabled) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${""} + ${" "} + ${"\t\t"} + ${"Refactor the taxi module"} + ${" Unsubscribe from the service"} + ${"Formatting."} + ${"WIP"} + ${"Fix the bug"} + ${"Update the documentation"} + ${"Add new feature"} + ${"Make the commit scream fixup! again"} + ${"there's no squash! to see here"} + ${"All done!"} + ${"#7044: Solve the problem"} + ${'Revert "Release the robot butler"'} + ${" Hunt down the bugs "} + ${"Compare the list of items to the objects downloaded from the server"} + ${"Resolve issues in #21 to make the code work better"} + ${"Finally..."} + ${"Let `SoftIceMachineAdapter` produce the goods that we need"} + ${"fixup the code"} + ${"squash the commits"} + ${"amend the message"} +`( + "when the subject line of $subjectLine does not contain a squash marker", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers([commit], enabled) + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers([commit], disabled) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe("when verifying a set of multiple commits and some commits have squash markers", () => { + const commits: Vector = [ + fakeCommit({ message: "fixup! resolve a bug that thought it was a feature" }), + fakeCommit({ message: "Squash! Make the program act like a clown" }), + fakeCommit({ message: "Refactor the taxi module" }), + fakeCommit({ message: "WIP" }), + fakeCommit({ message: " amend! update the code" }), + fakeCommit({ message: " Unsubscribe from the service" }), + fakeCommit({ message: "Fix the bug" }), + ] + + describe("and the rule is enabled", () => { + it("raises concerns about the commits with squash markers", () => { + const actualConcerns = noSquashMarkers(commits, enabled) + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commits[0].sha, { range: [0, 6] }), + subjectLineConcern(rule, commits[1].sha, { range: [0, 7] }), + subjectLineConcern(rule, commits[4].sha, { range: [1, 7] }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers(commits, disabled) + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and no commits have squash markers", () => { + const commits: Vector = [ + fakeCommit({ message: "let's go!!" }), + fakeCommit({ message: "squash the bugs" }), + fakeCommit({ message: 'Revert "Bugfix"' }), + fakeCommit({ message: "Make the commit scream fixup! again" }), + fakeCommit({ message: "1 2 3 this is a test" }), + ] + + describe("and the rule is enabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers(commits, enabled) + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = noSquashMarkers(commits, disabled) + expect(actualConcerns).toEqual([]) + }) + }) +}) diff --git a/src/domains/rules/NoSquashMarkers.ts b/src/domains/rules/NoSquashMarkers.ts index 76124763..2d80f15a 100644 --- a/src/domains/rules/NoSquashMarkers.ts +++ b/src/domains/rules/NoSquashMarkers.ts @@ -1,7 +1,28 @@ -import type { Commits } from "#commits/Commit.ts" -import type { Concerns } from "#rules/concerns/Concern.ts" +import type { Commit, Commits } from "#commits/Commit.ts" +import { isSquashMarker } from "#commits/tokens/SquashMarkerToken.ts" +import type { RuleKey } from "#configurations/Configuration.ts" +import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" import type { EmptyObject } from "#types/EmptyObject.ts" +import { notNullish } from "#utilities/Arrays.ts" -export function noSquashMarkers(_commits: Commits, _configuration: EmptyObject | null): Concerns { - throw new Error("The `noSquashMarkers` rule has not been implemented yet") // TODO: To be implemented. +const rule: RuleKey = "noSquashMarkers" + +export function noSquashMarkers(commits: Commits, configuration: EmptyObject | null): Concerns { + return configuration !== null ? commits.map(verifyCommit).filter(notNullish) : [] +} + +function verifyCommit(commit: Commit): Concern | null { + const [firstToken] = commit.subjectLine + + if (firstToken && isSquashMarker(firstToken)) { + const leadingWhitespaceOffset = firstToken.value.length - firstToken.value.trimStart().length + const trimmedTokenLength = firstToken.value.trim().length + + return subjectLineConcern(rule, commit.sha, { + range: [leadingWhitespaceOffset, leadingWhitespaceOffset + trimmedTokenLength], + }) + } + + return null } diff --git a/src/domains/rules/UseCapitalisedSubjectLines.tests.ts b/src/domains/rules/UseCapitalisedSubjectLines.tests.ts index c5c3ec21..0abddcd3 100644 --- a/src/domains/rules/UseCapitalisedSubjectLines.tests.ts +++ b/src/domains/rules/UseCapitalisedSubjectLines.tests.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest" import { fakeCommit } from "#commits/Commit.fixtures.ts" import type { Commit } from "#commits/Commit.ts" -import type { RuleKey } from "#configurations/Configuration.ts" +import type { RuleKey, RuleOptionsOf } from "#configurations/Configuration.ts" import type { Concerns } from "#rules/concerns/Concern.ts" import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" import { useCapitalisedSubjectLines } from "#rules/UseCapitalisedSubjectLines.ts" @@ -10,6 +10,9 @@ import type { Vector } from "#types/Vector.ts" const rule: RuleKey = "useCapitalisedSubjectLines" +const enabled: RuleOptionsOf = {} +const disabled: RuleOptionsOf = null + describe.each` subjectLine | expectedRange ${"test"} | ${[0, 1]} @@ -33,7 +36,7 @@ describe.each` describe("and the rule is enabled", () => { it("raises a concern about the first non-capitalised character", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], {}) + const actualConcerns = useCapitalisedSubjectLines([commit], enabled) expect(actualConcerns).toEqual([ subjectLineConcern(rule, commit.sha, { range: props.expectedRange }), ]) @@ -42,7 +45,7 @@ describe.each` describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], null) + const actualConcerns = useCapitalisedSubjectLines([commit], disabled) expect(actualConcerns).toEqual([]) }) }) @@ -72,14 +75,14 @@ describe.each` describe("and the rule is enabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], {}) + const actualConcerns = useCapitalisedSubjectLines([commit], enabled) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], null) + const actualConcerns = useCapitalisedSubjectLines([commit], disabled) expect(actualConcerns).toEqual([]) }) }) @@ -101,14 +104,14 @@ describe.each` describe("and the rule is enabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], {}) + const actualConcerns = useCapitalisedSubjectLines([commit], enabled) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], null) + const actualConcerns = useCapitalisedSubjectLines([commit], disabled) expect(actualConcerns).toEqual([]) }) }) @@ -127,14 +130,14 @@ describe.each` describe("and the rule is enabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], {}) + const actualConcerns = useCapitalisedSubjectLines([commit], enabled) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], null) + const actualConcerns = useCapitalisedSubjectLines([commit], disabled) expect(actualConcerns).toEqual([]) }) }) @@ -155,7 +158,7 @@ describe("when verifying a set of multiple commits and some commits have non-cap describe("and the rule is enabled", () => { it("raises concerns about the commits with non-capitalised subject lines", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, {}) + const actualConcerns = useCapitalisedSubjectLines(commits, enabled) expect(actualConcerns).toEqual([ subjectLineConcern(rule, commits[0].sha, { range: [0, 1] }), subjectLineConcern(rule, commits[3].sha, { range: [0, 1] }), @@ -166,7 +169,7 @@ describe("when verifying a set of multiple commits and some commits have non-cap describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, null) + const actualConcerns = useCapitalisedSubjectLines(commits, disabled) expect(actualConcerns).toEqual([]) }) }) @@ -182,14 +185,14 @@ describe("when verifying a set of multiple commits and all commits have capitali describe("and the rule is enabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, {}) + const actualConcerns = useCapitalisedSubjectLines(commits, enabled) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, null) + const actualConcerns = useCapitalisedSubjectLines(commits, disabled) expect(actualConcerns).toEqual([]) }) }) diff --git a/src/domains/rules/UseCapitalisedSubjectLines.ts b/src/domains/rules/UseCapitalisedSubjectLines.ts index 5f39dca5..6ad96bfd 100644 --- a/src/domains/rules/UseCapitalisedSubjectLines.ts +++ b/src/domains/rules/UseCapitalisedSubjectLines.ts @@ -25,7 +25,9 @@ function verifyCommit(commit: Commit): Concern | null { const leadingWhitespaceOffset = token.length - trimmedToken.length const startIndex = index + leadingWhitespaceOffset - return subjectLineConcern(rule, commit.sha, { range: [startIndex, startIndex + 1] }) + return subjectLineConcern(rule, commit.sha, { + range: [startIndex, startIndex + 1], + }) } return null diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index e29d2566..4c214ed4 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -56,6 +56,46 @@ describe("when 'useCapitalisedSubjectLines' has a concern about characters 7-8", }) }) +describe("when 'noSquashMarkers' has a concern about characters 0-6", () => { + const commit = fakeCommit({ + sha: "ffebad193fe7d02aa9b19b70ee132a26f14f8caf", + message: "amend!Apply strawberry jam to make the code sweeter", + }) + const concern = subjectLineConcern("noSquashMarkers", commit.sha, { range: [0, 6] }) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +ffebad1 amend!Apply strawberry jam to make the code sweeter + ──┬─── + ╰─ Commits with squash markers must be combined with their ancestors. + (noSquashMarkers) +`.trim(), + ) + }) +}) + +describe("when 'noSquashMarkers' has a concern about characters 1-14", () => { + const commit = fakeCommit({ + sha: "56c750b0811fbcad2b237b2b99fc7d3fc91b926", + message: " fixup! fixup! found a funny easter egg", + }) + const concern = subjectLineConcern("noSquashMarkers", commit.sha, { range: [1, 14] }) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +56c750b fixup! fixup! found a funny easter egg + ──────┬────── + ╰─ Commits with squash markers must be combined with their ancestors. + (noSquashMarkers) +`.trim(), + ) + }) +}) + describe.todo("when there are multiple concerns of different types", () => { const commits: Vector = [fakeCommit(), fakeCommit(), fakeCommit()] const concerns: Concerns = [] diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index 05efbc3a..2204722a 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -2,6 +2,7 @@ import type { Commit, Commits } from "#commits/Commit.ts" import { formatTokenisedLine } from "#commits/tokens/Token.ts" import type { RuleKey } from "#configurations/Configuration.ts" import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import type { CharacterRange } from "#types/CharacterRange.ts" import { requireNotNullish } from "#utilities/Assertions.ts" import { indentString } from "#utilities/Strings.ts" @@ -12,11 +13,7 @@ export function commitwiseReport(commits: Commits, concerns: Concerns): string { const SHORT_SHA_LENGTH = 7 function formatConcern(commits: Commits, concern: Concern): string { - const [startIndex] = concern.range - const messageOffset = SHORT_SHA_LENGTH + " ".length + startIndex - - const message = `┬\n╰─ ${formatRule(concern.rule)}\n (${concern.rule})` - return `${formatCommit(commits, concern)}\n${indentString(message, messageOffset)}` + return `${formatCommit(commits, concern)}\n${formatMessage(concern)}` } function formatCommit(commits: Commits, concern: Concern): string { @@ -32,6 +29,33 @@ function formatSubjectLine(commit: Commit): string { return formatTokenisedLine(commit.subjectLine) } +function formatMessage(concern: Concern): string { + const [start] = concern.range + + const [rangeMarker, rangeOffset] = formatRangeMarker(concern.range) + const message = `${formatRule(concern.rule)}\n ${" ".repeat(rangeOffset)}(${concern.rule})` + + const messageOffset = SHORT_SHA_LENGTH + " ".length + start + return indentString(`${rangeMarker} ${message}`, messageOffset) +} + +function formatRangeMarker(range: CharacterRange): [string, offset: number] { + const [start, end] = range + const length = end - start + + if (length === 1) { + return ["┬\n╰─", 2] + } + + const firstHalfLength = Math.trunc((length - "┬".length) / 2) + const secondHalfLength = length - firstHalfLength - "┬".length + + return [ + `${"─".repeat(firstHalfLength)}┬${"─".repeat(secondHalfLength)}\n${indentString("╰─", firstHalfLength)}`, + firstHalfLength + 2, + ] +} + function formatRule(rule: RuleKey): string { switch (rule) { case "noExcessiveCommitsPerBranch": { @@ -53,7 +77,7 @@ function formatRule(rule: RuleKey): string { throw new Error(`Not implemented yet: ${rule}`) } case "noSquashMarkers": { - throw new Error(`Not implemented yet: ${rule}`) + return "Commits with squash markers must be combined with their ancestors." } case "noUnexpectedPunctuation": { throw new Error(`Not implemented yet: ${rule}`)