Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/http-server-csharp"
---

Emit nullable C# property and constructor parameter types for optional error model properties.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/http-server-csharp"
---

Treat operations returning only error union variants as bodyless responses.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Wrapper>
<ControllerAction operation={canonOp} implFieldName="PetStoreImpl" />
</Wrapper>,
).toRenderTo(`
using Microsoft.AspNetCore.Mvc;

class TestController
{
[HttpDelete]
[Route("/pets")]
[ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))]
public virtual async Task<IActionResult> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export function getSuccessStatusCode(
if (bodylessSuccess !== undefined) {
return bodylessSuccess;
}
if (hasVoidSuccess) {
if (hasVoidSuccess || !hasValueSuccess) {
return { statusCode: 204, hasBody: false };
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" &&
Expand All @@ -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(
<Output program={runner.program} namePolicy={cs.createCSharpNamePolicy()}>
<EmitterOptions.Provider value={{ collectionType: "array", serviceNamespace: "Test" }}>
<Models models={[model]} serviceNamespace={undefined} />
</EmitterOptions.Provider>
</Output>,
);

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
Expand Down Expand Up @@ -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<string, int> counts = default,
int attempts = default,
string[] tags = default,
string? param = default,

@timotheeguerin Timothee Guerin (timotheeguerin) Sep 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, is that correct? can optional parameters have default in c#?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes; the default will end up being null for optional types

ApiError[]? details = default,
JsonObject? additionalInfo = default,
IDictionary<string, int>? counts = default,
int? attempts = default,
string[]? tags = default,
int? retryAfter = default,
IDictionary<string, JsonObject> nested = default
IDictionary<string, JsonObject>? nested = default
) : base(
400,
value: new { message = message, param = param, details = details, additionalInfo = additionalInfo, counts = counts, attempts = attempts, tags = tags, retryAfter = retryAfter, nested = nested }
Expand Down Expand Up @@ -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<string, JsonObject> data = default");
expect(apiErrorFile).toContain("IDictionary<string, JsonObject>? 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; }");
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -54,10 +60,17 @@ export function getErrorConstructor(program: Program, model: Model, className: s
}

const csharpType = <TypeExpression type={prop.type} />;
const nullableUnionInnerType =
prop.type.kind === "Union" ? getNullableUnionInnerType(prop.type) : undefined;
const typeExpressionIncludesNullable =
getNullableValueTypeUnionInnerType($, prop.type) !== undefined;
const needsNullable =
!typeExpressionIncludesNullable && (prop.optional || nullableUnionInnerType !== undefined);
Comment thread
sophia-ramsey marked this conversation as resolved.
const defaultStr = defaultValue ? defaultValue : prop.optional ? "default" : undefined;
parameters.push({
name: prop.name,
type: csharpType,
optional: needsNullable,
default: defaultStr,
});
bodyParts.push(`${propName} = ${prop.name};`);
Expand Down
19 changes: 15 additions & 4 deletions packages/http-server-csharp/src/components/models/models.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<T>
const isUniqueItems = getUniqueItems($.program, props.type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/http-server-csharp/test/generation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })`,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading