Skip to content

Read roles declared with the Roles constructor form - #2419

Merged
woksin merged 2 commits into
mainfrom
fix/proxy-generator-roles-constructor-form
Jul 29, 2026
Merged

Read roles declared with the Roles constructor form#2419
woksin merged 2 commits into
mainfrom
fix/proxy-generator-roles-constructor-form

Conversation

@woksin

@woksin woksin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fixed

claude and others added 2 commits July 29, 2026 13:39
#2381)

The role extraction only inspected named arguments, so the idiomatic
[Roles("...")] attribute — which sets Roles via a params string[] constructor —
yielded no roles. Also read the constructor argument array so both the
constructor and named-argument forms produce roles.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B1mrPcRT2GYjRxrx4T1p8N
Cratis.Arc.Authorization and Microsoft.AspNetCore.Authorization both declare
an Authorize attribute, so importing both left every use of it ambiguous and
the fixture did not compile. The named-argument form is the ASP.NET Core one,
which is now written out in full.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@woksin woksin added the patch label Jul 29, 2026
@woksin

woksin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer context — deliberately kept out of the description.

Provenance. This is one commit extracted from #2417 (43ac8e46), rebased onto main and made to compile. It is the only part of that PR not already implemented in #2418 — the Screenplay fixes there (#2395, #2397, #2398, #2400) are all covered by #2418, with specs and measurements against a real application.

It did not compile as written. #2417's reviewer note says it was authored in an environment without a .NET SDK and relies on CI. Cherry-picked onto main, the fixture failed with CS0104: 'Authorize' is an ambiguous referenceCratis.Arc.Authorization and Microsoft.AspNetCore.Authorization both declare an Authorize attribute and both were imported. Aliasing it then failed with CS1614 (ambiguous between the attribute and its Attribute-suffixed form), so the ASP.NET Core attribute is now written out in full where the fixture needs its named-argument form. A third failure followed in Release only — RCS1181, comment must be a documentation comment — which is the usual reminder that Release is the real gate here, since analyzers are errors only there.

The production change itself (MethodInfoExtensions.cs, reading ConstructorArguments as well as NamedArguments) was correct as written and needed no change.

This is agent-authored code that no human has read. Do not merge without explicit review.

Verification

  • ProxyGenerator.csproj Release: 0 errors, 0 warnings.
  • ProxyGenerator.Specs Release: the four new when_extracting_roles specs pass on net8.0 and net9.0 — constructor form with one role, with several roles, the named-argument form, and both forms combined.

Note on #2417. Four of its five commits duplicate work already in #2418, in overlapping files, so the two cannot both merge cleanly. Its #2397 release note also overstates the effect: on a real application that recovers nothing, because every WithMessage there resolves to a resx designer property with no constant value — which is why #2409 exists and is implemented in #2418.

@woksin
woksin merged commit d3d2cdd into main Jul 29, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants