diff --git a/.chronus/changes/fix-csharp-optional-error-properties-2026-09-08.md b/.chronus/changes/fix-csharp-optional-error-properties-2026-09-08.md new file mode 100644 index 00000000000..365a557c49e --- /dev/null +++ b/.chronus/changes/fix-csharp-optional-error-properties-2026-09-08.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Emit nullable C# property and constructor parameter types for optional error model properties. diff --git a/.chronus/changes/sramsey-csharp-optional-error-properties-2026-8-14-17-1-42.md b/.chronus/changes/sramsey-csharp-optional-error-properties-2026-8-14-17-1-42.md new file mode 100644 index 00000000000..e1b2c85773c --- /dev/null +++ b/.chronus/changes/sramsey-csharp-optional-error-properties-2026-8-14-17-1-42.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Treat operations returning only error union variants as bodyless responses. \ No newline at end of file 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 417b803da06..2d752802aed 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 @@ -216,6 +216,51 @@ it("does not treat a named union of error responses as a value success", async ( `); }); +it("does not assign a result for unions containing only error responses", 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")}(): 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 { 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 a8b83d9c84a..964f2b9062f 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 @@ -74,7 +74,7 @@ export function getSuccessStatusCode( if (bodylessSuccess !== undefined) { return bodylessSuccess; } - if (hasVoidSuccess) { + if (hasVoidSuccess || !hasValueSuccess) { return { statusCode: 204, hasBody: false }; } } diff --git a/packages/http-server-csharp/src/components/models/error-models.test.tsx b/packages/http-server-csharp/src/components/models/error-models.test.tsx index d50ef8b5172..13f4289c74d 100644 --- a/packages/http-server-csharp/src/components/models/error-models.test.tsx +++ b/packages/http-server-csharp/src/components/models/error-models.test.tsx @@ -26,8 +26,8 @@ function Wrapper(props: { children: Children }) { } function findFileContent(output: any, pathSuffix: string): string | undefined { - function search(dir: any): string | undefined { - for (const item of dir.contents) { + function search(directory: any): string | undefined { + for (const item of directory.contents) { if ( "contents" in item && typeof item.contents === "string" && @@ -45,6 +45,18 @@ function findFileContent(output: any, pathSuffix: string): string | undefined { return search(output); } +function renderModel(model: import("@typespec/compiler").Model): string | undefined { + const output = render( + + + + + , + ); + + return findFileContent(output, `${model.name}.cs`); +} + it("uses generated property types for structured error constructor parameters", async () => { const { ApiError } = await runner.compile(t.code` @error @@ -72,14 +84,14 @@ it("uses generated property types for structured error constructor parameters", { public ApiError( string message, - string param = default, - ApiError[] details = default, - JsonObject additionalInfo = default, - IDictionary counts = default, - int attempts = default, - string[] tags = default, + string? param = default, + ApiError[]? details = default, + JsonObject? additionalInfo = default, + IDictionary? counts = default, + int? attempts = default, + string[]? tags = default, int? retryAfter = default, - IDictionary nested = default + IDictionary? nested = default ) : base( 400, value: new { message = message, param = param, details = details, additionalInfo = additionalInfo, counts = counts, attempts = attempts, tags = tags, retryAfter = retryAfter, nested = nested } @@ -122,5 +134,69 @@ it("adds the JsonObject using for inherited record error constructor parameters" expect(apiErrorFile).toBeDefined(); expect(apiErrorFile).toContain("using System.Text.Json.Nodes;"); - expect(apiErrorFile).toContain("IDictionary data = default"); + expect(apiErrorFile).toContain("IDictionary? data = default"); +}); + +it("makes optional error properties and constructor parameters nullable", async () => { + const { ApiError } = await runner.compile(t.code` + @error + model ${t.model("ApiError")} { + message: string; + optionalText?: string; + optionalCount?: int32; + @header optionalHeader?: string; + } + `); + + const content = renderModel(ApiError); + + expect(content).toContain("string? optionalText = default"); + expect(content).toContain("int? optionalCount = default"); + expect(content).toContain("string? optionalHeader = default"); + expect(content).toContain('headers: new() { {"optional-header", optionalHeader} }'); + expect(content).toContain("public string? OptionalText { get; set; }"); + expect(content).toContain("public int? OptionalCount { get; set; }"); + expect(content).toContain("public string? OptionalHeader { get; set; }"); +}); + +it("emits one nullable suffix for explicitly nullable error properties", async () => { + const { ApiError } = await runner.compile(t.code` + union MaybeInt { + int32, + null, + } + + @error + model ${t.model("ApiError")} { + context: string | null; + count: int32 | null; + optionalContext?: string | null; + nestedCount?: MaybeInt | null; + } + `); + + const content = renderModel(ApiError); + + expect(content).toContain("string? context"); + expect(content).toContain("int? count"); + expect(content).toContain("string? optionalContext = default"); + expect(content).toContain("int? nestedCount = default"); + expect(content).toContain("public string? Context { get; set; }"); + expect(content).toContain("public int? Count { get; set; }"); + expect(content).toContain("public string? OptionalContext { get; set; }"); + expect(content).toContain("public int? NestedCount { get; set; }"); + expect(content).not.toContain("??"); +}); + +it("keeps optional non-error reference properties unchanged", async () => { + const { Widget } = await runner.compile(t.code` + model ${t.model("Widget")} { + optionalText?: string; + } + `); + + const content = renderModel(Widget); + + expect(content).toContain("public string OptionalText { get; set; }"); + expect(content).not.toContain("public string? OptionalText { get; set; }"); }); diff --git a/packages/http-server-csharp/src/components/models/error-models.tsx b/packages/http-server-csharp/src/components/models/error-models.tsx index 2587e8489bd..00017f36fdb 100644 --- a/packages/http-server-csharp/src/components/models/error-models.tsx +++ b/packages/http-server-csharp/src/components/models/error-models.tsx @@ -2,8 +2,13 @@ import { type Children } from "@alloy-js/core"; import type { ParameterProps } from "@alloy-js/csharp"; import * as cs from "@alloy-js/csharp"; import { isErrorModel, type Model, type Program } from "@typespec/compiler"; +import { useTsp } from "@typespec/emitter-framework"; +import { getNullableUnionInnerType } from "@typespec/emitter-framework/csharp"; import { getHeaderFieldName, isHeader, isStatusCode } from "@typespec/http"; -import { TypeExpression } from "../type-expression/type-expression.jsx"; +import { + getNullableValueTypeUnionInnerType, + TypeExpression, +} from "../type-expression/type-expression.jsx"; import { getAllProperties, getDefaultValueString, @@ -14,6 +19,7 @@ import { /** Generates the constructor for an error model. */ export function getErrorConstructor(program: Program, model: Model, className: string): Children { + const { $ } = useTsp(); const statusCode = getErrorStatusCode(program, model); const isChild = model.baseModel && isErrorModel(program, model.baseModel); const namePolicy = cs.createCSharpNamePolicy(); @@ -54,10 +60,17 @@ export function getErrorConstructor(program: Program, model: Model, className: s } const csharpType = ; + const nullableUnionInnerType = + prop.type.kind === "Union" ? getNullableUnionInnerType(prop.type) : undefined; + const typeExpressionIncludesNullable = + getNullableValueTypeUnionInnerType($, prop.type) !== undefined; + const needsNullable = + !typeExpressionIncludesNullable && (prop.optional || nullableUnionInnerType !== undefined); const defaultStr = defaultValue ? defaultValue : prop.optional ? "default" : undefined; parameters.push({ name: prop.name, type: csharpType, + optional: needsNullable, default: defaultStr, }); bodyParts.push(`${propName} = ${prop.name};`); diff --git a/packages/http-server-csharp/src/components/models/models.tsx b/packages/http-server-csharp/src/components/models/models.tsx index 3444292e72b..d9e50756ca6 100644 --- a/packages/http-server-csharp/src/components/models/models.tsx +++ b/packages/http-server-csharp/src/components/models/models.tsx @@ -10,14 +10,18 @@ import { type Namespace as TspNamespace, } from "@typespec/compiler"; import { useTsp } from "@typespec/emitter-framework"; -import { getDocComments } from "@typespec/emitter-framework/csharp"; +import { getDocComments, getNullableUnionInnerType } from "@typespec/emitter-framework/csharp"; import { isStatusCode } from "@typespec/http"; import { getUniqueItems } from "@typespec/json-schema"; import { useEmitterOptions } from "../../context/emitter-options-context.js"; import { getPropertyAttributes } from "../../utils/attributes.jsx"; import { getSubNamespaceParts } from "../../utils/namespace-utils.js"; import { CSharpFile } from "../csharp-file.jsx"; -import { efRefkey, TypeExpression } from "../type-expression/type-expression.jsx"; +import { + efRefkey, + getNullableValueTypeUnionInnerType, + TypeExpression, +} from "../type-expression/type-expression.jsx"; import { getErrorConstructor } from "./error-models.jsx"; import { getDefaultValueString, @@ -176,7 +180,7 @@ interface ServerPropertyProps { /** * Server-specific property that matches old emitter output. - * No `required`, no `[JsonPropertyName]`, no nullable `?` for reference types. + * No `required` or `[JsonPropertyName]`. Nullable reference types are limited to error models. */ function ServerProperty(props: ServerPropertyProps): Children { const { $ } = useTsp(); @@ -235,7 +239,14 @@ function ServerProperty(props: ServerPropertyProps): Children { // But not for union variant types — those should resolve to the enum type const resolveToScalar = (isLiteralOnly && !unionVariantInit) || isErrorProp; const resolvedType = resolveToScalar ? getScalarForLiteral(propType) : propType; - const needsNullable = props.type.optional && (isFloatEnum || isValueType($, resolvedType)); + const nullableUnionInnerType = + propType.kind === "Union" ? getNullableUnionInnerType(propType) : undefined; + const typeExpressionIncludesNullable = + getNullableValueTypeUnionInnerType($, propType) !== undefined; + const needsNullable = isErrorProp + ? !typeExpressionIncludesNullable && + (props.type.optional || nullableUnionInnerType !== undefined) + : props.type.optional && (isFloatEnum || isValueType($, resolvedType)); // Check if this is a @uniqueItems array → ISet const isUniqueItems = getUniqueItems($.program, props.type); diff --git a/packages/http-server-csharp/src/components/serialization/http-service-exception-filter.tsx b/packages/http-server-csharp/src/components/serialization/http-service-exception-filter.tsx index a1464378236..a5ca2a82bcb 100644 --- a/packages/http-server-csharp/src/components/serialization/http-service-exception-filter.tsx +++ b/packages/http-server-csharp/src/components/serialization/http-service-exception-filter.tsx @@ -28,7 +28,10 @@ export function HttpServiceExceptionFilter(): Children { { foreach (var header in httpServiceException.Headers) { - context.HttpContext.Response.Headers.Append(header.Key, header.Value.ToString()); + if (header.Value is not null) + { + context.HttpContext.Response.Headers.Append(header.Key, header.Value); + } } context.Result = new ObjectResult(httpServiceException.Value) diff --git a/packages/http-server-csharp/src/components/serialization/json-converters.test.tsx b/packages/http-server-csharp/src/components/serialization/json-converters.test.tsx index 5bb2bdc5052..71f373bbe20 100644 --- a/packages/http-server-csharp/src/components/serialization/json-converters.test.tsx +++ b/packages/http-server-csharp/src/components/serialization/json-converters.test.tsx @@ -87,6 +87,7 @@ describe("HttpServiceExceptionFilter", () => { expect(content).toBeDefined(); expect(content).toContain("class HttpServiceExceptionFilter"); expect(content).toContain("IActionFilter, IOrderedFilter"); + expect(content).toContain("if (header.Value is not null)"); expect(content).not.toContain("class HttpServiceException : Exception"); }); }); diff --git a/packages/http-server-csharp/test/generation.test.ts b/packages/http-server-csharp/test/generation.test.ts index 55aff85056d..b218ec789c8 100644 --- a/packages/http-server-csharp/test/generation.test.ts +++ b/packages/http-server-csharp/test/generation.test.ts @@ -3116,7 +3116,7 @@ describe("emit correct code for `@error` models", () => { `public Error(`, `string code,`, `string message,`, - `string optionalMessage = default,`, + `string? optionalMessage = default,`, `string defined = "default message"`, `) : base(200, value: new { code = code, message = message, optionalMessage = optionalMessage, defined = defined })`, ], diff --git a/packages/http-server-csharp/test/snapshots/sample-service/generated/lib/HttpServiceExceptionFilter.cs b/packages/http-server-csharp/test/snapshots/sample-service/generated/lib/HttpServiceExceptionFilter.cs index d8b2429f537..1204c8c47ce 100644 --- a/packages/http-server-csharp/test/snapshots/sample-service/generated/lib/HttpServiceExceptionFilter.cs +++ b/packages/http-server-csharp/test/snapshots/sample-service/generated/lib/HttpServiceExceptionFilter.cs @@ -22,7 +22,10 @@ public void OnActionExecuted(ActionExecutedContext context) { foreach (var header in httpServiceException.Headers) { - context.HttpContext.Response.Headers.Append(header.Key, header.Value.ToString()); + if (header.Value is not null) + { + context.HttpContext.Response.Headers.Append(header.Key, header.Value); + } } context.Result = new ObjectResult(httpServiceException.Value)