Skip to content

Commit

Permalink
fix: resolve problem with severity change for configurable rules (#1811)
Browse files Browse the repository at this point in the history 
* fix: resolve problem with severity change for configurable rules

* chore: update snapshots

* Update .changeset/fuzzy-crews-boil.md

* Update .changeset/fuzzy-crews-boil.md

* chore: review comments

---------

Co-authored-by: Jacek Łękawa <[email protected]>
  • Loading branch information
@RomanHotsiy @JLekawa
RomanHotsiy and JLekawa authored Nov 25, 2024
1 parent 0b265e1 commit 72c4c07
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 46 deletions.
6 changes: 6 additions & 0 deletions .changeset/fuzzy-crews-boil.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@redocly/openapi-core": patch
"@redocly/cli": patch
---

Resolved an issue where overrides for the severity of configurable rules were ignored.
10 changes: 8 additions & 2 deletions packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ exports[`resolveConfig should ignore minimal from the root and read local file 1
"component-name-unique": "off",
"no-empty-servers": "error",
"no-example-value-and-externalValue": "error",
"no-invalid-media-type-examples": "error",
"no-invalid-media-type-examples": {
"allowAdditionalProperties": false,
"severity": "error",
},
"no-server-example.com": "warn",
"no-server-trailing-slash": "error",
"no-server-variables-empty-enum": "error",
Expand Down Expand Up @@ -208,7 +211,10 @@ exports[`resolveStyleguideConfig should resolve extends with local file config w
"component-name-unique": "off",
"no-empty-servers": "error",
"no-example-value-and-externalValue": "error",
"no-invalid-media-type-examples": "warn",
"no-invalid-media-type-examples": {
"allowAdditionalProperties": false,
"severity": "warn",
},
"no-server-example.com": "warn",
"no-server-trailing-slash": "error",
"no-server-variables-empty-enum": "error",
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/config/__tests__/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,24 @@ describe('transformConfig', () => {
});
});
});

describe('mergeExtends', () => {
it('should work with empty extends', () => {
expect(utils.mergeExtends([]).rules).toEqual({});
});

it('should work with configurable rules changing severity', () => {
expect(
utils.mergeExtends([
{
rules: { 'rule/abc': { severity: 'error', subject: 'Operation' } },
},
{
rules: { 'rule/abc': 'warn' },
},
]).rules
).toEqual({
'rule/abc': { severity: 'warn', subject: 'Operation' },
});
});
});
85 changes: 43 additions & 42 deletions packages/core/src/config/utils.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
assignExisting,
assignOnlyExistingConfig,
assignConfig,
isDefined,
isTruthy,
showErrorForDeprecatedField,
Expand Down Expand Up @@ -198,47 +199,47 @@ export function mergeExtends(rulesConfList: ResolvedStyleguideConfig[]) {
);
}

Object.assign(result.rules, rulesConf.rules);
Object.assign(result.oas2Rules, rulesConf.oas2Rules);
assignExisting(result.oas2Rules, rulesConf.rules || {});
Object.assign(result.oas3_0Rules, rulesConf.oas3_0Rules);
assignExisting(result.oas3_0Rules, rulesConf.rules || {});
Object.assign(result.oas3_1Rules, rulesConf.oas3_1Rules);
assignExisting(result.oas3_1Rules, rulesConf.rules || {});
Object.assign(result.async2Rules, rulesConf.async2Rules);
assignExisting(result.async2Rules, rulesConf.rules || {});
Object.assign(result.async3Rules, rulesConf.async3Rules);
assignExisting(result.async3Rules, rulesConf.rules || {});
Object.assign(result.arazzoRules, rulesConf.arazzoRules);
assignExisting(result.arazzoRules, rulesConf.rules || {});

Object.assign(result.preprocessors, rulesConf.preprocessors);
Object.assign(result.oas2Preprocessors, rulesConf.oas2Preprocessors);
assignExisting(result.oas2Preprocessors, rulesConf.preprocessors || {});
Object.assign(result.oas3_0Preprocessors, rulesConf.oas3_0Preprocessors);
assignExisting(result.oas3_0Preprocessors, rulesConf.preprocessors || {});
Object.assign(result.oas3_1Preprocessors, rulesConf.oas3_1Preprocessors);
assignExisting(result.oas3_1Preprocessors, rulesConf.preprocessors || {});
Object.assign(result.async2Preprocessors, rulesConf.async2Preprocessors);
assignExisting(result.async2Preprocessors, rulesConf.preprocessors || {});
Object.assign(result.async3Preprocessors, rulesConf.async3Preprocessors);
assignExisting(result.async3Preprocessors, rulesConf.preprocessors || {});
Object.assign(result.arazzoPreprocessors, rulesConf.arazzoPreprocessors);
assignExisting(result.arazzoPreprocessors, rulesConf.preprocessors || {});

Object.assign(result.decorators, rulesConf.decorators);
Object.assign(result.oas2Decorators, rulesConf.oas2Decorators);
assignExisting(result.oas2Decorators, rulesConf.decorators || {});
Object.assign(result.oas3_0Decorators, rulesConf.oas3_0Decorators);
assignExisting(result.oas3_0Decorators, rulesConf.decorators || {});
Object.assign(result.oas3_1Decorators, rulesConf.oas3_1Decorators);
assignExisting(result.oas3_1Decorators, rulesConf.decorators || {});
Object.assign(result.async2Decorators, rulesConf.async2Decorators);
assignExisting(result.async2Decorators, rulesConf.decorators || {});
Object.assign(result.async3Decorators, rulesConf.async3Decorators);
assignExisting(result.async3Decorators, rulesConf.decorators || {});
Object.assign(result.arazzoDecorators, rulesConf.arazzoDecorators);
assignExisting(result.arazzoDecorators, rulesConf.decorators || {});
assignConfig(result.rules, rulesConf.rules);
assignConfig(result.oas2Rules, rulesConf.oas2Rules);
assignOnlyExistingConfig(result.oas2Rules, rulesConf.rules);
assignConfig(result.oas3_0Rules, rulesConf.oas3_0Rules);
assignOnlyExistingConfig(result.oas3_0Rules, rulesConf.rules);
assignConfig(result.oas3_1Rules, rulesConf.oas3_1Rules);
assignOnlyExistingConfig(result.oas3_1Rules, rulesConf.rules);
assignConfig(result.async2Rules, rulesConf.async2Rules);
assignOnlyExistingConfig(result.async2Rules, rulesConf.rules);
assignConfig(result.async3Rules, rulesConf.async3Rules);
assignOnlyExistingConfig(result.async3Rules, rulesConf.rules);
assignConfig(result.arazzoRules, rulesConf.arazzoRules);
assignOnlyExistingConfig(result.arazzoRules, rulesConf.rules);

assignConfig(result.preprocessors, rulesConf.preprocessors);
assignConfig(result.oas2Preprocessors, rulesConf.oas2Preprocessors);
assignOnlyExistingConfig(result.oas2Preprocessors, rulesConf.preprocessors);
assignConfig(result.oas3_0Preprocessors, rulesConf.oas3_0Preprocessors);
assignOnlyExistingConfig(result.oas3_0Preprocessors, rulesConf.preprocessors);
assignConfig(result.oas3_1Preprocessors, rulesConf.oas3_1Preprocessors);
assignOnlyExistingConfig(result.oas3_1Preprocessors, rulesConf.preprocessors);
assignConfig(result.async2Preprocessors, rulesConf.async2Preprocessors);
assignOnlyExistingConfig(result.async2Preprocessors, rulesConf.preprocessors);
assignConfig(result.async3Preprocessors, rulesConf.async3Preprocessors);
assignOnlyExistingConfig(result.async3Preprocessors, rulesConf.preprocessors);
assignConfig(result.arazzoPreprocessors, rulesConf.arazzoPreprocessors);
assignOnlyExistingConfig(result.arazzoPreprocessors, rulesConf.preprocessors);

assignConfig(result.decorators, rulesConf.decorators);
assignConfig(result.oas2Decorators, rulesConf.oas2Decorators);
assignOnlyExistingConfig(result.oas2Decorators, rulesConf.decorators);
assignConfig(result.oas3_0Decorators, rulesConf.oas3_0Decorators);
assignOnlyExistingConfig(result.oas3_0Decorators, rulesConf.decorators);
assignConfig(result.oas3_1Decorators, rulesConf.oas3_1Decorators);
assignOnlyExistingConfig(result.oas3_1Decorators, rulesConf.decorators);
assignConfig(result.async2Decorators, rulesConf.async2Decorators);
assignOnlyExistingConfig(result.async2Decorators, rulesConf.decorators);
assignConfig(result.async3Decorators, rulesConf.async3Decorators);
assignOnlyExistingConfig(result.async3Decorators, rulesConf.decorators);
assignConfig(result.arazzoDecorators, rulesConf.arazzoDecorators);
assignOnlyExistingConfig(result.arazzoDecorators, rulesConf.decorators);

result.plugins!.push(...(rulesConf.plugins || []));
result.pluginPaths!.push(...(rulesConf.pluginPaths || []));
Expand Down
25 changes: 23 additions & 2 deletions packages/core/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,30 @@ export function isNotString<T>(value: string | T): value is T {
return !isString(value);
}

export function assignExisting<T>(target: Record<string, T>, obj: Record<string, T>) {
export const assignConfig = <T extends string | { severity?: string }>(
target: Record<string, T>,
obj?: Record<string, T>
) => {
if (!obj) return;
for (const k of Object.keys(obj)) {
if (target.hasOwnProperty(k)) {
if (isPlainObject(target[k]) && typeof obj[k] === 'string') {
target[k].severity = obj[k];
} else {
target[k] = obj[k];
}
}
};

export function assignOnlyExistingConfig<T extends string | { severity?: string }>(
target: Record<string, T>,
obj?: Record<string, T>
) {
if (!obj) return;
for (const k of Object.keys(obj)) {
if (!target.hasOwnProperty(k)) continue;
if (isPlainObject(target[k]) && typeof obj[k] === 'string') {
target[k].severity = obj[k];
} else {
target[k] = obj[k];
}
}
Expand Down

1 comment on commit 72c4c07

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Coverage report

St.❔
 Category Percentage Covered / Total
🟡 Statements 78.82% 5029/6380
🟡 Branches 67.19% 2050/3051
🟡 Functions 73.35% 831/1133
🟡 Lines 79.12% 4743/5995

Test suite run success

814 tests passing in 121 suites.

Report generated by 🧪jest coverage report action from 72c4c07

Please sign in to comment.