From 171eedd07f238eae91d3082e33da197294b620e0 Mon Sep 17 00:00:00 2001 From: sophia-ramsey Date: Tue, 8 Sep 2026 14:30:40 -0700 Subject: [PATCH 1/2] fix(http-server-csharp): avoid duplicate nullable suffixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...-duplicate-nullable-suffixes-2026-09-08.md | 7 +++ .../components/interfaces/interfaces.test.tsx | 34 ++++++++++++++- .../src/components/interfaces/interfaces.tsx | 10 ++++- .../scaffolding/mock-implementations.tsx | 21 ++++++--- .../type-expression/type-expression.tsx | 11 ++++- .../test/nullable-parameters.test.ts | 43 +++++++++++++++++++ 6 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 .chronus/changes/sramsey-csharp-duplicate-nullable-suffixes-2026-09-08.md create mode 100644 packages/http-server-csharp/test/nullable-parameters.test.ts diff --git a/.chronus/changes/sramsey-csharp-duplicate-nullable-suffixes-2026-09-08.md b/.chronus/changes/sramsey-csharp-duplicate-nullable-suffixes-2026-09-08.md new file mode 100644 index 00000000000..16727ede5e1 --- /dev/null +++ b/.chronus/changes/sramsey-csharp-duplicate-nullable-suffixes-2026-09-08.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Prevent optional nullable value parameters from emitting duplicate nullable suffixes in generated C# interfaces and mocks. diff --git a/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx b/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx index d48f245e5cf..de7ed1edac3 100644 --- a/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx +++ b/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx @@ -1,9 +1,10 @@ import { Tester } from "#test/tester.js"; import { type Children } from "@alloy-js/core"; -import { createCSharpNamePolicy, SourceFile } from "@alloy-js/csharp"; +import { createCSharpNamePolicy, EnumDeclaration, SourceFile } from "@alloy-js/csharp"; import { t, type TesterInstance } from "@typespec/compiler/testing"; import { Output } from "@typespec/emitter-framework"; import { beforeEach, expect, it } from "vitest"; +import { efRefkey } from "../type-expression/type-expression.jsx"; import { BusinessLogicInterface } from "./interfaces.jsx"; let runner: TesterInstance; @@ -61,3 +62,34 @@ it("renders an interface with void return type", async () => { } `); }); + +it("renders one nullable suffix for optional nullable value parameters", async () => { + const { Choice, PetStore } = await runner.compile(t.code` + enum ${t.enum("Choice")} { + one, + } + + interface ${t.interface("PetStore")} { + update(value?: int32 | null, choice?: Choice | null): void; + } + `); + + expect( + + + One + + + + , + ).toRenderTo(` + enum Choice + { + One + } + public interface IPetStore + { + Task UpdateAsync(int? value, Choice? choice); + } + `); +}); diff --git a/packages/http-server-csharp/src/components/interfaces/interfaces.tsx b/packages/http-server-csharp/src/components/interfaces/interfaces.tsx index 6d9374001e0..5eecce71cb6 100644 --- a/packages/http-server-csharp/src/components/interfaces/interfaces.tsx +++ b/packages/http-server-csharp/src/components/interfaces/interfaces.tsx @@ -7,7 +7,10 @@ import { getDocComments } from "@typespec/emitter-framework/csharp"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { getUniqueItems } from "@typespec/json-schema"; import { getSuccessReturnType } from "../../utils/return-type-helpers.js"; -import { TypeExpression } from "../type-expression/type-expression.jsx"; +import { + getNullableValueTypeUnionInnerType, + TypeExpression, +} from "../type-expression/type-expression.jsx"; const interfaceRefKeyPrefix = Symbol.for("http-server-csharp:interface"); @@ -129,6 +132,9 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children { .map(([pName, prop]) => { const isUnique = getUniqueItems($.program, prop); const isArrayType = prop.type.kind === "Model" && $.array.is(prop.type); + const nullableValueType = prop.optional + ? getNullableValueTypeUnionInnerType($, prop.type) + : undefined; let typeExpr: Children; if (isUnique && isArrayType && prop.type.kind === "Model" && prop.type.indexer?.value) { typeExpr = ( @@ -139,7 +145,7 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children { ); } else { - typeExpr = ; + typeExpr = ; } return { name: namePolicy.getName(pName, "parameter"), diff --git a/packages/http-server-csharp/src/components/scaffolding/mock-implementations.tsx b/packages/http-server-csharp/src/components/scaffolding/mock-implementations.tsx index d987a801986..36195ed7325 100644 --- a/packages/http-server-csharp/src/components/scaffolding/mock-implementations.tsx +++ b/packages/http-server-csharp/src/components/scaffolding/mock-implementations.tsx @@ -4,7 +4,10 @@ import type { Interface, Operation, Program } from "@typespec/compiler"; import { useTsp } from "@typespec/emitter-framework"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { CSharpFile } from "../csharp-file.jsx"; -import { TypeExpression } from "../type-expression/type-expression.jsx"; +import { + getNullableValueTypeUnionInnerType, + TypeExpression, +} from "../type-expression/type-expression.jsx"; import { getGetBodyPropNames, getMockReturnStatement, @@ -112,6 +115,7 @@ interface MockMethodsProps { function MockMethods(props: MockMethodsProps): Children { const namePolicy = cs.useCSharpNamePolicy(); + const { $ } = useTsp(); return ( {([name, op]) => { @@ -147,11 +151,16 @@ function MockMethods(props: MockMethodsProps): Children { const parameters = Array.from(op.parameters.properties.entries()) .filter(([pName]) => !bodyPropNames.has(pName)) .filter(([pName]) => !multipartBodyPropNames.has(pName)) - .map(([pName, prop]) => ({ - name: namePolicy.getName(pName, "parameter"), - type: , - optional: prop.optional, - })) + .map(([pName, prop]) => { + const nullableValueType = prop.optional + ? getNullableValueTypeUnionInnerType($, prop.type) + : undefined; + return { + name: namePolicy.getName(pName, "parameter"), + type: , + optional: prop.optional, + }; + }) // Required parameters must come before optional ones in C# .sort((a, b) => (a.optional === b.optional ? 0 : a.optional ? 1 : -1)); diff --git a/packages/http-server-csharp/src/components/type-expression/type-expression.tsx b/packages/http-server-csharp/src/components/type-expression/type-expression.tsx index be6a9860b29..4b4b15fc606 100644 --- a/packages/http-server-csharp/src/components/type-expression/type-expression.tsx +++ b/packages/http-server-csharp/src/components/type-expression/type-expression.tsx @@ -20,6 +20,12 @@ export interface TypeExpressionProps { // Re-export efRefkey for consumers that were using serverRefkey export { efRefkey } from "@typespec/emitter-framework/csharp"; +export function getNullableValueTypeUnionInnerType($: Typekit, type: Type): Type | undefined { + if (type.kind !== "Union" || isUnionEnum(type)) return undefined; + const innerType = getNullableUnionInnerType(type); + return innerType !== undefined && isValueType($, innerType) ? innerType : undefined; +} + /** * Wrapper around emitter-framework's TypeExpression that handles * additional type kinds the server emitter encounters. @@ -199,10 +205,11 @@ function resolveUnionType($: Typekit, union: import("@typespec/compiler").Union) return code`object`; } // Nullable value type → T? - if (isValueType($, innerType)) { + const nullableValueType = getNullableValueTypeUnionInnerType($, union); + if (nullableValueType) { return ( <> - ? + ? ); } diff --git a/packages/http-server-csharp/test/nullable-parameters.test.ts b/packages/http-server-csharp/test/nullable-parameters.test.ts new file mode 100644 index 00000000000..1645a199672 --- /dev/null +++ b/packages/http-server-csharp/test/nullable-parameters.test.ts @@ -0,0 +1,43 @@ +import { beforeEach, expect, it } from "vitest"; +import { ApiTester, compileAndDiagnose, getStandardService } from "./test-host.js"; + +let tester: Awaited>; + +beforeEach(async () => { + tester = await ApiTester.createInstance(); +}); + +it("emits one nullable suffix for optional nullable value parameters", async () => { + const [result] = await compileAndDiagnose( + tester, + getStandardService(` + enum Choice { + one, + } + + @route("/nullable") + interface NullableParameters { + @get test( + @query value?: int32 | null, + @query choice?: Choice | null, + ): void; + } + `), + { + "emit-mocks": "mocks-only", + "skip-format": true, + }, + ); + + const interfaceContent = [...result.fs.fs.entries()].find(([path]) => + path.endsWith("/INullableParameters.cs"), + )?.[1]; + const mockContent = [...result.fs.fs.entries()].find(([path]) => + path.endsWith("/NullableParameters.cs"), + )?.[1]; + + expect(interfaceContent).toContain("TestAsync(int? value, Choice? choice)"); + expect(mockContent).toContain("TestAsync(int? value, Choice? choice)"); + expect(interfaceContent).not.toMatch(/\w+\?\?\s+\w+/); + expect(mockContent).not.toMatch(/\w+\?\?\s+\w+/); +}); From 29a344c1c8285c0c4b490756acd80afc43231583 Mon Sep 17 00:00:00 2001 From: sophia-ramsey Date: Fri, 11 Sep 2026 16:50:05 -0700 Subject: [PATCH 2/2] add recursive fix, add regression coverage and update tests --- .../controller-action/controller-action.tsx | 12 +++-- .../type-expression/type-expression.tsx | 15 +++++- .../test/nullable-parameters.test.ts | 49 +++++++++++-------- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/http-server-csharp/src/components/controller-action/controller-action.tsx b/packages/http-server-csharp/src/components/controller-action/controller-action.tsx index a79b1f3ef76..8146aac19d6 100644 --- a/packages/http-server-csharp/src/components/controller-action/controller-action.tsx +++ b/packages/http-server-csharp/src/components/controller-action/controller-action.tsx @@ -8,7 +8,10 @@ import type { OperationHttpCanonicalization } from "@typespec/http-canonicalizat import { AspNetMvc } from "../../utils/csharp-libs.jsx"; import { getHttpVerbAttribute, getRouteTemplate } from "../../utils/http-helpers.js"; import type { RequestModelInfo } from "../request-models.jsx"; -import { TypeExpression } from "../type-expression/type-expression.jsx"; +import { + getNullableValueTypeUnionInnerType, + TypeExpression, +} from "../type-expression/type-expression.jsx"; import { getBindingAttribute, getLiteralDefaultValue } from "./parameter-binding.js"; import { getSuccessStatusCode } from "./response-analysis.js"; @@ -49,12 +52,15 @@ export function ControllerAction(props: ControllerActionProps): Children { if (p.property.isContentTypeProperty) continue; const isOptional = p.property.sourceType.optional; const literalDefault = getLiteralDefaultValue(p.property.sourceType.type); + const nullableValueType = isOptional + ? getNullableValueTypeUnionInnerType($, p.property.sourceType.type) + : undefined; if (p.kind === "path") { const paramName = namePolicy.getName(p.property.sourceType.name, "parameter"); const attr = getBindingAttribute(p, paramName); pathParams.push({ name: paramName, - type: , + type: , attributes: attr ? [attr] : undefined, optional: isOptional, default: literalDefault, @@ -63,7 +69,7 @@ export function ControllerAction(props: ControllerActionProps): Children { const attr = getBindingAttribute(p); queryHeaderParams.push({ name: namePolicy.getName(p.property.sourceType.name, "parameter"), - type: , + type: , attributes: attr ? [attr] : undefined, optional: isOptional, default: literalDefault, diff --git a/packages/http-server-csharp/src/components/type-expression/type-expression.tsx b/packages/http-server-csharp/src/components/type-expression/type-expression.tsx index 4b4b15fc606..866ac105f9f 100644 --- a/packages/http-server-csharp/src/components/type-expression/type-expression.tsx +++ b/packages/http-server-csharp/src/components/type-expression/type-expression.tsx @@ -22,8 +22,19 @@ export { efRefkey } from "@typespec/emitter-framework/csharp"; export function getNullableValueTypeUnionInnerType($: Typekit, type: Type): Type | undefined { if (type.kind !== "Union" || isUnionEnum(type)) return undefined; - const innerType = getNullableUnionInnerType(type); - return innerType !== undefined && isValueType($, innerType) ? innerType : undefined; + let current: Type = type; + const visited = new Set(); + + while (current.kind === "Union" && !isUnionEnum(current)) { + if (visited.has(current)) return undefined; + visited.add(current); + + const innerType = getNullableUnionInnerType(current); + if (innerType === undefined) return undefined; + current = innerType; + } + + return isValueType($, current) ? current : undefined; } /** diff --git a/packages/http-server-csharp/test/nullable-parameters.test.ts b/packages/http-server-csharp/test/nullable-parameters.test.ts index 1645a199672..db239f02c58 100644 --- a/packages/http-server-csharp/test/nullable-parameters.test.ts +++ b/packages/http-server-csharp/test/nullable-parameters.test.ts @@ -1,43 +1,52 @@ -import { beforeEach, expect, it } from "vitest"; -import { ApiTester, compileAndDiagnose, getStandardService } from "./test-host.js"; - -let tester: Awaited>; - -beforeEach(async () => { - tester = await ApiTester.createInstance(); -}); +import { expect, it } from "vitest"; +import { EmitterTester, getStandardService } from "./test-host.js"; it("emits one nullable suffix for optional nullable value parameters", async () => { - const [result] = await compileAndDiagnose( - tester, + const { outputs } = await EmitterTester.compile( getStandardService(` enum Choice { one, } + union MaybeInt { + int32, + null, + } + @route("/nullable") interface NullableParameters { @get test( @query value?: int32 | null, @query choice?: Choice | null, + @query maybeInt?: MaybeInt | null, ): void; } `), { - "emit-mocks": "mocks-only", - "skip-format": true, + compilerOptions: { + options: { + "@typespec/http-server-csharp": { + "emit-mocks": "mocks-only", + "skip-format": true, + }, + }, + }, }, ); - const interfaceContent = [...result.fs.fs.entries()].find(([path]) => - path.endsWith("/INullableParameters.cs"), - )?.[1]; - const mockContent = [...result.fs.fs.entries()].find(([path]) => - path.endsWith("/NullableParameters.cs"), - )?.[1]; + const interfaceContent = outputs["generated/operations/INullableParameters.cs"]; + const mockContent = outputs["mocks/NullableParameters.cs"]; + const controllerContent = outputs["generated/controllers/NullableParametersController.cs"]; - expect(interfaceContent).toContain("TestAsync(int? value, Choice? choice)"); - expect(mockContent).toContain("TestAsync(int? value, Choice? choice)"); + expect(interfaceContent).toBeDefined(); + expect(mockContent).toBeDefined(); + expect(controllerContent).toBeDefined(); + expect(interfaceContent).toContain("TestAsync(int? value, Choice? choice, int? maybeInt)"); + expect(mockContent).toContain("TestAsync(int? value, Choice? choice, int? maybeInt)"); + expect(controllerContent).toContain("int? value"); + expect(controllerContent).toContain("Choice? choice"); + expect(controllerContent).toContain("int? maybeInt"); expect(interfaceContent).not.toMatch(/\w+\?\?\s+\w+/); expect(mockContent).not.toMatch(/\w+\?\?\s+\w+/); + expect(controllerContent).not.toMatch(/\w+\?\?\s+\w+/); });