From e287a9196cae942b1837a134d3f93a73703beeb0 Mon Sep 17 00:00:00 2001 From: Joywambui-maina Date: Thu, 13 Aug 2026 22:20:30 -0700 Subject: [PATCH] feat(wrapper-generator): complete v1.0 request-body binding Request bodies bound only top-level primitives, so 4,466 property occurrences across the v1.0 specs had no parameter. Every shape the classifier reaches now binds: referenced models and enums, formatted strings, schema-less UntypedNode values (converted on assignment, nulls dropped to match the published SDK's AddIf), and the numeric INF/NaN union. The invented -Password pair is replaced by the published -PasswordProfile. New gates verify it - omission oracle, coverage sweep, inventory diff, runtime conversions: 0 unbound across all 38 specs, 35 modules build and import, 148 tests. The pre-existing naming-parity gap is tracked separately. --- .gitignore | 4 + tools/Build-WrapperModule.ps1 | 32 +- tools/Compare-WrapperCmdletNames.ps1 | 6 +- tools/Compare-WrapperOperationInventory.ps1 | 105 ++++ tools/Measure-BodyPropertyCoverage.ps1 | 106 ++++ tools/New-WrapperOutputManifest.ps1 | 98 ++++ tools/Test-BodyBindingCoverage.ps1 | 296 ++++++++++ tools/Test-WrapperModule.ps1 | 225 +++++++- tools/WrapperGenerator.Tests/EmitterTests.cs | 40 +- tools/WrapperGenerator.Tests/NamingTests.cs | 4 +- .../SchemaPropertiesTests.cs | 517 +++++++++++++++--- .../WrapperGenerator.Tests/SpecShapeTests.cs | 142 +++++ tools/WrapperGenerator/CmdletEmitter.cs | 151 ++++- .../PowerShellWrapperGenerationService.cs | 90 ++- tools/WrapperGenerator/Program.cs | 13 +- tools/WrapperGenerator/README.md | 77 ++- tools/WrapperGenerator/SchemaProperties.cs | 500 +++++++++++++++-- tools/WrapperGenerator/Singularizer.cs | 2 +- tools/WrapperGenerator/StderrLogger.cs | 9 +- .../docs/body-property-binding.md | 241 ++++++++ .../edge-cases/body-binding-edge-cases.md | 156 ++++++ .../edge-cases/crosspath-merge-edge-cases.md | 0 .../edge-cases/kiota-alignment-edge-cases.md | 0 .../edge-cases/naming-edge-cases.md | 22 + 24 files changed, 2623 insertions(+), 213 deletions(-) create mode 100644 tools/Compare-WrapperOperationInventory.ps1 create mode 100644 tools/Measure-BodyPropertyCoverage.ps1 create mode 100644 tools/New-WrapperOutputManifest.ps1 create mode 100644 tools/Test-BodyBindingCoverage.ps1 create mode 100644 tools/WrapperGenerator.Tests/SpecShapeTests.cs create mode 100644 tools/WrapperGenerator/docs/body-property-binding.md create mode 100644 tools/WrapperGenerator/docs/edge-cases/body-binding-edge-cases.md rename tools/WrapperGenerator/{ => docs}/edge-cases/crosspath-merge-edge-cases.md (100%) rename tools/WrapperGenerator/{ => docs}/edge-cases/kiota-alignment-edge-cases.md (100%) rename tools/WrapperGenerator/{ => docs}/edge-cases/naming-edge-cases.md (88%) diff --git a/.gitignore b/.gitignore index 3e41c4492f..0d734493bd 100644 --- a/.gitignore +++ b/.gitignore @@ -252,6 +252,10 @@ Generated_Code/ # because we have git ;-) _UpgradeReport_Files/ Backup*/ +# ...but src/BackupRestore is a Graph service module, not a Visual Studio backup folder. The +# rule above excludes the directory itself, so git never descends into it; re-including the +# directory is what makes its generated output committable at all. +!src/BackupRestore/ UpgradeLog*.XML UpgradeLog*.htm ServiceFabricBackup/ diff --git a/tools/Build-WrapperModule.ps1 b/tools/Build-WrapperModule.ps1 index 7630a2563e..571994e82c 100644 --- a/tools/Build-WrapperModule.ps1 +++ b/tools/Build-WrapperModule.ps1 @@ -23,9 +23,10 @@ Get-* dispatchers forward to the workers by name via InvokeCommand.InvokeScript, manifest that hides the workers breaks dispatch ("term not recognized"). Worker visibility needs its own dispatch design and is tracked in the module-wiring issue. -Everything is written under artifacts/ (gitignored); nothing this script produces is -committed. To check cmdlet-name parity for a built module, point the parity gate at its -cmdlets folder: +By default everything is written under artifacts/ (gitignored) for throwaway local runs. With +-IntoSource the same pipeline writes the committed layout under src///wrapper/, +where the client, the wrappers and the csproj live together so the folder builds standalone. +To check cmdlet-name parity for a built module, point the parity gate at its cmdlets folder: .\tools\Compare-WrapperCmdletNames.ps1 -GeneratedPath artifacts\wrapper-modules\\src\Cmdlets .PARAMETER Module @@ -51,11 +52,19 @@ dotnet build configuration. Default: Debug. .PARAMETER SkipKiota Reuse the previously generated client (fast inner loop when only the wrappers changed). +.PARAMETER IntoSource +Write the committed layout under src///wrapper/ instead of artifacts/: +Client/ + Cmdlets/ + the csproj, self-contained so the folder builds on its own. This is how +the generated output is checked in; omit it for throwaway local builds. + .EXAMPLE .\tools\Build-WrapperModule.ps1 -Module Mail .EXAMPLE .\tools\Build-WrapperModule.ps1 -Module Mail,Calendar -ApiVersion v1.0 + +.EXAMPLE +.\tools\Build-WrapperModule.ps1 -Module Mail -IntoSource #> [CmdletBinding()] param( @@ -66,7 +75,8 @@ param( [string]$SpecRoot, [string]$OutputRoot, [string]$Configuration = 'Debug', - [switch]$SkipKiota + [switch]$SkipKiota, + [switch]$IntoSource ) $ErrorActionPreference = 'Stop' @@ -116,7 +126,17 @@ function Build-OneModule { $moduleName = "Microsoft.Graph.Wrapper.$Name" $clientNs = "Microsoft.Graph.PowerShell.$Name.Client" - $srcDir = Join-Path $OutputRoot "$Name\src" + # -IntoSource writes the committed layout: one self-contained project folder per module + # and API version, holding the kiota client, the wrappers, and the csproj that compiles + # both into one assembly. Everything under it is committable as-is (the module + # .gitignore blocks a csproj at the version-folder root, and still ignores bin/obj at + # any depth). Without the switch, output stays in artifacts/ for throwaway local runs. + $srcDir = if ($IntoSource) { + Join-Path $repoRoot "src\$Name\$ApiVersion\wrapper" + } + else { + Join-Path $OutputRoot "$Name\src" + } $clientDir = Join-Path $srcDir 'Client' $cmdletsDir = Join-Path $srcDir 'Cmdlets' New-Item -ItemType Directory -Force -Path $srcDir | Out-Null @@ -164,7 +184,7 @@ function Build-OneModule { $csprojPath = Join-Path $srcDir "$moduleName.csproj" $authCsprojRelative = [System.IO.Path]::GetRelativePath($srcDir, $authCsproj) -replace '/', '\' @" - + diff --git a/tools/Compare-WrapperCmdletNames.ps1 b/tools/Compare-WrapperCmdletNames.ps1 index d1a6b0a5c2..de2564b708 100644 --- a/tools/Compare-WrapperCmdletNames.ps1 +++ b/tools/Compare-WrapperCmdletNames.ps1 @@ -13,7 +13,7 @@ emitted [Cmdlet(...)] name matches what the oracle says the published SDK calls operation. A small set of published names are known AutoRest defects the generator deliberately -corrects instead of reproducing (tools/WrapperGenerator/edge-cases/naming-edge-cases.md +corrects instead of reproducing (tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md is the catalog). Those are matched against the $deliberateCorrections table below and reported as [CORRECTED] rather than [MISMATCH]; they do not fail the gate. @@ -133,7 +133,7 @@ function Get-ModuleApiVersion { # Published names the generator deliberately corrects instead of reproducing. Each entry maps # the shipped (wrong) command to the corrected one the generator emits, and must have a matching -# entry in tools/WrapperGenerator/edge-cases/naming-edge-cases.md and a pinned naming test. The +# entry in tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md and a pinned naming test. The # gate reports these as [CORRECTED] instead of [MISMATCH] and does not fail on them. $deliberateCorrections = @{ # AutoRest inflected the trailing /whois segment to "Whoi"; the other 28 whois-family @@ -270,7 +270,7 @@ foreach ($module in $modules | Sort-Object Name) { $oracleCommand = $candidates | Select-Object -First 1 if ($deliberateCorrections[$oracleCommand] -eq $expectedCommand) { $moduleCorrected++ - $moduleCorrections += " [CORRECTED] $($file.Name): oracle ships '$oracleCommand'; generator deliberately emits '$expectedCommand' (see tools/WrapperGenerator/edge-cases/naming-edge-cases.md)." + $moduleCorrections += " [CORRECTED] $($file.Name): oracle ships '$oracleCommand'; generator deliberately emits '$expectedCommand' (see tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md)." } else { $moduleProblems += " [MISMATCH] $($file.Name): generated '$expectedCommand', oracle says '$oracleCommand' for $method $normalizedUri." diff --git a/tools/Compare-WrapperOperationInventory.ps1 b/tools/Compare-WrapperOperationInventory.ps1 new file mode 100644 index 0000000000..31e0847611 --- /dev/null +++ b/tools/Compare-WrapperOperationInventory.ps1 @@ -0,0 +1,105 @@ +<# +.SYNOPSIS +Captures, and compares, the set of operations the generator turns into cmdlets. + +.DESCRIPTION +A change meant to affect only cmdlet PARAMETERS must not change which OPERATIONS generate. +Comparing filenames alone cannot show that: two operations could exchange ownership of a +cmdlet name and leave the same set of files behind. This records the full identity of each +emitted cmdlet - module, verb, noun, request path (the kiota builder chain, which is the +operation's path) and file - and diffs two snapshots on that tuple. + +Use -Baseline to record the current state before a change, then -Compare afterwards. + +.EXAMPLE +.\tools\Compare-WrapperOperationInventory.ps1 -Path artifacts\wrapper-modules -Baseline before.csv +.EXAMPLE +.\tools\Compare-WrapperOperationInventory.ps1 -Path artifacts\wrapper-modules -Baseline before.csv -Compare after.csv +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string]$Path, + [Parameter(Mandatory)] + [string]$Baseline, + [string]$Compare +) + +$ErrorActionPreference = 'Stop' + +$cmdletAttrPattern = '\[Cmdlet\(Verbs\w+\.(\w+),\s*"((?:\\.|[^"\\])*)"' +$builderPattern = 'client\.([A-Za-z0-9_\[\]\.]+?)\.(?:Get|Post|Patch|Delete|Put)Async' + +# Finds every Cmdlets folder under the root at any depth and takes the module name from the +# first segment below it, so the artifacts layout (/src/Cmdlets) and the committed one +# (//wrapper/Cmdlets) can be compared against each other. +function Get-Inventory([string]$root) { + $rootFull = (Resolve-Path $root).Path + $rows = [System.Collections.Generic.List[object]]::new() + foreach ($cmdletsDir in (Get-ChildItem $rootFull -Directory -Recurse -Filter 'Cmdlets')) { + $relative = $cmdletsDir.FullName.Substring($rootFull.Length).TrimStart('\', '/') + $module = ($relative -split '[\\/]')[0] + foreach ($file in Get-ChildItem $cmdletsDir.FullName -Filter '*.g.cs' -File) { + if ($file.Name -eq 'Shared.g.cs') { continue } + $text = Get-Content $file.FullName -Raw + $m = [regex]::Match($text, $cmdletAttrPattern) + if (-not $m.Success) { continue } + $b = [regex]::Match($text, $builderPattern) + $rows.Add([pscustomobject]@{ + Module = $module + Cmdlet = "$($m.Groups[1].Value)-$([regex]::Unescape($m.Groups[2].Value))" + Verb = $m.Groups[1].Value + RequestPath = if ($b.Success) { $b.Groups[1].Value } else { '(dispatcher)' } + File = $file.Name + }) + } + } + return $rows | Sort-Object Module, File +} + +$inventory = Get-Inventory $Path +# An empty inventory means the path or layout is wrong. Left unchecked it compares nothing +# against nothing and reports "unchanged" - a pass that proves the opposite of what it claims. +if ($inventory.Count -eq 0) { + Write-Error "No cmdlets found under '$Path'. Expected /src/Cmdlets or //wrapper/Cmdlets." + exit 2 +} + +if (-not $Compare) { + $inventory | Export-Csv $Baseline -NoTypeInformation + "baseline: $($inventory.Count) cmdlets -> $Baseline" + exit 0 +} + +$inventory | Export-Csv $Compare -NoTypeInformation +$before = Import-Csv $Baseline +$after = Import-Csv $Compare + +# Identity is the whole tuple, so an operation swapping which cmdlet/file it owns shows up as +# one removal plus one addition rather than as no change at all. +function Key($r) { "{0}|{1}|{2}|{3}" -f $r.Module, $r.Cmdlet, $r.RequestPath, $r.File } +# Filled by Add, and built inline rather than in a helper function, for two separate reasons: +# the HashSet(IEnumerable) constructor is ambiguous against +# HashSet(IEqualityComparer) when a side is empty, and returning a set FROM a function +# makes PowerShell enumerate it back into an Object[] - whose Contains is a linear scan, turning +# this comparison into ~n^2 string compares over ~10k identities. +$beforeKeys = [System.Collections.Generic.HashSet[string]]::new() +foreach ($r in $before) { [void]$beforeKeys.Add((Key $r)) } +$afterKeys = [System.Collections.Generic.HashSet[string]]::new() +foreach ($r in $after) { [void]$afterKeys.Add((Key $r)) } + +$added = @($afterKeys | Where-Object { -not $beforeKeys.Contains($_) }) +$removed = @($beforeKeys | Where-Object { -not $afterKeys.Contains($_) }) + +"before: $($before.Count) cmdlets" +"after: $($after.Count) cmdlets" +"added: $($added.Count)" +"removed: $($removed.Count)" +if ($added) { ""; "ADDED:"; $added | Select-Object -First 25 | ForEach-Object { " $_" } } +if ($removed) { ""; "REMOVED:"; $removed | Select-Object -First 25 | ForEach-Object { " $_" } } + +if ($added.Count -eq 0 -and $removed.Count -eq 0) { + ""; "operation inventory unchanged." + exit 0 +} +exit 1 diff --git a/tools/Measure-BodyPropertyCoverage.ps1 b/tools/Measure-BodyPropertyCoverage.ps1 new file mode 100644 index 0000000000..cf0afa6d28 --- /dev/null +++ b/tools/Measure-BodyPropertyCoverage.ps1 @@ -0,0 +1,106 @@ +<# +.SYNOPSIS +Reports how request-body properties classify across every module: bound, excluded, or +unsupported and why. + +.DESCRIPTION +Runs the generator over each spec and reads the per-property diagnostics it emits, so the +numbers come from the same classifier that decides what gets bound - not a second +reimplementation that could disagree with it. + +Output is a per-shape rollup (which unsupported shapes are worth implementing next) and a +per-module CSV. Required-but-unbound properties are called out separately: those are the ones +that make a cmdlet unable to complete its request at all. + +.EXAMPLE +.\tools\Measure-BodyPropertyCoverage.ps1 +#> +[CmdletBinding()] +param( + [ValidateSet('v1.0', 'beta')] + [string]$ApiVersion = 'v1.0', + [string]$OutCsv +) + +$ErrorActionPreference = 'Stop' +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path +if (-not $OutCsv) { $OutCsv = Join-Path $repoRoot "artifacts\body-property-coverage.$ApiVersion.csv" } + +$specRoot = Join-Path $repoRoot "openApiDocs_KiotaCompat\$ApiVersion" +$generator = Join-Path $repoRoot 'tools\WrapperGenerator' +$scratch = Join-Path $repoRoot "artifacts\body-coverage-scratch" +New-Item -ItemType Directory -Force $scratch | Out-Null +New-Item -ItemType Directory -Force (Split-Path $OutCsv) | Out-Null + +$rows = [System.Collections.Generic.List[object]]::new() +$specs = @(Get-ChildItem "$specRoot\*.yml" | Sort-Object Name) +# A run over no specs would report "0 unbound" - a clean bill of health from having measured +# nothing, which is the failure mode this whole sweep exists to avoid. +if ($specs.Count -eq 0) { Write-Error "No specs found under '$specRoot'."; exit 2 } +$failedSpecs = [System.Collections.Generic.List[string]]::new() + +foreach ($spec in $specs) { + $module = $spec.BaseName + $out = Join-Path $scratch $module + Remove-Item $out -Recurse -Force -ErrorAction SilentlyContinue + New-Item -ItemType Directory -Force $out | Out-Null + + # Information level so the per-property diagnostics are emitted. + $log = & dotnet run --project $generator -c Release -- ` + -d $spec.FullName -o $out -n "Microsoft.Graph.PowerShell.$module.Client" --api-version $ApiVersion --log-level Information 2>&1 + # A module that failed to generate emits no diagnostics, so its properties would silently + # count as zero unbound and flatter the total. + if ($LASTEXITCODE -ne 0) { + $failedSpecs.Add("$module (exit $LASTEXITCODE)") + Write-Warning "$module : generation failed; excluded from the totals" + continue + } + + foreach ($line in $log) { + if ("$line" -match 'Unbound body property (?[^.]+)\.(?\S+): (?\w+) \(required=(?\w+)\)') { + $rows.Add([pscustomobject]@{ + Module = $module + Noun = $Matches.noun + Property = $Matches.prop + Shape = $Matches.shape + Required = [bool]::Parse($Matches.req) + }) + } + } + Write-Host ("{0,-34} unbound: {1}" -f $module, @($rows | Where-Object Module -eq $module).Count) +} + +# A CSV written from a partial sweep reads exactly like a complete one, so it is only produced +# when every spec generated. The population is stated beside the totals for the same reason. +if ($failedSpecs.Count -gt 0) { + Write-Error "$($failedSpecs.Count) of $($specs.Count) specs failed to generate: $($failedSpecs -join ', '). No CSV written - these totals would understate the unbound surface." + exit 1 +} +$rows | Export-Csv $OutCsv -NoTypeInformation + +# The generator reports a property per OPERATION, so an inherited property on a widely reused +# model repeats across every cmdlet that binds it. Both figures matter and mean different +# things: occurrences size the noise in a run, distinct identities size the actual work. +$identity = { "$($_.Module)|$($_.Noun)|$($_.Property)|$($_.Shape)" } +$distinct = @($rows | ForEach-Object $identity | Sort-Object -Unique) + +""; "=== unbound body properties by shape (occurrences / distinct) ===" +$rows | Group-Object Shape | Sort-Object Count -Descending | + Select-Object Count, Name, + @{n = 'Distinct'; e = { @($_.Group | ForEach-Object $identity | Sort-Object -Unique).Count } } | + Format-Table -AutoSize | Out-String -Width 80 + +"=== distinct property names per shape (top 8 each) ===" +foreach ($g in ($rows | Group-Object Shape | Sort-Object Count -Descending)) { + $names = ($g.Group | Select-Object -ExpandProperty Property -Unique | Select-Object -First 8) -join ', ' + " {0,-16} {1}" -f $g.Name, $names +} + +""; "specs generated: $($specs.Count) of $($specs.Count)" +"total unbound occurrences: $($rows.Count)" +"distinct module/noun/prop/shape: $($distinct.Count)" +# Graph marks almost nothing required in its schemas (the overwhelming majority of required +# blocks list only @odata.type), so this count is reported for completeness and is not +# evidence that nothing important is unbound. +"flagged required in the spec: $(@($rows | Where-Object Required -eq 'True').Count)" +"csv: $OutCsv" diff --git a/tools/New-WrapperOutputManifest.ps1 b/tools/New-WrapperOutputManifest.ps1 new file mode 100644 index 0000000000..0d85270305 --- /dev/null +++ b/tools/New-WrapperOutputManifest.ps1 @@ -0,0 +1,98 @@ +<# +.SYNOPSIS +Writes a reviewable inventory of the committed wrapper output under src///wrapper/. + +.DESCRIPTION +The committed output is tens of thousands of generated files - far past what GitHub renders in +a diff and far past what anyone reads. This emits the summary a reviewer actually can read: +one CSV row per exported cmdlet (module, verb, noun, request path, source file) plus a +per-module rollup, so "what does this generator produce" and "what changed since last time" +are answerable from a diff of two small files instead of a diff of the tree. + +Cmdlet names come from the emitted [Cmdlet(VerbsX.Verb, "Noun")] attribute and the request +path from the emitted kiota builder chain - the generated source is the source of truth, so +the manifest cannot drift from what the module will actually export. + +Internal *_Get/*_List workers are listed with IsWorker = True: they are real emitted files but +not part of the surface a user calls, and separating them keeps the cmdlet count honest. (The +psd1 currently exports them anyway - the dispatcher resolves them by name at runtime - which is +a dispatch-design question tracked with the module-wiring work, not a manifest concern.) + +.EXAMPLE +.\tools\New-WrapperOutputManifest.ps1 +.EXAMPLE +.\tools\New-WrapperOutputManifest.ps1 -ApiVersion v1.0 +#> +[CmdletBinding()] +param( + [ValidateSet('v1.0', 'beta')] + [string]$ApiVersion = 'v1.0', + [string]$OutDir +) + +$ErrorActionPreference = 'Stop' +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path +# docs/ already holds versioned CSV inventories of the shipped surface +# (PowerShellBreakingChanges-V1.0.csv); this follows that placement and naming. +if (-not $OutDir) { $OutDir = Join-Path $repoRoot 'docs' } +$versionTag = if ($ApiVersion -eq 'v1.0') { 'V1.0' } else { 'Beta' } + +# Same attribute pattern the parity gate and Build-WrapperModule use. +$cmdletAttrPattern = '\[Cmdlet\(Verbs\w+\.(\w+),\s*"((?:\\.|[^"\\])*)"' +# The kiota chain the cmdlet calls, e.g. "client.Users[UserId].Messages.GetAsync()". +$builderPattern = 'client\.([A-Za-z0-9_\[\]\.]+?)\.(?:Get|Post|Patch|Delete|Put)Async' + +$rows = [System.Collections.Generic.List[object]]::new() +$moduleDirs = Get-ChildItem (Join-Path $repoRoot 'src') -Directory | + ForEach-Object { Join-Path $_.FullName "$ApiVersion\wrapper\Cmdlets" } | + Where-Object { Test-Path $_ } + +foreach ($dir in $moduleDirs) { + $module = (Get-Item $dir).Parent.Parent.Parent.Name + foreach ($file in Get-ChildItem $dir -Filter '*.g.cs' -File) { + if ($file.Name -eq 'Shared.g.cs') { continue } + $text = Get-Content $file.FullName -Raw + $m = [regex]::Match($text, $cmdletAttrPattern) + if (-not $m.Success) { continue } + $b = [regex]::Match($text, $builderPattern) + # A dispatcher makes no request itself - it forwards to its _Get/_List workers - so an + # absent builder chain identifies one rather than indicating a parse failure. + $isWorker = $file.Name -match '_(Get|List)\.g\.cs$' + $rows.Add([pscustomobject]@{ + Module = $module + ApiVersion = $ApiVersion + Cmdlet = "$($m.Groups[1].Value)-$([regex]::Unescape($m.Groups[2].Value))" + Verb = $m.Groups[1].Value + Noun = [regex]::Unescape($m.Groups[2].Value) + RequestPath = if ($b.Success) { $b.Groups[1].Value } elseif (-not $isWorker) { '(dispatcher)' } else { '' } + IsWorker = $isWorker + File = $file.Name + }) + } +} + +if ($rows.Count -eq 0) { throw "No committed wrapper output found for $ApiVersion under src/*/$ApiVersion/wrapper/Cmdlets." } + +$manifestPath = Join-Path $OutDir "WrapperCmdlets-$versionTag.csv" +$rows | Sort-Object Module, Cmdlet, File | Export-Csv $manifestPath -NoTypeInformation + +$summaryPath = Join-Path $OutDir "WrapperCmdlets-$versionTag-Summary.csv" +$rows | Group-Object Module | ForEach-Object { + $public = @($_.Group | Where-Object { -not $_.IsWorker }) + [pscustomobject]@{ + Module = $_.Name + Cmdlets = $public.Count + WorkerFiles = $_.Count - $public.Count + Get = @($public | Where-Object Verb -eq 'Get').Count + New = @($public | Where-Object Verb -eq 'New').Count + Update = @($public | Where-Object Verb -eq 'Update').Count + Remove = @($public | Where-Object Verb -eq 'Remove').Count + } +} | Sort-Object Cmdlets -Descending | Export-Csv $summaryPath -NoTypeInformation + +$publicTotal = @($rows | Where-Object { -not $_.IsWorker }).Count +"modules: $($rows | Group-Object Module | Measure-Object | Select-Object -ExpandProperty Count)" +"public cmdlets: $publicTotal" +"worker files: $($rows.Count - $publicTotal)" +"wrote $manifestPath" +"wrote $summaryPath" diff --git a/tools/Test-BodyBindingCoverage.ps1 b/tools/Test-BodyBindingCoverage.ps1 new file mode 100644 index 0000000000..7ac460c2e4 --- /dev/null +++ b/tools/Test-BodyBindingCoverage.ps1 @@ -0,0 +1,296 @@ +<# +.SYNOPSIS +Independent check that every settable Kiota request-body member is either bound by a cmdlet +parameter or accounted for by a named policy. + +.DESCRIPTION +Compilation proves that what we DO emit has the right CLR type - a wrong type name cannot +build. It cannot see what we FAIL to emit: a settable model member with no parameter, or a +parameter declared and never assigned, are both perfectly valid C#. This closes that gap. + +The invariant, per request-body model: + + settable kiota members == parameters with assignments + + properties excluded by a named policy + + properties reported as an unsupported shape + +Nothing here re-derives classification from the OpenAPI spec. The three inputs are produced +independently of each other: + + * the kiota client - generated by kiota, parsed here for its settable members and the + serialized (OpenAPI) name each one deserializes from + * the emitted cmdlet - parsed here for [Parameter] declarations and body.X = Y assignments + * the generator log - the classifier's own per-property exclusion/unsupported diagnostics + +A disagreement between them is a real defect in one of the three, which is the point. + +Failures reported: + MISSING a settable member with no parameter and no cited policy + NO-ASSIGNMENT a parameter that never assigns to the body + WRONG-TARGET an assignment to a member the model does not have + NO-PARAMETER an assignment reading an undeclared parameter + DUPLICATE two parameters assigning the same member + UNCITED a property excluded citing an unrecognised policy or shape + NO-MODEL a body type with no generated model file to check against + GENERATOR-FAILED generation failed, so its diagnostics cannot be trusted + +Modules whose spec or build output is absent are reported as skipped and counted, so a run +cannot look complete while silently covering less than it was asked to. + +.PARAMETER Module +Modules to check. Default: every module with generated cmdlets under -OutputRoot. + +.EXAMPLE +.\tools\Test-BodyBindingCoverage.ps1 -Module Users +.EXAMPLE +.\tools\Test-BodyBindingCoverage.ps1 +#> +[CmdletBinding()] +param( + [string[]]$Module, + [string]$OutputRoot, + [ValidateSet('v1.0', 'beta')] + [string]$ApiVersion = 'v1.0', + [string]$SpecRoot +) + +$ErrorActionPreference = 'Stop' +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path +if (-not $OutputRoot) { $OutputRoot = Join-Path $repoRoot 'artifacts\wrapper-modules' } +if (-not $SpecRoot) { $SpecRoot = Join-Path $repoRoot 'openApiDocs_KiotaCompat' } +$OutputRoot = (Resolve-Path -LiteralPath $OutputRoot).Path +$generator = Join-Path $repoRoot 'tools\WrapperGenerator' + +if (-not $Module) { + $Module = @(Get-ChildItem $OutputRoot -Directory | + Where-Object { Test-Path (Join-Path $_.FullName 'src\Cmdlets') } | + ForEach-Object { $_.Name } | Sort-Object) +} +if (-not $Module) { Write-Error "No modules found under '$OutputRoot'."; exit 2 } + +# --- kiota model members ----------------------------------------------------------------- +# The deserializer map is the authority for which OpenAPI name feeds which member: +# { "displayName", n => { DisplayName = n.GetStringValue(); } }, +# Reading it avoids re-deriving kiota's name-cleaning rules (underscores, Prop suffixing) +# here, which would just be a second implementation that could drift from the first. +$deserializerEntry = '\{\s*"(?[^"]+)"\s*,\s*n\s*=>\s*\{\s*(?\w+)\s*=' + +# Members are inherited: Message declares Subject but gets ChangeKey from OutlookItem and +# CreatedDateTime from Entity, each in its own file with its own deserializer map. Walking the +# base chain is required or every inherited assignment looks like it targets a member that does +# not exist. +# relativeName is everything after ".Models." - "CallRecords.Participant", not "Participant". +# Kiota nests a dotted schema name as a sub-namespace and a sub-folder, and several of those +# nested models share a simple name with a different model at the root +# (Models/Participant.cs vs Models/CallRecords/Participant.cs). Matching on the simple name +# silently compares a cmdlet against the wrong model, which is worse than not finding one. +function Get-ModelMembers([string]$modelsDir, [string]$relativeName, [System.Collections.Generic.HashSet[string]]$visited) { + if ($null -eq $visited) { $visited = [System.Collections.Generic.HashSet[string]]::new() } + if (-not $visited.Add($relativeName)) { return @{} } # defensive: never loop on a cyclic chain + + $segments = $relativeName -split '\.' + $typeName = $segments[-1] + $file = Join-Path $modelsDir ((($segments) -join [IO.Path]::DirectorySeparatorChar) + '.cs') + if (-not (Test-Path $file)) { return $null } + $text = Get-Content $file -Raw + $folder = if ($segments.Count -gt 1) { ($segments[0..($segments.Count - 2)] -join '.') + '.' } else { '' } + + $map = @{} + # The base type is fully qualified in the declaration; keep whatever sits after ".Models." + # so a nested base resolves to its own folder rather than the root. + $baseMatch = [regex]::Match($text, "public partial class $([regex]::Escape($typeName))\s*:\s*global::[A-Za-z0-9_.]*?\.Models\.(?[A-Za-z0-9_.]+)\s*,") + if ($baseMatch.Success) { + $inherited = Get-ModelMembers $modelsDir $baseMatch.Groups['base'].Value $visited + if ($null -ne $inherited) { + foreach ($k in $inherited.Keys) { $map[$k] = $inherited[$k] } + } + } + foreach ($m in [regex]::Matches($text, $deserializerEntry)) { + $map[$m.Groups['member'].Value] = $m.Groups['json'].Value + } + return $map +} + +# --- run the generator once per module to collect its diagnostics -------------------------- +$results = [System.Collections.Generic.List[object]]::new() +$failures = [System.Collections.Generic.List[object]]::new() + +# Only these may account for an unbound member. An exclusion naming anything else means the +# generator emitted a policy this check does not know about, which must fail rather than be +# accepted as a citation. +$knownPolicies = @( + 'ServerAssignedId', 'ODataControlData', 'KiotaAdditionalData', 'ReadOnlySchema', 'NavigationProperty' +) +$knownShapes = @('InlineEnum', 'UnknownFormat', 'InlineObject', 'Union', 'Dictionary', 'Unresolvable') + +$skipped = [System.Collections.Generic.List[string]]::new() + +foreach ($name in $Module) { + $spec = Join-Path $SpecRoot "$ApiVersion\$name.yml" + if (-not (Test-Path $spec)) { $spec = Join-Path $repoRoot "openApiDocs\$ApiVersion\$name.yml" } + if (-not (Test-Path $spec)) { $skipped.Add("$name (no spec)"); continue } + + $cmdletsDir = Join-Path $OutputRoot "$name\src\Cmdlets" + $modelsDir = Join-Path $OutputRoot "$name\src\Client\Models" + if (-not (Test-Path $cmdletsDir) -or -not (Test-Path $modelsDir)) { $skipped.Add("$name (not built)"); continue } + + $log = & dotnet run --project $generator -c Release -- ` + -d $spec -o (Join-Path $env:TEMP "binding-oracle-$name") -n "Microsoft.Graph.PowerShell.$name.Client" ` + --api-version $ApiVersion --log-level Information 2>&1 + # A failed generation produces no diagnostics, which would make every unbound member look + # like an uncited omission - or worse, make a module with no cmdlets look clean. + if ($LASTEXITCODE -ne 0) { + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = '(generation)'; Kind = 'GENERATOR-FAILED'; Detail = "exit $LASTEXITCODE; diagnostics unusable" }) + continue + } + + # noun -> set of OpenAPI property names the classifier deliberately did not bind. + # The two diagnostics are parsed separately and validated against DIFFERENT vocabularies: a + # policy exclusion and an unsupported shape are different claims, and one accepted set would + # let "Excluded ...: Untyped" or "Unbound ...: ServerAssignedId" pass as a citation. + $accounted = @{} + foreach ($line in $log) { + $kind = $null + $m = [regex]::Match("$line", 'Excluded body property (?[^.]+)\.(?\S+): (?\S+)') + if ($m.Success) { $kind = 'Excluded'; $allowed = $knownPolicies } + else { + $m = [regex]::Match("$line", 'Unbound body property (?[^.]+)\.(?\S+): (?\S+)') + if ($m.Success) { $kind = 'Unbound'; $allowed = $knownShapes } + } + if (-not $kind) { continue } + + $reason = $m.Groups['reason'].Value.Trim() + if ($reason -notin $allowed) { + $expected = if ($kind -eq 'Excluded') { 'an ExclusionPolicy' } else { 'an UnsupportedShape' } + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = '(diagnostics)'; Kind = 'UNCITED'; Detail = "$kind $($m.Groups['prop'].Value) cited '$reason'; expected $expected" }) + continue + } + $noun = $m.Groups['noun'].Value + if (-not $accounted.ContainsKey($noun)) { $accounted[$noun] = [System.Collections.Generic.HashSet[string]]::new() } + [void]$accounted[$noun].Add($m.Groups['prop'].Value) + } + + foreach ($file in Get-ChildItem $cmdletsDir -Filter '*.g.cs' -File) { + if ($file.Name -notmatch '^(New|Update)Mg') { continue } + $text = Get-Content $file.FullName -Raw + + # -match populates $Matches; -notmatch does not, so each pattern is matched explicitly. + $bodyMatch = [regex]::Match($text, 'var body = new ([A-Za-z0-9_.]+)\(\);') + if (-not $bodyMatch.Success) { continue } + $entityType = $bodyMatch.Groups[1].Value + # Keep the sub-namespace: "...Models.CallRecords.Participant" -> "CallRecords.Participant". + $modelsMarker = '.Models.' + $markerAt = $entityType.IndexOf($modelsMarker) + $relativeName = if ($markerAt -ge 0) { $entityType.Substring($markerAt + $modelsMarker.Length) } else { $entityType } + $simpleName = $relativeName.Substring($relativeName.LastIndexOf('.') + 1) + + $members = Get-ModelMembers $modelsDir $relativeName $null + if ($null -eq $members) { + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'NO-MODEL'; Detail = "no generated model file for $relativeName" }) + continue + } + + # The noun is used verbatim: the generator's diagnostics key on the same prefixed noun + # (MgUserMailFolder), so stripping the prefix here would match nothing and report every + # policy exclusion as an omission. + $cmdletMatch = [regex]::Match($text, '\[Cmdlet\(Verbs\w+\.(\w+),\s*"([^"]+)"') + if (-not $cmdletMatch.Success) { continue } + $noun = $cmdletMatch.Groups[2].Value + + # emitted parameters and the member each one assigns + $parameters = @([regex]::Matches($text, '(?m)^\s+public\s+[^\r\n]+?\s+(\w+)\s*\{\s*get;\s*set;\s*\}') | + ForEach-Object { $_.Groups[1].Value }) + # A schema-less property is assigned in two steps, so the right-hand side of the + # assignment is a local rather than the parameter: + # var untypedX = UntypedValue.From(X); + # if (untypedX is not null) body.X = untypedX; + # Mapping the local back to its parameter keeps the assignment attributable; without it + # the parameter looks unassigned and the local looks undeclared. + $localToParam = @{} + foreach ($c in [regex]::Matches($text, 'var\s+(?\w+)\s*=\s*UntypedValue\.From\((?\w+)\)')) { + $localToParam[$c.Groups['local'].Value] = $c.Groups['param'].Value + } + + $assignments = @{} + $assignedParams = [System.Collections.Generic.HashSet[string]]::new() + foreach ($a in [regex]::Matches($text, '(?m)^\s+body\.(?\w+)\s*=\s*(?\w+)')) { + $member = $a.Groups['member'].Value + $source = $a.Groups['param'].Value + if ($localToParam.ContainsKey($source)) { $source = $localToParam[$source] } + if ($assignments.ContainsKey($member)) { + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'DUPLICATE'; Detail = "two parameters assign body.$member" }) + } + $assignments[$member] = $source + [void]$assignedParams.Add($source) + } + + # A parameter that never reaches the body is silently inert: it binds, the user supplies + # a value, and the request goes out without it. The compiler is perfectly happy with it. + # Parameters that legitimately do not assign are identified by how they are emitted, not + # by a list of names: a path id carries an "= string.Empty" initializer, a header param + # is added to requestConfiguration.Headers, and AccessToken/Headers are the shared + # plumbing every cmdlet declares. + $pathParams = @([regex]::Matches($text, '(?m)^\s+public\s+string\s+(\w+)\s*\{\s*get;\s*set;\s*\}\s*=\s*string\.Empty;') | + ForEach-Object { $_.Groups[1].Value }) + $headerParams = @([regex]::Matches($text, 'requestConfiguration\.Headers\.Add\("[^"]*",\s*(\w+)!') | + ForEach-Object { $_.Groups[1].Value }) + $nonBody = [System.Collections.Generic.HashSet[string]]::new([string[]](@('AccessToken', 'Headers') + $pathParams + $headerParams)) + foreach ($p in $parameters) { + if ($assignedParams.Contains($p) -or $nonBody.Contains($p)) { continue } + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'NO-ASSIGNMENT'; Detail = "-$p is declared but never assigned to the body" }) + } + + foreach ($member in $assignments.Keys) { + if (-not $members.ContainsKey($member)) { + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'WRONG-TARGET'; Detail = "body.$member is not a member of $simpleName" }) + } + elseif ($assignments[$member] -notin $parameters) { + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'NO-PARAMETER'; Detail = "body.$member reads undeclared $($assignments[$member])" }) + } + } + + # every settable member must be assigned, or named in the classifier's diagnostics + # Plain assignment, not an if-expression: an empty HashSet returned through the pipeline + # enumerates to nothing and the variable lands as $null. + $cited = [System.Collections.Generic.HashSet[string]]::new() + if ($accounted.ContainsKey($noun)) { $cited = $accounted[$noun] } + foreach ($member in $members.Keys) { + if ($assignments.ContainsKey($member)) { continue } + if ($cited.Contains($members[$member])) { continue } + $failures.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Kind = 'MISSING'; Detail = "$simpleName.$member ('$($members[$member])') has no parameter and no cited policy" }) + } + + $results.Add([pscustomobject]@{ Module = $name; Cmdlet = $file.BaseName; Members = $members.Count; Assigned = $assignments.Count }) + } + Write-Host ("{0,-32} cmdlets {1,5} members {2,6} assigned {3,6}" -f $name, + @($results | Where-Object Module -eq $name).Count, + (($results | Where-Object Module -eq $name | Measure-Object Members -Sum).Sum), + (($results | Where-Object Module -eq $name | Measure-Object Assigned -Sum).Sum)) +} + +if ($results.Count -eq 0) { Write-Error 'No New/Update cmdlets were examined; the oracle proved nothing.'; exit 2 } + +$examinedModules = @($results | Select-Object -ExpandProperty Module -Unique) +# A module with no New/Update cmdlets has nothing for this oracle to check. Naming those +# explicitly keeps the population reconcilable without the reader subtracting two numbers. +# Compared against an explicit name list: a -match against an empty collection yields an empty +# array, which is falsy, so the filter would silently select nothing. +$skippedNames = @($skipped | ForEach-Object { ($_ -split ' ')[0] }) +$noBodyCmdlets = @($Module | Where-Object { $_ -notin $examinedModules -and $_ -notin $skippedNames }) +"" +"modules requested : $($Module.Count)" +"modules examined : $($examinedModules.Count)" +"modules with no New/Update: $($noBodyCmdlets.Count)$(if ($noBodyCmdlets.Count) { " -> $($noBodyCmdlets -join ', ')" })" +"modules skipped : $($skipped.Count)$(if ($skipped.Count) { " -> $($skipped -join ', ')" })" +"cmdlets examined : $($results.Count)" +"model members : $(($results | Measure-Object Members -Sum).Sum)" +"bound by a param : $(($results | Measure-Object Assigned -Sum).Sum)" +"failures : $($failures.Count)" +if ($failures.Count -gt 0) { + "" + $failures | Group-Object Kind | Sort-Object Count -Descending | Select-Object Count, Name | Format-Table -AutoSize | Out-String -Width 60 + $failures | Select-Object -First 25 | Format-Table Module, Cmdlet, Kind, Detail -AutoSize | Out-String -Width 200 + exit 1 +} +"binding coverage verified: every settable member is bound or cited." +exit 0 diff --git a/tools/Test-WrapperModule.ps1 b/tools/Test-WrapperModule.ps1 index 7460f4f2c6..0a19dd4154 100644 --- a/tools/Test-WrapperModule.ps1 +++ b/tools/Test-WrapperModule.ps1 @@ -8,6 +8,12 @@ Each module is tested in a CHILD pwsh process — a fresh process per module, be assemblies cannot be unloaded and Import-Module silently no-ops when a same-name module is already loaded. Checks, per module: + 0. the binary is not stale - the dll is compared against every compiled + input under src (the kiota client in Client/ + as well as Cmdlets/ and the csproj) and a + binary older than any of them is refused, + because every check below would pass against + a module built before the change under test 1. Import-Module succeeds - the user's first experience 2. exported cmdlet count == manifest count - nothing silently dropped at load 3. no orphan workers - every *_Get/*_List worker has its public @@ -16,8 +22,22 @@ already loaded. Checks, per module: PASS = NoGraphSession error (the call flowed dispatcher -> worker -> auth path) FAIL = CommandNotFound (dispatcher->worker forwarding broken: the manifest visibility trap) or any other unexpected error id + 5. each bound shape accepts the value a person would actually type, asserted against the + real compiled types rather than assumed: + complex - a model-typed parameter accepts a hashtable + enum - an enum-typed parameter accepts its own member name as a string + scalar - DateTimeOffset/Guid accept a string; kiota's Date/Time accept a [datetime] + (they have NO string conversion), reported as OK(n) where n is how many + cases the module actually exercised, so an empty pass is visible + untyped - 19 cases run through the module's OWN compiled UntypedValue helper, reached + by reflection so this gate cannot drift from a copy of the converter: every + numeric type, string, boolean, PSObject unwrapping, object, array, nesting, + nested-null drop, null-element drop, empty-object omission, and the throw on + an unsupported type. The helper is emitted into every module, so a missing + helper is a failure, never n/a Modules with no paired list+item GETs have no dispatcher; check 4 reports n/a for them. +A shape a module never binds reports n/a for that part of check 5, except untyped. .PARAMETER Module One or more module names previously built by Build-WrapperModule.ps1. @@ -43,6 +63,10 @@ $ErrorActionPreference = 'Stop' $repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path if (-not $OutputRoot) { $OutputRoot = Join-Path $repoRoot 'artifacts\wrapper-modules' } +# The psd1 path is embedded in a script run by a CHILD process with its own working directory, +# so a relative -OutputRoot would resolve there and Import-Module would fail with a confusing +# "module not found" rather than a path error. +$OutputRoot = (Resolve-Path -LiteralPath $OutputRoot).Path function Test-OneModule { param([string]$Name) @@ -51,13 +75,37 @@ function Test-OneModule { $psd1 = Join-Path $OutputRoot "$Name\src\bin\$Configuration\net10.0\$moduleName.psd1" $result = [pscustomobject]@{ Module = $Name; Pass = $false; Exported = 0; ManifestCount = 0 - OrphanWorkers = 0; Dispatcher = ''; ErrorId = ''; Detail = '' + OrphanWorkers = 0; Dispatcher = ''; ErrorId = ''; ComplexBinding = ''; EnumBinding = ''; ScalarBinding = '' + UntypedBinding = ''; Detail = '' } if (-not (Test-Path $psd1)) { $result.Detail = "not built: $psd1 missing (run Build-WrapperModule.ps1 first)" return $result } + + # A binary older than the sources it was built from passes every check below while proving + # nothing about the current generator. This gate loads whatever is on disk, so staleness is + # invisible unless it is refused here: the build and test defaults can drift apart, and a + # module last built under a different configuration is silently days old. + # + # Every compiled input counts, not just the cmdlets. A module is emitted sources plus the + # kiota client under Client/, and a regenerated client with an unchanged cmdlet is exactly + # the case where a parameter's CLR type moves out from under the assignment - so watching + # Cmdlets/ alone would miss the change most likely to invalidate a runtime result. + $dll = Join-Path $OutputRoot "$Name\src\bin\$Configuration\net10.0\$moduleName.dll" + $inputs = @(Get-ChildItem -Path (Join-Path $OutputRoot "$Name\src") -Recurse -File -Include *.cs, *.csproj -ErrorAction SilentlyContinue | + Where-Object { $_.FullName -notmatch '\\(bin|obj)\\' }) + if ((Test-Path $dll) -and $inputs) { + $newest = ($inputs | Sort-Object LastWriteTimeUtc -Descending | Select-Object -First 1) + $builtAt = (Get-Item $dll).LastWriteTimeUtc + if ($builtAt -lt $newest.LastWriteTimeUtc) { + $rel = $newest.FullName.Substring((Join-Path $OutputRoot "$Name\src").Length).TrimStart('\') + $result.Detail = "stale binary: $Configuration dll built $($builtAt.ToString('MM-dd HH:mm')) predates $rel ($($newest.LastWriteTimeUtc.ToString('MM-dd HH:mm'))); rebuild with -Configuration $Configuration" + return $result + } + } + $result.ManifestCount = (Import-PowerShellDataFile -Path $psd1).CmdletsToExport.Count # The child prints exactly one JSON line; everything else it may write is noise. @@ -83,11 +131,166 @@ if (`$dispatcher) { `$errorId = `$_.FullyQualifiedErrorId } } +# A model-typed parameter must accept a hashtable: that conversion is what makes +# -PasswordProfile @{ Password = '...' } work. Find one on any New-/Update- cmdlet and +# convert an empty hashtable to it; failure means typed binding is unusable from the shell. +`$complexBinding = 'N/A' +# Must be a model CLASS, not an enum: referenced enums are also .Client.Models.* types, and +# converting a hashtable to one is meaningless. Enums are covered by their own case below. +`$typed = `$cmds | + Where-Object { `$_.Name -like 'New-*' -or `$_.Name -like 'Update-*' } | + ForEach-Object { `$_.Parameters.Values } | + Where-Object { + `$_.ParameterType.FullName -like '*.Client.Models.*' -and -not `$_.ParameterType.IsArray -and + -not `$_.ParameterType.IsEnum -and -not ([System.Nullable]::GetUnderlyingType(`$_.ParameterType)) + } | + Select-Object -First 1 +if (`$typed) { + try { + `$converted = [System.Management.Automation.LanguagePrimitives]::ConvertTo(@{}, `$typed.ParameterType) + `$complexBinding = if (`$converted -and `$converted.GetType() -eq `$typed.ParameterType) { 'OK' } else { 'WRONG-TYPE' } + } + catch { + `$complexBinding = "FAILED: `$(`$_.Exception.Message)" + } +} + +# A referenced enum binds from the string a person would type. +`$enumBinding = 'N/A' +`$enumParam = `$cmds | + Where-Object { `$_.Name -like 'New-*' -or `$_.Name -like 'Update-*' } | + ForEach-Object { `$_.Parameters.Values } | + Where-Object { + `$u = [System.Nullable]::GetUnderlyingType(`$_.ParameterType) + `$u -and `$u.IsEnum + } | Select-Object -First 1 +if (`$enumParam) { + `$u = [System.Nullable]::GetUnderlyingType(`$enumParam.ParameterType) + `$sample = ([enum]::GetNames(`$u) | Select-Object -First 1) + try { + `$v = [System.Management.Automation.LanguagePrimitives]::ConvertTo(`$sample, `$enumParam.ParameterType) + `$enumBinding = if ("`$v" -eq `$sample) { 'OK' } else { "WRONG-VALUE(`$v)" } + } + catch { `$enumBinding = "FAILED: -`$(`$enumParam.Name)" } +} +# Scalar shapes are bound from a value a person would plausibly type. Kiota's Date and Time are +# the sharp edge: they are structs with no string conversion, so they take a [datetime] +# (what Get-Date returns) and binding a string fails. Pinning that here keeps the documented +# input contract honest - the parameter compiles either way, so only a runtime check can tell. +`$scalarBinding = 'N/A' +`$scalarCases = @{ + 'System.DateTimeOffset' = '2001-04-05T00:00:00Z' + 'System.Guid' = '00000000-0000-0000-0000-000000000000' + 'Microsoft.Kiota.Abstractions.Date' = [datetime]'2026-12-31' + 'Microsoft.Kiota.Abstractions.Time' = [datetime]'2026-12-31T14:30:00' +} +`$bodyParams = @(`$cmds | + Where-Object { `$_.Name -like 'New-*' -or `$_.Name -like 'Update-*' } | + ForEach-Object { `$_.Parameters.Values }) +`$scalarFailures = @() +`$scalarExercised = 0 +foreach (`$typeName in `$scalarCases.Keys) { + `$p = `$bodyParams | Where-Object { + `$u = [System.Nullable]::GetUnderlyingType(`$_.ParameterType) + `$u -and `$u.FullName -eq `$typeName + } | Select-Object -First 1 + if (-not `$p) { continue } + `$scalarExercised++ + try { + `$null = [System.Management.Automation.LanguagePrimitives]::ConvertTo(`$scalarCases[`$typeName], `$p.ParameterType) + } + catch { + `$scalarFailures += "-`$(`$p.Name) rejects `$typeName input" + } +} +# Reporting OK when no case matched would be a pass that proves nothing, so the count of +# cases actually exercised is carried in the result instead of being assumed. +if (`$scalarFailures) { `$scalarBinding = "FAILED: `$(`$scalarFailures -join ', ')" } +elseif (`$scalarExercised -gt 0) { `$scalarBinding = "OK(`$scalarExercised)" } + +# The schema-less converter is the one piece of emitted logic no compiler can check: every +# branch produces a UntypedNode, so a wrong branch sends a value the caller never wrote and +# still builds. The matrix runs the compiled helper inside the module under test - reached by +# reflection because it is internal - so it cannot drift from a copy kept in this script. +# UntypedValue is emitted into every module, so a module that cannot produce it is a failure, +# never N/A. +`$untypedBinding = 'NOT-FOUND' +`$untypedType = `$null +try { + `$impl = `$cmds | Where-Object { `$_.CommandType -eq 'Cmdlet' } | Select-Object -First 1 + `$untypedType = @(`$impl.ImplementingType.Assembly.GetTypes() | + Where-Object { `$_.Name -eq 'UntypedValue' })[0] +} +catch { `$untypedType = `$null } +if (`$untypedType) { + `$from = `$untypedType.GetMethod('From', [Reflection.BindingFlags]'Public,Static') + # Expect: node type name; '' means the property is omitted; 'THROW' means refused. + `$untypedCases = @( + @{ N = 'string'; V = 'hello'; T = 'UntypedString'; Val = 'hello' } + @{ N = 'boolean'; V = `$true; T = 'UntypedBoolean'; Val = 'True' } + @{ N = 'int32'; V = [int]42; T = 'UntypedInteger'; Val = '42' } + @{ N = 'int64'; V = [long]9000000000; T = 'UntypedLong'; Val = '9000000000' } + @{ N = 'float'; V = [float]1.5; T = 'UntypedFloat'; Val = '1.5' } + @{ N = 'double'; V = [double]2.5; T = 'UntypedDouble'; Val = '2.5' } + @{ N = 'decimal'; V = [decimal]3.5; T = 'UntypedDecimal'; Val = '3.5' } + @{ N = 'unsigned byte'; V = [byte]7; T = 'UntypedInteger'; Val = '7' } + @{ N = 'unsigned int'; V = [uint32]8; T = 'UntypedInteger'; Val = '8' } + @{ N = 'PSObject wrapper unwrapped'; V = [psobject]::AsPSObject('wrapped'); T = 'UntypedString'; Val = 'wrapped' } + @{ N = 'hashtable'; V = @{ a = 'x' }; T = 'UntypedObject'; Count = 1 } + @{ N = 'nested hashtable'; V = @{ o = @{ i = 'x' } }; T = 'UntypedObject'; Count = 1 } + @{ N = 'array'; V = @(1, 2); T = 'UntypedArray'; Count = 2 } + @{ N = 'null omitted'; V = `$null; T = '' } + @{ N = 'empty object omitted'; V = @{}; T = '' } + @{ N = 'all-null object omitted'; V = @{ a = `$null }; T = '' } + @{ N = 'nested null dropped, sibling kept'; V = @{ a = 'x'; b = `$null }; T = 'UntypedObject'; Count = 1 } + @{ N = 'null array element dropped'; V = @(1, `$null, 2); T = 'UntypedArray'; Count = 2 } + @{ N = 'unsupported type refused'; V = { 1 }; T = 'THROW' } + ) + `$untypedFailures = @() + foreach (`$case in `$untypedCases) { + # A one-element object[] built by hand: @(`$v) unrolls an array argument into the wrong arity. + `$callArgs = New-Object object[] 1 + `$callArgs[0] = `$case.V + `$threw = `$false + `$node = `$null + try { `$node = `$from.Invoke(`$null, `$callArgs) } + catch { `$threw = `$true } + + if (`$case.T -eq 'THROW') { + if (-not `$threw) { `$untypedFailures += "`$(`$case.N): accepted" } + continue + } + if (`$threw) { `$untypedFailures += "`$(`$case.N): threw"; continue } + if (`$case.T -eq '') { + if (`$null -ne `$node) { `$untypedFailures += "`$(`$case.N): sent `$(`$node.GetType().Name)" } + continue + } + if (`$null -eq `$node) { `$untypedFailures += "`$(`$case.N): omitted"; continue } + if (`$node.GetType().Name -ne `$case.T) { + `$untypedFailures += "`$(`$case.N): `$(`$node.GetType().Name) not `$(`$case.T)" + continue + } + if (`$case.ContainsKey('Val') -and "`$(`$node.GetValue())" -ne `$case.Val) { + `$untypedFailures += "`$(`$case.N): value `$(`$node.GetValue()) not `$(`$case.Val)" + } + if (`$case.ContainsKey('Count')) { + `$actual = @(`$node.GetValue()).Count + if (`$actual -ne `$case.Count) { `$untypedFailures += "`$(`$case.N): `$actual members not `$(`$case.Count)" } + } + } + `$untypedBinding = if (`$untypedFailures) { "FAILED: `$(`$untypedFailures -join '; ')" } + else { "OK(`$(`$untypedCases.Count))" } +} + [pscustomobject]@{ Exported = `$cmds.Count OrphanWorkers = `$orphans.Count Dispatcher = if (`$dispatcher) { `$dispatcher.Name } else { '' } ErrorId = `$errorId + ComplexBinding = `$complexBinding + EnumBinding = `$enumBinding + ScalarBinding = `$scalarBinding + UntypedBinding = `$untypedBinding } | ConvertTo-Json -Compress "@ @@ -106,6 +309,10 @@ if (`$dispatcher) { $result.OrphanWorkers = $r.OrphanWorkers $result.Dispatcher = $r.Dispatcher $result.ErrorId = $r.ErrorId + $result.ComplexBinding = $r.ComplexBinding + $result.EnumBinding = $r.EnumBinding + $result.ScalarBinding = $r.ScalarBinding + $result.UntypedBinding = $r.UntypedBinding if ($r.Exported -ne $result.ManifestCount) { $result.Detail = "exported $($r.Exported) != manifest $($result.ManifestCount)" @@ -120,6 +327,20 @@ if (`$dispatcher) { "unexpected error id: $($r.ErrorId)" } } + elseif ($r.ComplexBinding -notin @('OK', 'N/A')) { + $result.Detail = "complex parameter does not accept a hashtable: $($r.ComplexBinding)" + } + elseif ($r.EnumBinding -notin @('OK', 'N/A')) { + $result.Detail = "enum parameter does not accept its own member name: $($r.EnumBinding)" + } + elseif ($r.ScalarBinding -ne 'N/A' -and $r.ScalarBinding -notlike 'OK(*') { + $result.Detail = "scalar parameter rejects its documented input: $($r.ScalarBinding)" + } + # No N/A escape: UntypedValue is emitted into every module, so a missing helper or a zero + # count is a failure rather than a module that had nothing to test. + elseif ($r.UntypedBinding -notlike 'OK(*') { + $result.Detail = "schema-less conversion is wrong: $($r.UntypedBinding)" + } else { $result.Pass = $true } @@ -130,7 +351,7 @@ $results = foreach ($name in $Module) { Write-Host "=== $name ===" -ForegroundColor Cyan $r = Test-OneModule -Name $name if ($r.Pass) { - Write-Host " PASS: $($r.Exported) cmdlets; dispatcher $($r.Dispatcher) -> $($r.ErrorId)" -ForegroundColor Green + Write-Host " PASS: $($r.Exported) cmdlets; dispatcher $($r.Dispatcher) -> $($r.ErrorId); complex $($r.ComplexBinding); enum $($r.EnumBinding); scalar $($r.ScalarBinding); untyped $($r.UntypedBinding)" -ForegroundColor Green } else { Write-Host " FAIL: $($r.Detail)" -ForegroundColor Yellow diff --git a/tools/WrapperGenerator.Tests/EmitterTests.cs b/tools/WrapperGenerator.Tests/EmitterTests.cs index 24e8dc174b..fa906fe73d 100644 --- a/tools/WrapperGenerator.Tests/EmitterTests.cs +++ b/tools/WrapperGenerator.Tests/EmitterTests.cs @@ -32,17 +32,49 @@ public void DispatcherRethrowsTheWorkersOriginalErrorRecord() public void EmitsSuffixedParameterButAssignsRealModelProperty() { var naming = Naming.Resolve(new OperationInfo(HttpMethod.Patch, "/devices/{device-id}")); - var properties = SchemaProperties.ResolveParameterNameCollisions( + var (properties, _, _) = SchemaProperties.ResolveParameterNameCollisions( new[] { new CmdletProperty("deviceId", "DeviceId", "string", IsArray: false) }, + [], [], naming.PathParamNames); - var source = CmdletEmitter.EmitUpdate(naming, new EmitContext("Test.Client"), "Device", properties, hasPasswordProfile: false); + var source = CmdletEmitter.EmitUpdate(naming, new EmitContext("Test.Client"), "Device", properties, [], []); Assert.Contains("public string? DeviceId1 { get; set; }", source); Assert.Contains("body.DeviceId = DeviceId1;", source); Assert.Contains("IsParameterBound(nameof(DeviceId1))", source); } + // A complex body property binds as its kiota model type, fully qualified, and assigns + // straight to the model property. This is what lets a caller write + // New-MgUser -PasswordProfile @{ Password = '...' } - PowerShell converts the hashtable + // to the model on binding. Arrays land as T[] and convert with ToList() like scalar arrays. + [Fact] + public void EmitsComplexPropertyAsTypedModelParameter() + { + var naming = Naming.Resolve(new OperationInfo(HttpMethod.Post, "/users")); + var complex = new[] + { + new ComplexParameter("PasswordProfile", "PasswordProfile", "Test.Client.Models.PasswordProfile", IsArray: false, IsEnum: false), + new ComplexParameter("AssignedLicenses", "AssignedLicenses", "Test.Client.Models.AssignedLicense", IsArray: true, IsEnum: false), + // An enum collection needs nullable elements to assign to kiota's List. + new ComplexParameter("Roles", "Roles", "Test.Client.Models.RoleType", IsArray: true, IsEnum: true), + }; + + var source = CmdletEmitter.EmitNew(naming, new EmitContext("Test.Client"), "User", [], complex, []); + + Assert.Contains("public Test.Client.Models.PasswordProfile? PasswordProfile { get; set; }", source); + Assert.Contains("body.PasswordProfile = PasswordProfile;", source); + + Assert.Contains("public Test.Client.Models.AssignedLicense[]? AssignedLicenses { get; set; }", source); + Assert.Contains("body.AssignedLicenses = AssignedLicenses!.ToList();", source); + + Assert.Contains("public Test.Client.Models.RoleType?[]? Roles { get; set; }", source); + + // The removed hard-coded special case must not come back in any form. + Assert.DoesNotContain("ForceChangePasswordNextSignIn", source); + Assert.DoesNotContain("new PasswordProfile", source); + } + // PATCH-only resources (/places/{id}) have no GetAsync on their kiota builder, so the // 204 re-fetch must be emitted only when the path has a GET (found by compiling the // Calendar module). Without the re-fetch, a bodiless 204 writes nothing — same as the @@ -54,11 +86,11 @@ public void UpdateEmitsReFetchOnlyWhenPathHasGet() var props = new[] { new CmdletProperty("displayName", "DisplayName", "string", IsArray: false) }; var ctx = new EmitContext("Test.Client"); - var withGet = CmdletEmitter.EmitUpdate(naming, ctx, "Place", props, hasPasswordProfile: false, reFetchAfterUpdate: true); + var withGet = CmdletEmitter.EmitUpdate(naming, ctx, "Place", props, [], [], reFetchAfterUpdate: true); Assert.Contains("re-fetching the updated resource", withGet); Assert.Contains(".GetAsync()", withGet); - var withoutGet = CmdletEmitter.EmitUpdate(naming, ctx, "Place", props, hasPasswordProfile: false, reFetchAfterUpdate: false); + var withoutGet = CmdletEmitter.EmitUpdate(naming, ctx, "Place", props, [], [], reFetchAfterUpdate: false); Assert.DoesNotContain("re-fetching the updated resource", withoutGet); Assert.DoesNotContain(".GetAsync()", withoutGet); Assert.Contains("if (result is not null)", withoutGet); diff --git a/tools/WrapperGenerator.Tests/NamingTests.cs b/tools/WrapperGenerator.Tests/NamingTests.cs index 3e8c8ebca6..a065649b85 100644 --- a/tools/WrapperGenerator.Tests/NamingTests.cs +++ b/tools/WrapperGenerator.Tests/NamingTests.cs @@ -27,7 +27,7 @@ public sealed class SingularizerTests // "Whois" also hits the is-guard — a deliberate correction, not a parity pin: the SDK // ships Get-MgSecurityThreatIntelligenceHostWhoi (AutoRest inflected the trailing // "whois" segment) while its 28 whoisRecords/whoisHistoryRecords siblings keep "Whois". - // See edge-cases/naming-edge-cases.md. + // See docs/edge-cases/naming-edge-cases.md. [InlineData("Whois", "Whois")] // plain s [InlineData("Messages", "Message")] @@ -140,7 +140,7 @@ public void ResolvesPublishedSdkNames(string method, string path, string expecte [Theory] // Deliberate corrections: the published name is wrong (an AutoRest naming defect) and the // generator emits the corrected name instead of reproducing it. Every entry here must have - // an edge-cases/naming-edge-cases.md entry and a matching row in + // an docs/edge-cases/naming-edge-cases.md entry and a matching row in // Compare-WrapperCmdletNames.ps1's $deliberateCorrections table, so the parity gate // reports it as [CORRECTED], not a failure. // Shipped: Get-MgSecurityThreatIntelligenceHostWhoi — the only whois-family cmdlet (of 30) diff --git a/tools/WrapperGenerator.Tests/SchemaPropertiesTests.cs b/tools/WrapperGenerator.Tests/SchemaPropertiesTests.cs index 9d70471981..5adc167c64 100644 --- a/tools/WrapperGenerator.Tests/SchemaPropertiesTests.cs +++ b/tools/WrapperGenerator.Tests/SchemaPropertiesTests.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; +using System.Text.Json.Nodes; using Microsoft.OpenApi; using WrapperGenerator; using Xunit; @@ -8,26 +10,59 @@ namespace WrapperGenerator.Tests; public sealed class SchemaPropertiesTests { + private static OpenApiSchema Scalar(JsonSchemaType type, bool readOnly = false, string? format = null) => + new() { Type = type, ReadOnly = readOnly, Format = format }; + + // Component schemas the tests' $refs point at. Classification resolves a reference before + // deciding what it is, so the target's shape is what matters, not the reference itself. + private static readonly Dictionary Components = new(StringComparer.Ordinal) + { + ["graph.passwordProfile"] = new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = new Dictionary { ["password"] = Scalar(JsonSchemaType.String) }, + }, + ["graph.assignedLicense"] = new OpenApiSchema { Type = JsonSchemaType.Object }, + ["graph.importance"] = new OpenApiSchema { Type = JsonSchemaType.String, Enum = [new System.Text.Json.Nodes.JsonArray()] }, + // Graph's marker for "this numeric may also arrive as INF/-INF/NaN". The VALUES are what + // identify the encoding, so they are real here rather than a placeholder. + ["graph.referenceNumeric"] = new OpenApiSchema + { + Type = JsonSchemaType.String, + Enum = [JsonValue.Create("-INF")!, JsonValue.Create("INF")!, JsonValue.Create("NaN")!], + }, + // A string enum that is NOT the sentinel set: a meaningful alternative, not an encoding. + ["graph.currency"] = new OpenApiSchema + { + Type = JsonSchemaType.String, + Enum = [JsonValue.Create("usd")!, JsonValue.Create("eur")!], + }, + }; + + private static IOpenApiSchema? Resolve(string id) => Components.TryGetValue(id, out var s) ? s : null; + + private static BodyProperties ClassifyBody(Dictionary properties, params string[] required) => + SchemaProperties.Classify( + new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = properties, + Required = new HashSet(required, StringComparer.Ordinal), + }, + Resolve); + // Kiota strips underscores when naming model members: signIn's "riskEventTypes_v2" // becomes RiskEventTypesV2 (verified against a generated SignIn model). The body // assignment targets that member, so extraction must produce the same name. [Fact] public void MapsUnderscorePropertyNamesTheWayKiotaDoes() { - var schema = new OpenApiSchema + var classified = ClassifyBody(new Dictionary { - Type = JsonSchemaType.Object, - Properties = new Dictionary - { - ["riskEventTypes_v2"] = new OpenApiSchema - { - Type = JsonSchemaType.Array, - Items = new OpenApiSchema { Type = JsonSchemaType.String }, - }, - }, - }; + ["riskEventTypes_v2"] = new OpenApiSchema { Type = JsonSchemaType.Array, Items = Scalar(JsonSchemaType.String) }, + }); - var property = Assert.Single(SchemaProperties.ExtractPrimitiveProperties(schema)); + var property = Assert.Single(classified.Scalars); Assert.Equal("RiskEventTypesV2", property.PascalName); Assert.Equal("riskEventTypes_v2", property.OpenApiName); } @@ -39,13 +74,13 @@ public void MapsUnderscorePropertyNamesTheWayKiotaDoes() [Fact] public void SuffixesBodyPropertyThatCollidesWithPathParameter() { - var properties = new[] + var scalars = new[] { new CmdletProperty("deviceId", "DeviceId", "string", IsArray: false), new CmdletProperty("displayName", "DisplayName", "string", IsArray: false), }; - var resolved = SchemaProperties.ResolveParameterNameCollisions(properties, new[] { "DeviceId" }); + var (resolved, _, _) = SchemaProperties.ResolveParameterNameCollisions(scalars, [], [], ["DeviceId"]); var renamed = Assert.Single(resolved, p => p.OpenApiName == "deviceId"); Assert.Equal("DeviceId1", renamed.ParameterName); @@ -55,121 +90,423 @@ public void SuffixesBodyPropertyThatCollidesWithPathParameter() Assert.Equal("DisplayName", untouched.ParameterName); } - private static OpenApiSchema Scalar(JsonSchemaType type, bool readOnly = false, string? format = null) => - new() { Type = type, ReadOnly = readOnly, Format = format }; + // Scalars and complex properties share one C# property namespace on the emitted class, so + // a complex property must not be handed a name a scalar (or a path id) already took. + [Fact] + public void ResolvesCollisionsAcrossScalarAndComplexProperties() + { + var scalars = new[] { new CmdletProperty("photo", "Photo", "string", IsArray: false) }; + var complex = new[] { new ComplexProperty("photo2", "Photo", "graph.passwordProfile", IsArray: false, IsEnum: false) }; + + var (resolvedScalars, resolvedComplex, _) = SchemaProperties.ResolveParameterNameCollisions(scalars, complex, [], []); + + Assert.Equal("Photo", resolvedScalars[0].ParameterName); + Assert.Equal("Photo1", resolvedComplex[0].ParameterName); + Assert.Equal("Photo", resolvedComplex[0].PascalName); + } [Fact] - public void KeepsPrimitivesAndPrimitiveArrays_ExcludesServerManagedAndComplex() + public void KeepsPrimitivesAndPrimitiveArrays_ExcludesServerManagedAndNavigation() { - var body = new OpenApiSchema + var classified = ClassifyBody(new Dictionary { - Type = JsonSchemaType.Object, - Properties = new Dictionary + // bound + ["displayName"] = Scalar(JsonSchemaType.String), + ["accountEnabled"] = Scalar(JsonSchemaType.Boolean), + ["businessPhones"] = new OpenApiSchema { Type = JsonSchemaType.Array, Items = Scalar(JsonSchemaType.String) }, + // excluded + ["id"] = Scalar(JsonSchemaType.String), // server-assigned + ["@odata.type"] = Scalar(JsonSchemaType.String), // OData control data + ["createdDateTime"] = Scalar(JsonSchemaType.String, readOnly: true), // ReadOnly + ["manager"] = new OpenApiSchema // relationship, not a body field { - // kept - ["displayName"] = Scalar(JsonSchemaType.String), - ["accountEnabled"] = Scalar(JsonSchemaType.Boolean), - ["jobTitle"] = Scalar(JsonSchemaType.Integer), - ["businessPhones"] = new OpenApiSchema { Type = JsonSchemaType.Array, Items = Scalar(JsonSchemaType.String) }, - // excluded - ["id"] = Scalar(JsonSchemaType.String), // server-assigned - ["@odata.type"] = Scalar(JsonSchemaType.String), // @-prefixed OData control - ["createdDateTime"] = Scalar(JsonSchemaType.String, readOnly: true), // ReadOnly - ["assignedLicenses"] = new OpenApiSchema // nested complex - { - Type = JsonSchemaType.Object, - Properties = new Dictionary { ["skuId"] = Scalar(JsonSchemaType.String) }, - }, + Type = JsonSchemaType.Object, + Extensions = new Dictionary { ["x-ms-navigationProperty"] = new JsonNodeExtension(true) }, }, - }; - - var names = SchemaProperties.ExtractPrimitiveProperties(body).Select(p => p.OpenApiName).ToHashSet(); + }); + var names = classified.Scalars.Select(p => p.OpenApiName).ToHashSet(); Assert.Contains("displayName", names); Assert.Contains("accountEnabled", names); - Assert.Contains("jobTitle", names); Assert.Contains("businessPhones", names); - Assert.DoesNotContain("id", names); - Assert.DoesNotContain("@odata.type", names); - Assert.DoesNotContain("createdDateTime", names); - Assert.DoesNotContain("assignedLicenses", names); + Assert.Equal(4, classified.Excluded.Count); + Assert.Empty(classified.Complex); + Assert.Empty(classified.Unsupported); } + // The property that motivated typed binding: Graph writes a nullable complex property as + // anyOf[$ref, {type: object, nullable: true}], and it must bind to the referenced model. [Fact] - public void MapsScalarAndArrayShapes() + public void BindsNullableReferenceComposition() { - var body = new OpenApiSchema + var classified = ClassifyBody(new Dictionary { - Type = JsonSchemaType.Object, - Properties = new Dictionary + ["passwordProfile"] = new OpenApiSchema { - ["displayName"] = Scalar(JsonSchemaType.String), - ["businessPhones"] = new OpenApiSchema { Type = JsonSchemaType.Array, Items = Scalar(JsonSchemaType.String) }, + AnyOf = + [ + new OpenApiSchemaReference("graph.passwordProfile"), + new OpenApiSchema { Type = JsonSchemaType.Object }, + ], }, - }; + }, required: "passwordProfile"); - var props = SchemaProperties.ExtractPrimitiveProperties(body); + var complex = Assert.Single(classified.Complex); + Assert.Equal("passwordProfile", complex.OpenApiName); + Assert.Equal("PasswordProfile", complex.PascalName); + Assert.Equal("graph.passwordProfile", complex.ReferenceId); + Assert.False(complex.IsArray); + } - var scalar = props.Single(p => p.OpenApiName == "displayName"); - Assert.False(scalar.IsArray); - Assert.Equal("string", scalar.PsTypeName); - Assert.Equal("DisplayName", scalar.PascalName); + [Fact] + public void BindsDirectReferenceAndReferenceArray() + { + var classified = ClassifyBody(new Dictionary + { + ["passwordProfile"] = new OpenApiSchemaReference("graph.passwordProfile"), + ["assignedLicenses"] = new OpenApiSchema + { + Type = JsonSchemaType.Array, + Items = new OpenApiSchemaReference("graph.assignedLicense"), + }, + }); + + var single = Assert.Single(classified.Complex, p => p.OpenApiName == "passwordProfile"); + Assert.False(single.IsArray); - var array = props.Single(p => p.OpenApiName == "businessPhones"); + var array = Assert.Single(classified.Complex, p => p.OpenApiName == "assignedLicenses"); Assert.True(array.IsArray); - Assert.Equal("string[]", array.PsTypeName); - Assert.Equal("BusinessPhones", array.PascalName); + Assert.Equal("graph.assignedLicense", array.ReferenceId); } + // An enum reference binds like a model reference: kiota emits both as named types in the + // models namespace (Models/Importance.cs holds "public enum Importance"), so one path + // resolves both. PowerShell converts a string argument to the enum on binding. [Fact] - public void MapsNumericFormatsWithoutDataLoss() + public void BindsReferenceToEnumAsANamedType() { - var body = new OpenApiSchema + var classified = ClassifyBody(new Dictionary { - Type = JsonSchemaType.Object, - Properties = new Dictionary + ["importance"] = new OpenApiSchemaReference("graph.importance"), + }); + + Assert.Empty(classified.Unsupported); + var complex = Assert.Single(classified.Complex); + Assert.Equal("importance", complex.OpenApiName); + Assert.Equal("graph.importance", complex.ReferenceId); + } + + // Every mapping here was read off a generated Graph client; a wrong CLR name is a compile + // error in the module, so these are pinned rather than trusted to kiota's documentation. + [Theory] + [InlineData("date-time", "global::System.DateTimeOffset")] + [InlineData("uuid", "global::System.Guid")] + [InlineData("duration", "global::System.TimeSpan")] + [InlineData("date", "global::Microsoft.Kiota.Abstractions.Date")] + [InlineData("time", "global::Microsoft.Kiota.Abstractions.Time")] + [InlineData("base64url", "byte[]")] + [InlineData("binary", "byte[]")] + public void MapsFormattedStringsToTheTypeKiotaGenerates(string format, string expected) + { + var classified = ClassifyBody(new Dictionary + { + ["value"] = Scalar(JsonSchemaType.String, format: format), + }); + + Assert.Equal(expected, Assert.Single(classified.Scalars).PsTypeName); + } + + // An unrecognised format must be reported, never bound as plain string: kiota would have + // mapped it to some other CLR type and the assignment would not compile. + [Fact] + public void ReportsUnknownStringFormatRatherThanFallingBackToString() + { + var classified = ClassifyBody(new Dictionary + { + ["odd"] = Scalar(JsonSchemaType.String, format: "some-future-format"), + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.UnknownFormat, Assert.Single(classified.Unsupported).Shape); + } + + // uint8 generates as byte? (rgbColor.r/g/b). int16 has no short? anywhere in the generated + // clients, so kiota widens it to int and so must we. + [Theory] + [InlineData("uint8", JsonSchemaType.Integer, "byte")] + [InlineData("int16", JsonSchemaType.Integer, "int")] + public void MapsNarrowIntegerFormatsTheWayKiotaDoes(string format, JsonSchemaType type, string expected) + { + var classified = ClassifyBody(new Dictionary + { + ["n"] = Scalar(type, format: format), + }); + + Assert.Equal(expected, Assert.Single(classified.Scalars).PsTypeName); + } + + // Graph writes a numeric that may also carry OData's INF/NaN string as a three-way union. + // Kiota keeps the numeric (bookingService.price -> double?), so the numeric branch binds. + [Fact] + public void BindsTheNumericBranchOfGraphsInfinityUnion() + { + var classified = ClassifyBody(new Dictionary + { + ["price"] = new OpenApiSchema { - ["riskScore"] = Scalar(JsonSchemaType.Number), // fractions must survive - ["sizeInBytes"] = Scalar(JsonSchemaType.Integer, format: "int64"), // values > 2^31 must survive - ["retryCount"] = Scalar(JsonSchemaType.Integer, format: "int32"), - ["plainCount"] = Scalar(JsonSchemaType.Integer), - // Graph's docs declare Edm.Int32/Int64 as type "number" with the format carrying - // the real type (mailFolder.childFolderCount, messageRule.sequence). The format - // must win or the parameter type contradicts the Kiota model and won't compile. - ["childFolderCount"] = Scalar(JsonSchemaType.Number, format: "int32"), - ["quotaUsed"] = Scalar(JsonSchemaType.Number, format: "int64"), - ["confidence"] = Scalar(JsonSchemaType.Number, format: "float"), + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchema { Type = JsonSchemaType.String }, + new OpenApiSchemaReference("graph.referenceNumeric"), + ], }, - }; + }); - var props = SchemaProperties.ExtractPrimitiveProperties(body); + Assert.Empty(classified.Unsupported); + Assert.Equal("double", Assert.Single(classified.Scalars).PsTypeName); + } - Assert.Equal("double", props.Single(p => p.OpenApiName == "riskScore").PsTypeName); - Assert.Equal("long", props.Single(p => p.OpenApiName == "sizeInBytes").PsTypeName); - Assert.Equal("int", props.Single(p => p.OpenApiName == "retryCount").PsTypeName); - Assert.Equal("int", props.Single(p => p.OpenApiName == "plainCount").PsTypeName); - Assert.Equal("int", props.Single(p => p.OpenApiName == "childFolderCount").PsTypeName); - Assert.Equal("long", props.Single(p => p.OpenApiName == "quotaUsed").PsTypeName); - Assert.Equal("float", props.Single(p => p.OpenApiName == "confidence").PsTypeName); + // Without the sentinel enum, "number or string" is an ordinary union whose string arm means + // something. Binding the numeric would silently discard it, so the sentinel is required + // evidence that the string arm is only OData's non-finite encoding. + [Fact] + public void ReportsNumericAndPlainStringUnionWithNoSentinelEnum() + { + var classified = ClassifyBody(new Dictionary + { + ["amount"] = new OpenApiSchema + { + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchema { Type = JsonSchemaType.String }, + ], + }, + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); } + // A referenced string enum that is not the sentinel set is a real alternative too. [Fact] - public void HasPasswordProfile_DetectsDirectAndViaAllOf() + public void ReportsNumericUnionWhoseEnumIsNotTheSentinelSet() { - var withProfile = new OpenApiSchema + var classified = ClassifyBody(new Dictionary { - Properties = new Dictionary { ["passwordProfile"] = new OpenApiSchema { Type = JsonSchemaType.Object } }, + ["price"] = new OpenApiSchema + { + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchemaReference("graph.currency"), + ], + }, + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); + } + + // A numeric beside a MODEL is a real choice, not the INF encoding. Recognising only + // "exactly one numeric branch" would bind the numeric here and silently discard the model + // arm, so the whole structure has to match. + [Fact] + public void ReportsUnionOfANumericAndAModel() + { + var classified = ClassifyBody(new Dictionary + { + ["either"] = new OpenApiSchema + { + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchemaReference("graph.passwordProfile"), + ], + }, + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); + } + + // A numeric beside a formatted string is likewise not the INF encoding: the string arm + // carries its own CLR type rather than being a stringish alternative. + [Fact] + public void ReportsUnionOfANumericAndAFormattedString() + { + var classified = ClassifyBody(new Dictionary + { + ["either"] = new OpenApiSchema + { + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchema { Type = JsonSchemaType.String, Format = "date-time" }, + ], + }, + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); + } + + // Two numeric branches is a real choice, not the INF encoding: binding one would silently + // pick a type for the caller. + [Fact] + public void ReportsUnionWithMoreThanOneNumericBranch() + { + var classified = ClassifyBody(new Dictionary + { + ["ambiguous"] = new OpenApiSchema + { + OneOf = + [ + new OpenApiSchema { Type = JsonSchemaType.Number, Format = "double" }, + new OpenApiSchema { Type = JsonSchemaType.Integer, Format = "int64" }, + ], + }, + }); + + Assert.Empty(classified.Scalars); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); + } + + // Only a single reference plus pure nullability unwraps. A choice between two real + // schemas is a union: picking one arm silently would bind the caller to the wrong type. + [Fact] + public void ReportsGenuineUnionRatherThanChoosingABranch() + { + var classified = ClassifyBody(new Dictionary + { + ["either"] = new OpenApiSchema + { + AnyOf = + [ + new OpenApiSchemaReference("graph.passwordProfile"), + new OpenApiSchemaReference("graph.assignedLicense"), + ], + }, + }); + + Assert.Empty(classified.Complex); + Assert.Equal(UnsupportedShape.Union, Assert.Single(classified.Unsupported).Shape); + } + + // An anonymous object still has no name kiota would agree with, so it stays reported even + // though a formatted scalar beside it now binds. + [Fact] + public void ReportsInlineObjectWhileBindingAFormattedScalarBesideIt() + { + var classified = ClassifyBody(new Dictionary + { + ["anonymous"] = new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = new Dictionary { ["x"] = Scalar(JsonSchemaType.String) }, + }, + ["birthday"] = Scalar(JsonSchemaType.String, format: "date-time"), + }); + + Assert.Equal(UnsupportedShape.InlineObject, Assert.Single(classified.Unsupported).Shape); + Assert.Equal("global::System.DateTimeOffset", Assert.Single(classified.Scalars, p => p.OpenApiName == "birthday").PsTypeName); + } + + // Every property seen lands in exactly one bucket. The coverage sweep relies on this + // identity holding, so a shape that silently falls through would be caught here. + [Fact] + public void EveryPropertyIsAccountedForExactlyOnce() + { + var properties = new Dictionary + { + ["displayName"] = Scalar(JsonSchemaType.String), + ["businessPhones"] = new OpenApiSchema { Type = JsonSchemaType.Array, Items = Scalar(JsonSchemaType.String) }, + ["passwordProfile"] = new OpenApiSchemaReference("graph.passwordProfile"), + ["importance"] = new OpenApiSchemaReference("graph.importance"), + ["birthday"] = Scalar(JsonSchemaType.String, format: "date-time"), + ["id"] = Scalar(JsonSchemaType.String), }; - Assert.True(SchemaProperties.HasPasswordProfile(withProfile)); - var viaAllOf = new OpenApiSchema { AllOf = new List { withProfile } }; - Assert.True(SchemaProperties.HasPasswordProfile(viaAllOf)); + var classified = ClassifyBody(properties); - var without = new OpenApiSchema + Assert.Equal( + properties.Count, + classified.Scalars.Count + classified.Complex.Count + classified.Unsupported.Count + classified.Excluded.Count); + } + + // PropertiesSeen is counted independently of the buckets, so the reported total cannot be + // a restatement of their sum: every property reached is either routed or the classifier + // throws. Asserting it here keeps the runtime reconciliation line meaningful. + [Fact] + public void ReportsIndependentlyCountedTotalThatMatchesTheBuckets() + { + var properties = new Dictionary { - Properties = new Dictionary { ["displayName"] = Scalar(JsonSchemaType.String) }, + ["displayName"] = Scalar(JsonSchemaType.String), + ["passwordProfile"] = new OpenApiSchemaReference("graph.passwordProfile"), + ["importance"] = new OpenApiSchemaReference("graph.importance"), + ["id"] = Scalar(JsonSchemaType.String), }; - Assert.False(SchemaProperties.HasPasswordProfile(without)); + + var classified = ClassifyBody(properties); + + Assert.Equal(properties.Count, classified.PropertiesSeen); + Assert.Equal( + classified.PropertiesSeen, + classified.Scalars.Count + classified.Complex.Count + classified.Unsupported.Count + classified.Excluded.Count); + } + + // A property inherited through allOf and also restated on the child is one property, not + // two: the dedupe must be reflected in the independent count as well, or the invariant + // would fire spuriously on a perfectly normal Graph schema. + [Fact] + public void CountsAPropertyOnceWhenAllOfRestatesIt() + { + var classified = SchemaProperties.Classify( + new OpenApiSchema + { + Type = JsonSchemaType.Object, + AllOf = + [ + new OpenApiSchema + { + Properties = new Dictionary { ["displayName"] = Scalar(JsonSchemaType.String) }, + }, + ], + Properties = new Dictionary { ["displayName"] = Scalar(JsonSchemaType.String) }, + }, + Resolve); + + Assert.Equal(1, classified.PropertiesSeen); + Assert.Single(classified.Scalars); + } + + [Fact] + public void MapsNumericFormatsWithoutDataLoss() + { + var classified = ClassifyBody(new Dictionary + { + ["riskScore"] = Scalar(JsonSchemaType.Number), // fractions must survive + ["sizeInBytes"] = Scalar(JsonSchemaType.Integer, format: "int64"), // values > 2^31 must survive + ["retryCount"] = Scalar(JsonSchemaType.Integer, format: "int32"), + ["plainCount"] = Scalar(JsonSchemaType.Integer), + // Graph's docs declare Edm.Int32/Int64 as type "number" with the format carrying + // the real type (mailFolder.childFolderCount, messageRule.sequence). The format + // must win or the parameter type contradicts the Kiota model and won't compile. + ["childFolderCount"] = Scalar(JsonSchemaType.Number, format: "int32"), + ["quotaUsed"] = Scalar(JsonSchemaType.Number, format: "int64"), + ["confidence"] = Scalar(JsonSchemaType.Number, format: "float"), + }); + + var props = classified.Scalars; + Assert.Equal("double", props.Single(p => p.OpenApiName == "riskScore").PsTypeName); + Assert.Equal("long", props.Single(p => p.OpenApiName == "sizeInBytes").PsTypeName); + Assert.Equal("int", props.Single(p => p.OpenApiName == "retryCount").PsTypeName); + Assert.Equal("int", props.Single(p => p.OpenApiName == "plainCount").PsTypeName); + Assert.Equal("int", props.Single(p => p.OpenApiName == "childFolderCount").PsTypeName); + Assert.Equal("long", props.Single(p => p.OpenApiName == "quotaUsed").PsTypeName); + Assert.Equal("float", props.Single(p => p.OpenApiName == "confidence").PsTypeName); } } diff --git a/tools/WrapperGenerator.Tests/SpecShapeTests.cs b/tools/WrapperGenerator.Tests/SpecShapeTests.cs new file mode 100644 index 0000000000..70bf09758f --- /dev/null +++ b/tools/WrapperGenerator.Tests/SpecShapeTests.cs @@ -0,0 +1,142 @@ +using System.IO; +using System.Linq; +using System.Text; +using System.Threading; +using Microsoft.OpenApi; +using Microsoft.OpenApi.Reader; +using Xunit; + +namespace WrapperGenerator.Tests; + +// Pins the two spec facts complex-property binding depends on. Both are properties of the +// Graph documents AND of the reader that parses them, so an in-memory OpenApiSchema cannot +// prove either - these parse real YAML. +// +// If either breaks (a reader upgrade drops unknown extensions, or Graph changes how it marks +// navigation properties), binding would start emitting parameters for navigation properties +// like -AdhocCalls or -AppRoleAssignments, which are not request-body fields at all. That +// failure would be silent in the generator and only visible as nonsense cmdlet surface, so it +// is gated here instead. +public sealed class SpecShapeTests +{ + // The reader models a $ref as an OpenApiSchemaReference rather than an inlined schema; + // read it through the public API here so these tests pin the reader, not our helper. + private static string? ReferenceIdOf(IOpenApiSchema schema) => + schema is OpenApiSchemaReference reference ? reference.Reference?.Id : null; + + private static OpenApiDocument Parse(string yaml) + { + var settings = new OpenApiReaderSettings(); + settings.AddYamlReader(); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(yaml)); + var result = OpenApiDocument.LoadAsync(stream, settings: settings, cancellationToken: CancellationToken.None) + .GetAwaiter().GetResult(); + return result.Document!; + } + + // Graph marks navigation properties with x-ms-navigationProperty: true and does NOT set + // readOnly on them, so the extension is the only signal that keeps them out of the bound + // parameter set. + [Fact] + public void ReaderPreservesNavigationPropertyExtension() + { + const string yaml = """ + openapi: 3.0.1 + info: { title: probe, version: 1.0.0 } + paths: {} + components: + schemas: + probe.entity: + type: object + properties: + displayName: + type: string + adhocCalls: + type: array + items: + $ref: '#/components/schemas/probe.child' + x-ms-navigationProperty: true + probe.child: + type: object + properties: + id: { type: string } + """; + + var entity = Parse(yaml).Components!.Schemas!["probe.entity"]; + var nav = entity.Properties!["adhocCalls"]; + var structural = entity.Properties!["displayName"]; + + Assert.True(nav.Extensions is not null && nav.Extensions.ContainsKey("x-ms-navigationProperty"), + "Reader dropped x-ms-navigationProperty; navigation properties can no longer be excluded from body binding."); + Assert.False(nav.ReadOnly, "Graph does not set readOnly on navigation properties - the extension is the only signal."); + Assert.False(structural.Extensions?.ContainsKey("x-ms-navigationProperty") ?? false); + } + + // A nullable complex property is expressed as anyOf[ $ref, { type: object, nullable: true } ] + // - verified against user.passwordProfile, the property that motivated typed binding. The + // branch carrying the $ref must stay resolvable through the reader. + [Fact] + public void ReaderPreservesNullableRefCompositionShape() + { + const string yaml = """ + openapi: 3.0.1 + info: { title: probe, version: 1.0.0 } + paths: {} + components: + schemas: + probe.user: + type: object + properties: + passwordProfile: + anyOf: + - $ref: '#/components/schemas/probe.passwordProfile' + - type: object + nullable: true + probe.passwordProfile: + type: object + properties: + password: { type: string } + """; + + var property = Parse(yaml).Components!.Schemas!["probe.user"].Properties!["passwordProfile"]; + + Assert.NotNull(property.AnyOf); + Assert.Equal(2, property.AnyOf!.Count); + var refs = property.AnyOf.Where(b => ReferenceIdOf(b) is not null).ToList(); + Assert.Single(refs); + Assert.Equal("probe.passwordProfile", ReferenceIdOf(refs[0])); + } + + // A $ref does not imply an object: microsoft.graph.importance is a string enum reached the + // same way passwordProfile is. Classification has to resolve the reference and look at the + // target, or enums would be bound as model-typed parameters that do not compile. + [Fact] + public void ReferencedSchemaMayBeAnEnumNotAnObject() + { + const string yaml = """ + openapi: 3.0.1 + info: { title: probe, version: 1.0.0 } + paths: {} + components: + schemas: + probe.message: + type: object + properties: + importance: + $ref: '#/components/schemas/probe.importance' + probe.importance: + type: string + enum: [low, normal, high] + """; + + var document = Parse(yaml); + var importance = document.Components!.Schemas!["probe.message"].Properties!["importance"]; + + var referenceId = ReferenceIdOf(importance); + Assert.Equal("probe.importance", referenceId); + + var target = document.Components.Schemas[referenceId!]; + Assert.True((target.Type & ~JsonSchemaType.Null) == JsonSchemaType.String); + Assert.NotEmpty(target.Enum!); + } +} diff --git a/tools/WrapperGenerator/CmdletEmitter.cs b/tools/WrapperGenerator/CmdletEmitter.cs index 985309edfe..21b90c41cc 100644 --- a/tools/WrapperGenerator/CmdletEmitter.cs +++ b/tools/WrapperGenerator/CmdletEmitter.cs @@ -200,7 +200,7 @@ protected override void ProcessRecord() { {{AuthBlock}} - {{entityType}} result; + {{entityType}}? result; try { result = client.{{naming.BuilderExpression}}.GetAsync(requestConfiguration => @@ -295,7 +295,7 @@ protected override void ProcessRecord() { {{AuthBlock}} - {{collectionResponseType}} result; + {{collectionResponseType}}? result; try { result = client.{{naming.BuilderExpression}}.GetAsync(requestConfiguration => @@ -307,7 +307,10 @@ protected override void ProcessRecord() } {{CatchBlock(TargetId(naming))}} - WriteObject(result.Value, enumerateCollection: true); + // A collection response and its Value are both nullable on the kiota client; an + // empty page writes nothing rather than dereferencing null. + if (result?.Value is { } items) + WriteObject(items, enumerateCollection: true); } } } @@ -450,11 +453,13 @@ protected override void ProcessRecord() """; } - public static string EmitNew(CmdletNaming naming, EmitContext ctx, string entityType, IReadOnlyList properties, bool hasPasswordProfile) + public static string EmitNew(CmdletNaming naming, EmitContext ctx, string entityType, IReadOnlyList properties, IReadOnlyList complexProperties, IReadOnlyList untypedProperties) { ArgumentNullException.ThrowIfNull(naming); ArgumentNullException.ThrowIfNull(ctx); ArgumentNullException.ThrowIfNull(properties); + ArgumentNullException.ThrowIfNull(complexProperties); + ArgumentNullException.ThrowIfNull(untypedProperties); return $$""" #nullable enable @@ -476,7 +481,8 @@ public class {{naming.ClassName}} : PSCmdlet { {{PathParams(naming)}} {{EmitPropertyParameters(properties)}} -{{(hasPasswordProfile ? EmitPasswordProfileParameters() : "")}} +{{EmitComplexParameters(complexProperties)}} +{{EmitUntypedParameters(untypedProperties)}} {{HeaderParamDecls(naming)}} {{GenericHeadersParamDecl()}} @@ -489,7 +495,8 @@ protected override void ProcessRecord() var body = new {{entityType}}(); {{EmitPropertyAssignments(properties)}} -{{(hasPasswordProfile ? EmitPasswordProfileAssignment() : "")}} +{{EmitComplexAssignments(complexProperties)}} +{{EmitUntypedAssignments(untypedProperties)}} {{AuthBlock}} {{entityType}}? result; @@ -507,11 +514,13 @@ protected override void ProcessRecord() """; } - public static string EmitUpdate(CmdletNaming naming, EmitContext ctx, string entityType, IReadOnlyList properties, bool hasPasswordProfile, bool reFetchAfterUpdate = true) + public static string EmitUpdate(CmdletNaming naming, EmitContext ctx, string entityType, IReadOnlyList properties, IReadOnlyList complexProperties, IReadOnlyList untypedProperties, bool reFetchAfterUpdate = true) { ArgumentNullException.ThrowIfNull(naming); ArgumentNullException.ThrowIfNull(ctx); ArgumentNullException.ThrowIfNull(properties); + ArgumentNullException.ThrowIfNull(complexProperties); + ArgumentNullException.ThrowIfNull(untypedProperties); return $$""" #nullable enable @@ -533,7 +542,8 @@ public class {{naming.ClassName}} : PSCmdlet { {{PathParams(naming)}} {{EmitPropertyParameters(properties)}} -{{(hasPasswordProfile ? EmitPasswordProfileParameters() : "")}} +{{EmitComplexParameters(complexProperties)}} +{{EmitUntypedParameters(untypedProperties)}} {{HeaderParamDecls(naming)}} {{GenericHeadersParamDecl()}} @@ -546,7 +556,8 @@ protected override void ProcessRecord() var body = new {{entityType}}(); {{EmitPropertyAssignments(properties)}} -{{(hasPasswordProfile ? EmitPasswordProfileAssignment() : "")}} +{{EmitComplexAssignments(complexProperties)}} +{{EmitUntypedAssignments(untypedProperties)}} {{AuthBlock}} {{entityType}}? result; @@ -621,12 +632,14 @@ public static string EmitSharedAuth(EmitContext ctx) #nullable enable using System; +using System.Collections; using System.Collections.Generic; using System.Management.Automation; using System.Threading; using System.Threading.Tasks; using Microsoft.Kiota.Abstractions; using Microsoft.Kiota.Abstractions.Authentication; +using Microsoft.Kiota.Abstractions.Serialization; namespace {{ctx.CmdletNamespace}} { @@ -655,6 +668,80 @@ public Task AuthenticateRequestAsync(RequestInformation request, Dictionary(StringComparer.Ordinal); + foreach (DictionaryEntry entry in dictionary) + { + var key = entry.Key?.ToString(); + if (key is null) + continue; + // Dropped, not sent. The published SDK has no untyped bag to copy + // here, so this extends its top-level rule rather than inheriting it. + var member = From(entry.Value); + if (member is not null) + members[key] = member; + } + return members.Count == 0 ? null : new UntypedObject(members); + } + case IEnumerable sequence: + { + var items = new List(); + foreach (var item in sequence) + { + var node = From(item); + if (node is not null) + items.Add(node); + } + return items.Count == 0 ? null : new UntypedArray(items); + } + default: + // Stringifying an unrecognised type would send a value the caller never + // wrote; failing names the type so the gap is fixable. + throw new ArgumentException( + $"Cannot convert a value of type '{value.GetType().FullName}' to an untyped Graph value. Supported: string, boolean, number, hashtable, array."); + } + } + } } """; } @@ -693,25 +780,43 @@ private static string EmitPropertyAssignments(IReadOnlyList prop body.{{p.PascalName}} = {{(p.IsArray ? $"{p.ParameterName}!.ToList()" : p.ParameterName)}}; """)); - private static string EmitPasswordProfileParameters() => """ + // A complex property binds as its kiota model type. PowerShell converts a hashtable to that + // type on binding (the models have a parameterless constructor and settable properties), so + // the caller writes -PasswordProfile @{ Password = '...' } without constructing the type. + // TypeName is fully qualified: the models namespace is imported, but a model whose name + // matches a cmdlet parameter or BCL type would otherwise bind to the wrong symbol. + private static string EmitComplexParameters(IReadOnlyList properties) => + string.Join("\n", properties.Select(p => $$""" - [Parameter(Mandatory = false, - HelpMessage = "Required by Graph to create a user. Ignored if the resource has no passwordProfile.")] - public string? Password { get; set; } + [Parameter(Mandatory = false)] + public {{p.ElementNullableTypeName}}? {{p.ParameterName}} { get; set; } + """)); + + private static string EmitComplexAssignments(IReadOnlyList properties) => + string.Join("\n", properties.Select(p => $$""" + + if (this.IsParameterBound(nameof({{p.ParameterName}}))) + body.{{p.PascalName}} = {{(p.IsArray ? $"{p.ParameterName}!.ToList()" : p.ParameterName)}}; + """)); + + // A schema-less property takes object and converts, so the caller can pass an ordinary + // PowerShell value. A conversion result of null means "omit", which is why the assignment + // is guarded on the converted value and not merely on the parameter being bound. + private static string EmitUntypedParameters(IReadOnlyList properties) => + string.Join("\n", properties.Select(p => $$""" [Parameter(Mandatory = false)] - public bool? ForceChangePasswordNextSignIn { get; set; } - """; + public object? {{p.ParameterName}} { get; set; } + """)); - private static string EmitPasswordProfileAssignment() => """ + private static string EmitUntypedAssignments(IReadOnlyList properties) => + string.Join("\n", properties.Select(p => $$""" - if (this.IsParameterBound(nameof(Password)) || this.IsParameterBound(nameof(ForceChangePasswordNextSignIn))) + if (this.IsParameterBound(nameof({{p.ParameterName}}))) { - body.PasswordProfile = new PasswordProfile - { - Password = Password, - ForceChangePasswordNextSignIn = ForceChangePasswordNextSignIn ?? true, - }; + var {{p.LocalName}} = UntypedValue.From({{p.ParameterName}}); + if ({{p.LocalName}} is not null) + body.{{p.PascalName}} = {{p.LocalName}}; } - """; + """)); } diff --git a/tools/WrapperGenerator/PowerShellWrapperGenerationService.cs b/tools/WrapperGenerator/PowerShellWrapperGenerationService.cs index 82974e0db3..46690e5008 100644 --- a/tools/WrapperGenerator/PowerShellWrapperGenerationService.cs +++ b/tools/WrapperGenerator/PowerShellWrapperGenerationService.cs @@ -28,6 +28,16 @@ public sealed partial class PowerShellWrapperGenerationService private readonly List fileCollisions = []; private readonly Dictionary kiotaReservedRenames; + // Body-property classification totals for this run; reported as one reconciliation line. + // propertiesSeenCount is accumulated from the classifier's own independent count so the + // reported total is not merely the sum of the buckets beside it. + private int propertiesSeenCount; + private int boundScalarCount; + private int boundComplexCount; + private int boundUntypedCount; + private int unsupportedPropertyCount; + private int excludedPropertyCount; + public PowerShellWrapperGenerationService(OpenApiDocument document, GeneratorConfig configuration, ILogger logger) { ArgumentNullException.ThrowIfNull(document); @@ -208,7 +218,7 @@ public async Task GenerateAsync(CancellationToken cancellationToken) written += await EmitGetOperationsAsync(getOperations, ctx, cancellationToken).ConfigureAwait(false); // All collisions for the run are reported together so one generation surfaces the - // complete list; see edge-cases/naming-edge-cases.md for how each kind is resolved. + // complete list; see docs/edge-cases/naming-edge-cases.md for how each kind is resolved. if (fileCollisions.Count > 0) { throw new InvalidOperationException( @@ -216,6 +226,10 @@ public async Task GenerateAsync(CancellationToken cancellationToken) $"Resolve each with a NamingOverrides rename or suppression.\n " + string.Join("\n ", fileCollisions)); } + LogBodyPropertyReconciliation( + propertiesSeenCount, + boundScalarCount, boundComplexCount, boundUntypedCount, excludedPropertyCount, unsupportedPropertyCount); + LogWroteFiles(written + 1, config.OutputPath); } @@ -340,6 +354,14 @@ private async Task WriteCmdletFileAsync(CmdletNaming naming, string source, private partial void LogSuppressedOperation(string method, string pathTemplate); [LoggerMessage(Level = LogLevel.Warning, Message = "Skipped {Method} {PathTemplate}: {Reason}")] private partial void LogSkippedUnsupportedOperation(string method, string pathTemplate, string reason); + // Information, not Warning: an unbindable body property is a known coverage gap per shape, + // not a defect in this run, and at Graph scale these would drown the operation warnings. + [LoggerMessage(Level = LogLevel.Information, Message = "Unbound body property {Noun}.{Property}: {Shape} (required={IsRequired})")] + private partial void LogSkippedBodyProperty(string noun, string property, string shape, bool isRequired); + [LoggerMessage(Level = LogLevel.Information, Message = "Excluded body property {Noun}.{Property}: {Policy}")] + private partial void LogExcludedBodyProperty(string noun, string property, string policy); + [LoggerMessage(Level = LogLevel.Information, Message = "Body properties classified={Classified} = scalar={Scalars} + model={Complex} + untyped={Untyped} + excluded={Excluded} + unsupported={Unsupported}")] + private partial void LogBodyPropertyReconciliation(int classified, int scalars, int complex, int untyped, int excluded, int unsupported); // True when the path contains a segment shape the emitters cannot handle yet: an OData // $-segment ($count/$value/$ref) or a parameterized function/action call (any segment @@ -387,9 +409,8 @@ private bool TryResolveListEntityTypeName(IOpenApiSchema collectionValueSchema, return null; if (!TryResolveEntityTypeName(bodySchema, ctx.ModelsNamespace, out var entityType)) return null; - var properties = SchemaProperties.ResolveParameterNameCollisions( - SchemaProperties.ExtractPrimitiveProperties(bodySchema), naming.PathParamNames); - return CmdletEmitter.EmitNew(naming, ctx, entityType, properties, SchemaProperties.HasPasswordProfile(bodySchema)); + var (properties, complex, untyped) = BindBodyProperties(bodySchema, ctx, naming, entityType); + return CmdletEmitter.EmitNew(naming, ctx, entityType, properties, complex, untyped); } private string? EmitUpdateFor(CmdletNaming naming, EmitContext ctx, OpenApiOperation operation, bool canReFetch) @@ -400,11 +421,66 @@ private bool TryResolveListEntityTypeName(IOpenApiSchema collectionValueSchema, return null; if (!TryResolveEntityTypeName(bodySchema, ctx.ModelsNamespace, out var entityType)) return null; - var properties = SchemaProperties.ResolveParameterNameCollisions( - SchemaProperties.ExtractPrimitiveProperties(bodySchema), naming.PathParamNames); - return CmdletEmitter.EmitUpdate(naming, ctx, entityType, properties, SchemaProperties.HasPasswordProfile(bodySchema), canReFetch); + var (properties, complex, untyped) = BindBodyProperties(bodySchema, ctx, naming, entityType); + return CmdletEmitter.EmitUpdate(naming, ctx, entityType, properties, complex, untyped, canReFetch); + } + + // Classifies a request body and resolves each complex property's component-schema key to + // the kiota CLR type name, reusing ResolveModelTypeName so reserved-name renames and + // sub-namespace moves are applied in exactly one place. A property whose reference does not + // resolve is dropped with a diagnostic rather than emitted against a guessed type name, + // which would fail the module compile. + private (IReadOnlyList Scalars, IReadOnlyList Complex, IReadOnlyList Untyped) BindBodyProperties( + IOpenApiSchema bodySchema, EmitContext ctx, CmdletNaming naming, string entityType) + { + var classified = SchemaProperties.Classify(bodySchema, ResolveComponentSchema); + var (scalars, complex, untyped) = SchemaProperties.ResolveParameterNameCollisions( + classified.Scalars, classified.Complex, classified.Untyped, naming.PathParamNames); + + // C# forbids a member sharing its enclosing type's name, so kiota suffixes such a + // property with "Prop": microsoft.graph.list's own "list" property generates as + // List.ListProp (verified in a generated Files client). The assignment target has to + // match the member kiota emitted, or the module does not compile. + var enclosingTypeName = entityType[(entityType.LastIndexOf('.') + 1)..]; + scalars = [.. scalars.Select(p => p.PascalName == enclosingTypeName ? p with { PascalName = p.PascalName + "Prop" } : p)]; + complex = [.. complex.Select(p => p.PascalName == enclosingTypeName ? p with { PascalName = p.PascalName + "Prop" } : p)]; + untyped = [.. untyped.Select(p => p.PascalName == enclosingTypeName ? p with { PascalName = p.PascalName + "Prop" } : p)]; + + foreach (var skipped in classified.Unsupported) + LogSkippedBodyProperty(naming.Noun, skipped.OpenApiName, skipped.Shape.ToString(), skipped.IsRequired); + + // Named so an external reconciliation can tell a policy exclusion from an omission + // without re-deriving the policy from the spec. + foreach (var dropped in classified.Excluded) + LogExcludedBodyProperty(naming.Noun, dropped.OpenApiName, dropped.Policy.ToString()); + + // Totals for the run's reconciliation line: every classified property must end up in + // exactly one of these buckets, so a shape that silently fell through the classifier + // would show up as a mismatch at Graph scale, not just in the unit test. + propertiesSeenCount += classified.PropertiesSeen; + boundScalarCount += classified.Scalars.Count; + boundComplexCount += classified.Complex.Count; + boundUntypedCount += classified.Untyped.Count; + unsupportedPropertyCount += classified.Unsupported.Count; + excludedPropertyCount += classified.Excluded.Count; + + var parameters = new List(complex.Count); + foreach (var property in complex) + { + parameters.Add(new ComplexParameter( + property.PascalName, + property.ParameterName, + ResolveModelTypeName(property.ReferenceId, ctx.ModelsNamespace, modelSubNamespaces, kiotaReservedRenames), + property.IsArray, + property.IsEnum)); + } + var untypedParameters = untyped.Select(p => new UntypedParameter(p.PascalName, p.ParameterName)).ToList(); + return (scalars, parameters, untypedParameters); } + private IOpenApiSchema? ResolveComponentSchema(string referenceId) => + document.Components?.Schemas?.TryGetValue(referenceId, out var schema) == true ? schema : null; + private static bool HasNonJsonSuccessContent(OpenApiOperation operation) { if (operation.Responses is null) diff --git a/tools/WrapperGenerator/Program.cs b/tools/WrapperGenerator/Program.cs index 15d43b5dff..19c588fd58 100644 --- a/tools/WrapperGenerator/Program.cs +++ b/tools/WrapperGenerator/Program.cs @@ -4,6 +4,7 @@ using System.Text.Json; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using Microsoft.OpenApi; using Microsoft.OpenApi.Reader; @@ -22,6 +23,7 @@ private static async Task Main(string[] args) string? clientNamespace = null; var apiVersion = "v1.0"; var useCollisionData = true; + var logLevel = LogLevel.Warning; var includePaths = new List(); for (var i = 0; i < args.Length; i++) @@ -40,6 +42,13 @@ private static async Task Main(string[] args) case "--api-version": apiVersion = ArgValue(args, ref i); break; + case "--log-level": + if (!Enum.TryParse(ArgValue(args, ref i), ignoreCase: true, out logLevel)) + { + Console.Error.WriteLine("--log-level expects one of: Trace, Debug, Information, Warning, Error, Critical, None"); + return 2; + } + break; case "--no-collision-data": // Derivation mode: tools/Derive-CollisionResolutions.ps1 needs the raw // collision inventory, so the derived resolutions must not mask it. @@ -62,7 +71,7 @@ private static async Task Main(string[] args) if (specPath is null || outputPath is null || clientNamespace is null) { Console.Error.WriteLine( - "Usage: WrapperGenerator -d -o -n [--api-version v1.0|beta] [--no-collision-data] [--include-path '#GET,POST' ...]"); + "Usage: WrapperGenerator -d -o -n [--api-version v1.0|beta] [--no-collision-data] [--log-level Information] [--include-path '#GET,POST' ...]"); return 2; } @@ -78,7 +87,7 @@ private static async Task Main(string[] args) var config = new GeneratorConfig(ClientNamespaceName: clientNamespace, OutputPath: outputPath, ApiVersion: apiVersion, UseCollisionData: useCollisionData); - var service = new PowerShellWrapperGenerationService(document, config, new StderrLogger()); + var service = new PowerShellWrapperGenerationService(document, config, new StderrLogger(logLevel)); await service.GenerateAsync(CancellationToken.None).ConfigureAwait(false); // The generation service writes only *.g.cs. Also write a minimal kiota-lock.json recording diff --git a/tools/WrapperGenerator/README.md b/tools/WrapperGenerator/README.md index 60e4aa796a..d8e5b15f7e 100644 --- a/tools/WrapperGenerator/README.md +++ b/tools/WrapperGenerator/README.md @@ -6,7 +6,7 @@ Generates the PowerShell **cmdlets** for the Microsoft Graph SDK from Graph's Op The Microsoft Graph PowerShell SDK is thousands of cmdlets, and customers have scripts that depend on their exact names — `Get-MgUserMessage`, not `Get-MgUsersMessages`. Those names follow conventions, but the conventions are fiddly (singular nouns, a `Mg` prefix, a handful of hand-tuned exceptions), and the SDK's current generator (AutoRest) has quietly dropped cmdlets when names collided. -This tool regenerates those cmdlets from the same OpenAPI description **while reproducing the published names exactly**, so a regenerated module is a drop-in replacement. Because name parity is the hard part, most of the tool is a naming engine; the rest is a straightforward C# code emitter. The one exception to "exactly": a handful of published names are AutoRest naming defects (e.g. `Get-MgSecurityThreatIntelligenceHostWhoi`, where "Whois" lost its `s`) that the generator deliberately corrects — each is pinned by a test, allowlisted in the parity gate, and documented in the edge-case catalog ([edge-cases/naming-edge-cases.md](edge-cases/naming-edge-cases.md), one file per class of issue). +This tool regenerates those cmdlets from the same OpenAPI description **while reproducing the published names exactly**, so a regenerated module is a drop-in replacement. Because name parity is the hard part, most of the tool is a naming engine; the rest is a straightforward C# code emitter. The one exception to "exactly": a handful of published names are AutoRest naming defects (e.g. `Get-MgSecurityThreatIntelligenceHostWhoi`, where "Whois" lost its `s`) that the generator deliberately corrects — each is pinned by a test, allowlisted in the parity gate, and documented in the edge-case catalog ([docs/edge-cases/naming-edge-cases.md](docs/edge-cases/naming-edge-cases.md), one file per class of issue). ## What it produces @@ -54,7 +54,7 @@ Singularization runs per camel-case word (so `termsAndConditions` → `TermAndCo A few published names aren't algorithmic, and the spec publishes some routes the SDK never shipped. Both live as data in `NamingOverrides.cs` — renames mirroring the SDK's hand-written AutoRest directives, and suppressions for routes that ship nothing — each entry citing its evidence: the directive when one exists, otherwise the shipped-command inventory. Examples: the `GET /users/{id}/calendar` rename to `…UserDefaultCalendar` (Calendar.md), the `Solution` prefix strip under `/solutions/*` with the BackupRestore exception (Bookings.md), and the self-referential `sites/{id}/sites` rename to `SubSite`/`GroupSubSite` (Sites.md) — without which the sub-sites cmdlets would collide with `Get-MgSite` itself. The generator fails loudly on any such file collision rather than silently overwriting. -On top of the curated entries sits a **derived** layer: `tools/Derive-CollisionResolutions.ps1` replays every route from the checked-in collision inventory (`data/collision-inventory.v1.0.txt`, captured with `--no-collision-data`) against the shipped-command inventory and emits `data/collision-suppressions.v1.0.json` and `data/collision-renames.v1.0.json` — one exact-match entry per contested method+route, each carrying its oracle evidence. The files are embedded into the generator at build time (generation never reads the 22 MB oracle), applied only when `GeneratorConfig.UseCollisionData` is set, and the script's `-Validate` mode fails if the checked-in files drift from a fresh derivation. Two routes in all of v1.0 are **deferred cross-path merges** — the published SDK serves `Get-MgGroupPhoto` from both `/photo` and `/photos`, and `Get-MgShareListItem` from both `/listItem` and `/list/items`, as parameter-set variants of one cmdlet; the generator keeps the singleton side and suppresses the collection side until cross-path parameter sets land (see [edge-cases/crosspath-merge-edge-cases.md](edge-cases/crosspath-merge-edge-cases.md)). With the derived data applied, a full v1.0 generation across all 39 modules produces zero collisions. +On top of the curated entries sits a **derived** layer: `tools/Derive-CollisionResolutions.ps1` replays every route from the checked-in collision inventory (`data/collision-inventory.v1.0.txt`, captured with `--no-collision-data`) against the shipped-command inventory and emits `data/collision-suppressions.v1.0.json` and `data/collision-renames.v1.0.json` — one exact-match entry per contested method+route, each carrying its oracle evidence. The files are embedded into the generator at build time (generation never reads the 22 MB oracle), applied only when `GeneratorConfig.UseCollisionData` is set, and the script's `-Validate` mode fails if the checked-in files drift from a fresh derivation. Two routes in all of v1.0 are **deferred cross-path merges** — the published SDK serves `Get-MgGroupPhoto` from both `/photo` and `/photos`, and `Get-MgShareListItem` from both `/listItem` and `/list/items`, as parameter-set variants of one cmdlet; the generator keeps the singleton side and suppresses the collection side until cross-path parameter sets land (see [docs/edge-cases/crosspath-merge-edge-cases.md](docs/edge-cases/crosspath-merge-edge-cases.md)). With the derived data applied, a full v1.0 generation across all 39 modules produces zero collisions. ## The one subtle part: list + item GET become one cmdlet @@ -100,8 +100,18 @@ Whatever the shape, a generated cmdlet has the same skeleton: - **Auth**: every cmdlet takes an optional `-AccessToken`; without it, the cmdlet uses the active `Connect-MgGraph` session. (The shared auth helpers are written once per module into `Shared.g.cs`.) - **GETs** expose the OData query options the operation supports — `-Filter`, `-Property` (alias`-Select`), `-Sort` (alias `-OrderBy`), `-Top`, `-Skip`, `-Count`. -- **`New`/`Update`** flatten the request body's top-level primitive properties into parameters (`-Subject`, `-IsRead`, …). Nested/complex properties are skipped, with one special case: - `passwordProfile` is exposed as `-Password`/`-ForceChangePasswordNextSignIn` because creating a user requires it. `Update` also re-fetches after a `204 No Content` so it still returns the updated object. +- **`New`/`Update`** bind the request body's properties as parameters. Primitives flatten directly + (`-Subject`, `-IsRead`, …); a referenced model binds as its kiota type, so + `New-MgUser -PasswordProfile @{ Password = '...' }` works — PowerShell converts the hashtable on + binding. Referenced enums bind as the generated enum (`-Importance high`), and formatted strings + bind as the CLR type kiota uses (`date-time` → `DateTimeOffset`, `uuid` → `Guid`, `base64url` → + `byte[]`). A property the spec gives no type at all — which kiota emits as `UntypedNode` — takes + an ordinary PowerShell value (`-Maximum 100`, `-ContentInfo @{ … }`) and is converted on + assignment. Navigation properties, `id`, `additionalData` and `readOnly` properties are + deliberately excluded: they are relationships or serializer infrastructure, not body fields. + `Update` also re-fetches after a `204 No Content` so it still returns the updated object. + See [docs/body-property-binding.md](docs/body-property-binding.md) for the full mapping, the + `date`/`time` input contract, and what remains unbound. - **`New`/`Update`/`Remove`** are gated by `ShouldProcess`, so `-WhatIf` and `-Confirm` work. - **The actual request** is the Kiota client's fluent chain built from the path: `client.Users[UserId].Messages[MessageId].GetAsync(...)`. @@ -154,23 +164,68 @@ dotnet run --project tools/WrapperGenerator -- ` `-d` is the spec, `-o` the output folder, `-n` the namespace of the step-1 client the wrappers call. Each `--include-path` is a glob with an optional `#METHOD,METHOD` filter; omit them to generate every operation in the document. Output: `Shared.g.cs`, one `*.g.cs` per cmdlet (in a namespace derived from `-n` by dropping its trailing `.Client`, e.g. `-n Microsoft.Graph.PowerShell.Mail.Client` emits into `Microsoft.Graph.PowerShell.Mail`), and a small `kiota-lock.json` noting the source spec. -**Test** — two layers: +## The committed output + +The generated modules are checked in under `src/{Module}/{v1.0|beta}/wrapper/`, one +self-contained project per module and API version: + +``` +src/Mail/v1.0/wrapper/ + Client/ kiota client (models + request builders) + Cmdlets/ the wrapper cmdlets, one *.g.cs each, plus Shared.g.cs + Microsoft.Graph.Wrapper.Mail.csproj compiles both into one assembly +``` + +Everything needed to build is in that folder, so no generation step is required to try it: ```powershell -# 1. Naming rules pinned to published Microsoft.Graph names +dotnet build src/Mail/v1.0/wrapper # produces the dll + psd1 under bin/ +Import-Module src/Mail/v1.0/wrapper/bin/Release/net10.0/Microsoft.Graph.Wrapper.Mail.psd1 +``` + +To regenerate it after a generator change — this rewrites the committed folder in place, so the +diff shows exactly what the change did to the output: + +```powershell +.\tools\Build-WrapperModule.ps1 -Module Mail -IntoSource -Configuration Release +.\tools\New-WrapperOutputManifest.ps1 # refresh docs/WrapperCmdlets-V1.0*.csv +``` + +`docs/WrapperCmdlets-V1.0.csv` is the reviewable inventory of that output — one row per emitted +cmdlet with its module, verb, noun and request path — with per-module totals in +`docs/WrapperCmdlets-V1.0-Summary.csv`. The generated tree is far larger than GitHub renders in +a diff, so those two files, not the tree, are what a reviewer reads. + +**Test** — several layers, each proving something the others cannot: + +```powershell +# 1. Naming and classification rules pinned to published Microsoft.Graph names dotnet test tools/WrapperGenerator.Tests -# => Passed! - Failed: 0, Passed: 120, Total: 120 +# => Passed! - Failed: 0, Passed: 148, Total: 148 # 2. Parity gate: generate, then check every cmdlet name against Graph's own command inventory .\tools\Compare-WrapperCmdletNames.ps1 -GeneratedPath # => Mail [v1.0]: 4 of 4 cmdlets match the oracle ... EXIT CODE: 0 + +# 3. Compile gate: every module builds against the kiota client it was generated with +.\tools\Build-WrapperModule.ps1 -Module -Configuration Release + +# 4. Omission oracle: every settable kiota body member is bound or cited by a named policy +.\tools\Test-BodyBindingCoverage.ps1 + +# 5. Runtime gate: the module imports and each bound shape accepts what a person would type +.\tools\Test-WrapperModule.ps1 -Module -Configuration Release ``` -The unit tests guard the naming rules (their expected values are real published names from `src/Authentication/Authentication/custom/common/MgCommandMetadata.json`). The parity gate checks actual generated output against that same inventory; names on the deliberate-corrections list ([edge-cases/naming-edge-cases.md](edge-cases/naming-edge-cases.md)) are reported as `[CORRECTED]` instead of failing. There is **no** test yet that the generated cmdlets *compile* — that needs step 1's client to compile against. +The unit tests guard the naming and classification rules (their expected values are real published names from `src/Authentication/Authentication/custom/common/MgCommandMetadata.json`). The parity gate checks actual generated output against that same inventory; names on the deliberate-corrections list ([docs/edge-cases/naming-edge-cases.md](docs/edge-cases/naming-edge-cases.md)) are reported as `[CORRECTED]` instead of failing. + +The generated cmdlets **are** compiled: `Build-WrapperModule.ps1` builds each module against the kiota client it was generated with, the only authority on whether an emitted parameter's CLR type matches the member it assigns. Compilation cannot see an *omitted* member, so the omission oracle exists separately; neither can see whether PowerShell converts a value at runtime, so the runtime gate exists separately again. + +**Known failing gate at this commit:** the naming parity gate reports 5,689 of 7,434 comparable names matching the published SDK. Those mismatches predate this change (this commit's only naming edit is a doc-path comment) and are tracked for a separate oracle-derived naming change; they are disclosed here rather than hidden from the gate list. ## Gaps / not done yet -- **Output isn't committed into `src/{Module}/`.** `tools/Build-WrapperModule.ps1` now turns a module into an importable build under `artifacts/` (kiota client + wrappers + csproj + PSD1; `tools/Test-WrapperModule.ps1` smoke-tests it), with generation reading the Kiota-compatible docs (`openApiDocs_KiotaCompat`) by default. Cmdlets now emit into a per-module namespace (not `MgPoC`) and the generated csproj references Authentication by a relative path, so both are ready to move; the exact target folder under `src/{Module}/{v1.0|beta}/` is still open — the existing AutoRest modules' `.gitignore` there excludes a folder literally named `generated`, so the wrapper output needs a different folder name or that pattern needs updating, or a commit there would silently produce an empty diff. +- **Only v1.0 output is committed.** The beta docs exist (`openApiDocs_KiotaCompat/beta`) but no beta output is generated or checked in yet; the layout already accommodates it at `src/{Module}/beta/wrapper/`. - **No runtime base classes or real auth flow.** Shared paging, a proper `Connect-MgGraph`/session integration, and base cmdlet classes are a later phase. -- **Body binding is shallow** — top-level primitive properties only; no nested/complex types beyond the `passwordProfile` special case. -- **Some operation shapes aren't generated** — `$count`/`$ref`/`$value`, delta, OData actions/functions, and cast endpoints. +- **Body binding covers every shape reaching the classifier** — the sweep reports 0 unbound properties across all 38 specs, and the omission oracle reports 0 failures across 2,633 body-writing cmdlets. That is a statement about the operations that generate, not about v1.0 (see the next bullet). Classifications for shapes that do not occur (inline objects and enums, genuine unions, dictionaries, unresolvable references, unknown formats) are retained so a future corpus change is reported rather than silently mis-bound. `tools/Measure-BodyPropertyCoverage.ps1` counts them; [docs/edge-cases/body-binding-edge-cases.md](docs/edge-cases/body-binding-edge-cases.md) records each with its exit criteria. +- **Only 57.8% of v1.0 operations generate.** Of 14,131 operations across the 38 specs: 8,164 become cmdlets, 767 are suppressed because the published SDK ships no cmdlet for them, and 5,200 are unsupported — 3,495 OData path segments (`$count`/`$ref`/`$value`, delta, cast, and parameterized functions), 1,528 POST actions whose request schema is not a named entity, 93 PUT, 78 media/stream, 6 unresolvable responses. The three populations sum to 14,131 by construction; emitted files are not operations (9,608 files include 1,444 GET dispatchers that issue no request of their own). diff --git a/tools/WrapperGenerator/SchemaProperties.cs b/tools/WrapperGenerator/SchemaProperties.cs index 30d3ac2302..af6f914695 100644 --- a/tools/WrapperGenerator/SchemaProperties.cs +++ b/tools/WrapperGenerator/SchemaProperties.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.OpenApi; @@ -12,18 +12,118 @@ public sealed record CmdletProperty(string OpenApiName, string PascalName, strin public string ParameterName { get; init; } = PascalName; } -// Maps a body schema's top-level primitive properties onto cmdlet parameters. Deliberately -// shallow, per team decision: nested complex properties (assignedLicenses, employeeOrgData, -// and the like) are skipped rather than modeled. Two special cases: "id" is excluded because -// the server assigns it, and passwordProfile is flagged separately via HasPasswordProfile -// because creating a user requires it. +// A body property whose type is a model in the spec's component schemas, bound as a parameter +// typed as the corresponding kiota model class. ReferenceId is the component schema key +// ("microsoft.graph.passwordProfile"); the generation service turns it into a CLR type name, +// because only it knows kiota's namespace and reserved-name rules. +// IsEnum carries through because an enum is a value type: kiota declares a collection of one +// with nullable elements (List), unlike a collection of models. +public sealed record ComplexProperty(string OpenApiName, string PascalName, string ReferenceId, bool IsArray, bool IsEnum) +{ + public string ParameterName { get; init; } = PascalName; +} + +// A property the spec gives no type, which kiota emits as UntypedNode. It binds as object and +// is converted at assignment, so a caller passes an ordinary PowerShell value. +public sealed record UntypedProperty(string OpenApiName, string PascalName) +{ + public string ParameterName { get; init; } = PascalName; +} + +// The same after parameter-name collision resolution. LocalName names the converted value in +// the emitted assignment; it is derived from the parameter so two properties in one cmdlet +// cannot declare the same local. +public sealed record UntypedParameter(string PascalName, string ParameterName) +{ + public string LocalName => "untyped" + ParameterName; +} + +// A complex property after the generation service has turned its ReferenceId into a kiota CLR +// type name. Emission takes this rather than ComplexProperty so a parameter cannot be emitted +// with an unresolved type. +public sealed record ComplexParameter(string PascalName, string ParameterName, string TypeName, bool IsArray, bool IsEnum) +{ + // The declared parameter type. An array of enums needs nullable elements to assign to + // kiota's List; an array of models must not, matching List. + public string ElementNullableTypeName => IsArray + ? TypeName + (IsEnum ? "?[]" : "[]") + : TypeName; +} + +// Why a property could not be bound. Each value is a distinct spec shape rather than a generic +// "unsupported", so a sweep says which shapes are worth implementing next instead of just how +// many were missed. +public enum UnsupportedShape +{ + InlineEnum, // enum declared inline; kiota synthesises the type name from the parent + UnknownFormat, // a format with no verified kiota CLR mapping + InlineObject, // anonymous object; kiota synthesises the type name from the parent + Union, // anyOf/oneOf that is a real choice, not the numeric/INF encoding + Dictionary, // free-form map (additionalProperties) + Unresolvable, // an array with no item schema, or a reference with no bindable target +} + +public sealed record UnsupportedProperty(string OpenApiName, UnsupportedShape Shape, bool IsRequired); + +// Why a property is deliberately not a parameter. Each is a protocol or framework rule, never a +// Graph corpus special case, and each is named so an external check can tell a policy exclusion +// apart from an omission. +public enum ExclusionPolicy +{ + ServerAssignedId, // "id" is assigned by the service + ODataControlData, // "@"-prefixed, e.g. @odata.type; kiota's serializer supplies it + KiotaAdditionalData, // the IAdditionalDataHolder bag every kiota model already exposes + ReadOnlySchema, // readOnly: true - the OpenAPI signal for server-managed + NavigationProperty, // x-ms-navigationProperty - a relationship with its own request path +} + +public sealed record ExcludedProperty(string OpenApiName, ExclusionPolicy Policy); + +// The full classification of one request body. PropertiesSeen is counted independently of the +// buckets, so Scalars + Complex + Unsupported + Excluded == PropertiesSeen is a real +// invariant rather than an identity that holds by construction; Classify throws if it breaks. +public sealed record BodyProperties( + IReadOnlyList Scalars, + IReadOnlyList Complex, + IReadOnlyList Untyped, + IReadOnlyList Unsupported, + IReadOnlyList Excluded, + int PropertiesSeen); + +// Maps a request body schema onto cmdlet parameters. Scalars bind directly; properties whose +// type is a referenced model bind as that model's kiota type (PowerShell coerces a hashtable +// into it). Shapes whose kiota type name cannot be derived from the spec - inline objects and +// enums, unions, dictionaries - are reported rather than guessed, because a wrong type name is +// a compile error in the generated module. public static class SchemaProperties { - public static IReadOnlyList ExtractPrimitiveProperties(IOpenApiSchema schema) + // resolveReference maps a component schema key to its schema, so a $ref can be inspected + // before deciding what it is: a reference to microsoft.graph.importance is a string enum, + // not a model, and binding it as a model would not compile. + public static BodyProperties Classify(IOpenApiSchema schema, Func resolveReference) { ArgumentNullException.ThrowIfNull(schema); - var result = new List(); + ArgumentNullException.ThrowIfNull(resolveReference); + + var scalars = new List(); + var complex = new List(); + var untyped = new List(); + var unsupported = new List(); + var excluded = new List(); var seen = new HashSet(StringComparer.Ordinal); + var required = new HashSet(StringComparer.Ordinal); + + void CollectRequired(IOpenApiSchema s) + { + foreach (var inherited in s.AllOf ?? []) + CollectRequired(inherited); + if (s.Required is { } names) + { + foreach (var name in names) + required.Add(name); + } + } + CollectRequired(schema); void Walk(IOpenApiSchema s) { @@ -32,82 +132,351 @@ void Walk(IOpenApiSchema s) foreach (var (name, propSchema) in s.Properties ?? new Dictionary()) { - if (IsProtocolOrServerManagedProperty(name, propSchema) || !seen.Add(name)) + if (!seen.Add(name)) continue; - if (IsPlainScalar(propSchema)) + if (TryGetExclusionPolicy(name, propSchema) is { } policy) { - result.Add(new CmdletProperty(name, ToKiotaPropertyName(name), MapPsType(propSchema), IsArray: false)); + excluded.Add(new ExcludedProperty(name, policy)); + continue; } - else if (propSchema.Type == JsonSchemaType.Array && propSchema.Items is { } items && IsPlainScalar(items)) + + var isRequired = required.Contains(name); + var pascal = ToKiotaPropertyName(name); + + switch (ClassifyProperty(propSchema, resolveReference)) { - result.Add(new CmdletProperty(name, ToKiotaPropertyName(name), MapPsType(items) + "[]", IsArray: true)); + case ScalarShape scalarShape: + scalars.Add(new CmdletProperty(name, pascal, scalarShape.PsTypeName, scalarShape.IsArray)); + break; + case ModelShape modelShape: + // Requiredness is deliberately not carried here: every bound parameter + // is optional, and Graph's schemas do not mark requiredness usefully. + // The measurement behind that is in docs/body-property-binding.md. + complex.Add(new ComplexProperty(name, pascal, modelShape.ReferenceId, modelShape.IsArray, modelShape.IsEnum)); + break; + case UntypedShape: + untyped.Add(new UntypedProperty(name, pascal)); + break; + case UnsupportedNativeShape u: + unsupported.Add(new UnsupportedProperty(name, u.Shape, isRequired)); + break; } } } Walk(schema); - return result; + + // seen counts every distinct property the walk reached, without reference to where it + // was routed. A property that fell through the shape switch would show up here and + // nowhere else, which is precisely the failure a summed total could never reveal. + var accountedFor = scalars.Count + complex.Count + untyped.Count + unsupported.Count + excluded.Count; + if (seen.Count != accountedFor) + { + throw new InvalidOperationException( + $"Body property classification is not exhaustive: reached {seen.Count} properties but accounted for " + + $"{accountedFor} (scalar {scalars.Count} + model {complex.Count} + untyped {untyped.Count} + unsupported {unsupported.Count} + excluded {excluded.Count})."); + } + + return new BodyProperties(scalars, complex, untyped, unsupported, excluded, seen.Count); + } + + private abstract record PropertyShape; + private sealed record UntypedShape : PropertyShape; + private sealed record ScalarShape(string PsTypeName, bool IsArray) : PropertyShape; + private sealed record ModelShape(string ReferenceId, bool IsArray, bool IsEnum) : PropertyShape; + private sealed record UnsupportedNativeShape(UnsupportedShape Shape) : PropertyShape; + + private static PropertyShape ClassifyProperty(IOpenApiSchema propSchema, Func resolveReference) + { + // An array is classified by its item schema, at the same single level of nesting the + // scalar case allows - arrays of arrays are not a shape Graph request bodies use. + if ((propSchema.Type & ~JsonSchemaType.Null) == JsonSchemaType.Array) + { + if (propSchema.Items is not { } items) + return new UnsupportedNativeShape(UnsupportedShape.Unresolvable); + return ClassifyLeaf(items, resolveReference, isArray: true); + } + + return ClassifyLeaf(propSchema, resolveReference, isArray: false); + } + + private static PropertyShape ClassifyLeaf(IOpenApiSchema leaf, Func resolveReference, bool isArray) + { + if (TryMapScalar(leaf, out var scalarType, out var badFormat)) + return new ScalarShape(ArrayAware(scalarType, isArray), isArray); + if (badFormat) + return new UnsupportedNativeShape(UnsupportedShape.UnknownFormat); + + // A reference, either directly or as the single meaningful branch of a nullable union. + // An enum reference resolves the same way a model does - kiota emits both as named types + // in the models namespace - so both bind through the same path. + var referenceId = leaf.GetReferenceId() ?? SingleReferenceOfNullableUnion(leaf); + if (referenceId is not null) + { + var target = resolveReference(referenceId); + if (target is null) + return new UnsupportedNativeShape(UnsupportedShape.Unresolvable); + var targetType = target.Type & ~JsonSchemaType.Null; + // Graph model schemas are objects; some declare no type at all and are objects by + // virtue of carrying properties or an allOf chain. Enums are named types too. + var isEnum = IsEnumSchema(target); + if (isEnum || targetType == JsonSchemaType.Object || targetType is null) + return new ModelShape(referenceId, isArray, isEnum); + // A reference to a bare scalar carries no kiota type of its own to bind to. + return new UnsupportedNativeShape(UnsupportedShape.Unresolvable); + } + + if ((leaf.AnyOf?.Count ?? 0) > 0 || (leaf.OneOf?.Count ?? 0) > 0) + { + return TryMapNumericUnion(leaf, resolveReference, out var unionType) + ? new ScalarShape(ArrayAware(new ScalarType(unionType, IsValueType: true), isArray), isArray) + : new UnsupportedNativeShape(UnsupportedShape.Union); + } + if (IsEnumSchema(leaf)) + return new UnsupportedNativeShape(UnsupportedShape.InlineEnum); + if (leaf.AdditionalProperties is not null) + return new UnsupportedNativeShape(UnsupportedShape.Dictionary); + if ((leaf.Properties?.Count ?? 0) > 0 || (leaf.Type & ~JsonSchemaType.Null) == JsonSchemaType.Object) + return new UnsupportedNativeShape(UnsupportedShape.InlineObject); + + // Nothing left to go on: no type, reference, enum, format or members - Graph writes + // these with only a description (workbookChartAxis.maximum) and kiota emits UntypedNode. + // An array is not treated this way; its element shape is decided by ClassifyProperty. + return isArray ? new UnsupportedNativeShape(UnsupportedShape.Unresolvable) : new UntypedShape(); + } + + // OData's non-finite doubles: a numeric property that may instead arrive as one of these + // sentinel strings. Their presence in a referenced enum is what identifies the encoding. + private static readonly HashSet NonFiniteNumericSentinels = + new(StringComparer.Ordinal) { "-INF", "INF", "NaN" }; + + // Graph encodes a numeric that may also carry an OData non-finite value as a union of the + // numeric, a bare string, and a reference to a string enum of the sentinels. Kiota keeps the + // numeric and drops the rest (bookingService.price generates as double?), so the numeric + // branch is the type to bind. + // + // All three conditions are required, because each rules out a different real choice: + // one numeric branch (two numerics is a choice of precision), every other branch merely + // stringish (a model or formatted-string arm would be silently discarded), and at least one + // sentinel enum (without it, "number or string" is an ordinary union whose string arm means + // something). Recognition is by enum VALUES, never by the name of the schema carrying them. + private static bool TryMapNumericUnion(IOpenApiSchema schema, Func resolveReference, out string mapped) + { + mapped = string.Empty; + var branches = schema.AnyOf ?? schema.OneOf; + if (branches is null) + return false; + + var sawSentinelEnum = false; + foreach (var branch in branches) + { + if ((branch.Type & ~JsonSchemaType.Null) is JsonSchemaType.Integer or JsonSchemaType.Number) + { + if (mapped.Length > 0) + return false; // two numerics is a genuine choice + mapped = MapNumericType(branch); + continue; + } + if (!IsStringishAlternative(branch, resolveReference, ref sawSentinelEnum)) + return false; + } + return mapped.Length > 0 && sawSentinelEnum; } + private static bool IsStringishAlternative(IOpenApiSchema branch, Func resolveReference, ref bool sawSentinelEnum) + { + if (branch.GetReferenceId() is { } id) + { + var target = resolveReference(id); + if (target is null || !IsEnumSchema(target) || (target.Type & ~JsonSchemaType.Null) != JsonSchemaType.String) + return false; + // A referenced string enum only qualifies when it carries the sentinels; any other + // enum is a meaningful alternative, not the non-finite encoding. + if (!EnumValues(target).All(NonFiniteNumericSentinels.Contains)) + return false; + sawSentinelEnum = true; + return true; + } + if (IsNullabilityPlaceholder(branch)) + return true; + return (branch.Type & ~JsonSchemaType.Null) == JsonSchemaType.String + && string.IsNullOrEmpty(branch.Format) + && (branch.Enum?.Count ?? 0) == 0; + } + + // Enum members are JSON nodes; a quoted string node renders with quotes through ToString, + // so the value is read directly where possible and unquoted otherwise. + private static IEnumerable EnumValues(IOpenApiSchema schema) + { + foreach (var node in schema.Enum ?? []) + { + if (node is null) + continue; + string? value; + try { value = node.GetValue(); } + catch (InvalidOperationException) { value = node.ToString().Trim('"'); } + catch (FormatException) { value = node.ToString().Trim('"'); } + if (value is not null) + yield return value; + } + } + + // Graph writes a nullable complex property as anyOf[ $ref, { type: object, nullable: true } ] + // (user.passwordProfile). Only that exact shape is unwrapped: exactly one branch resolves to + // a reference and every other branch is an empty nullability placeholder. Two references, or + // a branch with real content, is a genuine union and stays unsupported rather than having + // one arm silently chosen for the caller. + private static string? SingleReferenceOfNullableUnion(IOpenApiSchema schema) + { + var branches = schema.AnyOf ?? schema.OneOf; + if (branches is null || branches.Count == 0) + return null; + + string? referenceId = null; + foreach (var branch in branches) + { + var id = branch.GetReferenceId(); + if (id is not null) + { + if (referenceId is not null) + return null; + referenceId = id; + continue; + } + if (!IsNullabilityPlaceholder(branch)) + return null; + } + return referenceId; + } + + // A branch that adds nullability and nothing else: no reference, no members, no enum, no + // items, no format. + private static bool IsNullabilityPlaceholder(IOpenApiSchema schema) => + (schema.Properties?.Count ?? 0) == 0 + && (schema.Enum?.Count ?? 0) == 0 + && schema.Items is null + && schema.AdditionalProperties is null + && string.IsNullOrEmpty(schema.Format) + && (schema.AnyOf?.Count ?? 0) == 0 + && (schema.OneOf?.Count ?? 0) == 0; + + private static bool IsEnumSchema(IOpenApiSchema schema) => (schema.Enum?.Count ?? 0) > 0; + // A body property whose Pascal name matches a path parameter would emit a duplicate C# // property (PATCH /devices/{device-id} has a path id AND a body property "deviceId" — // different values: the URL takes the object id, the body carries Entra's deviceId). // The published SDK keeps both reachable by suffixing the body one with "1" // (Update-MgDevice ships -DeviceId and -DeviceId1); reproduce that convention rather - // than dropping a settable property. - public static IReadOnlyList ResolveParameterNameCollisions( - IReadOnlyList properties, IReadOnlyList pathParamNames) + // than dropping a settable property. Scalars and complex properties share one parameter + // namespace, so they are resolved together. + public static (IReadOnlyList Scalars, IReadOnlyList Complex, IReadOnlyList Untyped) ResolveParameterNameCollisions( + IReadOnlyList scalars, IReadOnlyList complex, IReadOnlyList untyped, IReadOnlyList pathParamNames) { - ArgumentNullException.ThrowIfNull(properties); + ArgumentNullException.ThrowIfNull(scalars); + ArgumentNullException.ThrowIfNull(complex); + ArgumentNullException.ThrowIfNull(untyped); ArgumentNullException.ThrowIfNull(pathParamNames); + var taken = new HashSet(pathParamNames, StringComparer.Ordinal); - return properties - .Select(p => taken.Contains(p.PascalName) ? p with { ParameterName = p.PascalName + "1" } : p) - .ToList(); - } + string Unique(string pascal) + { + var candidate = pascal; + while (!taken.Add(candidate)) + candidate += "1"; + return candidate; + } - // passwordProfile is a nested complex type, so ExtractPrimitiveProperties skips it, but - // Graph requires it to create a user. This flag lets the emitter add the two flattened - // parameters (-Password, -ForceChangePasswordNextSignIn) that make New-MgUser usable. - public static bool HasPasswordProfile(IOpenApiSchema schema) - { - ArgumentNullException.ThrowIfNull(schema); - if (schema.Properties?.ContainsKey("passwordProfile") ?? false) - return true; - return schema.AllOf?.Any(HasPasswordProfile) ?? false; + var resolvedScalars = scalars.Select(p => p with { ParameterName = Unique(p.PascalName) }).ToList(); + var resolvedComplex = complex.Select(p => p with { ParameterName = Unique(p.PascalName) }).ToList(); + var resolvedUntyped = untyped.Select(p => p with { ParameterName = Unique(p.PascalName) }).ToList(); + return (resolvedScalars, resolvedComplex, resolvedUntyped); } - // A "format" on a string (date-time, uuid, byte, ...) means Kiota maps it to a non-string - // CLR type, and an enum-valued string becomes a real enum type. Both are left out rather - // than guessing Kiota's mapping and risking a type mismatch. Schema.Type is a flags enum - // and nullable unions set the Null bit, so it is masked off before comparing. - private static bool IsPlainScalar(IOpenApiSchema schema) => (schema.Type & ~JsonSchemaType.Null) switch + // The CLR type kiota gives a formatted string. Every entry is taken from a generated Graph + // client rather than from kiota's documentation, because only the generated member type has + // to match: a wrong name is a compile error in the module. Fully qualified so a Graph model + // called Date or Time cannot capture the name, and because the emitted cmdlets do not import + // Microsoft.Kiota.Abstractions. + // IsValueType travels with the mapping rather than in a parallel set: kiota declares a + // collection of a value type with nullable elements and a reference type without, so the two + // facts have to move together or a new mapping silently gets the wrong element contract. + private sealed record ScalarType(string Name, bool IsValueType); + + private static readonly Dictionary StringFormatTypes = new(StringComparer.OrdinalIgnoreCase) { - JsonSchemaType.String => string.IsNullOrEmpty(schema.Format) && (schema.Enum?.Count ?? 0) == 0, - JsonSchemaType.Boolean or JsonSchemaType.Integer or JsonSchemaType.Number => true, - _ => false, + ["date-time"] = new("global::System.DateTimeOffset", IsValueType: true), + ["uuid"] = new("global::System.Guid", IsValueType: true), + ["duration"] = new("global::System.TimeSpan", IsValueType: true), + ["date"] = new("global::Microsoft.Kiota.Abstractions.Date", IsValueType: true), + ["time"] = new("global::Microsoft.Kiota.Abstractions.Time", IsValueType: true), + ["base64url"] = new("byte[]", IsValueType: false), // an array is a reference type + ["binary"] = new("byte[]", IsValueType: false), // no Stream members exist in the generated clients }; + // "string" -> "string[]", "int" -> "int?[]", and a non-array passes through unchanged. + // ToList() on T[] yields List, which will not assign to kiota's List, so an element + // that is a value type must be declared nullable. + private static string ArrayAware(ScalarType scalar, bool isArray) => + !isArray ? scalar.Name + : scalar.IsValueType ? scalar.Name + "?[]" + : scalar.Name + "[]"; + + // Maps a scalar schema to its CLR type. badFormat distinguishes "not a scalar at all" from + // "a scalar whose format has no verified mapping" — the latter must be reported rather than + // silently bound as string, which would compile against the wrong kiota member type. + private static bool TryMapScalar(IOpenApiSchema schema, out ScalarType mapped, out bool badFormat) + { + mapped = default!; + badFormat = false; + + // Schema.Type is a flags enum and nullable unions set the Null bit, so mask it off. + switch (schema.Type & ~JsonSchemaType.Null) + { + case JsonSchemaType.Boolean: + mapped = new ScalarType("bool", IsValueType: true); + return true; + case JsonSchemaType.Integer or JsonSchemaType.Number: + // Every numeric CLR type is a struct. + mapped = new ScalarType(MapNumericType(schema), IsValueType: true); + return true; + case JsonSchemaType.String: + // An enum-valued string is a named kiota type, not a scalar; it binds through + // its $ref like a model does. + if ((schema.Enum?.Count ?? 0) > 0) + return false; + if (string.IsNullOrEmpty(schema.Format)) + { + mapped = new ScalarType("string", IsValueType: false); + return true; + } + if (StringFormatTypes.TryGetValue(schema.Format, out var formatted)) + { + mapped = formatted; + return true; + } + badFormat = true; + return false; + default: + return false; + } + } + // Numeric mapping: when a format is present it decides the CLR type, mirroring Kiota's // own mapping, so a wrapper parameter always matches the Kiota model property it is // assigned to. Graph's docs declare Edm.Int32 as "type: number, format: int32" — going by - // the type alone would emit double? against Kiota's int? and not compile. Without a - // format, integer stays int and number stays double (fraction and 64-bit safety). - private static string MapPsType(IOpenApiSchema schema) => (schema.Type & ~JsonSchemaType.Null) switch + // the type alone would emit double? against Kiota's int? and not compile. int16 is absent + // deliberately: no generated client contains a short member, so kiota widens it to int. + // Without a format, integer stays int and number stays double (fraction and 64-bit safety). + private static string MapNumericType(IOpenApiSchema schema) => schema.Format?.ToLowerInvariant() switch { - JsonSchemaType.String => "string", - JsonSchemaType.Boolean => "bool", - JsonSchemaType.Integer or JsonSchemaType.Number => schema.Format?.ToLowerInvariant() switch - { - "int64" => "long", - "int32" => "int", - "float" => "float", - "double" => "double", - "decimal" => "decimal", - _ => (schema.Type & ~JsonSchemaType.Null) == JsonSchemaType.Integer ? "int" : "double", - }, - _ => "string", + "int64" => "long", + "int32" => "int", + "float" => "float", + "double" => "double", + "decimal" => "decimal", + "uint8" => "byte", // kiota emits byte? (rgbColor.r/g/b) + _ => (schema.Type & ~JsonSchemaType.Null) == JsonSchemaType.Integer ? "int" : "double", }; // Kiota cleans property symbols when generating model members: underscores are dropped @@ -122,8 +491,23 @@ private static string ToKiotaPropertyName(string openApiName) // Excludes properties a caller cannot or should not set. "id" is server-assigned. // "@"-prefixed names like "@odata.type" are OData control data that Kiota's serializer - // fills in from the model type, and they are not legal C# identifiers anyway. ReadOnly is - // the general OpenAPI signal for server-managed. Future exclusions of this kind belong here. - private static bool IsProtocolOrServerManagedProperty(string name, IOpenApiSchema propSchema) => - name == "id" || name.StartsWith('@') || propSchema.ReadOnly; + // fills in from the model type, and they are not legal C# identifiers anyway. + // "additionalData" is the open-type bag every kiota model already exposes through + // IAdditionalDataHolder as IDictionary; where a spec also declares it (for + // example security.alertV2) kiota does not emit a second member, so binding it would assign + // a model type to the interface's dictionary and fail to compile. ReadOnly is the general + // OpenAPI signal for server-managed. Navigation properties are relationships (user.manager, + // user.messages), addressed through their own request paths and not settable in a body; + // Graph marks them with x-ms-navigationProperty and does NOT set readOnly, so that + // extension is the only signal that keeps them out. + private static ExclusionPolicy? TryGetExclusionPolicy(string name, IOpenApiSchema propSchema) => + name switch + { + "id" => ExclusionPolicy.ServerAssignedId, + "additionalData" => ExclusionPolicy.KiotaAdditionalData, + _ when name.StartsWith('@') => ExclusionPolicy.ODataControlData, + _ when propSchema.ReadOnly => ExclusionPolicy.ReadOnlySchema, + _ when propSchema.Extensions?.ContainsKey("x-ms-navigationProperty") ?? false => ExclusionPolicy.NavigationProperty, + _ => null, + }; } diff --git a/tools/WrapperGenerator/Singularizer.cs b/tools/WrapperGenerator/Singularizer.cs index 098a7ff687..752fe839ce 100644 --- a/tools/WrapperGenerator/Singularizer.cs +++ b/tools/WrapperGenerator/Singularizer.cs @@ -100,7 +100,7 @@ public static string SingularizeWord(string word) return word[..^2]; // Businesses -> Business, Mailboxes -> Mailbox if (word.EndsWith("ss", StringComparison.Ordinal) || word.EndsWith("us", StringComparison.Ordinal) || word.EndsWith("is", StringComparison.Ordinal)) return word; // Access, Status, Analysis stay put; keeping "Whois" is a deliberate - // fix of shipped ...HostWhoi (edge-cases/naming-edge-cases.md) + // fix of shipped ...HostWhoi (docs/edge-cases/naming-edge-cases.md) if (word.EndsWith('s')) return word[..^1]; // Messages -> Message, Plans -> Plan return word; diff --git a/tools/WrapperGenerator/StderrLogger.cs b/tools/WrapperGenerator/StderrLogger.cs index 4f863fa9f0..6a2b46aee5 100644 --- a/tools/WrapperGenerator/StderrLogger.cs +++ b/tools/WrapperGenerator/StderrLogger.cs @@ -5,13 +5,14 @@ namespace WrapperGenerator; // Minimal stderr logger for CLI runs, so the generation service's skip diagnostics (for // example unsupported OData path shapes) are actually visible without taking a -// console-logging package dependency. Warning and above only: the per-file "Wrote ..." -// chatter stays quiet, and Program prints its own one-line summary. -internal sealed class StderrLogger : ILogger +// console-logging package dependency. Defaults to Warning and above: the per-file "Wrote ..." +// chatter stays quiet, and Program prints its own one-line summary. --log-level lowers the +// threshold to surface the per-property diagnostics the coverage sweep reads. +internal sealed class StderrLogger(LogLevel minimumLevel = LogLevel.Warning) : ILogger { public IDisposable? BeginScope(TState state) where TState : notnull => null; - public bool IsEnabled(LogLevel logLevel) => logLevel >= LogLevel.Warning; + public bool IsEnabled(LogLevel logLevel) => logLevel >= minimumLevel; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) { diff --git a/tools/WrapperGenerator/docs/body-property-binding.md b/tools/WrapperGenerator/docs/body-property-binding.md new file mode 100644 index 0000000000..7252247c58 --- /dev/null +++ b/tools/WrapperGenerator/docs/body-property-binding.md @@ -0,0 +1,241 @@ +# Request-body property binding + +How a request body's properties become cmdlet parameters, what each spec shape maps to, and +what is deliberately not bound. This is the durable record behind those decisions; the code +comments state the current rule, the measurements and reasoning live here. + +## The authoritative chain + +``` +OpenAPI shape -> generated Kiota member -> emitted parameter + assignment +``` + +The generated Kiota member is the contract that matters. Every type in the mapping table below +was read off a generated Graph client, not from documentation: a parameter whose CLR type +disagrees with the member it assigns is a compile error in the module, so the generated code is +the only authority worth trusting. + +The published (AutoRest) SDK is **not** part of this chain. It is a useful reference for cmdlet +and parameter *naming*, but its `IMicrosoftGraph*` interfaces are a different type system and +must never be used to decide a Kiota parameter type. + +## Classification outcomes + +Every property reached by the classifier lands in exactly one bucket. `Classify` counts the +properties it reaches independently of the buckets and throws if they disagree, so a shape that +fell through the switch fails generation rather than quietly disappearing. + +| Outcome | Meaning | +|---|---| +| scalar | bound as a CLR scalar (see mapping) | +| model | bound as a named Kiota type - a model class or an enum | +| excluded | deliberately not a parameter, under a named policy | +| unsupported | a shape with no verified Kiota type; reported per property | + +## Type mapping + +### Referenced types + +A `$ref` is resolved before it is classified: a reference is not automatically an object. +`microsoft.graph.importance` resolves to a string enum, and binding it as a model class would +not compile. Enums and models then bind through the same path, because Kiota emits both as +named types in the models namespace (`Models/Importance.cs` contains `public enum Importance`). + +The CLR name comes from `ResolveModelTypeName`, which already encodes Kiota's reserved-name +renames (`File` -> `FileObject`) and sub-namespace moves. There is deliberately no second +resolver. + +A nullable complex property is written by Graph as `anyOf[$ref, {type: object, nullable: true}]` +and is unwrapped only in that exact shape: exactly one branch resolves to a reference and every +other branch is an empty nullability placeholder. Two references, or a branch with real content, +is a genuine union and stays unsupported rather than having an arm chosen for the caller. + +### Scalars + +| OpenAPI | Kiota member type | Verified against | +|---|---|---| +| `boolean` | `bool?` | ubiquitous | +| `integer`/`number`, `int32` | `int?` | `mailFolder.childFolderCount` | +| `integer`/`number`, `int64` | `long?` | `drive.quotaUsed` | +| `number`, `float` / `double` / `decimal` | `float?` / `double?` / `decimal?` | ubiquitous | +| `integer`, `uint8` | `byte?` | `rgbColor.r/g/b` | +| `integer`, `int16` | `int?` | no `short` member exists in any generated client - Kiota widens | +| `string`, no format | `string` | ubiquitous | +| `string`, `date-time` | `DateTimeOffset?` | `user.birthday` | +| `string`, `uuid` | `Guid?` | `servicePrincipal.appId` | +| `string`, `duration` | `TimeSpan?` | `event.duration` | +| `string`, `date` | `Microsoft.Kiota.Abstractions.Date?` | `todoTask.startDate` | +| `string`, `time` | `Microsoft.Kiota.Abstractions.Time?` | `todoTask.dueTime` | +| `string`, `base64url` / `binary` | `byte[]?` | `application.logo`; no `Stream` member exists in any generated client | + +Format types are emitted fully qualified. `Date` and `Time` come from +`Microsoft.Kiota.Abstractions`, which the emitted cmdlets do not import, and qualification also +prevents a Graph model named `Date` from capturing the name. + +**Input contract for `date` and `time`.** Unlike every other scalar, these do not convert from a +string — `Date` and `Time` are Kiota's own structs and PowerShell has no string conversion for +them. They do convert from `[datetime]` (and from `[DateOnly]`/`[TimeOnly]`), which is what +`Get-Date` produces, so the usable form is: + +```powershell +-ExpirationDate (Get-Date '2026-12-31') # works +-ExpirationDate '2026-12-31' # fails to bind +``` + +Measured against a compiled module; see the runtime gate below. This is a real sharp edge and is +the reason the runtime conversion check exists as a separate gate — the parameter compiles and +satisfies the coverage oracle either way. + +An unrecognised format is reported as `UnknownFormat`, never silently bound as `string`: Kiota +would have mapped it to some other CLR type and the assignment would not compile. + +### Collections + +Kiota declares a collection of a **value** type with nullable elements and a collection of a +**reference** type without: + +```csharp +List? // value type +List? // enum - also a value type +List? // reference type +List? // reference type +``` + +`ToList()` on `T[]` yields `List`, which will not assign to `List`, so an array parameter +whose element is a value type is declared `T?[]`. Value-ness travels with each mapping rather +than in a parallel list, so a new mapping cannot acquire the wrong element contract by omission. + +This distinction was found by compiling the full 35-module population; a six-module sample +passed without it. + +### The numeric/INF union + +Graph encodes a numeric that may also carry OData's `INF`/`-INF`/`NaN` string as: + +```yaml +price: + oneOf: + - { type: number, format: double, nullable: true } + - { type: string, nullable: true } + - $ref: '#/components/schemas/ReferenceNumeric' +``` + +Kiota keeps the numeric and drops the rest (`bookingService.price` generates as `double?`), so +the numeric branch is what binds. Recognition requires all three conditions, and names no schema +or property — the referenced enum is identified by its **values**: + +1. exactly one numeric branch (two is a choice of precision); +2. every other branch merely stringish — a nullability placeholder or a plain string (a model or + formatted-string arm would otherwise be silently discarded); +3. at least one referenced string enum whose values are drawn from `-INF`, `INF`, `NaN`. + +Condition 3 is what makes this specific to the protocol encoding. Without it, an ordinary +`number | string` union — where the string arm means something — would collapse to the numeric. +All three are pinned by negative tests, none of which today's corpus exercises. + +## Exclusion policies + +These are protocol and framework rules, not Graph corpus exceptions. No endpoint, module, noun, +or incidental property name influences classification. + +| Policy | Rule | Why | +|---|---|---| +| `ServerAssignedId` | property named `id` | assigned by the service | +| `ODataControlData` | name starts with `@` | OData metadata; Kiota's serializer supplies it, and the name is not a legal C# identifier | +| `KiotaAdditionalData` | property named `additionalData` | every Kiota model already exposes this through `IAdditionalDataHolder` as `IDictionary`; binding it assigns a model type to that dictionary and fails to compile | +| `ReadOnlySchema` | `readOnly: true` | the OpenAPI signal for server-managed | +| `NavigationProperty` | `x-ms-navigationProperty: true` | a relationship addressed through its own request path, not a body field. Graph does **not** set `readOnly` on these, so the extension is the only signal that keeps them out | + +Each exclusion is emitted as a named diagnostic so an external check can distinguish a policy +exclusion from an omission without re-deriving the policy. + +## Requiredness + +No bound parameter is declared mandatory. Graph's schemas do not carry usable requiredness: +across the v1.0 specs, **10,604 of 10,742** `required:` blocks list only `@odata.type`. Any +count of "required but unbound" properties derived from the spec is therefore close to +meaningless and must not be cited as evidence that nothing important is missing. + +## Verification + +Five gates, each proving something the others cannot: + +| Gate | Proves | Cannot prove | +|---|---|---| +| `dotnet test tools/WrapperGenerator.Tests` | classification and emission rules | anything about the corpus | +| `tools/Build-WrapperModule.ps1` over every module | emitted CLR types match the generated Kiota members | that a member was omitted | +| `tools/Test-BodyBindingCoverage.ps1` | every settable member is bound or cited by a named policy | that a bound value converts at runtime | +| `tools/Test-WrapperModule.ps1` | PowerShell converts a hashtable to a model, a string to an enum, a `[datetime]` to a kiota `Date`, and 19 schema-less cases through the module's own compiled `UntypedValue` | compile-time type agreement | +| `tools/Compare-WrapperOperationInventory.ps1` | a parameter-level change did not alter which operations generate | anything about parameters | + +`tools/Measure-BodyPropertyCoverage.ps1` reports what remains unbound, by shape, as both +occurrences and distinct identities - the same inherited property repeats across every cmdlet +that binds its model, so occurrences overstate the remaining work. + +Compilation is the authority for type compatibility; the omission oracle is the authority for +omissions; runtime tests are the authority for PowerShell conversion. + +**Only some of these are independent of the classifier, and the distinction matters.** +`Test-BodyBindingCoverage.ps1` builds its expectation from the *generated kiota models* and joins +it against the *emitted parameters*, so the classifier is the subject rather than the judge — it +catches a member the classifier never mentioned. `Measure-BodyPropertyCoverage.ps1` is different: +it consumes the generator's own `Unbound`/`Excluded` diagnostics, so it reports what the +classifier says about itself. That makes it a measurement instrument, not a gate, and a zero from +it means "the classifier reported nothing unbound", never "nothing is unbound". Cite the oracle +for that claim. + +**The runtime gate refuses a stale binary.** It loads whatever is on disk, so a module last built +before the change under test would pass every check while proving nothing. `Build-` and `Test-` +both default to `Debug`, so a deliberate `-Configuration Release` build leaves a months-old +`Debug` binary in place for the test run to find — which is exactly what happened here, and it +reported green. The gate now compares the assembly's timestamp against the newest generated +source and fails with both dates rather than testing the wrong artifact. + +## Schema-less properties + +A property Graph writes with only a `description` — no type, reference, enum or format +(`workbookChartAxis.maximum`) — generates as `UntypedNode`, a base class PowerShell cannot +convert to. It binds as `object` and is converted on assignment by `UntypedValue.From` in +`Shared.g.cs`: string to `UntypedString`, integral to `UntypedInteger`/`UntypedLong`, fractional +to `UntypedDouble`/`UntypedDecimal`/`UntypedFloat`, bool to `UntypedBoolean`, array to +`UntypedArray`, hashtable recursively to `UntypedObject`. An unrecognised CLR type throws with +the type named rather than being stringified. + +**Null handling.** The published SDK's `AddIf` helper adds a value only when it is non-null and +not an empty JSON object, and no model serializer has an explicit-null path — so `{"prop": null}` +was never sendable and clearing a field that way was never possible here. Emitting an explicit +null would invent a capability the published surface does not have, so the converter omits a null +and an empty hashtable. That much is parity. + +The nested rules are an extension, and worth separating from the parity claim. AutoRest applies +`AddIf` at every level it generates — including per array element +(`autorest.powershell/powershell/llcsharp/schema/array.ts:227,235`) — so "drop, don't send null" +is its consistent behaviour rather than a top-level special case. But a caller-supplied untyped +bag has no analogue in the published SDK: every AutoRest body is a generated type, so there is no +precedent for what a null *inside* a hashtable should do. Extending the same rule is a choice, not +an inherited one. The converter therefore also drops a null nested among other members while its +siblings survive, drops a null array element, and omits an object whose members all drop out. +This is the wrapper's own input contract; it is pinned by the runtime gate rather than asserted. + +## Residual debt + +**None among the operations the generator emits: the sweep reports 0 unbound properties across all 38 specs, and the oracle 0 failures across 2,633 body-writing cmdlets.** Of the 14,131 operations in those specs only 8,164 (57.8%) generate - 767 are suppressed (the published SDK ships no cmdlet) and 5,200 are unsupported (path segments, actions, PUT, streams) - so a zero here says nothing about an operation refused upstream. The +classifications for shapes that do not occur — `Union`, `UnknownFormat`, `InlineObject`, +`InlineEnum`, `Dictionary`, `Unresolvable` — are retained deliberately so a future corpus change +is reported rather than silently mis-bound. + +See [edge-cases/body-binding-edge-cases.md](edge-cases/body-binding-edge-cases.md) for each +shape, its population, and its exit criteria. + +## Measured effect + +| | Occurrences | Distinct | +|---|---:|---:| +| Unbound before this work | 4,466 | 2,426 | +| After typed models, enums, formats and unions | 28 | 20 | +| After schema-less properties | **0** | **0** | + +`New-MgUser` went from 59 parameters to 82 over the same change **in a freshly generated tree**, +and the operation inventory is unchanged at 9,608 cmdlets — these slices altered which +*properties* bind, never which *operations* generate. The committed output under `src/` predates +this work and still shows 59; it has to be regenerated before the same figure applies there. diff --git a/tools/WrapperGenerator/docs/edge-cases/body-binding-edge-cases.md b/tools/WrapperGenerator/docs/edge-cases/body-binding-edge-cases.md new file mode 100644 index 0000000000..9204454b91 --- /dev/null +++ b/tools/WrapperGenerator/docs/edge-cases/body-binding-edge-cases.md @@ -0,0 +1,156 @@ +# Body-binding edge cases + +Request-body property shapes the generator classifies but does not bind. **Every shape reaching +the classifier now binds — the sweep reports 0 unbound properties across all 38 specs.** That is +a statement about the 8,164 operations (57.8% of 14,131) that generate: an operation refused +upstream — 767 suppressed because the published SDK ships no cmdlet, 5,200 unsupported shapes — +contributes no properties to any count in this file. What +remains here is one closed entry recording how the last gap was shut, and several classifications +with zero population that are retained deliberately: they exist so a future corpus change is +reported accurately instead of being silently mis-bound, and each is reported per property at +`--log-level Information` and counted by `tools/Measure-BodyPropertyCoverage.ps1`. + +The type evidence and the policies behind what is bound live in +[../body-property-binding.md](../body-property-binding.md). + +Entry template (keep field names exact so the file converts cleanly): + +``` +## +- **Class:** unsupported-shape +- **Status:** deferred | blocked | investigating +- **Counts:** occurrences / distinct (v1.0, ) +- **Evidence:** +- **Why unsafe today:** +- **Intended representation:** +- **Exit criteria:** +- **References:** +``` + +## Untyped (`UntypedNode`) — CLOSED + +- **Class:** unsupported-shape +- **Status:** closed 2026-08-13; kept as a record of how it was closed +- **Counts:** was 28 occurrences / 20 distinct (26 Files workbook internals — `maximum`, + `minimum`, `majorUnit`, `minorUnit`, `value`, `values` — and 2 + `CrossDeviceExperiences.MgUserActivity.contentInfo`). Now **0**. +- **Evidence:** the schema carries no type, reference, enum or format at all — Graph writes these + with only a `description` (`workbookChartAxis.maximum`) — and kiota emits `UntypedNode?`. + `UntypedNode` is a non-abstract base with ten subclasses; PowerShell cannot convert to the base + (`[UntypedNode]'hello'` fails), so the parameter could not be typed as the model member. +- **How it was closed:** the parameter binds as `object` and a shared `UntypedValue.From` helper + in `Shared.g.cs` converts on assignment — string to `UntypedString`, integral to + `UntypedInteger`/`UntypedLong`, fractional to `UntypedDouble`/`UntypedDecimal`/`UntypedFloat`, + bool to `UntypedBoolean`, array to `UntypedArray`, hashtable recursively to `UntypedObject`. + A `PSObject` wrapper is unwrapped first. An unrecognised CLR type throws with the type named + rather than being stringified, so an unsupported value cannot be silently sent. +- **Null handling, and how much of it is parity.** The published SDK's `AddIf` helper + (`src///generated/runtime/Extensions.cs`) adds a value only when it is + non-null **and not an empty JSON object**, and no model serializer has any explicit-null path — + so `{"prop": null}` was never sendable from this SDK and clearing a field that way was never + possible. Omitting a null and an empty hashtable is therefore parity. The *nested* rules are an + extension: AutoRest applies `AddIf` at every level it generates, including per array element + (`autorest.powershell/powershell/llcsharp/schema/array.ts:227,235`), but every AutoRest body is + a generated type, so a caller-supplied untyped bag has no published analogue and no precedent + for what a null inside it should do. Extending the same rule — dropping a nested null while its + siblings survive, dropping a null array element, omitting an object whose members all drop out — + is the wrapper's own documented contract, chosen for consistency and pinned by the runtime gate. +- **Verified:** 19 conversions runtime-tested by `tools/Test-WrapperModule.ps1` against each + module's own compiled `UntypedValue` (reached by reflection, so the gate cannot drift from a + copy of the converter), covering every branch: the seven numeric types, string, boolean, + `PSObject` unwrapping, object, array, nesting, nested-null drop, null array element drop, + empty-object omission, all-null-object omission, and the throw on an unsupported type. The + helper is emitted into every module, so the gate reports `OK(19)` for all 35 that produce a + manifest and treats a missing helper as a failure rather than N/A. Negative-tested: removing the + empty-object omission from a + module and rebuilding produced + `FAILED: empty object omitted: sent UntypedObject; all-null object omitted: sent UntypedObject`. +- **References:** issue #3707; `UntypedValue` in `CmdletEmitter.EmitSharedAuth`; + `SchemaProperties.UntypedProperty`. + +## Genuine unions + +- **Class:** unsupported-shape +- **Status:** deferred — zero population in v1.0 +- **Counts:** 0. Before the numeric/INF family was bound this shape reported 56 occurrences / + 33 distinct; every one of them was that family, so nothing remains + (`Measure-BodyPropertyCoverage.ps1`, 2026-08-12). +- **Evidence:** Graph's only union in the v1.0 corpus is a numeric with OData's `INF`/`NaN` + string alternative, which kiota resolves to the numeric and which the generator now binds. + A union whose branches are materially different schemas does not occur here — but the + classification is retained so one would be reported rather than silently mis-bound. +- **Why unsafe today:** binding one arm silently commits the caller to a type the API may not + want. Unlike the numeric family there is no branch kiota itself privileges, so there is no + evidence for which arm is right. +- **Intended representation:** most likely a parameter per arm, or a single parameter typed as + the shared base where one exists. Needs published-surface evidence before choosing. +- **Exit criteria:** the residual unions are enumerated, grouped by shape, and each group has a + kiota member type that a chosen representation demonstrably matches. +- **References:** `UnsupportedShape.Union`; `SchemaProperties.TryMapNumericUnion`. + +## Inline objects and inline enums + +- **Class:** unsupported-shape +- **Status:** deferred — zero population **among the operations that generate**, which is not + the same as zero in v1.0. +- **Counts:** 0 occurrences (`Measure-BodyPropertyCoverage.ps1`, 2026-08-12, all 38 specs). +- **Evidence:** the sweep produced no `InlineObject` or `InlineEnum` classification. For entity + CRUD bodies that is a real property of the corpus: Graph declares those objects and enums as + component `$ref`s, which is why referenced-type binding covers them. +- **Why the count is conditional:** action bodies are where Graph *does* write inline objects, + and they never reach the property classifier — an action's `requestBody` is a `$ref` to a + **requestBodies** component whose schema is an inline `type: object`, and the generator skips + the whole operation first (1,528 POSTs corpus-wide, logged as `missing supported request JSON + schema`). This population becomes non-zero the moment action generation lands. +- **Why unsafe today:** kiota synthesises a type name for an anonymous schema from its parent + and property, and that name cannot be derived from the spec alone. Guessing it is the failure + mode that produced 39 compile errors when numeric formats were first mapped. +- **Intended representation:** none required while the population is zero. The classifications + are retained deliberately so a future spec shape is reported accurately instead of being + mislabelled as something else. +- **Exit criteria:** revisit only if a corpus sweep reports a non-zero count — at which point + the kiota name must be read from a generated client before any mapping is written. +- **References:** `UnsupportedShape.InlineObject`, `UnsupportedShape.InlineEnum`. + +## Unknown string formats + +- **Class:** unsupported-shape +- **Status:** deferred — zero population in v1.0 +- **Counts:** 0 occurrences (`Measure-BodyPropertyCoverage.ps1`, 2026-08-12, all 38 specs); + every format present in the corpus is mapped. +- **Evidence:** the format inventory across all v1.0 specs is `date-time`, `int32`, `int64`, + `double`, `binary`, `base64url`, `uuid`, `time`, `date`, `duration`, `int16`, `float`, + `uint8`, `decimal` — all mapped. +- **Why unsafe today:** an unmapped format bound as `string` would compile against whatever + other CLR type kiota chose, or not compile at all. Reporting keeps the failure visible. +- **Exit criteria:** a new format appears in a sweep; its kiota member type is read from a + generated client and added to the mapping with a pinned test. +- **References:** `UnsupportedShape.UnknownFormat`; `SchemaProperties.StringFormatTypes`. + +## Dictionaries (`additionalProperties`) + +- **Class:** unsupported-shape +- **Status:** deferred — zero population in v1.0 +- **Counts:** 0 occurrences (`Measure-BodyPropertyCoverage.ps1`, 2026-08-12, all 38 specs). +- **Evidence:** no property classified as `Dictionary` in the sweep. Free-form + bags in Graph reach the caller through the `additionalData` member instead, which is excluded + by policy. +- **Why unsafe today:** untested; kiota's representation of an open map property has not been + observed in a generated client here, so any mapping would be a guess. +- **Exit criteria:** a non-zero count, then the same read-it-from-the-client procedure. +- **References:** `UnsupportedShape.Dictionary`; `ExclusionPolicy.KiotaAdditionalData`. + +## Unresolvable references and untyped arrays + +- **Class:** unsupported-shape +- **Status:** deferred — zero population in v1.0 +- **Counts:** 0 occurrences (`Measure-BodyPropertyCoverage.ps1`, 2026-08-13, all 38 specs). +- **Evidence:** three distinct situations share this classification — an array whose `items` + schema is absent, a `$ref` whose target is not in the document, and a `$ref` to a bare scalar + that has no kiota type of its own. None occurs in the corpus. +- **Why unsafe today:** unlike a schema-less property, which reliably generates as + `UntypedNode` and can therefore be bound, these have no predictable kiota member type at all. + A broken reference in particular is a spec defect; binding past it would hide the defect. +- **Exit criteria:** a non-zero count, then read the generated member type from a client and + decide per situation — they may not share one answer. +- **References:** `UnsupportedShape.Unresolvable`; `SchemaProperties.ClassifyLeaf`. diff --git a/tools/WrapperGenerator/edge-cases/crosspath-merge-edge-cases.md b/tools/WrapperGenerator/docs/edge-cases/crosspath-merge-edge-cases.md similarity index 100% rename from tools/WrapperGenerator/edge-cases/crosspath-merge-edge-cases.md rename to tools/WrapperGenerator/docs/edge-cases/crosspath-merge-edge-cases.md diff --git a/tools/WrapperGenerator/edge-cases/kiota-alignment-edge-cases.md b/tools/WrapperGenerator/docs/edge-cases/kiota-alignment-edge-cases.md similarity index 100% rename from tools/WrapperGenerator/edge-cases/kiota-alignment-edge-cases.md rename to tools/WrapperGenerator/docs/edge-cases/kiota-alignment-edge-cases.md diff --git a/tools/WrapperGenerator/edge-cases/naming-edge-cases.md b/tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md similarity index 88% rename from tools/WrapperGenerator/edge-cases/naming-edge-cases.md rename to tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md index 309c64f5e8..f0505140b2 100644 --- a/tools/WrapperGenerator/edge-cases/naming-edge-cases.md +++ b/tools/WrapperGenerator/docs/edge-cases/naming-edge-cases.md @@ -184,6 +184,28 @@ Entry template (keep the field names exact so the file converts cleanly): - **References:** issue #3704 (remainder inventory + resolver evidence); `NamingOverrides.cs` "Collision resolutions" section. +## `-Password` / `-ForceChangePasswordNextSignIn` replaced by typed `-PasswordProfile` + +- **Class:** wrapper-surface-change +- **Status:** corrected +- **Evidence:** while body binding was primitives-only, `passwordProfile` was hard-coded into + two invented parameters so `New-MgUser` was usable at all. Neither name is published: the + shipped SDK exposes `-PasswordProfile` as a typed parameter taking a hashtable, and + `passwordProfile` is an ordinary complex property in the spec + (`anyOf[$ref microsoft.graph.passwordProfile, nullable]`), not a special case. +- **Decision:** typed binding covers every referenced-model property, so the hard-coded pair + was deleted along with the flag that emitted it. `New-MgUser -PasswordProfile @{ Password = + '...'; ForceChangePasswordNextSignIn = $true }` replaces them and matches the published + surface. +- **Migration impact:** breaking for anyone using the prototype's `-Password` / + `-ForceChangePasswordNextSignIn`. This changes only the wrapper prototype's own surface - + no published cmdlet had these parameters - and it moves toward parity rather than away. + One behaviour note: the removed code defaulted `ForceChangePasswordNextSignIn` to `true` + when only `-Password` was supplied; the typed parameter passes through exactly what the + caller sets, matching Graph's own default handling. +- **References:** issue #3707; `SchemaProperties.Classify`; `EmitsComplexPropertyAsTypedModelParameter` + in EmitterTests. + ## Watch list Cases spotted but deliberately not acted on yet, so they aren't lost: