Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A rule that rejects obvious words in the commit title #67

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions commitlint.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ module.exports = {
// 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"],
"reject-obvious-words": [RuleConfigSeverity.Error, "always"],
},
plugins: [
// TODO (ideas for more rules):
Expand Down Expand Up @@ -81,6 +82,21 @@ module.exports = {
return Plugins.commitHashAlone(rawStr);
},

"reject-obvious-words": ({
header,
body,
}: {
header: any;
body: any;
}) => {
let headerStr = Helpers.convertAnyToString(
header,
"header"
);
let bodyStr = Helpers.convertAnyToString(body, "header");
return Plugins.rejectObviousWords(headerStr, bodyStr);
},

"empty-wip": ({ header }: { header: any }) => {
let headerStr = Helpers.assertNotNull(
Helpers.convertAnyToString(header, "header"),
Expand Down
71 changes: 71 additions & 0 deletions commitlint/plugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,77 @@ test("proper-issue-refs5", () => {
expect(properIssueRefs5.status).not.toBe(0);
});

test("reject-obvious-words1", () => {
let commitMsgWithObviousWordAfterColon = "foo: change";
let rejectObviousWords1 = runCommitLintOnMsg(
commitMsgWithObviousWordAfterColon
);
expect(rejectObviousWords1.status).not.toBe(0);
});

test("reject-obvious-words2", () => {
let commitMsgWithObviousWordAfterColon = "foo: change bla bla";
let rejectObviousWords2 = runCommitLintOnMsg(
commitMsgWithObviousWordAfterColon
);
expect(rejectObviousWords2.status).not.toBe(0);
});

test("reject-obvious-words3", () => {
let commitMsgWithObviousWordAfterColon = "foo: change bla bla\n\nBla blah.";
let rejectObviousWords3 = runCommitLintOnMsg(
commitMsgWithObviousWordAfterColon
);
expect(rejectObviousWords3.status).not.toBe(0);
});
tehraninasab marked this conversation as resolved.
Show resolved Hide resolved

test("reject-obvious-words4", () => {
let commitMsgWithoutObviousWordAfterColon = "foo: bla bla bla";
let rejectObviousWords4 = runCommitLintOnMsg(
commitMsgWithoutObviousWordAfterColon
);
expect(rejectObviousWords4.status).toBe(0);
});

test("reject-obvious-words5", () => {
// In general using `update` in commit message is a bad practice but there is
// exception in cases that the area refers to a file which is only modified when
// it needs updating and there's no need for further explanation of why it needs
// updating such as:
// README: update or Backend/servers.json: update

let commitMsgWithUpdateWordAfterColon = "foo: update";
let rejectObviousWords5 = runCommitLintOnMsg(
commitMsgWithUpdateWordAfterColon
);
expect(rejectObviousWords5.status).toBe(0);
});

test("reject-obvious-words6", () => {
let commitMsgWithUpdateWordAfterColon = "foo: update bla bla";
let rejectObviousWords6 = runCommitLintOnMsg(
commitMsgWithUpdateWordAfterColon
);
expect(rejectObviousWords6.status).not.toBe(0);
});

test("reject-obvious-words7", () => {
let commitMsgWithUpdateWordAfterColon = "foo: update bla bla\n\nBla blah.";
let rejectObviousWords7 = runCommitLintOnMsg(
commitMsgWithUpdateWordAfterColon
);
expect(rejectObviousWords7.status).toBe(0);
});

test("reject-obvious-words8", () => {
let commitMsgWithoutObviousWordAfterColon =
"scripts/detectNotUsingGitPush1by1.fsx: fix";
let rejectObviousWords8 = runCommitLintOnMsg(
commitMsgWithoutObviousWordAfterColon
);
expect(rejectObviousWords8.status).not.toBe(0);
});

test("subject-lowercase1", () => {
let commitMsgWithUppercaseAfterColon = "foo: Bar baz";
let subjectLowerCase1 = runCommitLintOnMsg(
Expand Down
32 changes: 32 additions & 0 deletions commitlint/plugins.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { abbr } from "./abbreviations";
import { Helpers } from "./helpers";
const obviousWords = ["change", "update", "modify"];

export abstract class Plugins {
public static bodyProse(rawStr: string) {
Expand Down Expand Up @@ -278,6 +279,37 @@ export abstract class Plugins {
];
}

public static rejectObviousWords(
headerStr: string | null,
bodyStr: string | null
) {
let offence = false;
let firstWordInTitle = "";

if (headerStr !== null) {
let colonFirstIndex = headerStr.indexOf(":");
let titleStartIndex = Math.max(0, colonFirstIndex + 1);
let title = headerStr
.substring(titleStartIndex, headerStr.length)
.trim();
let titleWords = title.split(" ");
firstWordInTitle = titleWords[0];

if (firstWordInTitle === "update") {
offence = titleWords.length > 1 && bodyStr === null;
} else if (firstWordInTitle === "fix") {
offence = bodyStr === null;
} else {
offence = obviousWords.includes(firstWordInTitle);
}
}

return [
!offence,
`Please don't use obvious words such as ${firstWordInTitle} in the commit title`,
];
}

public static titleUppercase(headerStr: string) {
let firstWord = headerStr.split(" ")[0];
let offence =
Expand Down
Loading