Skip to content

Commit

Permalink
fix: add handling of absolute url path in ignore-file (#1014)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SmoliyY
SmoliyY authored Feb 20, 2023
1 parent 4d8811c commit 28e7e83
Show file tree
Hide file tree
Showing 5 changed files with 208 additions and 10 deletions.
144 changes: 144 additions & 0 deletions packages/core/src/config/__tests__/__snapshots__/config.test.ts.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`generation ignore object should generate config with absoluteUri for ignore 1`] = `
StyleguideConfig {
"_usedRules": Set {},
"_usedVersions": Set {},
"configFile": undefined,
"decorators": Object {
"oas2": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"oas3_0": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"oas3_1": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
},
"doNotResolveExamples": false,
"extendPaths": Array [],
"ignore": Object {
"https://some-path.yaml": Object {
"no-unused-components": Set {
"#/components/schemas/Foo",
},
},
"some-path/openapi.yaml": Object {
"no-unused-components": Set {
"#/components/schemas/Foo",
},
},
},
"pluginPaths": Array [],
"plugins": Array [],
"preprocessors": Object {
"oas2": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"oas3_0": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"oas3_1": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
},
"rawConfig": Object {
"_usedRules": Set {},
"_usedVersions": Set {},
"configFile": "redocly.yaml",
"decorators": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"doNotResolveExamples": false,
"ignore": Object {},
"plugins": Array [],
"preprocessors": Object {
"oas2": Object {},
"oas3_0": Object {},
"oas3_1": Object {},
},
"rawConfig": Object {
"plugins": Array [],
"rules": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
},
"recommendedFallback": false,
"rules": Object {
"oas2": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_0": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_1": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
},
},
"recommendedFallback": false,
"rules": Object {
"oas2": Object {
"oas2": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_0": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_1": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
},
"oas3_0": Object {
"oas2": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_0": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_1": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
},
"oas3_1": Object {
"oas2": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_0": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
"oas3_1": Object {
"no-empty-servers": "error",
"operation-summary": "error",
},
},
},
}
`;
25 changes: 25 additions & 0 deletions packages/core/src/config/__tests__/config.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import { OasVersion } from '../../oas-types';
import { Config, StyleguideConfig } from '../config';
import { getMergedConfig } from '../utils';
import { doesYamlFileExist } from '../../utils';
import { parseYaml } from '../../js-yaml';
import { readFileSync } from 'fs';
import { ignoredFileStub } from './fixtures/ingore-file';
import * as path from 'path';
import { NormalizedProblem } from '../../walk';
import { Source } from '../../resolve';

jest.mock('../../utils');
jest.mock('../../js-yaml');
jest.mock('fs');

const testConfig: Config = {
rawConfig: {
Expand Down Expand Up @@ -280,3 +291,17 @@ describe('StyleguideConfig.extendTypes', () => {
);
});
});

describe('generation ignore object', () => {
it('should generate config with absoluteUri for ignore', () => {
(readFileSync as jest.Mock<any, any>).mockImplementationOnce(() => '');
(parseYaml as jest.Mock<any, any>).mockImplementationOnce(() => ignoredFileStub);
(doesYamlFileExist as jest.Mock<any, any>).mockImplementationOnce(() => true);

jest.spyOn(path, 'resolve').mockImplementationOnce((_, filename) => `some-path/${filename}`);

const styleguideConfig = new StyleguideConfig(testConfig.styleguide);

expect(styleguideConfig).toMatchSnapshot();
});
});
8 changes: 8 additions & 0 deletions packages/core/src/config/__tests__/fixtures/ingore-file.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export const ignoredFileStub = {
'openapi.yaml': {
'no-unused-components': ['#/components/schemas/Foo'],
},
'https://some-path.yaml': {
'no-unused-components': ['#/components/schemas/Foo'],
},
};
18 changes: 14 additions & 4 deletions packages/core/src/config/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
ThemeRawConfig,
} from './types';
import { getResolveConfig } from './utils';
import { isAbsoluteUrl } from '../ref-utils';

export const IGNORE_FILE = '.redocly.lint-ignore.yaml';
const IGNORE_BANNER =
Expand Down Expand Up @@ -115,11 +116,17 @@ export class StyleguideConfig {

// resolve ignore paths
for (const fileName of Object.keys(this.ignore)) {
this.ignore[path.resolve(path.dirname(ignoreFile), fileName)] = this.ignore[fileName];
this.ignore[
isAbsoluteUrl(fileName) ? fileName : path.resolve(path.dirname(ignoreFile), fileName)
] = this.ignore[fileName];

for (const ruleId of Object.keys(this.ignore[fileName])) {
this.ignore[fileName][ruleId] = new Set(this.ignore[fileName][ruleId]);
}
delete this.ignore[fileName];

if (!isAbsoluteUrl(fileName)) {
delete this.ignore[fileName];
}
}
}

Expand All @@ -128,8 +135,11 @@ export class StyleguideConfig {
const ignoreFile = path.join(dir, IGNORE_FILE);
const mapped: Record<string, any> = {};
for (const absFileName of Object.keys(this.ignore)) {
const ignoredRules = (mapped[slash(path.relative(dir, absFileName))] =
this.ignore[absFileName]);
const mappedDefinitionName = isAbsoluteUrl(absFileName)
? absFileName
: slash(path.relative(dir, absFileName));
const ignoredRules = (mapped[mappedDefinitionName] = this.ignore[absFileName]);

for (const ruleId of Object.keys(ignoredRules)) {
ignoredRules[ruleId] = Array.from(ignoredRules[ruleId]) as any;
}
Expand Down
23 changes: 17 additions & 6 deletions packages/core/src/format/format.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const coreVersion = require('../../package.json').version;
import { NormalizedProblem, ProblemSeverity, LineColLocationObject, LocationObject } from '../walk';
import { getCodeframe, getLineColLocation } from './codeframes';
import { env } from '../env';
import { isAbsoluteUrl } from '../ref-utils';

export type Totals = {
errors: number;
Expand Down Expand Up @@ -120,7 +121,7 @@ export function formatProblems(
for (const [file, { ruleIdPad, locationPad: positionPad, fileProblems }] of Object.entries(
groupedByFile
)) {
logger.info(`${colorize.blue(path.relative(cwd, file))}:\n`);
logger.info(`${colorize.blue(isAbsoluteUrl(file) ? file : path.relative(cwd, file))}:\n`);

for (let i = 0; i < fileProblems.length; i++) {
const problem = fileProblems[i];
Expand All @@ -138,7 +139,9 @@ export function formatProblems(
output.write('<checkstyle version="4.3">\n');

for (const [file, { fileProblems }] of Object.entries(groupedByFile)) {
output.write(`<file name="${xmlEscape(path.relative(cwd, file))}">\n`);
output.write(
`<file name="${xmlEscape(isAbsoluteUrl(file) ? file : path.relative(cwd, file))}">\n`
);
fileProblems.forEach(formatCheckstyle);
output.write(`</file>\n`);
}
Expand Down Expand Up @@ -169,7 +172,9 @@ export function formatProblems(
return {
description: p.message,
location: {
path: path.relative(cwd, location.source.absoluteRef),
path: isAbsoluteUrl(location.source.absoluteRef)
? location.source.absoluteRef
: path.relative(cwd, location.source.absoluteRef),
lines: {
begin: lineCol.start.line,
},
Expand All @@ -191,14 +196,18 @@ export function formatProblems(
location: p.location.map((location: any) => ({
...location,
source: {
ref: path.relative(cwd, location.source.absoluteRef),
ref: isAbsoluteUrl(location.source.absoluteRef)
? location.source.absoluteRef
: path.relative(cwd, location.source.absoluteRef),
},
})),
from: p.from
? {
...p.from,
source: {
ref: path.relative(cwd, p.from?.source.absoluteRef || cwd),
ref: isAbsoluteUrl(p.from?.source.absoluteRef)
? p.from?.source.absoluteRef
: path.relative(cwd, p.from?.source.absoluteRef || cwd),
},
}
: undefined,
Expand Down Expand Up @@ -226,7 +235,9 @@ export function formatProblems(
function formatCodeframe(problem: NormalizedProblem, idx: number) {
const bgColor = getBgColor(problem);
const location = problem.location[0]; // TODO: support multiple locations
const relativePath = path.relative(cwd, location.source.absoluteRef);
const relativePath = isAbsoluteUrl(location.source.absoluteRef)
? location.source.absoluteRef
: path.relative(cwd, location.source.absoluteRef);
const loc = getLineColLocation(location);
const atPointer = location.pointer ? colorize.gray(`at ${location.pointer}`) : '';
const fileWithLoc = `${relativePath}:${loc.start.line}:${loc.start.col}`;
Expand Down

1 comment on commit 28e7e83

@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 73.82% 3437/4656
🟡 Branches 65% 1959/3014
🟡 Functions 66.17% 579/875
🟡 Lines 73.84% 3196/4328

Test suite run success

566 tests passing in 85 suites.

Report generated by 🧪jest coverage report action from 28e7e83

Please sign in to comment.