Skip to content

Commit

Permalink
refactor(plugin-stylelint): change getSeverityFromRuleConfig logic pl…
Browse files Browse the repository at this point in the history
…us add test
  • Loading branch information
aramirezj committed Jan 7, 2025
1 parent a11dfbc commit 071cbd8
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 20 deletions.
15 changes: 14 additions & 1 deletion packages/plugin-stylelint/src/lib/runner/model.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
import type { ConfigRuleSettings, Primary, Severity } from 'stylelint';

// Typing resource https://stylelint.io/user-guide/configure/
/** Config rule setting of Stylelint excluding null and undefined values */
export type ActiveConfigRuleSetting = Exclude<
ConfigRuleSettings<Primary, Record<string, unknown>>,
null | undefined
>;

/** Output of the `getNormalizedConfigForFile` function. Config file of Stylelint */
export type NormalizedStyleLintConfig = {
config: { rules: Record<string, unknown[]> };
config: {
rules: Record<string, ConfigRuleSettings<Primary, Record<string, any>>>;
defaultSeverity?: Severity;
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function getNormalizedConfigForFile({
cwd,
}: Required<Pick<StyleLintTarget, 'stylelintrc'>> & {
cwd?: string;
}) {
}): Promise<NormalizedStyleLintConfig> {
const _linter = stylelint._createLinter({ configFile: stylelintrc });
const configFile =
stylelintrc ?? path.join(cwd ?? process.cwd(), '.stylelintrc.json');
Expand Down
48 changes: 32 additions & 16 deletions packages/plugin-stylelint/src/lib/runner/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ConfigRuleSettings, LintResult, Warning } from 'stylelint';
import type { LintResult, Secondary, Severity, Warning } from 'stylelint';
import type { Audit, AuditOutputs, AuditReport } from '@code-pushup/models';
import type { ActiveConfigRuleSetting } from './model.js';

export function mapStylelintResultsToAudits(
results: LintResult[],
Expand Down Expand Up @@ -73,24 +74,39 @@ export function getSeverityFromWarning(warning: Warning): 'error' | 'warning' {
throw new Error(`Unknown severity: ${severity}`);
}

/**
* Function that returns the severity from a ruleConfig.
* If the ruleConfig is not an array, the default severity of the config file must be returned, since the custom severity can be only specified in an array.
* If the ruleConfig is an array, a custom severity might have been set, in that case, it must be returned
* @param ruleConfig - The Stylelint rule config value
* @param defaultSeverity - The default severity of the config file. By default, it's 'error'
* @returns The severity (EX: 'error' | 'warning')
*/
export function getSeverityFromRuleConfig(
ruleConfig: ConfigRuleSettings<unknown, { severity?: 'error' | 'warning' }>,
): 'error' | 'warning' {
if (!ruleConfig) {
// Default severity if the ruleConfig is null or undefined
return 'error';
ruleConfig: ActiveConfigRuleSetting,
defaultSeverity: Severity = 'error',
): Severity {
//If it's not an array, the default severity of the config file must be returned, since the custom severity can be only specified in an array.
if (!Array.isArray(ruleConfig)) {
return defaultSeverity;
}

if (Array.isArray(ruleConfig)) {
const options = ruleConfig[1];
if (options && typeof options === 'object' && 'severity' in options) {
const severity = options.severity;
if (severity === 'warning') {
return 'warning';
}
}
// If it's an array, a custom severity might have been set, in that case, it must be returned

const secondary: Secondary = ruleConfig.at(1);

if (secondary == null) {
return defaultSeverity;
}

if (!secondary['severity']) {
return defaultSeverity;
}

if (typeof secondary['severity'] === 'function') {
console.warn('Function severity is not supported');
return defaultSeverity;
}

// Default severity if severity is not explicitly set
return 'error';
return secondary['severity'];
}
54 changes: 54 additions & 0 deletions packages/plugin-stylelint/src/lib/runner/utils.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { describe, expect, it } from 'vitest';
import type { ActiveConfigRuleSetting } from './model.js';
import { getSeverityFromRuleConfig } from './utils.js';

describe('getSeverityFromRuleConfig', () => {
it('should respect the default severity when from the default', () => {
expect(getSeverityFromRuleConfig([true])).toBe('error');
});

it('should consider the default severity when its different from the default', () => {
expect(getSeverityFromRuleConfig([true], 'warning')).toBe('warning');
});

it.each([true, 5, 'percentage', ['/\\[.+]/', 'percentage'], { a: 1 }])(
'should return the default severity for a primary value %s',
ruleConfig => {
expect(
getSeverityFromRuleConfig(ruleConfig as ActiveConfigRuleSetting),
).toBe('error');
},
);

it('should return the default severity when the rule config does not have a secondary item', () => {
expect(getSeverityFromRuleConfig([true])).toBe('error');
});

it('should return the default severity when the secondary item is missing the `severity` property', () => {
expect(getSeverityFromRuleConfig([true, {}])).toBe('error');
});

it('should return the default severity when `severity` property is of type function', () => {
expect(getSeverityFromRuleConfig([true, { severity: () => {} }])).toBe(
'error',
);
});

it.each([
{ ruleConfig: [true, { severity: 'warning' }], expected: 'warning' },
{ ruleConfig: [true, { severity: 'error' }], expected: 'error' },
])('should return the set severity `%s`', ({ ruleConfig, expected }) => {
expect(getSeverityFromRuleConfig(ruleConfig)).toBe(expected);
});

it.each([null, undefined])(
'should return the default severity for disabled rules %s',
ruleConfig => {
expect(
getSeverityFromRuleConfig(
ruleConfig as unknown as ActiveConfigRuleSetting,
),
).toBe('error');
},
);
});
19 changes: 17 additions & 2 deletions packages/plugin-stylelint/src/lib/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ConfigRuleSettings } from 'stylelint';
import type { Audit, CategoryRef } from '@code-pushup/models';
import {
type StyleLintPluginConfig,
Expand All @@ -9,6 +10,7 @@ import {
GROUPS,
STYLELINT_PLUGIN_SLUG,
} from './constants.js';
import type { ActiveConfigRuleSetting } from './runner/model.js';
import { getNormalizedConfigForFile } from './runner/normalize-config.js';
import { getSeverityFromRuleConfig } from './runner/utils.js';

Expand All @@ -31,12 +33,16 @@ export async function getGroups(
options: Required<Pick<StyleLintTarget, 'stylelintrc'>>,
) {
const { config } = await getNormalizedConfigForFile(options);
const { rules } = config;
const { rules, defaultSeverity } = config;
return GROUPS.map(group => ({
...group,
refs: Object.entries(rules)
.filter(filterNonNull) // TODO Type Narrowing is not fully working for the nulls / undefineds
.filter(([_, ruleConfig]) => {
const severity = getSeverityFromRuleConfig(ruleConfig);
const severity = getSeverityFromRuleConfig(
ruleConfig as ActiveConfigRuleSetting,
defaultSeverity,
);
if (severity === 'error' && group.slug === 'problems') {
return true;
} else if (severity === 'warning' && group.slug === 'suggestions') {
Expand Down Expand Up @@ -75,3 +81,12 @@ export async function getCategoryRefsFromAudits(
type: 'audit',
}));
}

function filterNonNull<T, O extends object = object>(
settings: Array<ConfigRuleSettings<T, O>>,
): Exclude<ConfigRuleSettings<T, O>, null | undefined>[] {
return settings.filter(
(setting): setting is Exclude<ConfigRuleSettings<T, O>, null | undefined> =>
setting !== null && setting !== undefined,
);
}

0 comments on commit 071cbd8

Please sign in to comment.