diff --git a/src/domains/configurations/Configuration.fixtures.ts b/src/domains/configurations/Configuration.fixtures.ts index 0fc88c45..f3094cc0 100644 --- a/src/domains/configurations/Configuration.fixtures.ts +++ b/src/domains/configurations/Configuration.fixtures.ts @@ -31,7 +31,7 @@ export function fakeConfiguration(overrides: ConfigurationTemplate = {}): Config useCapitalisedSubjectLines: {}, useCommitterEmailPatterns: {}, useCommitterNamePatterns: {}, - useConciseSubjectLines: {}, + useConciseSubjectLines: { maxLength: 50 }, useEmptyLineBeforeBodyLines: {}, useImperativeSubjectLines: {}, useIssueLinks: {}, diff --git a/src/domains/configurations/GetConfiguration.tests.ts b/src/domains/configurations/GetConfiguration.tests.ts index d2a859f2..2d5b7350 100644 --- a/src/domains/configurations/GetConfiguration.tests.ts +++ b/src/domains/configurations/GetConfiguration.tests.ts @@ -25,7 +25,7 @@ describe("the default configuration in the command-line", () => { ${"noUnexpectedPunctuation"} | ${{}} ${"noUnexpectedWhitespace"} | ${{}} ${"useCapitalisedSubjectLines"} | ${{}} - ${"useConciseSubjectLines"} | ${{}} + ${"useConciseSubjectLines"} | ${{ maxLength: 50 }} ${"useEmptyLineBeforeBodyLines"} | ${{}} ${"useImperativeSubjectLines"} | ${{}} ${"useLineWrapping"} | ${{}} @@ -78,7 +78,7 @@ describe("the default configuration in GitHub Actions", () => { ${"noUnexpectedPunctuation"} | ${{}} ${"noUnexpectedWhitespace"} | ${{}} ${"useCapitalisedSubjectLines"} | ${{}} - ${"useConciseSubjectLines"} | ${{}} + ${"useConciseSubjectLines"} | ${{ maxLength: 50 }} ${"useEmptyLineBeforeBodyLines"} | ${{}} ${"useImperativeSubjectLines"} | ${{}} ${"useLineWrapping"} | ${{}} diff --git a/src/domains/configurations/GetDefaultConfiguration.ts b/src/domains/configurations/GetDefaultConfiguration.ts index 6fcf1e4e..40ae9cd3 100644 --- a/src/domains/configurations/GetDefaultConfiguration.ts +++ b/src/domains/configurations/GetDefaultConfiguration.ts @@ -35,7 +35,7 @@ function getDefaultCliConfiguration(): Configuration { useCapitalisedSubjectLines: {}, useCommitterEmailPatterns: null, useCommitterNamePatterns: null, - useConciseSubjectLines: {}, + useConciseSubjectLines: { maxLength: 50 }, useEmptyLineBeforeBodyLines: {}, useImperativeSubjectLines: {}, useIssueLinks: null, @@ -63,7 +63,7 @@ function getDefaultGhaConfiguration(): Configuration { useCapitalisedSubjectLines: {}, useCommitterEmailPatterns: null, useCommitterNamePatterns: null, - useConciseSubjectLines: {}, + useConciseSubjectLines: { maxLength: 50 }, useEmptyLineBeforeBodyLines: {}, useImperativeSubjectLines: {}, useIssueLinks: null, diff --git a/src/domains/rules/UseCapitalisedSubjectLines.ts b/src/domains/rules/UseCapitalisedSubjectLines.ts index 73f77ebc..19b2e67b 100644 --- a/src/domains/rules/UseCapitalisedSubjectLines.ts +++ b/src/domains/rules/UseCapitalisedSubjectLines.ts @@ -1,4 +1,5 @@ import type { Commit, Commits } from "#commits/Commit.ts" +import { isText } from "#commits/tokens/Token.ts" import type { Concern, Concerns } from "#rules/concerns/Concern.ts" import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" import { type RuleContext, ruleContext } from "#rules/Rule.ts" @@ -21,7 +22,7 @@ function verifyCommit(commit: Commit, rule: RuleContext): Concern | null { let index = 0 for (const token of commit.subjectLine) { - if (typeof token === "string") { + if (isText(token)) { const trimmedToken = token.trimStart() if (startsWithLowercaseLetter(trimmedToken)) { diff --git a/src/domains/rules/UseConciseSubjectLines.tests.ts b/src/domains/rules/UseConciseSubjectLines.tests.ts new file mode 100644 index 00000000..205aa173 --- /dev/null +++ b/src/domains/rules/UseConciseSubjectLines.tests.ts @@ -0,0 +1,375 @@ +import { describe, expect, it } from "vitest" +import { fakeCommitFactory } from "#commits/Commit.fixtures.ts" +import type { Commit } from "#commits/Commit.ts" +import { fakeConfiguration } from "#configurations/Configuration.fixtures.ts" +import type { Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" +import { type RuleContext, ruleContext } from "#rules/Rule.ts" +import { useConciseSubjectLines } from "#rules/UseConciseSubjectLines.ts" +import type { CharacterRange } from "#types/CharacterRange.ts" +import { fakeCommitSha } from "#types/CommitSha.fixtures.ts" +import type { CommitSha } from "#types/CommitSha.ts" +import type { Vector } from "#types/Vector.ts" + +const enabled20 = ruleContext("useConciseSubjectLines", { maxLength: 20 }) +const enabled50 = ruleContext("useConciseSubjectLines", { maxLength: 50 }) +const enabled72 = ruleContext("useConciseSubjectLines", { maxLength: 72 }) + +const fakeCommit = fakeCommitFactory(fakeConfiguration()) + +describe.each` + subjectLine | expectedRange20 | expectedRange50 | expectedRange72 + ${"Upgrade dependency @opentelemetry/exporter-metrics-otlp-http to the newest version"} | ${[20, 82]} | ${[50, 82]} | ${[72, 82]} + ${"make a genuine attempt to fix the bugs that the users were complaining about"} | ${[20, 76]} | ${[50, 76]} | ${[72, 76]} + ${"shelve the broken cursor until the warm summer weather makes it shine again"} | ${[20, 75]} | ${[50, 75]} | ${[72, 75]} +`( + "when the subject line of $subjectLine exceeds 72 characters", + (props: { + subjectLine: string + expectedRange20: CharacterRange + expectedRange50: CharacterRange + expectedRange72: CharacterRange + }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with a maximum length of 20 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled20.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled20, commit.sha, { range: props.expectedRange20 }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 50 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled50.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled50, commit.sha, { range: props.expectedRange50 }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 72 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled72.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled72, commit.sha, { range: props.expectedRange72 }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRange20 | expectedRange50 + ${"retrieve data from the exclusive third-party service"} | ${[20, 52]} | ${[50, 52]} + ${"Forget to close the backtick section in `RapidTransportService"} | ${[20, 62]} | ${[50, 62]} + ${"Compare the list of items to the objects downloaded from the server"} | ${[20, 67]} | ${[50, 67]} + ${"Resolve memory issues to make the code work better than it did last year"} | ${[20, 72]} | ${[50, 72]} +`( + "when the subject line of $subjectLine exceeds 50 characters, but not 72 characters", + (props: { + subjectLine: string + expectedRange20: CharacterRange + expectedRange50: CharacterRange + }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with a maximum length of 20 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled20.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled20, commit.sha, { range: props.expectedRange20 }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 50 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled50.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled50, commit.sha, { range: props.expectedRange50 }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 72 characters", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled72.options) + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRange20 + ${"Make the user interface less chaotic (GH-11) (GL-17)"} | ${[20, 36]} + ${"<#71238> free up unclaimed disk space"} | ${[29, 37]} + ${"#1104: Upgrade rainstormy/updraft in GitHub Actions flows"} | ${[27, 57]} + ${"`pnpm dlx` is the preferred way now"} | ${[30, 35]} + ${"revisit the boolean properties in the `IceCreamMachine` constructor"} | ${[20, 67]} + ${"Remove redundant call to `wrapper`"} | ${[20, 25]} + ${"Enable `firewall` protection again"} | ${[30, 34]} + ${"(GH-72): fix security vulnerability in `BridgeService`"} | ${[29, 39]} + ${"Make a smile to the `camera` again"} | ${[28, 34]} + ${"revisit the glorious `MaxDPS` algorithm"} | ${[20, 39]} +`( + "when the subject line of $subjectLine contains tokens to be disregarded and still exceeds 20 characters, but not 50 characters", + (props: { subjectLine: string; expectedRange20: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with a maximum length of 20 characters", () => { + it("raises a concern about the characters that exceed the limit", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled20.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled20, commit.sha, { range: props.expectedRange20 }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 50 characters", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled50.options) + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with a maximum length of 72 characters", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], enabled72.options) + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${'Revert "retrieve data from the exclusive third-party service"'} + ${'Revert "Revert "Revert "Fix the nasty bug from yesterday"""'} + ${"fixup! Resolve memory issues to make the code work better than it did last year"} + ${"Squash! Make the program act like a clown"} +`( + "when the subject line of $subjectLine makes the commit disregardable", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe.each` + options + ${enabled20.options} + ${enabled50.options} + ${enabled72.options} + `( + "and the rule is enabled with a maximum length of $options.maxLength characters", + (context: RuleContext<"useConciseSubjectLines">) => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], context.options) + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + parents | subjectLine + ${[fakeCommitSha(), fakeCommitSha()]} | ${"Merge branch 'main' into bugfix/dance-party-playlist"} + ${[fakeCommitSha(), fakeCommitSha(), fakeCommitSha()]} | ${"Keep my branch up to date"} +`( + "when the commit is a merge commit with $parents.length parents", + (props: { parents: Array; subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine, parents: props.parents }) + + describe.each` + options + ${enabled20.options} + ${enabled50.options} + ${enabled72.options} + `( + "and the rule is enabled with a maximum length of $options.maxLength characters", + (context: RuleContext<"useConciseSubjectLines">) => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], context.options) + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${""} + ${" "} + ${"\t\t"} + ${"clean up"} + ${"test"} + ${"Refactor some stuff"} + ${"Move `RapidTransportService` up"} + ${"Upgrade React"} +`( + "when the subject line of $subjectLine does not exceed 20 characters", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe.each` + options + ${enabled20.options} + ${enabled50.options} + ${enabled72.options} + `( + "and the rule is enabled with a maximum length of $options.maxLength characters", + (context: RuleContext<"useConciseSubjectLines">) => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], context.options) + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines([commit], null) + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe("when verifying a set of multiple commits and some commits have long subject lines", () => { + const commits: Vector = [ + fakeCommit({ message: "Retrieve data from the exclusive third-party service" }), + fakeCommit({ message: "Refactor the taxi module" }), + fakeCommit({ + message: "Downgrade dependency @opentelemetry/exporter-metrics-otlp-http to 0.100.5-beta.3", + }), + fakeCommit({ message: "revisit the boolean properties in the `IceCreamMachine` constructor" }), + fakeCommit({ message: "test" }), + fakeCommit({ message: "#728 free up unclaimed disk space" }), + fakeCommit({ message: "Fix the bug" }), + fakeCommit({ + message: "Compare the list of items to the objects downloaded from the premium server", + }), + fakeCommit({ message: "Unsubscribe from the service" }), + fakeCommit({ message: "fixup! Unsubscribe from the service" }), + fakeCommit({ + message: + "Upgrade `rainstormy/github-action-validate-commit-messages in GitHub Actions` to the newest version", + }), + fakeCommit({ + message: "make a genuine attempt to fix the bugs that the users were complaining about", + }), + ] + + describe("and the rule is enabled with a maximum length of 20 characters", () => { + it("raises concerns about the commits whose subject lines exceed 20 characters", () => { + const actualConcerns = useConciseSubjectLines(commits, enabled20.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled20, commits[0].sha, { range: [20, 52] }), + subjectLineConcern(enabled20, commits[1].sha, { range: [20, 24] }), + subjectLineConcern(enabled20, commits[3].sha, { range: [20, 67] }), + subjectLineConcern(enabled20, commits[5].sha, { range: [25, 33] }), + subjectLineConcern(enabled20, commits[7].sha, { range: [20, 75] }), + subjectLineConcern(enabled20, commits[8].sha, { range: [20, 28] }), + subjectLineConcern(enabled20, commits[10].sha, { range: [89, 99] }), + subjectLineConcern(enabled20, commits[11].sha, { range: [20, 76] }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 50 characters", () => { + it("raises concerns about the commits whose subject lines exceed 50 characters", () => { + const actualConcerns = useConciseSubjectLines(commits, enabled50.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled50, commits[0].sha, { range: [50, 52] }), + subjectLineConcern(enabled50, commits[7].sha, { range: [50, 75] }), + subjectLineConcern(enabled50, commits[11].sha, { range: [50, 76] }), + ]) + }) + }) + + describe("and the rule is enabled with a maximum length of 72 characters", () => { + it("raises concerns about the commits whose subject lines exceed 72 characters", () => { + const actualConcerns = useConciseSubjectLines(commits, enabled72.options) + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled72, commits[7].sha, { range: [72, 75] }), + subjectLineConcern(enabled72, commits[11].sha, { range: [72, 76] }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines(commits, null) + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and all commits have concise subject lines", () => { + const commits: Vector = [ + fakeCommit({ message: "Fix the bug (GH-27)" }), + fakeCommit({ message: "#41: Refactor the factory" }), + fakeCommit({ message: "Upgrade Vitest to 4.1.2" }), + fakeCommit({ message: "Subscribe to the `RapidTransportService`" }), + ] + + describe.each` + options + ${enabled20.options} + ${enabled50.options} + ${enabled72.options} + `( + "and the rule is enabled with a maximum length of $options.maxLength characters", + (context: RuleContext<"useConciseSubjectLines">) => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines(commits, context.options) + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + it("does not raise any concerns", () => { + const actualConcerns = useConciseSubjectLines(commits, null) + expect(actualConcerns).toEqual([]) + }) + }) +}) diff --git a/src/domains/rules/UseConciseSubjectLines.ts b/src/domains/rules/UseConciseSubjectLines.ts index c405aa7d..6cb52130 100644 --- a/src/domains/rules/UseConciseSubjectLines.ts +++ b/src/domains/rules/UseConciseSubjectLines.ts @@ -1,7 +1,65 @@ -import type { Commits } from "#commits/Commit.ts" -import type { Concerns } from "#rules/concerns/Concern.ts" -import type { EmptyObject } from "#types/EmptyObject.ts" +import type { Commit, Commits } from "#commits/Commit.ts" +import { isDependencyVersion } from "#commits/tokens/DependencyVersionToken.ts" +import { isRevertMarker } from "#commits/tokens/RevertMarkerToken.ts" +import { isSquashMarker } from "#commits/tokens/SquashMarkerToken.ts" +import { isText } from "#commits/tokens/Token.ts" +import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" +import { type RuleContext, ruleContext } from "#rules/Rule.ts" +import { notNullish } from "#utilities/Arrays.ts" -export function useConciseSubjectLines(_commits: Commits, _options: EmptyObject | null): Concerns { - throw new Error("The `useConciseSubjectLines` rule has not been implemented yet") // TODO: To be implemented. +/** + * Verifies that the subject line does not exceed a given number of characters (default: 50 characters). + * + * Keeping the subject line short helps to preserve the readability of the commit history in various Git clients. + * + * It ignores merge commits and commits with dependency versions, revert markers, or squash markers. + * Issue links and inline code phrases do not count towards the limit. + */ +export function useConciseSubjectLines( + commits: Commits, + options: { maxLength: number } | null, +): Concerns { + if (options === null) { + return [] + } + + const rule = ruleContext("useConciseSubjectLines", options) + return commits.map((commit) => verifyCommit(commit, rule)).filter(notNullish) +} + +function verifyCommit(commit: Commit, rule: RuleContext<"useConciseSubjectLines">): Concern | null { + if (commit.isMergeCommit) { + return null + } + + const maxLength = rule.options.maxLength + + let textLength = 0 + let startOffset = 0 + let endOffset = 0 + let endOffsetPotential = 0 + + for (const token of commit.subjectLine) { + if (isDependencyVersion(token) || isRevertMarker(token) || isSquashMarker(token)) { + return null + } + if (isText(token)) { + textLength += token.length + endOffset += endOffsetPotential + endOffsetPotential = 0 + } else if (textLength <= maxLength) { + startOffset += token.value.length + } else { + endOffsetPotential += token.value.length + } + } + + if (textLength > maxLength) { + return subjectLineConcern(rule, commit.sha, { + range: [startOffset + maxLength, startOffset + textLength + endOffset], + }) + } + + return null } diff --git a/src/domains/rules/concerns/Concern.ts b/src/domains/rules/concerns/Concern.ts index f512df1d..00b0b5f8 100644 --- a/src/domains/rules/concerns/Concern.ts +++ b/src/domains/rules/concerns/Concern.ts @@ -1,4 +1,4 @@ -import type { Commits } from "#commits/Commit.ts" +import type { Commit, Commits } from "#commits/Commit.ts" import type { Configuration } from "#configurations/Configuration.ts" import type { AuthorEmailAddressConcern } from "#rules/concerns/AuthorEmailAddressConcern.ts" import type { AuthorNameConcern } from "#rules/concerns/AuthorNameConcern.ts" @@ -26,6 +26,7 @@ import { useImperativeSubjectLines } from "#rules/UseImperativeSubjectLines.ts" import { useIssueLinks } from "#rules/UseIssueLinks.ts" import { useLineWrapping } from "#rules/UseLineWrapping.ts" import { useSignedCommits } from "#rules/UseSignedCommits.ts" +import { requireNotNullish } from "#utilities/Assertions.ts" export type Concern = | AuthorEmailAddressConcern @@ -37,6 +38,13 @@ export type Concern = export type Concerns = Array +export function concernedCommit(concern: Concern, commits: Commits): Commit { + return requireNotNullish( + commits.find(({ sha }) => sha === concern.commitSha), + () => `Concerned commit ${concern.commitSha} not found`, + ) +} + export function mapCommitsToConcerns(commits: Commits, configuration: Configuration): Concerns { const rules = configuration.rules return [ diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index 94b61ab6..46acf90c 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -151,6 +151,102 @@ describe("when 'useCapitalisedSubjectLines' has a concern about characters 7-8", }) }) +describe("when 'useConciseSubjectLines' has a concern about characters 20-25", () => { + const commit = fakeCommit({ + sha: "68e921648c4a19e93d72f42a5d39c3eba704e41", + message: "Remove redundant call to `wrapper`", + }) + const concern = subjectLineConcern( + ruleContext("useConciseSubjectLines", { maxLength: 20 }), + commit.sha, + { range: [20, 25] }, + ) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +68e9216 Remove redundant call to \`wrapper\` + ──┬── + ╰─ Subject lines must not exceed 20 characters. + (useConciseSubjectLines) +`.trim(), + ) + }) +}) + +describe("when 'useConciseSubjectLines' has a concern about characters 20-67", () => { + const commit = fakeCommit({ + sha: "9bed522bd48f0aee7574635bb23f5decdc4999", + message: "revisit the boolean properties in the `IceCreamMachine` constructor", + }) + const concern = subjectLineConcern( + ruleContext("useConciseSubjectLines", { maxLength: 20 }), + commit.sha, + { range: [20, 67] }, + ) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +9bed522 revisit the boolean properties in the \`IceCreamMachine\` constructor + ───────────────────────┬─────────────────────── + Subject lines must not exceed 20 characters. ─╯ + (useConciseSubjectLines) +`.trim(), + ) + }) +}) + +describe("when 'useConciseSubjectLines' has a concern about characters 50-52", () => { + const commit = fakeCommit({ + sha: "e8c95d69587a51685070837aaf3a8746e3cbba8", + message: "Retrieve data from the exclusive third-party service", + }) + const concern = subjectLineConcern( + ruleContext("useConciseSubjectLines", { maxLength: 50 }), + commit.sha, + { range: [50, 52] }, + ) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +e8c95d6 Retrieve data from the exclusive third-party service + ─┬ + Subject lines must not exceed 50 characters. ─╯ + (useConciseSubjectLines) +`.trim(), + ) + }) +}) + +describe("when 'useConciseSubjectLines' has a concern about characters 72-76", () => { + const commit = fakeCommit({ + sha: "be86674322213fb408d176589fadbcd44a2df", + message: "make a genuine attempt to fix the bugs that the users were complaining about", + }) + const concern = subjectLineConcern( + ruleContext("useConciseSubjectLines", { maxLength: 72 }), + commit.sha, + { range: [72, 76] }, + ) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +be86674 make a genuine attempt to fix the bugs that the users were complaining about + ──┬─ + Subject lines must not exceed 72 characters. ─╯ + (useConciseSubjectLines) +`.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 4e0cca25..0fb44ee5 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -1,62 +1,50 @@ import type { Commit, Commits } from "#commits/Commit.ts" import { formatTokenisedLine } from "#commits/tokens/Token.ts" -import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import { type Concern, type Concerns, concernedCommit } from "#rules/concerns/Concern.ts" import type { RuleContext } from "#rules/Rule.ts" -import type { CharacterRange } from "#types/CharacterRange.ts" -import { requireNotNullish } from "#utilities/Assertions.ts" +import { formatCharacterRange } from "#types/CharacterRange.ts" import { indentString } from "#utilities/Strings.ts" export function commitwiseReport(commits: Commits, concerns: Concerns): string { - return concerns.map((concern) => formatConcern(commits, concern)).join("\n\n") + return concerns + .map((concern) => formatConcern(concern, concernedCommit(concern, commits))) + .join("\n\n") } const SHORT_SHA_LENGTH = 7 -function formatConcern(commits: Commits, concern: Concern): string { - return `${formatCommit(commits, concern)}\n${formatMessage(concern)}` -} +const MESSAGE_PREFIX = "╰─" +const MESSAGE_SUFFIX = "─╯" -function formatCommit(commits: Commits, concern: Concern): string { - const commit = requireNotNullish( - commits.find(({ sha }) => sha === concern.commitSha), - () => `Concerned commit ${concern.commitSha} not found`, - ) +function formatConcern(concern: Concern, commit: Commit): string { + const commitLine = `${commit.sha.slice(0, SHORT_SHA_LENGTH)} ${formatTokenisedLine(commit.subjectLine)}` - return `${commit.sha.slice(0, SHORT_SHA_LENGTH)} ${formatSubjectLine(commit)}` -} + const [rangeStart, rangeEnd] = concern.range + const length = rangeEnd - rangeStart -function formatSubjectLine(commit: Commit): string { - return formatTokenisedLine(commit.subjectLine) -} + const offset = SHORT_SHA_LENGTH + " ".length + rangeStart + const longHalfLength = Math.trunc(length / 2) + const shortHalfLength = length - longHalfLength - 1 -function formatMessage(concern: Concern): string { - const [start] = concern.range + const message = ruleMessage(concern.rule) + const anchoredRight = message.length + MESSAGE_SUFFIX.length < offset + longHalfLength - const [rangeMarker, rangeOffset] = formatRangeMarker(concern.range) - const message = `${formatRule(concern.rule)}\n ${" ".repeat(rangeOffset)}(${concern.rule.key})` - - const messageOffset = SHORT_SHA_LENGTH + " ".length + start - return indentString(`${rangeMarker} ${message}`, messageOffset) -} + const rangeLine = indentString(formatCharacterRange(concern.range, anchoredRight), offset) -function formatRangeMarker(range: CharacterRange): [string, offset: number] { - const [start, end] = range - const length = end - start + const messageLine = anchoredRight + ? indentString( + `${message} ${MESSAGE_SUFFIX}\n(${concern.rule.key})`, + offset + longHalfLength - message.length - MESSAGE_SUFFIX.length, + ) + : indentString( + `${MESSAGE_PREFIX} ${message}\n (${concern.rule.key})`, + offset + shortHalfLength, + ) - 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, - ] + return `${commitLine}\n${rangeLine}\n${messageLine}` } -function formatRule(rule: RuleContext): string { +function ruleMessage(rule: RuleContext): string { switch (rule.key) { case "noExcessiveCommitsPerBranch": { throw new Error(`Not implemented yet: ${rule.key}`) @@ -101,7 +89,7 @@ function formatRule(rule: RuleContext): string { throw new Error(`Not implemented yet: ${rule.key}`) } case "useConciseSubjectLines": { - throw new Error(`Not implemented yet: ${rule.key}`) + return `Subject lines must not exceed ${rule.options.maxLength} characters.` } case "useEmptyLineBeforeBodyLines": { throw new Error(`Not implemented yet: ${rule.key}`) diff --git a/src/types/CharacterRange.ts b/src/types/CharacterRange.ts index c16a2503..6dac4f57 100644 --- a/src/types/CharacterRange.ts +++ b/src/types/CharacterRange.ts @@ -1 +1,19 @@ export type CharacterRange = [startIndex: number, endIndex: number] + +export function formatCharacterRange(range: CharacterRange, anchoredRight: boolean): string { + const [start, end] = range + const length = end - start + + if (length === 1) { + return "┬" + } + + const longHalfLength = Math.trunc(length / 2) + const shortHalfLength = length - longHalfLength - 1 + + const longHalf = "─".repeat(longHalfLength) + const shortHalf = "─".repeat(shortHalfLength) + + return anchoredRight ? `${longHalf}┬${shortHalf}` : `${shortHalf}┬${longHalf}` + // anchored right ─╯ ╰─ anchored left +}