-
Notifications
You must be signed in to change notification settings - Fork 934
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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, | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
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'; | ||||||||||||||||||||
|
||||||||||||||||||||
/** | ||||||||||||||||||||
|
@@ -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; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
@@ -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; | ||||||||||||||||||||
|
@@ -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.
Sorry, something went wrong. |
||||||||||||||||||||
const [, value] = rule; | ||||||||||||||||||||
if (value && Array.isArray(value)) { | ||||||||||||||||||||
return value[1] === 'always'; | ||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this guard is invalidas its only check |
||||||||||||||||||||
|
||||||||||||||||||||
export function enumRuleIsActive( | ||||||||||||||||||||
rule: RuleEntry | ||||||||||||||||||||
rule: RuleEntry<EnumRuleOptions> | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||||||||||
); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
|
There was a problem hiding this comment.
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