Skip to content

feat: allow scope-enum to also config delimiter #2407

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions @commitlint/prompt/src/library/get-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getHasName,
getMaxLength,
} from './utils';
import {EnumRuleOptions} from '@commitlint/types';

/**
* Get a cli prompt based on rule configuration
Expand Down Expand Up @@ -68,7 +69,9 @@ export default function getPrompt(
throw new TypeError('getPrompt: prompt.show is not a function');
}

const enumRule = rules.filter(getHasName('enum')).find(enumRuleIsActive);
const enumRule = (rules as Array<RuleEntry<EnumRuleOptions>>)
.filter(getHasName('enum'))
.find(enumRuleIsActive);

const emptyRule = rules.find(getHasName('empty'));

Expand Down Expand Up @@ -118,7 +121,7 @@ export default function getPrompt(
if (enumRule) {
const [, [, , enums]] = enumRule;

enums.forEach((enumerable) => {
(enums as string[]).forEach((enumerable) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should not do typecasts as types should be correct here, you can do something like this

Suggested change
(enums as string[]).forEach((enumerable) => {
(Array.isArray(enums) ? enums : enums.values).forEach((enumerable) => {

const enumSettings = (settings.enumerables || {})[enumerable] || {};
prompt
.command(enumerable)
Expand Down
4 changes: 2 additions & 2 deletions @commitlint/prompt/src/library/types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {RuleConfigCondition, RuleConfigSeverity} from '@commitlint/types';

export type RuleEntry =
export type RuleEntry<C = unknown> =
| [string, Readonly<[RuleConfigSeverity.Disabled]>]
| [string, Readonly<[RuleConfigSeverity, RuleConfigCondition]>]
| [string, Readonly<[RuleConfigSeverity, RuleConfigCondition, unknown]>];
| [string, Readonly<[RuleConfigSeverity, RuleConfigCondition, C]>];

export type InputSetting = {
description?: string;
Expand Down
25 changes: 18 additions & 7 deletions @commitlint/prompt/src/library/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {RuleConfigSeverity} from '@commitlint/types';
import {EnumRuleOptions, RuleConfigSeverity} from '@commitlint/types';
import {RuleEntry} from './types';

import {
enumRuleIsActive,
Expand Down Expand Up @@ -85,37 +86,47 @@ test('getMaxLength', () => {
});

test('check enum rule filters', () => {
const rules: any = {
const rules: Record<string, RuleEntry<EnumRuleOptions>[1]> = {
Comment on lines -88 to +89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

'enum-string': [RuleConfigSeverity.Warning, 'always', ['1', '2', '3']],
'type-enum': [RuleConfigSeverity.Error, 'always', ['build', 'chore', 'ci']],
'scope-enum': [RuleConfigSeverity.Error, 'never', ['cli', 'core', 'lint']],
'bar-enum': [RuleConfigSeverity.Disabled, 'always', ['foo', 'bar', 'baz']],
'extendable-scope-enum': [
RuleConfigSeverity.Disabled,
'always',
{values: ['foo', 'bar', 'baz']},
],
};

let enumRule = getRules('type', rules)
let enumRule = getRules<EnumRuleOptions>('type', rules)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of those forced types should be removed

.filter(getHasName('enum'))
.find(enumRuleIsActive);
expect(enumRule).toEqual([
'type-enum',
[2, 'always', ['build', 'chore', 'ci']],
]);

enumRule = getRules('string', rules)
enumRule = getRules<EnumRuleOptions>('string', rules)
.filter(getHasName('enum'))
.find(enumRuleIsActive);
expect(enumRule).toEqual(undefined);

enumRule = getRules('enum', rules)
enumRule = getRules<EnumRuleOptions>('enum', rules)
.filter(getHasName('string'))
.find(enumRuleIsActive);
expect(enumRule).toEqual(['enum-string', [1, 'always', ['1', '2', '3']]]);

enumRule = getRules('bar', rules)
enumRule = getRules<EnumRuleOptions>('bar', rules)
.filter(getHasName('enum'))
.find(enumRuleIsActive);
expect(enumRule).toEqual(undefined);

enumRule = getRules<EnumRuleOptions>('scope', rules)
.filter(getHasName('enum'))
.find(enumRuleIsActive);
expect(enumRule).toEqual(undefined);

enumRule = getRules('scope', rules)
enumRule = getRules<EnumRuleOptions>('extendable-scope', rules)
.filter(getHasName('enum'))
.find(enumRuleIsActive);
expect(enumRule).toEqual(undefined);
Expand Down
48 changes: 36 additions & 12 deletions @commitlint/prompt/src/library/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import {QualifiedRules, RuleConfigSeverity} from '@commitlint/types';
import {
QualifiedRules,
RuleConfigSeverity,
EnumRuleOptions,
EnumRuleExtendableOptions,
} from '@commitlint/types';
import {RuleEntry} from './types';

/**
Expand All @@ -25,7 +30,7 @@ export function getRulePrefix(id: string): string | null {
* Get a predicate matching rule definitions with a given name
*/
export function getHasName(name: string) {
return <T extends RuleEntry>(
return <C = unknown, T extends RuleEntry<C> = RuleEntry<C>>(
rule: RuleEntry
): rule is Exclude<T, [string, undefined]> => getRuleName(rule[0]) === name;
}
Expand All @@ -35,7 +40,10 @@ export function getHasName(name: string) {
* @param rule to check
* @return if the rule definition is active
*/
export function ruleIsActive<T extends RuleEntry>(
export function ruleIsActive<
C = unknown,
T extends RuleEntry<C> = RuleEntry<C>
>(
rule: T
): rule is Exclude<T, [string, Readonly<[RuleConfigSeverity.Disabled]>]> {
const [, value] = rule;
Expand All @@ -50,11 +58,11 @@ export function ruleIsActive<T extends RuleEntry>(
* @param rule to check
* @return if the rule definition is applicable
*/
export function ruleIsApplicable(
rule: RuleEntry
export function ruleIsApplicable<C>(
rule: RuleEntry<C>
): rule is
| [string, Readonly<[RuleConfigSeverity, 'always']>]
| [string, Readonly<[RuleConfigSeverity, 'always', unknown]>] {
| [string, Readonly<[RuleConfigSeverity, 'always', C]>] {

This comment was marked as outdated.

const [, value] = rule;
if (value && Array.isArray(value)) {
return value[1] === 'always';
Expand All @@ -79,19 +87,32 @@ export function ruleIsNotApplicable(
return false;
}

function enumConfigIsExtendable(
config?: EnumRuleOptions
): config is EnumRuleExtendableOptions {
return !Array.isArray(config);
}
Comment on lines +90 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this guard is invalidas its only check is not any[]


export function enumRuleIsActive(
rule: RuleEntry
rule: RuleEntry<EnumRuleOptions>
Copy link
Contributor

@armano2 armano2 Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is not valid, as rule, we should require RuleEntry, rn, you basically force to have correct values, than type-guard is semi useless, this works only because you force expected type mapping in getRules, and all guards are useless now

): rule is [
string,
Readonly<
[RuleConfigSeverity.Warning | RuleConfigSeverity.Error, 'always', string[]]
[
RuleConfigSeverity.Warning | RuleConfigSeverity.Error,
'always',
EnumRuleOptions
]
>
] {
let config: EnumRuleOptions | undefined;
return (
ruleIsActive(rule) &&
ruleIsApplicable(rule) &&
Array.isArray(rule[1][2]) &&
rule[1][2].length > 0
!!(config = rule[1][2]) &&
(!enumConfigIsExtendable(config)
? config.length > 0
: Array.isArray(config.values) && config.values.length > 0)
Comment on lines +112 to +115
Copy link
Contributor

@armano2 armano2 Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
!!(config = rule[1][2]) &&
(!enumConfigIsExtendable(config)
? config.length > 0
: Array.isArray(config.values) && config.values.length > 0)
(Array.isArray(rule[1][2])
? rule[1][2].length > 0
: typeof rule[1][2] === 'object' &&
'values' in rule[1][2] &&
rule[1][2].length > 0)

);
}

Expand All @@ -101,9 +122,12 @@ export function enumRuleIsActive(
* @param rules rules to search in
* @return rules matching the prefix search
*/
export function getRules(prefix: string, rules: QualifiedRules): RuleEntry[] {
export function getRules<C = unknown>(
prefix: string,
rules: QualifiedRules
): RuleEntry<C>[] {
Comment on lines +125 to +128
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function returns array of unknown rules and you can't know what types thy are going to be, all what you know is that prefix stats with something

return Object.entries(rules).filter(
(rule): rule is RuleEntry => getRulePrefix(rule[0]) === prefix
(rule): rule is RuleEntry<C> => getRulePrefix(rule[0]) === prefix
);
}

Expand Down
11 changes: 11 additions & 0 deletions @commitlint/rules/src/scope-enum.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ const messages = {
superfluous: 'foo(): baz',
empty: 'foo: baz',
multiple: 'foo(bar,baz): qux',
scoped: 'foo(@a/b): test',
};

const parsed = {
plain: parse(messages.plain),
superfluous: parse(messages.superfluous),
empty: parse(messages.empty),
multiple: parse(messages.multiple),
scoped: parse(messages.scoped),
};

test('scope-enum with plain message and always should succeed empty enum', async () => {
Expand All @@ -21,6 +23,15 @@ test('scope-enum with plain message and always should succeed empty enum', async
expect(actual).toEqual(expected);
});

test('scope-enum allows custom delimiters', async () => {
const [actual] = scopeEnum(await parsed.scoped, 'always', {
values: ['@a/b'],
delimiter: /,/g,
});
const expected = true;
expect(actual).toEqual(expected);
});

test('scope-enum with plain message and never should error empty enum', async () => {
const [actual] = scopeEnum(await parsed.plain, 'never', []);
const expected = false;
Expand Down
21 changes: 15 additions & 6 deletions @commitlint/rules/src/scope-enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,27 @@ import * as ensure from '@commitlint/ensure';
import message from '@commitlint/message';
import {SyncRule} from '@commitlint/types';

export const scopeEnum: SyncRule<string[]> = (
parsed,
when = 'always',
value = []
) => {
export const scopeEnum: SyncRule<
string[] | {delimiter?: RegExp; values?: string[]}
> = (parsed, when = 'always', config = []) => {
if (!parsed.scope) {
return [true, ''];
}

let delimiters = /\/|\\|,/g;
let value: string[] = [];
if (!Array.isArray(config)) {
if (config.delimiter) {
delimiters = config.delimiter;
}
value = config.values || [];
} else {
value = config;
}

// Scopes may contain slash or comma delimiters to separate them and mark them as individual segments.
// This means that each of these segments should be tested separately with `ensure`.
const delimiters = /\/|\\|,/g;

const scopeSegments = parsed.scope.split(delimiters);

const negated = when === 'never';
Expand Down
9 changes: 8 additions & 1 deletion @commitlint/types/src/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,16 @@ export type LengthRuleConfig<V = RuleConfigQuality.User> = RuleConfig<
V,
number
>;
export interface EnumRuleExtendableOptions {
values: string[];
[k: string]: any;
}

export type EnumRuleOptions = EnumRuleExtendableOptions | string[];

export type EnumRuleConfig<V = RuleConfigQuality.User> = RuleConfig<
V,
string[]
EnumRuleOptions
>;

export type RulesConfig<V = RuleConfigQuality.User> = {
Expand Down
7 changes: 7 additions & 0 deletions docs/reference-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ Infinity
```
[]
```
or if you want to customize delimiter regex for [multiple scopes](https://commitlint.js.org/#/concepts-commit-conventions?id=multiple-scopes)
```
{
values: [],
delimiter: /,/g
}
```

#### scope-case

Expand Down