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

Rule produces false positives very frequently #2

Open
RoystonS opened this issue Jun 8, 2023 · 1 comment
Open

Rule produces false positives very frequently #2

RoystonS opened this issue Jun 8, 2023 · 1 comment

Comments

@RoystonS
Copy link

RoystonS commented Jun 8, 2023

Lots of valid commit messages are rejected incorrectly:

  • Reduce duplication in codebase - this is incorrectly flagged as past tense because the word 'Reduce' contains the letters 'ed'. I presume the code is attempting to detect words ending 'ed', but the regex is matching words that contain 'ed'.
  • Add missing directive to foobar - this is incorrectly flagged as a gerund because the word 'missing' (which isn't a verb) ends 'ing'. Adjectives like 'missing' or nouns like 'warning' occur quite frequently in commit messages, so checking anything other than the first word in the commit produces a lot of false positives.
  • Fix compiler warnings in foobar - this is incorrectly flagged as a gerund because the word 'warning' contains the letters 'ing'.
@AndreMarquesDev
Copy link

Managed to fix it by basically just forking the logic from https://github.com/ngx-devs/commitlint-plugin-imperative/blob/main/src/rules.ts and applying the plugin locally.

So before I had it like this:

// commitlint.config.ts

const Configuration: UserConfig = {
  ...
  rules: {
    ...
    'imperative-rule/en': [RuleConfigSeverity.Error, 'always'],
  },
  plugins: ['@ngx-devs/commitlint-plugin-imperative'],
};

export default Configuration;

And I improved the regex to check only the last letters from the word and only the first word after the commit type:

// commitlint.config.ts

const Configuration: UserConfig = {
  ...
  rules: {
    ...
    'subject-imperative': [RuleConfigSeverity.Error, 'always'],
  },
  plugins: [
    {
      rules: {
        // inspired by https://github.com/ngx-devs/commitlint-plugin-imperative
        'subject-imperative': ({ subject }) => {
          const subjectFirstWord = subject?.trim().split(' ')[0] ?? '';
          const pastTenseRegex = /\b\w+ed\b/;
          const gerundRegex = /((\w)*(ing))/;
          const pastTenseErrorMessage = `The word "${subjectFirstWord}" is in the past tense. Please use the imperative form. More info at https://github.com/ngx-devs/commitlint-plugin-imperative`;
          const gerundErrorMessage = `The word "${subjectFirstWord}" is in the gerund. Please use the imperative form. More info at https://github.com/ngx-devs/commitlint-plugin-imperative`;

          const isInPastTense = pastTenseRegex.test(subjectFirstWord);
          if (isInPastTense) {
            return [false, pastTenseErrorMessage];
          }

          const isInGerund = gerundRegex.test(subjectFirstWord);
          if (isInGerund) {
            return [false, gerundErrorMessage];
          }

          return [true];
        },
      },
    },
  ]
};

export default Configuration;

Also cleaned up the code a little bit and renamed the rule to subject-imperative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants