From b19857650f85bc9cdd1c6895dbc232f83aeddab1 Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Sun, 26 Apr 2026 23:35:27 +0200 Subject: [PATCH 1/4] Fix pluralisation in `useConciseSubjectLines` message --- src/domains/rules/reports/CommitwiseReport.ts | 5 +++-- src/utilities/Strings.ts | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index 6a3c6fba..dae239d1 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -7,7 +7,7 @@ import type { SubjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" import type { RuleKey, RuleOptions } from "#rules/Rule.ts" import { formatCharacterRange } from "#types/CharacterRange.ts" import { requireNotNullish } from "#utilities/Assertions.ts" -import { indentString } from "#utilities/Strings.ts" +import { formatCount, indentString } from "#utilities/Strings.ts" export function commitwiseReport( concerns: Concerns, @@ -151,7 +151,8 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { } case "useConciseSubjectLines": { const options = getRuleOptions(rule, configuration) - return `Subject lines must not exceed ${options.maxLength} characters.` + const characterPhrase = formatCount(options.maxLength, "character", "characters") + return `Subject lines must not exceed ${characterPhrase}.` } case "useEmptyLineBeforeBodyLines": { throw new Error(`Not implemented yet: ${rule}`) diff --git a/src/utilities/Strings.ts b/src/utilities/Strings.ts index 2223c10b..5e2c4c9d 100644 --- a/src/utilities/Strings.ts +++ b/src/utilities/Strings.ts @@ -13,3 +13,7 @@ export function countOccurrences( const regex = new RegExp(RegExp.escape(search), caseInsensitive ? "giu" : "gu") return value.match(regex)?.length ?? 0 } + +export function formatCount(count: number, singular: string, plural: string): string { + return `${count} ${count === 1 ? singular : plural}` +} From 3b529f0923fbe1e5641bba5f2f16a7345fe28216 Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Sat, 2 May 2026 18:02:00 +0200 Subject: [PATCH 2/4] Add unit tests for dependency upgrade commits --- src/domains/rules/UseConciseSubjectLines.tests.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/domains/rules/UseConciseSubjectLines.tests.ts b/src/domains/rules/UseConciseSubjectLines.tests.ts index 338ee5c6..7b90f3d3 100644 --- a/src/domains/rules/UseConciseSubjectLines.tests.ts +++ b/src/domains/rules/UseConciseSubjectLines.tests.ts @@ -234,12 +234,19 @@ describe.each` describe.each` subjectLine + ${"Bump vite from 4.1.1-beta.0 to 4.3.2"} + ${"Upgrade tsgo to 7.0.0-dev.20260131.1"} + ${"squash! Downgrade the grumpy cat module from 3.1.4 to 3.0.0"} + ${"Pre-release v10.12.22-next"} + ${"Pin the Node.js image to 4af617c"} ${'Revert "bugfix"'} - ${'Revert "retrieve data from the exclusive third-party service"'} + ${'revert "Organise the quarterly chaos into a spreadsheet"'} + ${'amend! Revert "retrieve data from the exclusive third-party service"'} ${'Revert "Revert "Revert "Fix the nasty bug from yesterday"""'} - ${'fixup! Revert "keep the lowercase story around long enough to exceed the limit"'} + ${'fixup! revert "keep the lowercase story around long enough to exceed the limit"'} + ${'Revert "Upgrade nginx image digest to 9d739ff1ada6"'} `( - "when the subject line of $subjectLine contains a revert marker", + "when the subject line of $subjectLine contains a dependency version or a revert marker", (props: { subjectLine: string }) => { const commit = fakeCommit({ message: props.subjectLine }) From b218d0d2d2ec3ffb1c9d3a3d19de3d585e5ecff3 Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Sun, 3 May 2026 14:42:30 +0200 Subject: [PATCH 3/4] Implement `useIssueLinks` --- .../configurations/Configuration.fixtures.ts | 2 +- src/domains/rules/UseIssueLinks.tests.ts | 626 ++++++++++++++++++ src/domains/rules/UseIssueLinks.ts | 104 ++- .../rules/reports/CommitwiseReport.tests.ts | 75 +++ src/domains/rules/reports/CommitwiseReport.ts | 10 +- 5 files changed, 809 insertions(+), 8 deletions(-) create mode 100644 src/domains/rules/UseIssueLinks.tests.ts diff --git a/src/domains/configurations/Configuration.fixtures.ts b/src/domains/configurations/Configuration.fixtures.ts index 987a0e6b..a80f11c8 100644 --- a/src/domains/configurations/Configuration.fixtures.ts +++ b/src/domains/configurations/Configuration.fixtures.ts @@ -35,7 +35,7 @@ export function fakeConfiguration(overrides: ConfigurationTemplate = {}): Config useConciseSubjectLines: { maxLength: 50 }, useEmptyLineBeforeBodyLines: {}, useImperativeSubjectLines: {}, - useIssueLinks: {}, + useIssueLinks: { position: "anywhere" }, useLineWrapping: {}, useSignedCommits: {}, ...overrides.rules, diff --git a/src/domains/rules/UseIssueLinks.tests.ts b/src/domains/rules/UseIssueLinks.tests.ts new file mode 100644 index 00000000..57a5f355 --- /dev/null +++ b/src/domains/rules/UseIssueLinks.tests.ts @@ -0,0 +1,626 @@ +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 { RuleKey, RuleOptions } from "#rules/Rule.ts" +import { useIssueLinks } from "#rules/UseIssueLinks.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 rule = "useIssueLinks" satisfies RuleKey + +const enabledAnywhere: RuleOptions = { position: "anywhere" } +const enabledPrefix: RuleOptions = { position: "prefix" } +const enabledSuffix: RuleOptions = { position: "suffix" } + +const fakeCommit = fakeCommitFactory(fakeConfiguration()) + +describe.each` + subjectLine | expectedRangeAnywhere | expectedRangePrefix | expectedRangeSuffix + ${"bugfix"} | ${[0, 1]} | ${[0, 1]} | ${[6, 7]} + ${"Convince the office printer to print in colour"} | ${[0, 1]} | ${[0, 1]} | ${[46, 47]} + ${"accept `pseudocode:` as a valid keyword in the compiler"} | ${[0, 1]} | ${[0, 1]} | ${[55, 56]} + ${" squash! "} | ${[9, 10]} | ${[9, 10]} | ${[9, 10]} + ${"fixup! Smoothen the rough edges"} | ${[7, 8]} | ${[7, 8]} | ${[31, 32]} +`( + "when the subject line of $subjectLine does not contain an issue link", + (props: { + subjectLine: string + expectedRangeAnywhere: CharacterRange + expectedRangePrefix: CharacterRange + expectedRangeSuffix: CharacterRange + }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("raises a concern about the beginning of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeAnywhere }), + ]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("raises a concern about the beginning of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangePrefix }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("raises a concern about the end of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeSuffix }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRangeSuffix + ${"(GH-7) hotfix"} | ${[13, 14]} + ${"#42 Convince the office printer to print in colour"} | ${[50, 51]} + ${"GL-1024 keep the hamsters on the wheel"} | ${[38, 39]} + ${"#1 #2 three go"} | ${[14, 15]} + ${"fixup! GH-88 is the issue that explains why this code exists"} | ${[60, 61]} +`( + "when the subject line of $subjectLine starts with an issue link", + (props: { subjectLine: string; expectedRangeSuffix: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("raises a concern about the end of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeSuffix }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRangePrefix + ${"hotfix (GH-7)"} | ${[0, 1]} + ${"Convince the office printer to print in colour #42"} | ${[0, 1]} + ${"keep the hamsters on the wheel GL-1024"} | ${[0, 1]} + ${"one two #3 #4"} | ${[0, 1]} + ${"squash! the issue that explains why this code exists is GH-88"} | ${[8, 9]} +`( + "when the subject line of $subjectLine ends with an issue link", + (props: { subjectLine: string; expectedRangePrefix: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("raises a concern about the start of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangePrefix }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRangePrefix | expectedRangeSuffix + ${"Replace GH-42 the hamster wheel with a turbine"} | ${[0, 1]} | ${[46, 47]} + ${"promote #7 from staging to production on a friday"} | ${[0, 1]} | ${[49, 50]} + ${"fixup! Override GL-88 by shouting at the monitor"} | ${[7, 8]} | ${[48, 49]} +`( + "when the subject line of $subjectLine has an issue link in the middle", + (props: { + subjectLine: string + expectedRangePrefix: CharacterRange + expectedRangeSuffix: CharacterRange + }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("raises a concern about the start of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangePrefix }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("raises a concern about the end of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeSuffix }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${"#7 polish the rough edges off the api GH-7"} + ${"(GH-42) Admit that the bug was intentional all along [#50]"} + ${"amend! GL-9 Deploy to production on a wing and a prayer GL-9"} +`( + "when the subject line of $subjectLine starts and ends with an issue link", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe.each` + options + ${enabledAnywhere} + ${enabledPrefix} + ${enabledSuffix} + `( + "and the rule is enabled with position '$options.position'", + (options: RuleOptions) => { + const actualConcerns = useIssueLinks([commit], options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${"Bump vite from 4.1.1-beta.0 to 4.3.2"} + ${"Upgrade tsgo to 7.0.0-dev.20260131.1"} + ${"squash! Downgrade the grumpy cat module from 3.1.4 to 3.0.0"} + ${"Pre-release v10.12.22-next"} + ${"Pin the Node.js image to 4af617c"} + ${'Revert "bugfix"'} + ${'revert "Organise the quarterly chaos into a spreadsheet"'} + ${'amend! Revert "retrieve data from the exclusive third-party service"'} + ${'Revert "Revert "Revert "Fix the nasty bug from yesterday"""'} + ${'fixup! revert "keep the lowercase story around long enough to exceed the limit"'} + ${'Revert "Upgrade nginx image digest to 9d739ff1ada6"'} +`( + "when the subject line of $subjectLine contains a dependency version or a revert marker", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe.each` + options + ${enabledAnywhere} + ${enabledPrefix} + ${enabledSuffix} + `( + "and the rule is enabled with position '$options.position'", + (options: RuleOptions) => { + const actualConcerns = useIssueLinks([commit], options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + parents | subjectLine + ${[fakeCommitSha(), fakeCommitSha()]} | ${"Merge branch 'main' into feature/time-machine"} + ${[fakeCommitSha(), fakeCommitSha(), fakeCommitSha()]} | ${"Merge branch 'feature/robot-butler' into feature/office-overhaul"} + ${[fakeCommitSha(), fakeCommitSha()]} | ${"Keep the branch up to date (no ticket required)"} +`( + "when the commit is a merge commit with $parents.length parents and has a subject line of $subjectLine", + (props: { parents: Array; subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine, parents: props.parents }) + + describe.each` + options + ${enabledAnywhere} + ${enabledPrefix} + ${enabledSuffix} + `( + "and the rule is enabled with position '$options.position'", + (options: RuleOptions) => { + const actualConcerns = useIssueLinks([commit], options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe("when verifying a set of multiple commits and some commits are missing issue links", () => { + const commits: Vector = [ + fakeCommit({ message: "#42 Convince the office printer to print in colour" }), + fakeCommit({ message: "Organise the quarterly chaos into a spreadsheet" }), + fakeCommit({ message: "fixup! GL-3 Polish the rough edges off the API" }), + fakeCommit({ message: "GH-88 is the issue that explains why this code exists" }), + fakeCommit({ message: "squash! accept `pseudocode:` as a valid keyword in the compiler" }), + fakeCommit({ message: "Add a message in the error log GL-99" }), + fakeCommit({ message: 'revert "Add an emergency eject button to the legacy module"' }), + fakeCommit({ message: "Replace #21 the hamster wheel with a turbine" }), + ] + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks(commits, enabledAnywhere) + + it("raises concerns about commits without an issue link", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commits[1].sha, { range: [0, 1] }), + subjectLineConcern(rule, commits[4].sha, { range: [8, 9] }), + ]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks(commits, enabledPrefix) + + it("raises concerns about commits whose issue link is not at the prefix", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commits[1].sha, { range: [0, 1] }), + subjectLineConcern(rule, commits[4].sha, { range: [8, 9] }), + subjectLineConcern(rule, commits[5].sha, { range: [0, 1] }), + subjectLineConcern(rule, commits[7].sha, { range: [0, 1] }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks(commits, enabledSuffix) + + it("raises concerns about commits whose issue link is not at the suffix", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commits[0].sha, { range: [50, 51] }), + subjectLineConcern(rule, commits[1].sha, { range: [47, 48] }), + subjectLineConcern(rule, commits[2].sha, { range: [46, 47] }), + subjectLineConcern(rule, commits[3].sha, { range: [53, 54] }), + subjectLineConcern(rule, commits[4].sha, { range: [63, 64] }), + subjectLineConcern(rule, commits[7].sha, { range: [44, 45] }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks(commits, null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and all commits start with an issue link", () => { + const commits: Vector = [ + fakeCommit({ message: "#1 Convince the office printer to print in colour" }), + fakeCommit({ message: "(GH-2) Organise the quarterly chaos into a spreadsheet" }), + fakeCommit({ message: "fixup! GL-3 Polish the rough edges off the API" }), + fakeCommit({ message: 'revert "Add an emergency eject button to the legacy module"' }), + fakeCommit({ message: "GH-4 is the issue that explains why this code exists" }), + ] + + describe.each` + options + ${enabledAnywhere} + ${enabledPrefix} + `( + "and the rule is enabled with position '$options.position'", + (options: RuleOptions) => { + const actualConcerns = useIssueLinks(commits, options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks(commits, null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and all commits end with an issue link", () => { + const commits: Vector = [ + fakeCommit({ message: "Convince the office printer to print in colour #1" }), + fakeCommit({ message: "Organise the quarterly chaos into a spreadsheet (GH-2)" }), + fakeCommit({ message: "fixup! Polish the rough edges off the API GL-3" }), + fakeCommit({ message: 'revert "Add an emergency eject button to the legacy module"' }), + fakeCommit({ message: "the issue that explains why this code exists is GH-4" }), + ] + + describe.each` + options + ${enabledAnywhere} + ${enabledSuffix} + `( + "and the rule is enabled with position '$options.position'", + (options: RuleOptions) => { + const actualConcerns = useIssueLinks(commits, options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }, + ) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks(commits, null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +const fakeJiraStyleCommit = fakeCommitFactory( + fakeConfiguration({ + tokens: { issueLinkPrefixes: ["UNICORN-"] }, + }), +) + +describe.each` + subjectLine | expectedRangeAnywhere | expectedRangePrefix | expectedRangeSuffix + ${"#1 bugfix"} | ${[0, 1]} | ${[0, 1]} | ${[9, 10]} + ${"Convince the office printer to print in colour #2"} | ${[0, 1]} | ${[0, 1]} | ${[49, 50]} + ${"#3 accept `pseudocode:` as a valid keyword in the compiler"} | ${[0, 1]} | ${[0, 1]} | ${[58, 59]} + ${" squash! "} | ${[9, 10]} | ${[9, 10]} | ${[9, 10]} + ${"fixup! #41 Smoothen the rough edges #42"} | ${[7, 8]} | ${[7, 8]} | ${[39, 40]} +`( + "when the subject line of $subjectLine does not contain a Jira-style issue link", + (props: { + subjectLine: string + expectedRangeAnywhere: CharacterRange + expectedRangePrefix: CharacterRange + expectedRangeSuffix: CharacterRange + }) => { + const commit = fakeJiraStyleCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("raises a concern about the beginning of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeAnywhere }), + ]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("raises a concern about the beginning of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangePrefix }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("raises a concern about the end of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeSuffix }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRangeSuffix + ${"(UNICORN-7) hotfix"} | ${[18, 19]} + ${"UNICORN-42 Convince the office printer to print in colour"} | ${[57, 58]} + ${"UNICORN-1024 keep the hamsters on the wheel"} | ${[43, 44]} + ${"UNICORN-1 UNICORN-2 three go"} | ${[28, 29]} + ${"fixup! UNICORN-88 is the issue that explains why this code exists"} | ${[65, 66]} +`( + "when the subject line of $subjectLine starts with a Jira-style issue link", + (props: { subjectLine: string; expectedRangeSuffix: CharacterRange }) => { + const commit = fakeJiraStyleCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("raises a concern about the end of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangeSuffix }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine | expectedRangePrefix + ${"hotfix (UNICORN-7)"} | ${[0, 1]} + ${"Convince the office printer to print in colour UNICORN-42"} | ${[0, 1]} + ${"keep the hamsters on the wheel UNICORN-1024"} | ${[0, 1]} + ${"one two UNICORN-3 UNICORN-4"} | ${[0, 1]} + ${"squash! the issue that explains why this code exists is UNICORN-88"} | ${[8, 9]} +`( + "when the subject line of $subjectLine ends with a Jira-style issue link", + (props: { subjectLine: string; expectedRangePrefix: CharacterRange }) => { + const commit = fakeJiraStyleCommit({ message: props.subjectLine }) + + describe("and the rule is enabled with position 'anywhere'", () => { + const actualConcerns = useIssueLinks([commit], enabledAnywhere) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is enabled with position 'prefix'", () => { + const actualConcerns = useIssueLinks([commit], enabledPrefix) + + it("raises a concern about the start of the subject line", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(rule, commit.sha, { range: props.expectedRangePrefix }), + ]) + }) + }) + + describe("and the rule is enabled with position 'suffix'", () => { + const actualConcerns = useIssueLinks([commit], enabledSuffix) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = useIssueLinks([commit], null) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) diff --git a/src/domains/rules/UseIssueLinks.ts b/src/domains/rules/UseIssueLinks.ts index 7e54ae17..1aa9bbbb 100644 --- a/src/domains/rules/UseIssueLinks.ts +++ b/src/domains/rules/UseIssueLinks.ts @@ -1,20 +1,112 @@ import type { Commit, Commits } from "#commits/Commit.ts" +import type { Token } from "#commits/tokens/Token.ts" import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" import type { RuleKey, RuleOptions } from "#rules/Rule.ts" -import type { EmptyObject } from "#types/EmptyObject.ts" import { notNullish } from "#utilities/Arrays.ts" -const _rule = "useIssueLinks" satisfies RuleKey +const rule = "useIssueLinks" satisfies RuleKey /** - * It ignores revert commits. + * Verifies that the subject line contains at least one issue link. + * + * Linking commits to issues in a project management system like GitHub or Jira + * provides traceability between code changes and the work that motivated them, + * making it easier to understand the project history and to find related changes. + * + * It ignores merge commits, revert commits, and dependency upgrade commits. + * It disregards squash markers when the position is `prefix`. */ -export function useIssueLinks(commits: Commits, options: EmptyObject | null): Concerns { +export function useIssueLinks( + commits: Commits, + options: { position: "anywhere" | "prefix" | "suffix" } | null, +): Concerns { return options !== null ? commits.map((commit) => verifyCommit(commit, options)).filter(notNullish) : [] } -function verifyCommit(_commit: Commit, _options: RuleOptions): Concern | null { - throw new Error("The `useIssueLinks` rule has not been implemented yet") // TODO: To be implemented. +function verifyCommit(commit: Commit, options: RuleOptions): Concern | null { + if (commit.isMergeCommit) { + return null + } + + switch (options.position) { + case "anywhere": { + return verifyAnywhere(commit) + } + case "prefix": { + return verifyPrefix(commit) + } + case "suffix": { + return verifySuffix(commit) + } + } +} + +function verifyAnywhere(commit: Commit): Concern | null { + let lastInsignificantToken: Token | null = null + + for (const token of commit.subjectLine) { + if ( + token.type === "dependency-version" || + token.type === "issue-link" || + token.type === "revert-marker" + ) { + return null + } + if (token.type === "squash-marker") { + lastInsignificantToken = token + } + } + + const firstSignificantIndex = lastInsignificantToken?.range[1] ?? 0 + + return subjectLineConcern(rule, commit.sha, { + range: [firstSignificantIndex, firstSignificantIndex + 1], + }) +} + +function verifyPrefix(commit: Commit): Concern | null { + let firstSignificantToken: Token | null = null + let lastInsignificantToken: Token | null = null + + for (const token of commit.subjectLine) { + if (token.type === "dependency-version" || token.type === "revert-marker") { + return null + } + if (firstSignificantToken === null) { + if (token.type === "issue-link") { + return null + } + if (token.type !== "squash-marker") { + firstSignificantToken = token + } else { + lastInsignificantToken = token + } + } + } + + const firstSignificantIndex = lastInsignificantToken?.range[1] ?? 0 + + return subjectLineConcern(rule, commit.sha, { + range: [firstSignificantIndex, firstSignificantIndex + 1], + }) +} + +function verifySuffix(commit: Commit): Concern | null { + for (const token of commit.subjectLine) { + if (token.type === "dependency-version" || token.type === "revert-marker") { + return null + } + } + + const lastToken = commit.subjectLine.at(-1) + + if (lastToken?.type === "issue-link") { + return null + } + + const lastIndex = lastToken?.range[1] ?? 0 + return subjectLineConcern(rule, commit.sha, { range: [lastIndex, lastIndex + 1] }) } diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index 3315a38d..1c6eb295 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -399,6 +399,81 @@ be86674 make a genuine attempt to fix the bugs that the users were complaining a }) }) +describe("when 'useIssueLinks' with position 'anywhere' has a concern about characters 0-1 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "anywhere" } }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "c861aeae9dcdea99776d1e56c4de100ba29effb", + message: "Organise the robot uprising without a ticket", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [0, 1] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +c861aea Organise the robot uprising without a ticket + ┬ + ╰─ Subject lines must include an issue link. + (useIssueLinks) +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'prefix' has a concern about characters 7-8 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "prefix" } }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "fb100238cf55fdcc7c48044df4b5922c0886f5c2d", + message: "amend! Teach the unit tests to write themselves", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [7, 8] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +fb10023 amend! Teach the unit tests to write themselves + ┬ + ╰─ Subject lines must start with an issue link. + (useIssueLinks) +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'suffix' has a concern about characters 49-50 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "suffix" } }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "d9a30bb22c78cf24fc6a79a3131a33829792bd4", + message: "make the automated tests question their existence", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [49, 50] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +d9a30bb make the automated tests question their existence + ┬ + Subject lines must end with an issue link. ─╯ + (useIssueLinks) +`.trim(), + ) + }) +}) + describe.todo("when there are multiple concerns of different types", () => { const configuration = fakeConfiguration() const fakeCommit = fakeCommitFactory(configuration) diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index dae239d1..9d3621b3 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -161,7 +161,15 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { throw new Error(`Not implemented yet: ${rule}`) } case "useIssueLinks": { - throw new Error(`Not implemented yet: ${rule}`) + const options = getRuleOptions(rule, configuration) + const positionPhrase = + options.position === "prefix" + ? "start with" + : options.position === "suffix" + ? "end with" + : "include" + + return `Subject lines must ${positionPhrase} an issue link.` } case "useLineWrapping": { throw new Error(`Not implemented yet: ${rule}`) From 4af8bae47d312aa0496e393bee9e13194145a16e Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Sun, 3 May 2026 18:34:36 +0200 Subject: [PATCH 4/4] Include issue link examples in reports --- .../rules/reports/CommitwiseReport.tests.ts | 117 ++++++++++++++++++ src/domains/rules/reports/CommitwiseReport.ts | 41 +++--- 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index 1c6eb295..b692cc19 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -419,6 +419,8 @@ c861aea Organise the robot uprising without a ticket ┬ ╰─ Subject lines must include an issue link. (useIssueLinks) + + Examples: #123, GH-123, GL-123 `.trim(), ) }) @@ -444,6 +446,8 @@ fb10023 amend! Teach the unit tests to write themselves ┬ ╰─ Subject lines must start with an issue link. (useIssueLinks) + + Examples: #123, GH-123, GL-123 `.trim(), ) }) @@ -469,6 +473,119 @@ d9a30bb make the automated tests question their existence ┬ Subject lines must end with an issue link. ─╯ (useIssueLinks) + + Examples: #123, GH-123, GL-123 +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'suffix' has a concern about characters 26-27 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "suffix" } }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "5761bad8f4bbdd9f22eac552ca15a42dd547692", + message: "Cooked this commit at 3 AM", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [26, 27] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +5761bad Cooked this commit at 3 AM + ┬ + ╰─ Subject lines must end with an issue link. + (useIssueLinks) + + Examples: #123, GH-123, GL-123 +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'anywhere' and Jira-style issue links has a concern about characters 10-11 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "anywhere" } }, + tokens: { issueLinkPrefixes: ["ABC-"] }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "d0709d2e4d2c55bf37ec7e7632f655e8e9b3eb", + message: " squash! made the code so clean that it sparkles", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [10, 11] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +d0709d2 squash! made the code so clean that it sparkles + ┬ + ╰─ Subject lines must include an issue link. + (useIssueLinks) + + Example: ABC-123 +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'prefix' and custom-style issue links has a concern about characters 0-1 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "prefix" } }, + tokens: { issueLinkPrefixes: ["test#", "experiment#"] }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "f6fc2399d62caefc3e4bfd8bf2a8da28fffafe", + message: "Refactored code, now it’s overpowered", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [0, 1] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +f6fc239 Refactored code, now it’s overpowered + ┬ + ╰─ Subject lines must start with an issue link. + (useIssueLinks) + + Examples: test#123, experiment#123 +`.trim(), + ) + }) +}) + +describe("when 'useIssueLinks' with position 'suffix' and Jira-style issue links has a concern about characters 41-42 of the subject line", () => { + const configuration = fakeConfiguration({ + rules: { useIssueLinks: { position: "suffix" } }, + tokens: { issueLinkPrefixes: ["AWESOME-", "UNICORN-", "PROJECT-"] }, + }) + const fakeCommit = fakeCommitFactory(configuration) + + const commit = fakeCommit({ + sha: "cccee2c633f0e65d939df7d9953f59ee9322c323", + message: "Fixed a bad typo in comment (yes, really)", + }) + const concern = subjectLineConcern("useIssueLinks", commit.sha, { range: [41, 42] }) + + it("describes the rule violation", () => { + const actualOutput = commitwiseReport([concern], [commit], configuration) + expect(actualOutput).toBe( + ` +cccee2c Fixed a bad typo in comment (yes, really) + ┬ + Subject lines must end with an issue link. ─╯ + (useIssueLinks) + + Examples: AWESOME-123, UNICORN-123, PROJECT-123 `.trim(), ) }) diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index 9d3621b3..9a961cdf 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -1,6 +1,7 @@ import type { Commit, Commits } from "#commits/Commit.ts" import { formatTokenisedLine } from "#commits/tokens/Token.ts" import type { Configuration } from "#configurations/Configuration.ts" +import { pluralise } from "#legacy-v1/utilities/StringUtilities.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" @@ -58,7 +59,7 @@ function formatCommitConcern( ): string { const formattedSubjectLine = formatTokenisedLine(commit.subjectLine) const commitLine = `${commit.sha.slice(0, SHORT_SHA_LENGTH)} ${formattedSubjectLine}` - const message = ruleMessage(concern.rule, configuration) + const [message, sidenote = null] = ruleMessage(concern.rule, configuration) const rangeLine = indentString( RANGE_PREFIX + "─".repeat(formattedSubjectLine.length), @@ -66,7 +67,7 @@ function formatCommitConcern( ) const messageLine = indentString( - `${MESSAGE_PREFIX} ${message}\n (${concern.rule})`, + `${MESSAGE_PREFIX} ${message}\n (${concern.rule})${sidenote !== null ? `\n \n ${sidenote}` : ""}`, SHORT_SHA_LENGTH - MESSAGE_PREFIX.length + 1, ) @@ -87,31 +88,37 @@ function formatSubjectLineConcern( const longHalfLength = Math.trunc(length / 2) const shortHalfLength = length - longHalfLength - 1 - const message = ruleMessage(concern.rule, configuration) + const [message, sidenote = null] = ruleMessage(concern.rule, configuration) const anchoredRight = message.length + MESSAGE_SUFFIX.length < offset + longHalfLength const rangeLine = indentString(formatCharacterRange(concern.range, anchoredRight), offset) const messageLine = anchoredRight ? indentString( - `${message} ${MESSAGE_SUFFIX}\n(${concern.rule})`, + `${message} ${MESSAGE_SUFFIX}\n(${concern.rule})${sidenote !== null ? `\n\n${sidenote}` : ""}`, offset + longHalfLength - message.length - MESSAGE_SUFFIX.length, ) - : indentString(`${MESSAGE_PREFIX} ${message}\n (${concern.rule})`, offset + shortHalfLength) + : indentString( + `${MESSAGE_PREFIX} ${message}\n (${concern.rule})${sidenote !== null ? `\n \n ${sidenote}` : ""}`, + offset + shortHalfLength, + ) return `${commitLine}\n${rangeLine}\n${messageLine}` } -function ruleMessage(rule: RuleKey, configuration: Configuration): string { +function ruleMessage( + rule: RuleKey, + configuration: Configuration, +): [main: string, sidenote?: string] { switch (rule) { case "noBlankSubjectLines": { - return "Subject lines must contain at least one non-whitespace character." + return ["Subject lines must contain at least one non-whitespace character."] } case "noExcessiveCommitsPerBranch": { throw new Error(`Not implemented yet: ${rule}`) } case "noMergeCommits": { - return "Merge commits are not allowed." + return ["Merge commits are not allowed."] } case "noRepeatedSubjectLines": { throw new Error(`Not implemented yet: ${rule}`) @@ -120,13 +127,13 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { throw new Error(`Not implemented yet: ${rule}`) } case "noRevertRevertCommits": { - return "Cherry-pick the original commit instead of reverting it over." + return ["Cherry-pick the original commit instead of reverting it over."] } case "noSingleWordSubjectLines": { - return "Subject lines must contain at least two words." + return ["Subject lines must contain at least two words."] } case "noSquashMarkers": { - return "Combine squash commits with their ancestors." + return ["Combine squash commits with their ancestors."] } case "noUnexpectedPunctuation": { throw new Error(`Not implemented yet: ${rule}`) @@ -141,7 +148,7 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { throw new Error(`Not implemented yet: ${rule}`) } case "useCapitalisedSubjectLines": { - return "The first letter in subject lines must be in uppercase." + return ["The first letter in subject lines must be in uppercase."] } case "useCommitterEmailPatterns": { throw new Error(`Not implemented yet: ${rule}`) @@ -152,7 +159,7 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { case "useConciseSubjectLines": { const options = getRuleOptions(rule, configuration) const characterPhrase = formatCount(options.maxLength, "character", "characters") - return `Subject lines must not exceed ${characterPhrase}.` + return [`Subject lines must not exceed ${characterPhrase}.`] } case "useEmptyLineBeforeBodyLines": { throw new Error(`Not implemented yet: ${rule}`) @@ -169,7 +176,13 @@ function ruleMessage(rule: RuleKey, configuration: Configuration): string { ? "end with" : "include" - return `Subject lines must ${positionPhrase} an issue link.` + const examples = configuration.tokens.issueLinkPrefixes.map((prefix) => `${prefix}123`) + const examplePhrase = pluralise(examples.length, "Example", "Examples") + + return [ + `Subject lines must ${positionPhrase} an issue link.`, + `${examplePhrase}: ${examples.join(", ")}`, + ] } case "useLineWrapping": { throw new Error(`Not implemented yet: ${rule}`)