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

fix: Message scope validation for forbidden scopes is incorrect #3779

Open
2 of 4 tasks
joberstein opened this issue Nov 17, 2023 · 3 comments
Open
2 of 4 tasks

fix: Message scope validation for forbidden scopes is incorrect #3779

joberstein opened this issue Nov 17, 2023 · 3 comments
Labels

Comments

@joberstein
Copy link
Contributor

joberstein commented Nov 17, 2023

Reference documentation: https://github.com/conventional-changelog/commitlint/blob/36c58e3d0ba677280a17388cb5bbf58f4627a56a/docs/reference-rules.md#scope-enum

Expected Behavior

If any of the forbidden ('never') scopes are supplied as message scopes, the scope-enum validator should fail. Otherwise, it should pass.

Current Behavior

The scope-enum validator fails only if all of the message scopes are listed as forbidden ('never') scopes. Otherwise, it passes.

Affected packages

  • cli
  • core
  • prompt
  • config-angular

Possible Solution

Rework the scope-enum 'never' validation to be it's own condition, not simply the negation of the 'always' validation.

Steps to Reproduce

{
    "rules": {
        "scope-enum": [2, "never", [
            "action",
        ]]
    },
};

Test 1:
- Command: echo 'fix(different): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Pass
- Expected Result: Pass

Test 2:
- Command: echo 'fix(action): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Failure
- Expected Result: Failure

Test 3:
- Command: echo 'fix(action,other): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Pass
- Expected Result: Failure ('action' is a forbidden scope)

---

{
    "rules": {
        "scope-enum": [2, "never", [
            "action",
            "other"
        ]]
    },
};

Test 1:
- Command: echo 'fix(action): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Failure
- Expected Result: Failure

Test 2:
- Command: echo 'fix(action,other): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Failure 
- Expected Result: Failure

Test 3:
- Command: echo 'fix(action,different): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Pass
- Expected Result: Failure ('action' is a forbidden scope)

Test 4:
- Command: echo 'fix(action,different,other): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Failure
- Expected Result: Failure

---

{
    "rules": {
        "scope-enum": [2, "never", []]
    },
};

Test 1:
- Command: echo 'fix(action): Test message.' | npx commitlint@18.4.2 -V
- Actual Result: Fail
- Expected Result: Pass (there aren't any forbidden scopes)

Context

Discovered in PR: #3725 as a result of failing tests.

commitlint --version

@commitlint/rules@18.4.0

git --version

2.41.0

node --version

20.9.0

@joberstein joberstein added the bug label Nov 17, 2023
@joberstein joberstein changed the title fix: Message scopes validation for forbidden scopes is incorrect fix: Message scope validation for forbidden scopes is incorrect Nov 17, 2023
@silversonicaxel
Copy link
Contributor

As extra info:

Fixing this error will require a refactoring of tests here too https://github.com/conventional-changelog/commitlint/blob/renovate/conventional-changelog-angular-7.x/%40commitlint/rules/src/scope-enum.test.ts

Once this error is covered, conventional changelog angular 7.x could be finally released and this could resolve these issues #3698 and release-it/conventional-changelog#70

@joberstein
Copy link
Contributor Author

As extra info:

Fixing this error will require a refactoring of tests here too https://github.com/conventional-changelog/commitlint/blob/renovate/conventional-changelog-angular-7.x/%40commitlint/rules/src/scope-enum.test.ts

Once this error is covered, conventional changelog angular 7.x could be finally released and this could resolve these issues #3698 and release-it/conventional-changelog#70

@silversonicaxel this issue was pre-existing so the conventional changelog angular upgrade can be done without addressing it.

@silversonicaxel
Copy link
Contributor

Looking forward to new release! Thanks

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

No branches or pull requests

2 participants