diff --git a/.chronus/changes/csharp-void-success-2026-09-08.md b/.chronus/changes/csharp-void-success-2026-09-08.md new file mode 100644 index 00000000000..7fedc36d524 --- /dev/null +++ b/.chronus/changes/csharp-void-success-2026-09-08.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Handle `void | @error` responses as bodyless success responses in generated C# controllers. diff --git a/packages/http-server-csharp/src/components/controller-action/controller-action.test.tsx b/packages/http-server-csharp/src/components/controller-action/controller-action.test.tsx index 79208a4ecd1..417b803da06 100644 --- a/packages/http-server-csharp/src/components/controller-action/controller-action.test.tsx +++ b/packages/http-server-csharp/src/components/controller-action/controller-action.test.tsx @@ -96,3 +96,266 @@ it("renders a DELETE action with path param", async () => { } `); }); + +it("does not assign a result for void success unions with error responses", async () => { + const { deletePet } = await runner.compile(t.code` + @error + model ErrorResponse { + code: string; + } + + op ServiceOperation(): Response | ErrorResponse; + + interface PetStore { + @route("/pets") @delete ${t.op("deletePet")} is ServiceOperation; + } + `); + + const canonOp = canonicalizeOp(deletePet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpDelete] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))] + public virtual async Task DeletePet() + { + await PetStoreImpl.DeletePetAsync(); + return NoContent(); + } + } + `); +}); + +it("preserves result handling for value success unions with error responses", async () => { + const { getPet } = await runner.compile(t.code` + @error + model ErrorResponse { + code: string; + } + + op ServiceOperation(): Response | ErrorResponse; + + interface PetStore { + @route("/pets") @get ${t.op("getPet")} is ServiceOperation; + } + `); + + const canonOp = canonicalizeOp(getPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpGet] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task GetPet() + { + var result = await PetStoreImpl.GetPetAsync(); + return Ok(result); + } + } + `); +}); + +it("does not treat a named union of error responses as a value success", async () => { + const { deletePet } = await runner.compile(t.code` + @error + model NotFound { + code: string; + } + + @error + model Conflict { + code: string; + } + + union ApiError { + NotFound, + Conflict, + } + + interface PetStore { + @route("/pets") @delete ${t.op("deletePet")}(): void | ApiError; + } + `); + + const canonOp = canonicalizeOp(deletePet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpDelete] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))] + public virtual async Task DeletePet() + { + await PetStoreImpl.DeletePetAsync(); + return NoContent(); + } + } + `); +}); + +it("preserves explicit success status codes after scalar variants", async () => { + const { createPet } = await runner.compile(t.code` + model CreatedPet { + @statusCode statusCode: 201; + id: string; + } + + interface PetStore { + @route("/pets") @post ${t.op("createPet")}(): string | CreatedPet; + } + `); + + const canonOp = canonicalizeOp(createPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpPost] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task CreatePet() + { + var result = await PetStoreImpl.CreatePetAsync(); + return StatusCode(201, result); + } + } + `); +}); + +it("uses a nested union success type in response metadata", async () => { + const { getPet } = await runner.compile(t.code` + @error + model NotFound { + code: string; + } + + union PetResult { + string, + NotFound, + } + + interface PetStore { + @route("/pets") @get ${t.op("getPet")}(): void | PetResult; + } + `); + + const canonOp = canonicalizeOp(getPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpGet] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task GetPet() + { + var result = await PetStoreImpl.GetPetAsync(); + return Ok(result); + } + } + `); +}); + +it("prefers value success variants over status-code-only models", async () => { + const { getPet } = await runner.compile(t.code` + model EmptyResponse { + @statusCode statusCode: 204; + } + + interface PetStore { + @route("/pets") @get ${t.op("getPet")}(): EmptyResponse | string; + } + `); + + const canonOp = canonicalizeOp(getPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpGet] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task GetPet() + { + var result = await PetStoreImpl.GetPetAsync(); + return Ok(result); + } + } + `); +}); + +it("does not assign a result for direct error responses", async () => { + const { getPet } = await runner.compile(t.code` + @error + model ErrorResponse { + code: string; + } + + interface PetStore { + @route("/pets") @get ${t.op("getPet")}(): ErrorResponse; + } + `); + + const canonOp = canonicalizeOp(getPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpGet] + [Route("/pets")] + [ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))] + public virtual async Task GetPet() + { + await PetStoreImpl.GetPetAsync(); + return NoContent(); + } + } + `); +}); 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 8146aac19d6..087f9d4c078 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 @@ -1,12 +1,12 @@ import { code, type Children } from "@alloy-js/core"; import * as cs from "@alloy-js/csharp"; import { Attribute } from "@alloy-js/csharp"; -import { isErrorModel, isVoidType } from "@typespec/compiler"; import { useTsp } from "@typespec/emitter-framework"; import { getDocComments } from "@typespec/emitter-framework/csharp"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { AspNetMvc } from "../../utils/csharp-libs.jsx"; import { getHttpVerbAttribute, getRouteTemplate } from "../../utils/http-helpers.js"; +import { getSuccessReturnType } from "../../utils/return-type-helpers.js"; import type { RequestModelInfo } from "../request-models.jsx"; import { getNullableValueTypeUnionInnerType, @@ -150,31 +150,13 @@ export function ControllerAction(props: ControllerActionProps): Children { } // Determine the success status code from the response - const { statusCode, hasBody } = getSuccessStatusCode(props.operation); + const { statusCode, hasBody } = getSuccessStatusCode($.program, props.operation); // Determine response type for ProducesResponseType attribute const returnType = props.operation.sourceType.returnType; const responseStatusCode = hasBody ? "OK" : "NoContent"; - let responseTypeExpr: Children | undefined = undefined; - - if (hasBody) { - if (returnType.kind === "Union") { - for (const variant of returnType.variants.values()) { - const vt = variant.type; - if (isVoidType(vt)) continue; - if (vt.kind === "Model") { - try { - if (isErrorModel($.program, vt)) continue; - } catch {} - if (vt.name?.toLowerCase() === "error") continue; - } - responseTypeExpr = ; - break; - } - } else if (!isVoidType(returnType)) { - responseTypeExpr = ; - } - } + const successType = hasBody ? getSuccessReturnType($.program, returnType) : undefined; + const responseTypeExpr = successType ? : undefined; const attributes: Children[] = [ , diff --git a/packages/http-server-csharp/src/components/controller-action/response-analysis.ts b/packages/http-server-csharp/src/components/controller-action/response-analysis.ts index d8baefccfca..a8b83d9c84a 100644 --- a/packages/http-server-csharp/src/components/controller-action/response-analysis.ts +++ b/packages/http-server-csharp/src/components/controller-action/response-analysis.ts @@ -1,11 +1,14 @@ -import { isVoidType } from "@typespec/compiler"; +import { isErrorModel, isVoidType, type Program, type Type } from "@typespec/compiler"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; /** * Determines the success HTTP status code and whether the response has a body. * Checks the original return type for @statusCode properties. */ -export function getSuccessStatusCode(operation: OperationHttpCanonicalization): { +export function getSuccessStatusCode( + program: Program, + operation: OperationHttpCanonicalization, +): { statusCode: number | undefined; hasBody: boolean; } { @@ -13,20 +16,66 @@ export function getSuccessStatusCode(operation: OperationHttpCanonicalization): // Check direct model response if (returnType.kind === "Model") { + if (isErrorModel(program, returnType) || returnType.name?.toLowerCase() === "error") { + return { statusCode: 204, hasBody: false }; + } return analyzeResponseModel(returnType); } // Check union responses - find the first non-error success response if (returnType.kind === "Union") { - for (const variant of returnType.variants.values()) { - const vt = variant.type; - if (isVoidType(vt)) continue; - if (vt.kind === "Model") { + let hasVoidSuccess = false; + let hasValueSuccess = false; + let bodylessSuccess: { statusCode: number | undefined; hasBody: boolean } | undefined; + const visitedUnions = new Set(); + + function analyzeVariant( + type: Type, + ): { statusCode: number | undefined; hasBody: boolean } | undefined { + if (isVoidType(type)) { + hasVoidSuccess = true; + return undefined; + } + + if (type.kind === "Union") { + if (visitedUnions.has(type)) return undefined; + visitedUnions.add(type); + + for (const variant of type.variants.values()) { + const result = analyzeVariant(variant.type); + if (result !== undefined) return result; + } + return undefined; + } + + if (type.kind === "Model") { // Skip models with @error decorator or error-range status codes - const result = analyzeResponseModel(vt); - if (result.statusCode !== undefined && result.statusCode >= 400) continue; + if (isErrorModel(program, type) || type.name?.toLowerCase() === "error") return undefined; + const result = analyzeResponseModel(type); + if (result.statusCode !== undefined && result.statusCode >= 400) return undefined; + if (!result.hasBody) { + bodylessSuccess ??= result; + return undefined; + } return result; } + + hasValueSuccess = true; + return undefined; + } + + const result = analyzeVariant(returnType); + if (result !== undefined) { + return result; + } + if (hasValueSuccess) { + return { statusCode: 200, hasBody: true }; + } + if (bodylessSuccess !== undefined) { + return bodylessSuccess; + } + if (hasVoidSuccess) { + return { statusCode: 204, hasBody: false }; } } 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 de7ed1edac3..268c28b818c 100644 --- a/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx +++ b/packages/http-server-csharp/src/components/interfaces/interfaces.test.tsx @@ -93,3 +93,56 @@ it("renders one nullable suffix for optional nullable value parameters", async ( } `); }); + +it("renders a non-generic task for void success with a named error union", async () => { + const { PetStore } = await runner.compile(t.code` + @error + model NotFound { + code: string; + } + + @error + model Conflict { + code: string; + } + + union ApiError { + NotFound, + Conflict, + } + + interface ${t.interface("PetStore")} { + deletePet(): void | ApiError; + } + `); + + expect( + + + , + ).toRenderTo(` + public interface IPetStore + { + Task DeletePetAsync(); + } + `); +}); + +it("renders a generic task for scalar success with void", async () => { + const { PetStore } = await runner.compile(t.code` + interface ${t.interface("PetStore")} { + getPet(): string | void; + } + `); + + expect( + + + , + ).toRenderTo(` + public interface IPetStore + { + Task GetPetAsync(); + } + `); +}); diff --git a/packages/http-server-csharp/src/utils/return-type-helpers.ts b/packages/http-server-csharp/src/utils/return-type-helpers.ts index 837a6355f78..5a5674442e0 100644 --- a/packages/http-server-csharp/src/utils/return-type-helpers.ts +++ b/packages/http-server-csharp/src/utils/return-type-helpers.ts @@ -7,37 +7,42 @@ import { isErrorModel, isVoidType } from "@typespec/compiler"; * If the return type is void, returns undefined. */ export function getSuccessReturnType(program: Program, returnType: Type): Type | undefined { - if (isVoidType(returnType)) return undefined; - - if (returnType.kind === "Union") { - for (const variant of returnType.variants.values()) { - const variantType = variant.type; - if (isVoidType(variantType)) continue; - // Skip error models by checking the @error decorator or name convention - if (variantType.kind === "Model") { - try { - if (isErrorModel(program, variantType)) continue; - } catch { - // isErrorModel may fail on certain types - } - if (variantType.name && variantType.name.toLowerCase() === "error") { - continue; - } - // Skip response-only models (only @statusCode, no body props) - if (isStatusCodeOnlyModel(variantType)) continue; + const visitedUnions = new Set(); + + function findSuccessType(type: Type): Type | undefined { + if (isVoidType(type)) return undefined; + + if (type.kind === "Union") { + if (visitedUnions.has(type)) return undefined; + visitedUnions.add(type); + + for (const variant of type.variants.values()) { + const successType = findSuccessType(variant.type); + if (successType !== undefined) return successType; + } + return undefined; + } + + // Skip error models by checking the @error decorator or name convention + if (type.kind === "Model") { + try { + if (isErrorModel(program, type)) return undefined; + } catch { + // isErrorModel may fail on certain types + } + if (type.name && type.name.toLowerCase() === "error") { + return undefined; + } + // Skip response-only models (only @statusCode, no body props) + if (isStatusCodeOnlyModel(type)) { + return undefined; } - return variantType; } - // All variants are errors or void - return undefined; - } - // Check if it's a status-code-only model (e.g., OkResponse, NoContentResponse) - if (returnType.kind === "Model" && isStatusCodeOnlyModel(returnType)) { - return undefined; + return type; } - return returnType; + return findSuccessType(returnType); } /** Returns true if the model only has statusCode-related properties (no body). Walks inherited properties too. */ diff --git a/packages/http-server-csharp/test/snapshots/sample-service/generated/controllers/PetsController.cs b/packages/http-server-csharp/test/snapshots/sample-service/generated/controllers/PetsController.cs index 98874e873d3..d983ec05463 100644 --- a/packages/http-server-csharp/test/snapshots/sample-service/generated/controllers/PetsController.cs +++ b/packages/http-server-csharp/test/snapshots/sample-service/generated/controllers/PetsController.cs @@ -87,10 +87,10 @@ Pet body /// [HttpDelete] [Route("/pets/{id}")] - [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(void))] + [ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))] public virtual async Task Delete(long id) { - var result = await PetsImpl.DeleteAsync(id); - return Ok(result); + await PetsImpl.DeleteAsync(id); + return NoContent(); } }