From 78b4b421c0c80d05edb7ef3bd1565467566e558d Mon Sep 17 00:00:00 2001 From: realmarv Date: Thu, 27 Oct 2022 13:29:12 +0330 Subject: [PATCH 1/4] commitlintplugings.test: add failing tests Add failing tests for default-revert-message rule. --- commitlint/tests/plugins.test.ts | 58 ++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/commitlint/tests/plugins.test.ts b/commitlint/tests/plugins.test.ts index 1bc111d1b..3df17e924 100644 --- a/commitlint/tests/plugins.test.ts +++ b/commitlint/tests/plugins.test.ts @@ -1025,6 +1025,64 @@ test("proper-issue-refs5", () => { expect(properIssueRefs5.status).not.toBe(0); }); +test("default-revert-message1", () => { + let commitMsgWithoutDefaultRevertMessage = + 'Revert "add abbreviations.ts"\n\n' + + "This reverts commit 0272f587c7eece147e8d1756116b0b43e11c34ac."; + let defaultRevertMessage1 = runCommitLintOnMsg( + commitMsgWithoutDefaultRevertMessage + ); + expect(defaultRevertMessage1.status).not.toBe(0); +}); + +test("default-revert-message2", () => { + let commitMsgWithDefaultRevertMessage = + 'Revert "add abbreviations.ts"\n\n' + + "This reverts commit 0272f587c7eece147e8d1756116b0b43e11c34ac\n" + + "because/otherwise bla bla."; + let defaultRevertMessage2 = runCommitLintOnMsg( + commitMsgWithDefaultRevertMessage + ); + expect(defaultRevertMessage2.status).toBe(0); +}); + +test("default-revert-message3", () => { + let commitMsgWithoutDefaultRevertMessage = 'Revert "add abbreviations.ts"'; + let defaultRevertMessage3 = runCommitLintOnMsg( + commitMsgWithoutDefaultRevertMessage + ); + expect(defaultRevertMessage3.status).not.toBe(0); +}); + +test("default-revert-message4", () => { + let commitMsgWithDefaultRevertMessage = "Revert .NET6 upd as it broke CI"; + let defaultRevertMessage4 = runCommitLintOnMsg( + commitMsgWithDefaultRevertMessage + ); + expect(defaultRevertMessage4.status).toBe(0); +}); + +test("default-revert-message5", () => { + let commitMsgWithoutDefaultRevertMessage = + 'Revert "add abbreviations.ts"\n\n' + + "This reverts commit 0272f587 because bla bla.\n"; + + let defaultRevertMessage5 = runCommitLintOnMsg( + commitMsgWithoutDefaultRevertMessage + ); + expect(defaultRevertMessage5.status).toBe(0); +}); + +test("default-revert-message6", () => { + let commitMsgWithDefaultRevertMessage = + 'Revert "process overhaul" to fix CI\n\n'; + + let defaultRevertMessage6 = runCommitLintOnMsg( + commitMsgWithDefaultRevertMessage + ); + expect(defaultRevertMessage6.status).toBe(0); +}); + test("subject-lowercase1", () => { let commitMsgWithUppercaseAfterColon = "foo: Bar baz"; let subjectLowerCase1 = runCommitLintOnMsg( From bdd9fca6fd42e594a9462ec165363b5de7c9082c Mon Sep 17 00:00:00 2001 From: realmarv Date: Thu, 27 Oct 2022 14:16:46 +0330 Subject: [PATCH 2/4] commitlint.config: add rule Add default-revert-message rule. --- commitlint.config.ts | 31 ++++++++++++++++++++++++++++++- commitlint/helpers.ts | 8 ++++++++ commitlint/plugins.ts | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/commitlint.config.ts b/commitlint.config.ts index 96ab0892f..d5bf6cf16 100644 --- a/commitlint.config.ts +++ b/commitlint.config.ts @@ -1,3 +1,4 @@ +import { None, OptionStatic } from "./commitlint/fpHelpers.js"; import { Helpers } from "./commitlint/helpers.js"; import { Plugins } from "./commitlint/plugins.js"; import { RuleConfigSeverity } from "@commitlint/types"; @@ -62,10 +63,14 @@ export default { // disabled because most of the time it doesn't work, due to https://github.com/conventional-changelog/commitlint/issues/3404 // and anyway we were using this rule only as a warning, not an error (because a scope is not required, e.g. when too broad) "type-empty": [RuleConfigSeverity.Disabled, "never"], + "default-revert-message": [RuleConfigSeverity.Error, "never"], }, + + // Commitlint automatically ignores some kinds of commits like Revert commit messages. + // We need to set this value to false to apply our rules on these messages. + defaultIgnores: false, plugins: [ // TODO (ideas for more rules): - // * Detect reverts which have not been elaborated. // * Reject some stupid obvious words: change, update, modify (if first word after colon, error; otherwise warning). // * Think of how to reject this shitty commit message: https://github.com/nblockchain/NOnion/pull/34/commits/9ffcb373a1147ed1c729e8aca4ffd30467255594 // * Workflow: detect if wip commit in a branch not named "wip/*" or whose name contains "squashed". @@ -141,6 +146,30 @@ export default { return Plugins.properIssueRefs(rawStr); }, + "default-revert-message": ( + { + header, + body, + }: { + header: any; + body: any; + }, + when: string + ) => { + Helpers.assertWhen(when); + + let bodyStr = Helpers.convertAnyToString(body); + let headerStr = Helpers.assertNotNone( + Helpers.convertAnyToString(header), + notStringErrorMessage("header") + ); + return Plugins.defaultRevertMessage( + headerStr, + bodyStr instanceof None ? null : bodyStr.value, + when + ); + }, + "title-uppercase": ({ header }: { header: any }) => { let headerStr = extractStringFromCommitlintParam( "header", diff --git a/commitlint/helpers.ts b/commitlint/helpers.ts index 3b88bf7b1..abefc3ee0 100644 --- a/commitlint/helpers.ts +++ b/commitlint/helpers.ts @@ -49,6 +49,14 @@ export abstract class Helpers { return text as string; } + public static assertWhen(when: string) { + if (when !== "never" && when !== "always") { + throw new Error( + 'Variable "when" should be either "never" or "always"' + ); + } + } + public static assertCharacter(letter: string) { if (letter.length !== 1) { throw Error("This function expects a character as input"); diff --git a/commitlint/plugins.ts b/commitlint/plugins.ts index da962c593..168764cd6 100644 --- a/commitlint/plugins.ts +++ b/commitlint/plugins.ts @@ -286,6 +286,44 @@ export abstract class Plugins { ]; } + public static defaultRevertMessage( + headerStr: string, + bodyStr: string | null, + when: string + ) { + let offence = false; + let isRevertCommitMessage = headerStr.toLowerCase().includes("revert"); + + const negated = when === "never"; + + if (isRevertCommitMessage) { + let isDefaultRevertHeader = + headerStr.match(/^[Rr]evert ".+"$/) !== null; + + if (isDefaultRevertHeader) { + if (bodyStr !== null) { + let lines = bodyStr.split("\n"); + offence = + lines.length == 1 && + // 40 is the length of git commit hash. + lines[0].match(/^This reverts commit [^ ]{40}\.$/) !== + null; + } else { + offence = true; + } + } + + offence = negated ? offence : !offence; + } + return [ + !offence, + (negated + ? `Please explain why you're reverting.` + : `Please don't change the default revert commit message.`) + + Helpers.errMessageSuffix, + ]; + } + public static titleUppercase(headerStr: string) { let firstWord = headerStr.split(" ")[0]; let offence = From 00d66ef1ed1f3a0b11a6e03f91edc37ff292f5e0 Mon Sep 17 00:00:00 2001 From: realmarv Date: Mon, 20 Feb 2023 16:11:19 +0330 Subject: [PATCH 3/4] commitlint/plugins.test: add another test --- commitlint/tests/plugins.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/commitlint/tests/plugins.test.ts b/commitlint/tests/plugins.test.ts index 3df17e924..abddab5e3 100644 --- a/commitlint/tests/plugins.test.ts +++ b/commitlint/tests/plugins.test.ts @@ -1083,6 +1083,16 @@ test("default-revert-message6", () => { expect(defaultRevertMessage6.status).toBe(0); }); +test("default-revert-message7", () => { + let commitMsgWithoutDefaultRevertMessage = + "This is a revert commit\n\nBla bla."; + + let defaultRevertMessage7 = runCommitLintOnMsg( + commitMsgWithoutDefaultRevertMessage + ); + expect(defaultRevertMessage7.status).toBe(0); +}); + test("subject-lowercase1", () => { let commitMsgWithUppercaseAfterColon = "foo: Bar baz"; let subjectLowerCase1 = runCommitLintOnMsg( From cdc729af5ced56fd8a1c82047ff2a349c5ff8c41 Mon Sep 17 00:00:00 2001 From: realmarv Date: Sat, 25 Feb 2023 12:53:25 +0330 Subject: [PATCH 4/4] commitlint: add When enum --- commitlint.config.ts | 57 ++++++++++++++++++++++--------------------- commitlint/helpers.ts | 8 +++++- commitlint/plugins.ts | 4 +-- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/commitlint.config.ts b/commitlint.config.ts index d5bf6cf16..1a2414856 100644 --- a/commitlint.config.ts +++ b/commitlint.config.ts @@ -1,5 +1,5 @@ import { None, OptionStatic } from "./commitlint/fpHelpers.js"; -import { Helpers } from "./commitlint/helpers.js"; +import { Helpers, When } from "./commitlint/helpers.js"; import { Plugins } from "./commitlint/plugins.js"; import { RuleConfigSeverity } from "@commitlint/types"; @@ -25,45 +25,48 @@ function extractStringFromCommitlintParam( export default { parserPreset: "conventional-changelog-conventionalcommits", rules: { - "body-leading-blank": [RuleConfigSeverity.Error, "always"], + "body-leading-blank": [RuleConfigSeverity.Error, When.Always], "body-soft-max-line-length": [ RuleConfigSeverity.Error, - "always", + When.Always, bodyMaxLineLength, ], - "body-paragraph-line-min-length": [RuleConfigSeverity.Error, "always"], - "empty-wip": [RuleConfigSeverity.Error, "always"], - "footer-leading-blank": [RuleConfigSeverity.Warning, "always"], + "body-paragraph-line-min-length": [ + RuleConfigSeverity.Error, + When.Always, + ], + "empty-wip": [RuleConfigSeverity.Error, When.Always], + "footer-leading-blank": [RuleConfigSeverity.Warning, When.Always], "footer-max-line-length": [ RuleConfigSeverity.Error, - "always", + When.Always, footerMaxLineLength, ], - "footer-notes-misplacement": [RuleConfigSeverity.Error, "always"], - "footer-refs-validity": [RuleConfigSeverity.Error, "always"], + "footer-notes-misplacement": [RuleConfigSeverity.Error, When.Always], + "footer-refs-validity": [RuleConfigSeverity.Error, When.Always], "header-max-length-with-suggestions": [ RuleConfigSeverity.Error, - "always", + When.Always, headerMaxLineLength, ], - "subject-full-stop": [RuleConfigSeverity.Error, "never", "."], - "type-space-after-colon": [RuleConfigSeverity.Error, "always"], - "subject-lowercase": [RuleConfigSeverity.Error, "always"], - "body-prose": [RuleConfigSeverity.Error, "always"], - "type-space-after-comma": [RuleConfigSeverity.Error, "always"], - "trailing-whitespace": [RuleConfigSeverity.Error, "always"], - "prefer-slash-over-backslash": [RuleConfigSeverity.Error, "always"], - "type-space-before-paren": [RuleConfigSeverity.Error, "always"], - "type-with-square-brackets": [RuleConfigSeverity.Error, "always"], - "proper-issue-refs": [RuleConfigSeverity.Error, "always"], - "too-many-spaces": [RuleConfigSeverity.Error, "always"], - "commit-hash-alone": [RuleConfigSeverity.Error, "always"], - "title-uppercase": [RuleConfigSeverity.Error, "always"], + "subject-full-stop": [RuleConfigSeverity.Error, When.Never, "."], + "type-space-after-colon": [RuleConfigSeverity.Error, When.Always], + "subject-lowercase": [RuleConfigSeverity.Error, When.Always], + "body-prose": [RuleConfigSeverity.Error, When.Always], + "type-space-after-comma": [RuleConfigSeverity.Error, When.Always], + "trailing-whitespace": [RuleConfigSeverity.Error, When.Always], + "prefer-slash-over-backslash": [RuleConfigSeverity.Error, When.Always], + "type-space-before-paren": [RuleConfigSeverity.Error, When.Always], + "type-with-square-brackets": [RuleConfigSeverity.Error, When.Always], + "proper-issue-refs": [RuleConfigSeverity.Error, When.Always], + "too-many-spaces": [RuleConfigSeverity.Error, When.Always], + "commit-hash-alone": [RuleConfigSeverity.Error, When.Always], + "title-uppercase": [RuleConfigSeverity.Error, When.Always], // disabled because most of the time it doesn't work, due to https://github.com/conventional-changelog/commitlint/issues/3404 // and anyway we were using this rule only as a warning, not an error (because a scope is not required, e.g. when too broad) - "type-empty": [RuleConfigSeverity.Disabled, "never"], - "default-revert-message": [RuleConfigSeverity.Error, "never"], + "type-empty": [RuleConfigSeverity.Disabled, When.Never], + "default-revert-message": [RuleConfigSeverity.Error, When.Never], }, // Commitlint automatically ignores some kinds of commits like Revert commit messages. @@ -156,8 +159,6 @@ export default { }, when: string ) => { - Helpers.assertWhen(when); - let bodyStr = Helpers.convertAnyToString(body); let headerStr = Helpers.assertNotNone( Helpers.convertAnyToString(header), @@ -166,7 +167,7 @@ export default { return Plugins.defaultRevertMessage( headerStr, bodyStr instanceof None ? null : bodyStr.value, - when + Helpers.assertWhen(when) ); }, diff --git a/commitlint/helpers.ts b/commitlint/helpers.ts index abefc3ee0..2f0cb8c25 100644 --- a/commitlint/helpers.ts +++ b/commitlint/helpers.ts @@ -1,5 +1,10 @@ import { None, Some, Option, OptionStatic, TypeHelpers } from "./fpHelpers.js"; +export enum When { + Never = "never", + Always = "always", +} + export abstract class Helpers { public static errMessageSuffix = "\nFor reference, these are the guidelines that include our commit message conventions: https://github.com/nblockchain/conventions/blob/master/docs/WorkflowGuidelines.md"; @@ -50,11 +55,12 @@ export abstract class Helpers { } public static assertWhen(when: string) { - if (when !== "never" && when !== "always") { + if (when !== When.Never && when !== When.Always) { throw new Error( 'Variable "when" should be either "never" or "always"' ); } + return when as When; } public static assertCharacter(letter: string) { diff --git a/commitlint/plugins.ts b/commitlint/plugins.ts index 168764cd6..ffe40de0d 100644 --- a/commitlint/plugins.ts +++ b/commitlint/plugins.ts @@ -1,6 +1,6 @@ import { Option, Some, None, OptionStatic } from "./fpHelpers.js"; import { abbr } from "./abbreviations.js"; -import { Helpers } from "./helpers.js"; +import { Helpers, When } from "./helpers.js"; export abstract class Plugins { public static bodyProse(rawStr: string) { @@ -289,7 +289,7 @@ export abstract class Plugins { public static defaultRevertMessage( headerStr: string, bodyStr: string | null, - when: string + when: When ) { let offence = false; let isRevertCommitMessage = headerStr.toLowerCase().includes("revert");