Skip to content

fix: clear every build warning and move generation fixups into templates - #39

Merged
ariesclark merged 4 commits into
mainfrom
fix/generated-build-warnings
Sep 3, 2026
Merged

fix: clear every build warning and move generation fixups into templates#39
ariesclark merged 4 commits into
mainfrom
fix/generated-build-warnings

Conversation

@binn

@binn binn commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

@ariesclark for review.

Closes #32.
Closes #37 — supersedes it, fix incorporated with credit to @Powerbyte7.

The CI build emits 674 warnings. This fixes all of them at their source, and removes the post-generation sed passes that caused several.

Verified by regenerating against spec v1.20.9-nightly.24 with generator 7.17.0 and building the solution: 0 warnings, 0 errors, all six assemblies. src/ is untouched here — the fixes land on the next generation.

Warnings

Code Count Cause Fix
CS8073 416 Equals/GetHashCode gate their null checks on vendorExtensions.x-is-value-type, which openapi-generator computes from a hardcoded list that omits DateTime and DateOnly Use x-csharp-value-type, derived from the emitted C# type, and already used by the constructor block in the same template
CS0436 4 generate.sh copied wrapper/VRChat.API/Client/* into src/, so VRChat.API.Wrapper compiled those files and referenced VRChat.API, which already contained them Compile the wrapper straight from wrapper/; one definition each
CS1591 60 VRChatClient and IVRChatClientFactory.TryAddClient undocumented <inheritdoc/> for the 23 IVRChat implementations, prose for the rest
CS1572 2 <param name="auth"> on WithCredentials, which has no such parameter Removed, and the empty summary filled in

Four of the CS8073 sites are visible in the failing job.

generate.sh: 92 → 59 lines, 19 seds → 2, 9 rms → 0

Each sed keyed on a string the generator may rename at any time, and sed exits 0 when it matches nothing, so a stale one failed silently. Four were already dead:

  • new Cookie(cookie.Name, cookie.Value) — the httpclient library's ApiClient.mustache uses transformed.Cookies.Add(cookie); that string is nowhere in the output.
  • the /System.ComponentModel.Annotations/a README injection — that PackageReference is {{^net60OrLater}}-guarded, so it never exists on net8.0.
  • rm build.sh / build.bat / mono_nunit_test.sh / nuget.exe — generator 7.17.0 emits none of them.

Where the rest went:

  • .openapi-generator-ignore now covers docs/, api/, src/VRChat.API.Test/, appveyor.yml and git_push.sh. They are never written, so there is nothing to delete.
  • netcore_project.additions.mustache, the generator's own csproj extension point, carries the wrapper sources, the Otp.NET reference, the packaged assets and the property overrides. It renders last, so its <PropertyGroup> wins on MSBuild's last-assignment-wins rule — which is why netcore_project.mustache goes back to completely stock, one fewer forked template to re-merge on the next generator upgrade.
  • Package metadata moved to --additional-properties. This also fixes the copyright: its © was UTF-8 encoded twice by the sed, so the package shipped the bytes c3 82 c2 a9 where c2 a9 was meant. packageTags and releaseNote cannot go there — the CLI splits that flag on commas — so they are set in the additions file.
  • Cookie path/domain and credential URL-encoding moved into templates/libraries/httpclient/api.mustache. Note this is the library-specific template; with --library httpclient it shadows templates/api.mustache, so editing the root one compiles fine and changes nothing.
  • ITwoFactorCode and CurrentUser.RequiresTwoFactorAuth are now partial classes in wrapper/VRChat.API/Model/. The latter is a spec gap: the specification models requiresTwoFactorAuth on TwoFactorRecoveryCodes, not CurrentUser.
  • The banner strip over ~370 files became a four-line deletion in partial_header.mustache.

One patch to generated output survives: the IsRequired relaxation on CurrentUser, which rewrites attributes on generated members, so neither a template nor a partial class can reach it. Closing it properly needs the spec to mark those fields optional. It now runs through a helper that probes with grep -qF first and aborts with a message, so a patch that stops applying is loud rather than silent. set -euo pipefail is on.

Percent-encoded parentheses (#37)

VRChat answers 400 "malformed url" when ( and ) arrive percent-encoded, and instance IDs contain them:

wrld_0000:12345~group(grp_0000)~groupAccessType(plus)~region(use)

#37 by @Powerbyte7 restored them with a sed over the generated WebRequestPathBuilder.cs. That file comes from templates/WebRequestPathBuilder.mustache, so the fix goes there instead — behind one Escape helper used by both the path and query paths, rather than a chained Replace at each call site.

Un-escaping is safe because Uri.EscapeDataString encodes % first, so a literal %28 in a value becomes %2528 and cannot be corrupted by the replacement.

VRChatClientBuilder XML docs (#32)

Twelve members carried empty <summary>, <param> and <returns> tags. Those do not raise CS1591 — a tag is present — so they survived the warning sweep while being just as useless to a caller. Now written out, including what WithApplication is for and why VRChat wants it.

Two things surfaced while writing them:

  • Fixed: WithAuthCookie set the twoFactorAuth key from the auth argument, so a two-factor cookie passed to it was discarded and the auth token stored twice.
  • Fixed: WithAuthCookie stored cookie values with AddApiKeyPrefix rather than AddApiKey. GetApiKeyWithPrefix returns prefix + " " + value whenever a prefix exists, and no value was ever set, so both cookies went out as the token followed by a trailing space — Cookie: auth=authcookie_0000 . VRChat evidently tolerated it, which is why it went unnoticed. These two calls were the only writers of ApiKeyPrefix in the repository, so nothing depended on the old placement.

Also

  • VRChat.API.Wrapper.csproj is deleted (and its .sln entry). Its sources now belong to a real project for the IDE too, and its stale <Version>1.20.5</Version> goes with it.
  • DefaultVRChatClientFactory.TryAddClient called Dictionary.Add, which throws when the key exists — so overrideIfExists: true never overrode, it threw ArgumentException. Uses the indexer now.
  • The publish job checks out the repo and overlays the artifact, so anything the generator stops emitting survives forever. It now clears src/ first, which removes the orphaned Model/GroupRoleTemplateValuesRoles.cs — unreferenced, and absent from .openapi-generator/FILES.

Verification

Regenerated from a clean repo copy, then built VRChat.API.sln -c Release. Normalising every changed line in the src/ diff leaves exactly four shapes: 208 CS8073 removals, 348 CS0612 pragmas (your commit f345bf3 postdates the last generation), 386 stripped banner lines, and the doc comments. VRChatClient, IVRChat, VRChatClientBuilder, VRChatLoginResult, ITwoFactorCode and CurrentUser.RequiresTwoFactorAuth all appear in the shipped VRChat.API.xml, so nothing was dropped on the way.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CpDekr34WgDkSS5myqf8m9

The CI build emitted 674 warnings. All of them are now fixed at their source,
and the post-generation sed passes that caused several of them are gone.

Warnings

  CS8073 (416) Equals/GetHashCode gated their null checks on
    vendorExtensions.x-is-value-type, which openapi-generator computes from a
    hardcoded list that omits DateTime and DateOnly, so required date properties
    got a null check the compiler proved dead. Switched to
    x-csharp-value-type, which is derived from the emitted C# type and is
    already used by the constructor block in the same template.

  CS0436 (4) generate.sh copied wrapper/VRChat.API/Client/* into
    src/VRChat.API/Client/, so VRChat.API.Wrapper compiled those files and also
    referenced VRChat.API, which already contained them. The wrapper is now
    compiled straight from wrapper/ by the generated csproj, so each type has a
    single definition and VRChat.API.Wrapper.csproj is deleted.

  CS1591 (60) / CS1572 (2) Documented VRChatClient, whose IVRChat members take
    <inheritdoc/> from the already-documented interface, plus
    IVRChatClientFactory.TryAddClient, and dropped a <param name="auth"> tag on
    WithCredentials, which has no such parameter.

generate.sh: 92 lines to 59, 19 seds to 2, 9 rm lines to 0

  .openapi-generator-ignore now covers docs/, api/, src/VRChat.API.Test/,
  appveyor.yml and git_push.sh, so they are never written rather than deleted
  afterwards. rm build.sh, build.bat, mono_nunit_test.sh and nuget.exe were
  dead: generator 7.17.0 does not emit them.

  netcore_project.additions.mustache, the generator's own csproj extension
  point, now carries the wrapper sources, the Otp.NET reference, the packaged
  assets and the property overrides. It renders last, so its PropertyGroup wins
  and netcore_project.mustache stays stock.

  Package metadata moved to --additional-properties. This also fixes the
  copyright: its (c) sign was UTF-8 encoded twice by the sed, so the package
  shipped the bytes c3 82 c2 a9 where c2 a9 was meant. Passing the string once
  through a generator property encodes it correctly. packageTags and
  releaseNote cannot go there, because the CLI splits that flag on commas, so
  they are set in the additions file.

  Cookie path/domain and credential URL-encoding moved into the httpclient
  api.mustache. A third cookie sed and a README injection sed were dead, both
  keyed on strings absent from the output.

  ITwoFactorCode and CurrentUser.RequiresTwoFactorAuth are now partial classes
  in wrapper/VRChat.API/Model/. The latter is a spec gap: the specification
  models requiresTwoFactorAuth on TwoFactorRecoveryCodes, not CurrentUser.

  One patch to generated output survives, the IsRequired relaxation on
  CurrentUser, which no template or partial class can express. It now runs
  through a helper that probes with grep first, since sed exits 0 when it
  matches nothing and set -e would never catch a patch that stopped applying.

Also

  DefaultVRChatClientFactory.TryAddClient called Dictionary.Add, which throws
  when the key exists, so overrideIfExists: true never overrode. Uses the
  indexer now.

  The publish job overlays the artifact onto a checkout, so files the generator
  stops emitting survived forever. It now clears src/ first. This removes the
  orphaned Model/GroupRoleTemplateValuesRoles.cs.

Verified by regenerating against spec v1.20.9-nightly.24 with generator 7.17.0
and building the solution: 0 warnings, 0 errors, all six assemblies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CpDekr34WgDkSS5myqf8m9
@binn
binn requested a review from ariesclark September 3, 2026 04:56
Two fixes that the template restructuring in the previous commit makes
straightforward.

Parentheses (#37, by @Powerbyte7)

  VRChat answers 400 "malformed url" when "(" and ")" arrive percent-encoded,
  and instance IDs contain them:

    wrld_0000:12345~group(grp_0000)~groupAccessType(plus)~region(use)

  #37 restored them with a sed over the generated WebRequestPathBuilder.cs.
  That file comes from templates/WebRequestPathBuilder.mustache, so the fix
  goes there instead, behind one Escape helper used by both the path and query
  paths rather than a chained Replace at each call site.

  Un-escaping is safe because Uri.EscapeDataString encodes "%" first, so a
  literal "%28" in a value becomes "%2528" and cannot be corrupted.

XML docs (#32)

  Twelve VRChatClientBuilder members carried empty <summary>, <param> and
  <returns> tags. These do not raise CS1591, because a tag is present, so they
  survived the warning sweep while being just as useless to a caller. Written
  out, including what WithApplication is for and why VRChat wants it.

  While writing them: WithAuthCookie set the twoFactorAuth key from the auth
  argument. Two-factor cookies passed to it were discarded and the auth token
  was stored twice.

  Left alone, because it changes authentication behaviour and cannot be
  verified without a live account: WithAuthCookie stores cookie values with
  AddApiKeyPrefix rather than AddApiKey, so GetApiKeyWithPrefix returns
  "<token> ", the token with a trailing space, since no key value is ever set.

Regenerated and rebuilt: still 0 warnings, 0 errors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CpDekr34WgDkSS5myqf8m9
binn and others added 2 commits September 3, 2026 00:01
WithAuthCookie called AddApiKeyPrefix, which writes to ApiKeyPrefix rather than
ApiKey. GetApiKeyWithPrefix returns `prefix + " " + value` whenever a prefix
exists, and no value was ever set, so both cookies were sent as the token
followed by a trailing space:

    Cookie: auth=authcookie_0000<space>

VRChat evidently tolerated the trailing space, which is why this went
unnoticed. AddApiKey stores the value where GetApiKeyWithPrefix reads it, and
the header is now exact.

These two calls were the only writers of ApiKeyPrefix in the repository, so
nothing else depended on the old placement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CpDekr34WgDkSS5myqf8m9
@ariesclark
ariesclark merged commit b00b096 into main Sep 3, 2026
9 checks passed
@ariesclark
ariesclark deleted the fix/generated-build-warnings branch September 3, 2026 05:26
@ariesclark
ariesclark restored the fix/generated-build-warnings branch September 3, 2026 05:43
@ariesclark
ariesclark deleted the fix/generated-build-warnings branch September 3, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Update XMLDoc for VRChatClientBuilder

2 participants