diff --git a/src/domains/commits/tokens/Token.ts b/src/domains/commits/tokens/Token.ts index a9d02ee8..c4c1bd43 100644 --- a/src/domains/commits/tokens/Token.ts +++ b/src/domains/commits/tokens/Token.ts @@ -27,6 +27,10 @@ export function formatTokenisedLine(tokens: TokenisedLine): string { return tokens.map((token) => token.value).join("") } +export function isNonBlankTokenisedLine(tokens: TokenisedLine): boolean { + return tokens.some((token) => token.type === "text" && token.value.trim() !== "") +} + export function splitTextTokens( tokens: TokenisedLine, regex: RegExp, diff --git a/src/domains/rules/UseEmptyLineBeforeBodyLines.tests.ts b/src/domains/rules/UseEmptyLineBeforeBodyLines.tests.ts new file mode 100644 index 00000000..5dffce26 --- /dev/null +++ b/src/domains/rules/UseEmptyLineBeforeBodyLines.tests.ts @@ -0,0 +1,227 @@ +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 { bodyLineConcern } from "#rules/concerns/BodyLineConcern.ts" +import type { Concerns } from "#rules/concerns/Concern.ts" +import type { RuleKey, RuleOptions } from "#rules/Rule.ts" +import { useEmptyLineBeforeBodyLines } from "#rules/UseEmptyLineBeforeBodyLines.ts" +import type { CharacterRange } from "#types/CharacterRange.ts" +import type { Vector } from "#types/Vector.ts" + +const rule = "useEmptyLineBeforeBodyLines" satisfies RuleKey +const enabled: RuleOptions = {} + +const fakeCommit = fakeCommitFactory(fakeConfiguration()) + +describe.each` + message + ${"init"} + ${"fix some bugs"} + ${"Upgrade Vite to 8.0.0"} + ${"Consolidate `BadgeFactory` with `BadgeService` and count to ten"} + ${'#1 Revert "Add an amazing feature"'} + ${" squash! organise the bookshelf"} +`("when the commit message of $message does not have a body", (props: { message: string }) => { + const commit = fakeCommit({ message: props.message }) + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], enabled) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe.each` + message + ${"\n\n"} + ${" \n\n\n "} + ${"Establish the repository\n"} + ${"GH-34 open the hotel room\n\n"} + ${"a majestic bottle of water \n \n "} + ${"fixup! Apply strawberry jam to make the code sweeter\n\n\n"} +`("when the commit message of $message only has a blank body", (props: { message: string }) => { + const commit = fakeCommit({ message: props.message }) + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], enabled) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe.each` + message | expectedLine | expectedRange + ${"bugfix #614\nattempt 2"} | ${0} | ${[0, 1]} + ${"Write unit tests\n Finally..."} | ${0} | ${[0, 1]} + ${"Refactor the taxi module\nThe roof sign now flashes politely."} | ${0} | ${[0, 1]} + ${"Install React\n```\npnpm add --save \\\n @types/react \\\n @types/react-dom \\\n react \\\n react-dom\n```"} | ${0} | ${[0, 1]} + ${'Revert "Repair the soft ice machine"\nThis commit undoes the changes made in revision 6227ca0.'} | ${0} | ${[0, 1]} + ${"squash! serve the tiny sandwiches\n guests get fewer crumbs in the keyboard"} | ${0} | ${[0, 1]} +`( + "when the commit message $message does not have an empty line before the body", + (props: { message: string; expectedLine: number; expectedRange: CharacterRange }) => { + const commit = fakeCommit({ message: props.message }) + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], enabled) + + it("raises a concern about the first body line", () => { + expect(actualConcerns).toEqual([ + bodyLineConcern(rule, commit.sha, { + line: props.expectedLine, + range: props.expectedRange, + }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + message | expectedLine | expectedRange + ${"Bring it on \n\n\nThe code is prepared for anything."} | ${1} | ${[0, 1]} + ${"Replace guesswork with a chart\n\n\n\nThe chart uses three excellent dots."} | ${2} | ${[0, 1]} + ${"Update src/main.ts\n \n \nCo-Authored-By: Everloving Easter Bunny "} | ${1} | ${[0, 1]} + ${"Bump @typescript-eslint/parser from 5.52.0 to 5.59.1\n\n\n\n\nBumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.52.0 to 5.59.1.\n- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)\n- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)"} | ${3} | ${[0, 1]} + ${"transport the fragile and expensive goods\n\n\n\n \n\nThis commit moves the goods from one place to another via the `RapidTransportService`."} | ${4} | ${[0, 1]} + ${"Install Vite\n\n\n```shell\npnpm add --save-dev vite\n```\n\nThis commit also uses Vite as bundler."} | ${1} | ${[0, 1]} +`( + "when the commit message $message has multiple blank lines before the body", + (props: { message: string; expectedLine: number; expectedRange: CharacterRange }) => { + const commit = fakeCommit({ message: props.message }) + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], enabled) + + it("raises a concern about the first non-blank body line", () => { + expect(actualConcerns).toEqual([ + bodyLineConcern(rule, commit.sha, { + line: props.expectedLine, + range: props.expectedRange, + }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + message + ${"fix a longstanding bug\n\nit was just a matter of time before it would cause customers to complain "} + ${"Refactor the taxi module\n\nThe meter now accepts exact change."} + ${"Resolve the conflicts\n\nConflicts:\n\n src/grumpy-cat.ts\n src/summer-vacation-plans.ts"} + ${"Teach the changelog to whisper\n\nThe noisy bits moved to the release notes.\n\nFooter: charming"} +`( + "when the commit message $message has exactly one empty line before the body", + (props: { message: string }) => { + const commit = fakeCommit({ message: props.message }) + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], enabled) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe("when verifying a set of multiple commits and some commits do not have exactly one empty line before the body", () => { + const commits: Vector = [ + fakeCommit({ message: "Refactor the taxi module" }), + fakeCommit({ message: "Install a quieter keyboard\nThe old one sounded like hail." }), + fakeCommit({ message: "Update the docs\n\nThe example now uses a bowl of fresh hot soup." }), + fakeCommit({ message: "Clean the tiny dashboard\n\n\nThe widgets sparkle." }), + fakeCommit({ message: "Use precise names\n" }), + ] + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines(commits, enabled) + + it("raises concerns about the commits with unexpected body lines", () => { + expect(actualConcerns).toEqual([ + bodyLineConcern(rule, commits[1].sha, { line: 0, range: [0, 1] }), + bodyLineConcern(rule, commits[3].sha, { line: 1, range: [0, 1] }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines(commits, null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and all commits has exactly one empty line before the body", () => { + const commits: Vector = [ + fakeCommit({ message: "Refactor the taxi module" }), + fakeCommit({ message: "Install a quieter keyboard\n\nThe old one sounded like hail." }), + fakeCommit({ message: "Update the docs\n\nThe example now uses a bowl of fresh hot soup." }), + fakeCommit({ message: "Clean the tiny dashboard\n\nThe widgets sparkle." }), + fakeCommit({ message: "Use precise names\n" }), + ] + + describe("and the rule is enabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines(commits, enabled) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useEmptyLineBeforeBodyLines(commits, null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) diff --git a/src/domains/rules/UseEmptyLineBeforeBodyLines.ts b/src/domains/rules/UseEmptyLineBeforeBodyLines.ts index 9efc72a1..97ca1e43 100644 --- a/src/domains/rules/UseEmptyLineBeforeBodyLines.ts +++ b/src/domains/rules/UseEmptyLineBeforeBodyLines.ts @@ -1,11 +1,18 @@ import type { Commit, Commits } from "#commits/Commit.ts" +import { isNonBlankTokenisedLine } from "#commits/tokens/Token.ts" +import { bodyLineConcern } from "#rules/concerns/BodyLineConcern.ts" import type { Concern, Concerns } from "#rules/concerns/Concern.ts" import type { RuleKey } from "#rules/Rule.ts" import type { EmptyObject } from "#types/EmptyObject.ts" import { notNullish } from "#utilities/Arrays.ts" -const _rule = "useEmptyLineBeforeBodyLines" satisfies RuleKey +const rule = "useEmptyLineBeforeBodyLines" satisfies RuleKey +/** + * Verifies that the subject line and message body is separated by exactly one empty line. + * + * Standardising the commit message format helps to preserve the readability of the commit history in various Git clients. + */ export function useEmptyLineBeforeBodyLines( commits: Commits, options: EmptyObject | null, @@ -13,6 +20,28 @@ export function useEmptyLineBeforeBodyLines( return options !== null ? commits.map(verifyCommit).filter(notNullish) : [] } -function verifyCommit(_commit: Commit): Concern | null { - throw new Error("The `useEmptyLineBeforeBodyLines` rule has not been implemented yet") // TODO: To be implemented. +function verifyCommit(commit: Commit): Concern | null { + let firstNonBlankLineNumber: number | null = null + let lineNumber = 0 + + for (const bodyLine of commit.bodyLines) { + if (isNonBlankTokenisedLine(bodyLine)) { + firstNonBlankLineNumber = lineNumber + break + } + + lineNumber += 1 + } + + if (firstNonBlankLineNumber === null) { + return null + } + if (firstNonBlankLineNumber === 0) { + return bodyLineConcern(rule, commit.sha, { line: 0, range: [0, 1] }) + } + if (firstNonBlankLineNumber > 1) { + return bodyLineConcern(rule, commit.sha, { line: firstNonBlankLineNumber - 1, range: [0, 1] }) + } + + return null } diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index b63d1a3d..92da194b 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest" import { fakeCommitFactory } from "#commits/Commit.fixtures.ts" import type { Commit, Commits } from "#commits/Commit.ts" import { fakeConfiguration } from "#configurations/Configuration.fixtures.ts" +import { bodyLineConcern } from "#rules/concerns/BodyLineConcern.ts" import { commitConcern } from "#rules/concerns/CommitConcern.ts" import type { Concerns } from "#rules/concerns/Concern.ts" import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" @@ -798,6 +799,66 @@ be86674 make a genuine attempt to fix the bugs that the users were complaining a }) }) +describe("when 'useEmptyLineBeforeBodyLines' has a concern about the first body line", () => { + const configuration = fakeConfiguration() + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "f163851c187a568acc631fff402fcca43f41968d", + message: "Install a quieter keyboard\nThe old one sounded like hail.", + }) + const concern = bodyLineConcern("useEmptyLineBeforeBodyLines", commit.sha, { + line: 0, + range: [0, 1], + }) + + it("describes the rule violation in the body line", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +f163851 Install a quieter keyboard + ╭── +∙ 1 │ The old one sounded like hail. + · ┬ + · ╰─ Subject lines and message bodies must be separated by exactly one empty line. + · (useEmptyLineBeforeBodyLines) + ╰── +`.trim(), + ) + }) +}) + +describe("when 'useEmptyLineBeforeBodyLines' has a concern about an extra empty body line", () => { + const configuration = fakeConfiguration() + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "30fd57b19c55b3746a157f5981838e26865637f2", + message: "Clean the tiny dashboard\n\n\nThe widgets sparkle.\nAnd the birds are joyful.", + }) + const concern = bodyLineConcern("useEmptyLineBeforeBodyLines", commit.sha, { + line: 1, + range: [0, 1], + }) + + it("describes the rule violation in the body line", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +30fd57b Clean the tiny dashboard + ╭── + 1 │ +∙ 2 │ + · ┬ + · ╰─ Subject lines and message bodies must be separated by exactly one empty line. + · (useEmptyLineBeforeBodyLines) + 3 │ The widgets sparkle. + ╰── +`.trim(), + ) + }) +}) + describe("when 'useIssueLinks' with position 'anywhere' has a concern about characters 0-1 of the subject line", () => { const configuration = fakeConfiguration({ rules: { useIssueLinks: { position: "anywhere" } }, diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index bc39414c..fdc622b3 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -1,7 +1,8 @@ import type { Commit, Commits } from "#commits/Commit.ts" -import { formatTokenisedLine } from "#commits/tokens/Token.ts" +import { type TokenisedLines, formatTokenisedLine } from "#commits/tokens/Token.ts" import type { Configuration } from "#configurations/Configuration.ts" import { pluralise } from "#legacy-v1/utilities/StringUtilities.ts" +import type { BodyLineConcern } from "#rules/concerns/BodyLineConcern.ts" import type { CommitConcern } from "#rules/concerns/CommitConcern.ts" import { type Concern, type Concerns, concernedCommit } from "#rules/concerns/Concern.ts" import type { SubjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" @@ -9,7 +10,7 @@ import type { UserIdentityConcern } from "#rules/concerns/UserIdentityConcern.ts import type { RuleKey, RuleOptions } from "#rules/Rule.ts" import { formatCharacterRange } from "#types/CharacterRange.ts" import { requireNotNullish } from "#utilities/Assertions.ts" -import { formatCount, indentString } from "#utilities/Strings.ts" +import { formatCount, indentString, prefixStringLines } from "#utilities/Strings.ts" export function commitwiseReport( concerns: Concerns, @@ -24,7 +25,7 @@ export function commitwiseReport( function formatConcern(concern: Concern, commit: Commit, configuration: Configuration): string { switch (concern.location) { case "body-line": { - throw new Error(`Not implemented yet: ${concern.location}`) + return formatBodyLineConcern(concern, commit, configuration) } case "commit": { return formatCommitConcern(concern, commit, configuration) @@ -87,6 +88,64 @@ function formatSubjectLineConcern( return `${commitLine}\n${rangeLine}\n${messageLines}` } +function formatBodyLineConcern( + concern: BodyLineConcern, + commit: Commit, + configuration: Configuration, +): string { + const message = getRuleMessage(concern.rule, configuration) + + const [rangeStart, rangeEnd] = concern.range + const length = rangeEnd - rangeStart + + const gutterWidth = Math.ceil(Math.log10(concern.line + 2)) + 3 + const concernGutter = indentString("· ", gutterWidth) + + const offset = gutterWidth + 2 + rangeStart + const longHalfLength = Math.trunc(length / 2) + const shortHalfLength = length - longHalfLength - 1 + + const violationLength = message.violation.length + MESSAGE_SUFFIX.length + const anchoredRight = violationLength < offset + longHalfLength + + const commitLine = getCommitLine(commit) + + const precedingBodyLine = getBodyLine(commit.bodyLines, concern.line - 1, gutterWidth) + const blockHeadLines = `${indentString("╭──", gutterWidth)}\n${precedingBodyLine}` + + const concernedBodyLine = getBodyLine(commit.bodyLines, concern.line, gutterWidth, true) + + const rangeLine = `${concernGutter}${indentString( + formatCharacterRange(concern.range, anchoredRight), + rangeStart, + )}` + + const messageLines = anchoredRight + ? getMessageLines(message, rangeStart + longHalfLength - violationLength, true) + : getMessageLines(message, rangeStart + shortHalfLength) + + const succeedingBodyLine = getBodyLine(commit.bodyLines, concern.line + 1, gutterWidth) + const blockTailLines = `${succeedingBodyLine}${indentString("╰──", gutterWidth)}` + + return `${commitLine}\n${blockHeadLines}${concernedBodyLine}${rangeLine}\n${prefixStringLines(messageLines, concernGutter)}\n${blockTailLines}` +} + +function getBodyLine( + bodyLines: TokenisedLines, + lineNumber: number, + gutterWidth: number, + isConcernedLine = false, +): string { + const bodyLine = bodyLines[lineNumber] ?? null + + if (bodyLine === null) { + return "" + } + + const formattedLineNumber = (lineNumber + 1).toString().padStart(gutterWidth - 3, " ") + return `${isConcernedLine ? "∙" : " "} ${formattedLineNumber} │ ${formatTokenisedLine(bodyLine)}\n` +} + function formatUserIdentityConcern( concern: UserIdentityConcern, commit: Commit, @@ -218,7 +277,9 @@ function getRuleMessage(rule: RuleKey, configuration: Configuration): RuleMessag return ruleMessage(`Subject lines must not exceed ${characterPhrase}.`) } case "useEmptyLineBeforeBodyLines": { - throw new Error(`Not implemented yet: ${rule}`) + return ruleMessage( + "Subject lines and message bodies must be separated by exactly one empty line.", + ) } case "useImperativeSubjectLines": { return ruleMessage("Subject lines must start with a verb in the imperative mood.") diff --git a/src/utilities/Strings.ts b/src/utilities/Strings.ts index e65c8c47..cef3a825 100644 --- a/src/utilities/Strings.ts +++ b/src/utilities/Strings.ts @@ -22,6 +22,9 @@ export function formatCount(count: number, singular: string, plural: string): st } export function indentString(value: string, offset: number): string { - const indent = " ".repeat(offset) - return indent + value.replaceAll("\n", `\n${indent}`) + return prefixStringLines(value, " ".repeat(offset)) +} + +export function prefixStringLines(value: string, prefix: string): string { + return prefix + value.replaceAll("\n", `\n${prefix}`) }