conventional-changelog / commitlint

📓 Lint commit messages
https://commitlint.js.org
MIT License
16.75k stars 902 forks source link

Allow per-type scope-enum #395

Open ianwremmel opened 6 years ago

ianwremmel commented 6 years ago

Expected Behavior

I'd like to limit the valid scopes on a per-type basis. For example, the docs type should accept all and each of my lerna packages as scopes, but the build type should perhaps accept only npm, webpack, babel.

Current Behavior

I can specify valid types and valid scopes, but not tuples of which ones are valid together.

Affected packages

Possible Solution

Allow one (or both) of scopes-enum and types-enum to accept an object instead of an array. Alternatively, introduce a new rule that accepts an object.

marionebl commented 6 years ago

I can see how this might be useful, especially on larger projects. How about we take advantage of the fact values in commitlint.config.js may be of type <T>(ctx: CommitlintContext) => Promise<T> | T?

https://github.com/marionebl/commitlint/blob/1d79828427c19c72add82ff46a6c893c389cb4c7/%40commitlint/config-lerna-scopes/index.js#L8-L11

We could pass in the currently parsed message to said function, implementing your example would then looks like this:

const lernaConfig = require('@commitlint/config-lerna');

module.exports = {
  rules: {
    'scope-enum': async (ctx) => {
        // ctx.message not passed yet
        if (ctx.message.type === 'build') {
           return [2, 'always', ['npm', 'babel', 'webpack']]
        }

        const pkgs = await lernaConfig.utils.getPackages();
        return [2, 'always', ['all', ...pkgs]]
    }
  }
};

What do you think?

ianwremmel commented 6 years ago

I think that makes sense; are you saying that's achievable without any changes or are you proposing that support like this be added?

marionebl commented 6 years ago

ctx.message is not available to the config fns yet, so this is a proposal. We would have to pass it down to @commitlint/load via:

An additional complication is the fact we need to (partially) resolve configuration before we can parse a message because parserOpts might be set by the user.

We could work around this by introducing options.mode here:

https://github.com/marionebl/commitlint/blob/master/%40commitlint/load/src/index.js#L14

When options.mode === 'parser', https://github.com/marionebl/commitlint/blob/master/%40commitlint/load/src/index.js#L52-L78 could be skipped.

njlaw commented 3 years ago

I ran into the need for per-type scopes myself and after digging around a bit ended up implementing it as a local plugin. If there's interest, I can clean up the code and publish it on npmjs. In the meantime, here's my ugly working commitlint.config.js in case its helpful to anyone:

module.exports = {
  extends: ['@commitlint/config-conventional'],
  rules: {
    'scope-enums': [
      2, 'always', {
        feat: [/^frontend\/[^\/]+$/, 'backend'],
        perf: [],
        ci: [null, 'codebuild', 'jenkins']
      }
    ]
  },
  plugins: [
    {
      rules: {
        /**
         * - If a type does not appear in the rule, do not enforce scope
         * - If a type appears in the rule with an empty array,
         *   do not allow scope
         * - If a type appears in the rule with an non-empty array,
         *   only allow the values in the array for scope.
         * - If the array includes null, the scope is optional.
         *
         * E.g., {
         *   'allowed-scopes': [2, "always", {
         *     feat: [/^frontend\/[^\/]+$/, 'backend'],
         *     perf: [],
         *     ci: [null, 'codebuild', 'jenkins']
         *   }]
         * }
         *
         * In the above rules configuration,
         *  - if the type is 'feat', the scope must be either
         *    match the regex /frontend\/[^\/]+/ or be 'backend'
         *  - if the type if 'chore', the scope is optional and can
         *    be anything
         *  - if the type is 'perf', a scope is not allowed
         *  - if the type is 'ci', the scope must be 'codebuild' or
         *    'jenkins' if present, but is not required
         */
        'scope-enums': (ctx, applicable, rule) => {
          if (applicable === 'never') {
            return [false, 'the "allowed-scopes" rule does not support "never"'];
          }

          const allowedScopes = rule[ctx.type];

          // If the type does not appear in the rule config, allow any scope or no scope
          if (allowedScopes === undefined) {
            return [true];
          }

          if (Array.isArray(allowedScopes)) {
            // If the type maps to an empty array in the rule config, scope it not allowed
            if (allowedScopes.length === 0) {
              if (ctx.scope != null) {
                return [false, `commit messages with type "${ctx.type}" must not specify a scope`];
              }

              return [true];
            }

            // Otherwise attempt to match against either null, a string literal, or a RegExp
            if (
              allowedScopes.findIndex((s) => {
                if (
                  typeof ctx.scope === 'string' &&
                  Object.prototype.toString.call() === '[object RegExp]'
                ) {
                  return ctx.scope.match(s);
                }

                // Equality comparison works for both strings and null
                return s === ctx.scope;
              }) !== -1
            ) {
              return [true];
            } else if (allowedScopes.includes(null)) {
              return [
                false,
                `commit message with type "${
                  ctx.type
                }" may specify a scope, but if specified, it must be one of the following: ${allowedScopes
                  .filter((s) => s !== null)
                  .map((s) => `"${s}"`)
                  .join(', ')}`,
              ];
            } else {
              return [
                false,
                `commit message with type "${
                  ctx.type
                }" must specify one of the following scopes: ${allowedScopes
                  .map((s) => `"${s}"`)
                  .join(', ')}`,
              ];
            }
          }

          return [false, `invalid rule entry: { ${ctx.type}: ${JSON.stringify(allowedScopes)} }`];
        },
      },
    },
  ],
};
ridvanaltun commented 3 years ago

Thank you @njlaw

I converted it to an actual plugin, here: commitlint-plugin-selective-scope