commitlint icon indicating copy to clipboard operation
commitlint copied to clipboard

Allow per-type scope-enum

Open ianwremmel opened this issue 7 years ago • 5 comments

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

  • [ ] cli
  • [x] core
  • [ ] prompt
  • [ ] config-angular

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.

ianwremmel avatar Jul 13 '18 18:07 ianwremmel

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?

marionebl avatar Sep 04 '18 21:09 marionebl

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

ianwremmel avatar Sep 04 '18 22:09 ianwremmel

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:

  • https://github.com/marionebl/commitlint/blob/master/%40commitlint/cli/src/cli.js#L135
  • https://github.com/marionebl/commitlint/blob/master/%40commitlint/load/src/index.js#L60
  • https://github.com/marionebl/commitlint/blob/master/%40commitlint/execute-rule/src/index.js#L7

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.

marionebl avatar Sep 04 '18 22:09 marionebl

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)} }`];
        },
      },
    },
  ],
};

njlaw avatar Jul 24 '21 00:07 njlaw

Thank you @njlaw

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

ridvanaltun avatar Sep 30 '21 12:09 ridvanaltun