fix(csharp): skip mock test examples with empty path parameters#14879
fix(csharp): skip mock test examples with empty path parameters#14879Swimburger merged 3 commits intomainfrom
Conversation
When an endpoint example has an empty string path parameter, the mock server URL collapses the empty segment (e.g. /v0/tools/version/1) while the SDK client sends a double-slash URL (e.g. /v0/tools//version/1), causing WireMock to return 404. Filter out such examples from mock server test generation. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
… to 2.59.5 - Fix dynamic snippet generation for literal types when generate-literals is enabled: inline literals emit nop (relying on default initializer), named literal aliases emit `new TypeName()` instead of raw strings - Fix `list<file>` upload properties generating single FileParameter instead of List<FileParameter> - Move empty-path-parameter skip fix to new 2.59.5 release - Remove several fixtures from allowedFailures (now passing)
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (this.settings.generateLiterals) { | ||
| return this.csharp.Literal.nop(); | ||
| } |
There was a problem hiding this comment.
🔴 convertLiteral blanket nop() return drops literal path parameter arguments, causing compilation errors
When generateLiterals is enabled, convertLiteral returns nop() unconditionally (line 145-146). This is correct for inline object properties (where Class_ literal filters out nop fields at Literal.ts:115), but breaks path parameters that are passed as standalone method arguments. The MethodInvocation.write() method (MethodInvocation.ts:95-106) does NOT filter out nop arguments — it just calls arg.write(writer) which writes nothing. This causes the literal path parameter value to be silently dropped from the method call.
The generated snippet for the literal/readonly-constants fixture confirms this: await client.Path.SendAsync() is missing the required string id argument (see PathClient.cs:83-86 which requires string id). The fallbackToDefault parameter set at EndpointSnippetGenerator.ts:860 is never reached because the early return on line 146 fires first.
Affected code flow
getPathParameters(EndpointSnippetGenerator.ts:855-862) callsconvert()on each path param- For
literal<"123">,convert()dispatches toconvertLiteral() convertLiteral()returnsnop()at line 146 before any value logic- The nop is used as a method argument at EndpointSnippetGenerator.ts:779 or :584
MethodInvocationrenders the nop as empty output- Result:
SendAsync()instead ofSendAsync("123")— CS7036 compilation error
Prompt for agents
The convertLiteral method in DynamicLiteralMapper.ts has a blanket early return that returns nop() when generateLiterals is true. This is too aggressive — it works for inline object properties (which are filtered by Class_ literal's nop filtering in Literal.ts:115), but breaks literal values used as standalone method arguments (path parameters, body request arguments), because MethodInvocation does not filter out nop arguments.
The fix needs to preserve the nop behavior for inline properties while still emitting literal values when they're needed as method arguments. Possible approaches:
1. Remove the early return from convertLiteral and instead handle literal suppression at the object-property level. The Class_ literal already filters nop fields, so you could keep returning the actual literal value and add a separate check in convertObject or at the ConstructorField level to skip literal-typed properties when generateLiterals is on.
2. Add a context flag (e.g., suppressLiterals: boolean) to the DynamicLiteralMapper.Args interface, defaulting to false. Only callers that generate inline properties (convertObject, getInlinedRequestBodyPropertyConstructorFields, header/query fields) would pass suppressLiterals: true. Path parameter callers in EndpointSnippetGenerator.getPathParameters would not set this flag.
3. In EndpointSnippetGenerator.getPathParameters, special-case literal type references: when generateLiterals is true and the parameter typeReference.type is 'literal', emit the literal value directly instead of going through convert().
The affected callers are in EndpointSnippetGenerator.ts: getPathParameters (line 843), getMethodArgsForBodyRequest (line 767), and getMethodArgsForInlinedRequest (line 556). Files: generators/csharp/dynamic-snippets/src/context/DynamicLiteralMapper.ts and generators/csharp/dynamic-snippets/src/EndpointSnippetGenerator.ts.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…-empty-path-params
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
2 participants
Description
Refs #14875
Bundles several C# generator fixes into version 2.59.5:
Skip mock test examples with empty path parameters — When an OpenAPI spec provides an
x-fern-examplesentry with an empty string path parameter (e.g.,"id": ""), WireMock collapses the path while the SDK preserves the double-slash, causing a 404. These examples are now filtered out.Fix dynamic snippet generation for literal types with
generate-literalsenabled — Inline literal properties now emit a nop (relying on the= new()default initializer in the C# model), and named literal alias types emitnew TypeName()instead of a raw string, preventing CS0029 compilation errors.Fix
list<file>upload properties —fileArrayproperties were incorrectly generating a singleFileParameterfield instead ofList<FileParameter>.Add precision fallbacks to JsonElementComparer — Handles float/double values where the decimal representation differs from the original JSON string.
Changes Made
CsharpTypeMapper.ts: WrapfileArraycase inCollection.list()DynamicLiteralMapper.ts: Returnnop()for inline literals whengenerateLiteralsis on; emitnew TypeName()for named literal aliasesDynamicTypeMapper.ts: Return class reference for named literal alias types whengenerateLiteralsis onversions.yml: Bump to 2.59.5 with changelog entriesseed.yml: Remove several fixtures fromallowedFailures(now passing)Testing
Link to Devin session: https://app.devin.ai/sessions/1dcf1274fbe74a7cbac9f76e9a65ed28
Requested by: @Swimburger