Skip to content

fix(wrapper-generator): fail loudly on cmdlet file collisions - #3713

Open
Joywambui-maina wants to merge 1 commit into
feature/wrapper-module-packagingfrom
fix/cmdlet-file-collisions
Open

fix(wrapper-generator): fail loudly on cmdlet file collisions#3713
Joywambui-maina wants to merge 1 commit into
feature/wrapper-module-packagingfrom
fix/cmdlet-file-collisions

Conversation

@Joywambui-maina

Copy link
Copy Markdown

Changes proposed in this pull request

  • Fail generation loudly when two operations resolve to the same cmdlet file, listing every collision with both operations' request-builder expressions, instead of letting the later one silently overwrite the earlier. This is the silent-drop failure mode AutoRest had, and nothing could detect it before: writes are not logged at console level, the summary counts surviving files and the parity gate only inspects files that exist.
  • Merge OData cast list/item pairs (owners/graph.user with owners/{id}/graph.user) into one Get-* dispatcher the way plain list/item pairs already merge, matching the single cmdlet the published SDK ships for them.
  • Resolve the collisions the full-inventory sweep surfaced as NamingOverrides data: the termStore and agreement-file stitches, the default-singleton renames (SubSite, DefaultDrive, DefaultCalendarEvent, GroupLifecyclePolicyByGroup) and the nested navigations the SDK never shipped. Every entry cites the AutoRest directive or the published command inventory that proves it.
  • Add PathMatch.Suffix to NamingOverrides so a navigation that recurs under many roots takes one entry instead of one per root.
  • Surface the generator's exception instead of its skip warnings when Build-WrapperModule.ps1 captures a failure, so a collision failure is readable in the build summary.
  • Document both collision classes in the edge-case catalog, and pin the renamed names, the suppression matching and the guard itself in the unit tests (115 passing).
  • Six modules now fail generation instead of building green: Calendar 3, Files 1, Groups 8, Identity.Governance 80, Notes 18 and Sites 102 collisions. That is the guard working rather than a regression, because those modules were already losing cmdlets silently.

Other links

A second operation resolving to an already-written cmdlet file now fails
generation with the full collision list instead of silently overwriting it,
which is the silent-drop failure mode AutoRest had.

OData cast list/item pairs (owners/graph.user) now merge like plain pairs,
and the sweep's collisions land as cited NamingOverrides entries: termStore
and agreement-file stitches, default-singleton renames (SubSite,
DefaultDrive, DefaultCalendarEvent), and nested navs the SDK never shipped.
Remaining families are tracked on #3704.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the WrapperGenerator to prevent silent cmdlet loss by detecting and failing on cmdlet file name collisions, and expands the naming-override data/model to encode published-SDK renames/suppressions (including broad “suffix” matching for recurring navigations). It also adjusts GET list/item pairing to merge OData cast list/item pairs into a single dispatcher cmdlet, and updates tests/docs accordingly.

Changes:

  • Detect cmdlet .g.cs file collisions during generation and fail with a consolidated, operation-identifying error message.
  • Extend NamingOverrides to support exact/prefix/suffix path matching and add many oracle-/directive-cited rename/suppression entries to resolve known collisions.
  • Merge cast list/item GET pairs (e.g., .../owners/graph.user with .../owners/{id}/graph.user) into one Get-* dispatcher; add regression/unit tests and documentation updates.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/WrapperGenerator/README.md Updates documentation to reflect collision-guard behavior and expanded overrides/testing counts.
tools/WrapperGenerator/PowerShellWrapperGenerationService.cs Tracks written files to detect collisions and throws a consolidated exception at end of generation.
tools/WrapperGenerator/NamingOverrides.cs Reworks override matching to Exact/Prefix/Suffix and adds many new rename/suppression entries with citations.
tools/WrapperGenerator/edge-cases/naming-edge-cases.md Documents collision families and how they’re handled/resolved.
tools/WrapperGenerator/CmdletNaming.cs Enhances list/item merge detection to support OData cast list/item pairing.
tools/WrapperGenerator.Tests/NamingTests.cs Adds unit coverage for new overrides and cast list/item pairing behavior.
tools/WrapperGenerator.Tests/GenerationServiceRegressionTests.cs Adds regression test ensuring collisions fail loudly and identify both operations.
tools/Build-WrapperModule.ps1 Improves failure capture to surface the generator exception text in build output summaries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -298,6 +316,15 @@ private async Task<int> EmitGetOperationsAsync(List<GetOperationRecord> getOpera
private async Task<int> WriteCmdletFileAsync(CmdletNaming naming, string source, CancellationToken cancellationToken)
{
var fileName = naming.ClassName.Replace("Command", "", StringComparison.Ordinal) + ".g.cs";
Comment on lines +149 to +151
$lines = @($wrapperOut | ForEach-Object { "$_" })
$exception = $lines | Where-Object { $_ -match 'Unhandled exception|Exception:' } | Select-Object -First 1
$exceptionIndex = if ($exception) { $lines.IndexOf($exception) } else { -1 }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants