fix: report component renaming conflicts for same-name components from different files#2706
fix: report component renaming conflicts for same-name components from different files#2706kanoru3101 wants to merge 10 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: ae18522 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1774945843 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1774945843
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1774945843
# or
npm install @redocly/respect-core@0.0.0-snapshot.1774945843 |
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
The issue here is that the bundler doesn't apply renaming consistently. If the schema wasn't referenced with a JSON pointer inside the file, it would behave correctly.
…g-conflicts-severity-ignored-the-reference
kanoru3101
added
snapshot
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1775209981 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1775209981
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1775209981
# or
npm install @redocly/respect-core@0.0.0-snapshot.1775209981 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Hardcoded "schemas" in message for all component types
- Updated the conflict report text to use the actual
componentTypeand adjusted tests to assert the correct type-specific wording.
- Updated the conflict report text to use the actual
Or push these changes by commenting:
@cursor push c4dddad26a
Preview (c4dddad26a)
diff --git a/packages/core/src/__tests__/bundle-oas.test.ts b/packages/core/src/__tests__/bundle-oas.test.ts
--- a/packages/core/src/__tests__/bundle-oas.test.ts
+++ b/packages/core/src/__tests__/bundle-oas.test.ts
@@ -50,7 +50,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('warn');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed first to first-2.`
+ `Two examples are referenced with the same name but different content. Renamed first to first-2.`
);
expect(res.parsed).toMatchSnapshot();
});
@@ -62,7 +62,7 @@
});
expect(problems).toHaveLength(1);
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed param-b to param-b-2.`
+ `Two parameters are referenced with the same name but different content. Renamed param-b to param-b-2.`
);
expect(res.parsed).toMatchSnapshot();
});
diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts
--- a/packages/core/src/__tests__/bundle.test.ts
+++ b/packages/core/src/__tests__/bundle.test.ts
@@ -67,7 +67,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('warn');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed first to first-2.`
+ `Two examples are referenced with the same name but different content. Renamed first to first-2.`
);
expect(res.parsed).toMatchSnapshot();
});
@@ -96,7 +96,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('warn');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed param-b to param-b-2.`
+ `Two parameters are referenced with the same name but different content. Renamed param-b to param-b-2.`
);
expect(res.parsed).toMatchSnapshot();
});
@@ -119,7 +119,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('error');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed param-b to param-b-2.`
+ `Two parameters are referenced with the same name but different content. Renamed param-b to param-b-2.`
);
});
@@ -134,7 +134,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('warn');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed User to User-2.`
+ `Two parameters are referenced with the same name but different content. Renamed User to User-2.`
);
expect(res.parsed).toMatchSnapshot();
});
@@ -163,7 +163,7 @@
expect(problems).toHaveLength(1);
expect(problems[0].severity).toBe('error');
expect(problems[0].message).toEqual(
- `Two schemas are referenced with the same name but different content. Renamed User to User-2.`
+ `Two parameters are referenced with the same name but different content. Renamed User to User-2.`
);
});
diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts
--- a/packages/core/src/bundle/bundle-visitor.ts
+++ b/packages/core/src/bundle/bundle-visitor.ts
@@ -287,7 +287,7 @@
if (!componentsGroup[name] && prevName !== name) {
ctx.report({
- message: `Two schemas are referenced with the same name but different content. Renamed ${prevName} to ${name}.`,
+ message: `Two ${componentType} are referenced with the same name but different content. Renamed ${prevName} to ${name}.`,
location: ctx.location,
forceSeverity: componentRenamingConflicts,
});This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
…g-conflicts-severity-ignored-the-reference
kanoru3101
added
snapshot
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1775468574 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1775468574
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1775468574
# or
npm install @redocly/respect-core@0.0.0-snapshot.1775468574 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f78736. Configure here.
| export function pointerBaseName(pointer: string) { | ||
| const parts = pointer.split('/'); | ||
| return parts[parts.length - 1]; | ||
| return unescapePointerFragment(parts[parts.length - 1]); |
There was a problem hiding this comment.
Unescaped component names produce invalid $ref JSON pointers
Low Severity
pointerBaseName now calls unescapePointerFragment, which converts ~1 → / and ~0 → ~ (and decodes percent-encoding). The resulting name is stored correctly as an object key in components[componentType][name], but it's also interpolated directly into the $ref string (#/components/${componentType}/${name}) without re-escaping via escapePointerFragment. A component name containing / or ~ would produce a structurally broken JSON pointer reference.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8f78736. Configure here.
There was a problem hiding this comment.
This looks like a bug, but it's safe — the OAS spec only allows ^[a-zA-Z0-9\.\-_]+$ as component keys, so / and ~ are not supported in valid names.
…g-conflicts-severity-ignored-the-reference
kanoru3101
added
snapshot
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1775489865 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1775489865
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1775489865
# or
npm install @redocly/respect-core@0.0.0-snapshot.1775489865 |
kanoru3101
added
snapshot
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1775493945 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1775493945
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1775493945
# or
npm install @redocly/respect-core@0.0.0-snapshot.1775493945 |
| } | ||
|
|
||
| if (!componentsGroup[name]) { | ||
| if (!componentsGroup[name] && prevName !== name) { |
There was a problem hiding this comment.
| if (!componentsGroup[name] && prevName !== name) { | |
| if ( prevName !== name) { |
De we need the first part?
| export function pointerBaseName(pointer: string) { | ||
| const parts = pointer.split('/'); | ||
| return parts[parts.length - 1]; | ||
| return unescapePointerFragment(parts[parts.length - 1]); |
There was a problem hiding this comment.
Could you explain why do we need to unescape the pointer fragment.
3 participants
What/Why/How?
The
bundlecommand now reports component renaming conflicts whenever two external definitions share the same name but have different content.Also, another issue was that we created two types of uniq names. For
pointer- it was withprefix, and for other cases wassuffixRemoved logic to create prefix for the component name when the component name has the same name
Tested with other products
Reference
close #2691
Testing
Without
--component-renaming-conflicts-severityruleWith
--component-renaming-conflicts-severity=errorruleWith
--component-renaming-conflicts-severity=warnruleWith
--component-renaming-conflicts-severity=offruleScreenshots (optional)
Check yourself
Security
Note
Medium Risk
Changes component name generation during bundling, which can alter bundled
$refpaths and emitted warning/error behavior for existing specs. Risk is moderate because it affects core bundling output and conflict detection logic, though covered by expanded tests and snapshots.Overview
Bundling now detects and reports component renaming conflicts when two external refs resolve to the same component name but have different content (including pointer-style external refs), honoring
--component-renaming-conflicts-severity.Component naming logic was simplified to derive names from the JSON pointer basename (with proper unescaping) and then suffix
-2,-3, etc. on conflict, which may change resulting bundled$refpaths; tests/snapshots were updated and new fixtures added for pointer refs and dotted schema keys.Reviewed by Cursor Bugbot for commit 8f78736. Bugbot is set up for automated code reviews on this repo. Configure here.