NodeJS commit message validator

Question

I wrote a commit message validator + CLI in NodeJS.

Notes:

• My greatest concern is that the patten I use to represent the results of a validation is inadequate. Options:

  • Use adhoc enums
  • Create a ValidationResult class (?)
  • Extend Error and return an array of those. In this case, how would the --verbose option, which shows passed rules and informational rules work?

• Is the pattern I use in lib/rules.js to represent each of the rules adequate?

• Is there a better (performance and/or readability-wise) way of stripping commit messages than the approach I use in lib/strip.js?

• Other feedback is welcome too, of course.

index.js:

const strip = require('./lib/strip');
const rules = require('./lib/rules');

function validate(commitMessage) {
    const strippedMessage = strip(commitMessage);
    const results = [];

    for (const rule of rules) {
        let outcome;

        if (rule.test === undefined) {
            outcome = 'info';

            results.push({
                type: outcome,
                message: rule.message
            });

            continue;
        }

        if (rule.test(strippedMessage)) {
            outcome = 'pass';
        } else {
            outcome = 'fail';
        }

        results.push({
            type: outcome,
            message: rule.message
        });
    }

    return results;
}

module.exports = validate;

cli.js:

#!/usr/bin/env node

const meow = require('meow');
const fs = require('fs');
const stdin = require('get-stdin');

const validate = require('.');

const log = require('./lib/log');

const cli = meow({
    description: 'Validate commit messages against the seven rules of commit messages.',

    flags: {
        silent: {
            type: 'boolean',
            alias: 's'
        },

        verbose: {
            type: 'boolean',
            alias: 'v'
        },

        // TODO: Work with --file flag
        file: {
            type: 'string',
            alias: 'f'
        }

        // TODO: If nothing else, read stdin
    }
});

log(); // Leading new line


if (cli.input.length === 0) {
    // TODO: Cry
}

const [commitMessage] = cli.input;

const results = validate(commitMessage);
let exitCode = 0;

for (const result of results) {
    switch (result.type) {
    case 'fail':
        if (!cli.flags.silent) {
            log.error(result.message);
        }

        if (exitCode === 0) { exitCode = 1; }

        break;
    case 'pass':
    case 'info':
        if (!cli.flags.silent && cli.flags.verbose) {
            log[result.type](result.message);
        }

        break;
    default:
        throw new Error(`Internal Error: Invalid result type '${result.type}'`);
    }
}

process.exit(exitCode);

lib/

lib/rules.js:

const one = {
    message: 'Separate subject from body with a blank line',
    test: (commitMessage) => {
        const separatedByLf = /^.+(\n\n(?:.|\n)+|\n?)$/g;

        return separatedByLf.test(commitMessage);
    }
};

const two = {
    message: 'Limit the subject line to 50 characters',
    test: (commitMessage) => {
        const subjectLine = getSubjectLine(commitMessage);
        const cutOff = 50;

        return subjectLine.length < cutOff;
    }
};

const three = {
    message: 'Capitalize the subject line',
    test: (commitMessage) => {
        const subjectLine = getSubjectLine(commitMessage);
        const firstCharacter = subjectLine[0];

        return !isLowerCase(firstCharacter);
    }
};

const four = {
    message: 'Do not end the subject line with a period',
    test: (commitMessage) => {
        const subjectLine = getSubjectLine(commitMessage);
        const lastCharacter = subjectLine.substr(-1);

        return !(lastCharacter === '.');
    }
};

const five = {
    message: 'Use the imperative mood in the subject line'
    // We could, in theory, use NLP to check for this rule,
    // ...but it would take effort and would be error prone
};

const six = {
    message: 'Wrap the body at 72 characters',
    test: (commitMessage) => {
        const bodyLines = getBody(commitMessage).split('\n');

        return bodyLines.every(line => line.length < 72);
    }
};

const seven = {
    message: 'Use the body to explain _what_ and _why_ vs. _how_'
    // This is obviously not detectable programtically
};

const rules = [
    one, two, three, four, five, six, seven
];

module.exports = rules;

function getSubjectLine(commitMessage) {
    return commitMessage.split('\n')[0];
}

function getBody(commitMessage) {
    const [, ...body] = commitMessage.split('\n');
    return body.join('\n');
}

function isLowerCase(char) {
    return !(char.toUpperCase() === char);
}

lib/log.js:

const chalk = require('chalk');
const logSymbols = require('log-symbols');

function log(...args) {
    args = args.join(' ');
    console.log(args);
}

log.error = (...args) => {
    console.error(chalk.red(logSymbols.error, args));
};

log.success = (...args) => {
    log(chalk.green(logSymbols.success, args));
};

log.warn = (...args) => {
    console.warn(chalk.yellow(logSymbols.warn, args));
};

log.info = (...args) => {
    console.info(chalk.blue(logSymbols.info, args));
};

module.exports = log;

lib/strip.js:

// See: https://git-scm.com/docs/git-commit#git-commit-strip

// TODO: Respect git's core.commentchar

function strip(rawCommitMessage) {
    const trailingWhitespace = /[ \t\f\v]+$/gm;
    const commentary = /^#.*/gm;
    const consecutiveEmptyLines = /\n{3,}/g;
    const leadingTrailingEmptyLines = /^\n+|\n+$/g;

    return rawCommitMessage
        .replace(trailingWhitespace, '')
        .replace(commentary, '')
        .replace(consecutiveEmptyLines, '\n\n')
        .replace(leadingTrailingEmptyLines, '');
}

module.exports = strip;

Answer 1

Your code looks mostly good, but here are some things:

• lib/log.js: You don't need the log function. console.log does exactly what you do in that function (well not exactly, but it shouldn't make a difference in this context). You can just set log = (...args) => console.log(...args) or function log(...args) {console.log(...args)}
• lib/rules.js In the isLowerCase function, you are checking !(char.toUpperCase() === char). You should use !==