Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions .changeset/quiet-bears-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@redocly/cli': patch
'@redocly/openapi-core': patch
---

Fixed an issue where rule reported a duplicate parameter when two or more `$ref`s point to the same path item.
Comment thread
JLekawa marked this conversation as resolved.
Outdated
123 changes: 123 additions & 0 deletions packages/core/src/__tests__/walk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,129 @@ describe('walk order', () => {
`);
});

it('should not visit a nested rule when another rule re-enters a shared $ref-ed node', async () => {
const calls: string[] = [];

const testRuleSet: Oas3RuleSet = {
nested: () => ({
PathItem: {
Operation: {
Parameter(param: any, _ctx: any, parents: any) {
calls.push(`param ${param.name} > op ${parents.Operation.operationId}`);
},
},
},
}),
callbacks: () => ({
PathItem: {
Operation: {
Callback: {
PathItem: {
Operation(op: any) {
calls.push(`callback op ${op.operationId}`);
},
},
},
},
},
}),
};

const document = parseYamlToDocument(
outdent`
openapi: 3.1.0
paths:
/sample:
get:
operationId: get
callbacks:
first:
'uri-1':
$ref: '#/components/pathItems/shared'
second:
'uri-2':
$ref: '#/components/pathItems/shared'
components:
pathItems:
shared:
post:
operationId: notify
parameters:
- name: x-shared
in: header
`,
''
);

await lintDocument({
externalRefResolver: new BaseResolver(),
document,
config: await createConfig({
plugins: [{ id: 'test', rules: { oas3: testRuleSet } }],
rules: {
'test/nested': 'error',
'test/callbacks': 'error',
},
}),
});

expect(calls).toMatchInlineSnapshot(`
[
"callback op notify",
"param x-shared > op notify",
"callback op notify",
]
`);
});

it('should visit nested rules for keys written next to a $ref', async () => {
const calls: string[] = [];

const testRuleSet: Oas3RuleSet = {
test: () => ({
NamedSchemas: {
Schema: {
Schema(_schema: any, ctx: any) {
calls.push(ctx.location.pointer);
},
},
},
}),
};

const document = parseYamlToDocument(
outdent`
openapi: 3.1.0
paths: {}
components:
schemas:
Base:
type: object
WithSiblings:
$ref: '#/components/schemas/Base'
properties:
sibling:
type: string
`,
''
);

await lintDocument({
externalRefResolver: new BaseResolver(),
document,
config: await createConfig({
plugins: [{ id: 'test', rules: { oas3: testRuleSet } }],
rules: { 'test/test': 'error' },
}),
});

expect(calls).toMatchInlineSnapshot(`
[
"#/components/schemas/WithSiblings/properties/sibling",
]
`);
});

it('should visit and do not recurse for circular refs top-level', async () => {
const calls: string[] = [];

Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/walk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export function walkDocument<T extends BaseVisitor>(opts: {
const composedRefWalkId = (type: NormalizedNodeType, location: Location) =>
`${type.name}::${location.absolutePointer}`;
const ignoredNodes = new Set<string>();
const walkingNodeByType: Record<string, unknown> = {};

// Pre-compute combined enter/leave arrays per type to avoid per-node array allocations
const anyEnter = normalizedVisitors.any.enter as VisitorNode<any>[];
Expand Down Expand Up @@ -300,6 +301,8 @@ export function walkDocument<T extends BaseVisitor>(opts: {
if (
(context.parent && // if nested
context.parent.activatedOn &&
context.parent.activatedOn.value.node ===
walkingNodeByType[context.parent.type.name] &&
context.activatedOn?.value.withParentNode !== context.parent.activatedOn.value.node &&
// do not enter if visited by parent children (it works thanks because deeper visitors are sorted before)
context.parent.activatedOn.value.nextLevelTypeActivated?.value !== type) ||
Expand Down Expand Up @@ -343,6 +346,9 @@ export function walkDocument<T extends BaseVisitor>(opts: {
}
}

const prevWalkingNode = walkingNodeByType[type.name];
walkingNodeByType[type.name] = resolvedNode;

if (visitedBySome || !isNodeSeen) {
seenNodesPerType[type.name] = seenNodesPerType[type.name] || new Set();
seenNodesPerType[type.name].add(resolvedNode);
Expand Down Expand Up @@ -407,6 +413,8 @@ export function walkDocument<T extends BaseVisitor>(opts: {
}
}

walkingNodeByType[type.name] = prevWalkingNode;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why reassigning it back?


const currentLeaveVisitors =
combinedLeave[type.name] || (normalizedVisitors[type.name]?.leave || []).concat(anyLeave);

Expand Down
36 changes: 36 additions & 0 deletions tests/e2e/lint/shared-path-item-in-callbacks/openapi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
openapi: 3.1.0
info:
title: Example OpenAPI 3.1 definition.
version: 1.0

paths:
/sample:
get:
operationId: getSample
summary: example summary
responses:
'200':
description: example description
callbacks:
onEventA:
'https://callbacks.example.com/a':
$ref: '#/components/pathItems/notify'
onEventB:
'https://callbacks.example.com/b':
$ref: '#/components/pathItems/notify'

components:
pathItems:
notify:
post:
operationId: notify
summary: example summary
parameters:
- in: header
name: x-signature
required: true
schema:
type: string
responses:
'200':
description: example description
7 changes: 7 additions & 0 deletions tests/e2e/lint/shared-path-item-in-callbacks/redocly.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apis:
main:
root: ./openapi.yaml

rules:
operation-parameters-unique: error
path-parameters-defined: error
6 changes: 6 additions & 0 deletions tests/e2e/lint/shared-path-item-in-callbacks/snapshot.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

validating openapi.yaml using lint rules for api 'main'...
openapi.yaml: validated in <test>ms

Woohoo! Your API description is valid. 🎉

Loading