diff --git a/.chronus/changes/sramsey-csharp-server-nullable-2026-7-12-14-45-16.md b/.chronus/changes/sramsey-csharp-server-nullable-2026-7-12-14-45-16.md new file mode 100644 index 00000000000..dec16f982ab --- /dev/null +++ b/.chronus/changes/sramsey-csharp-server-nullable-2026-7-12-14-45-16.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +fix errors in emitter including duplicate nullable suffixes, unresolved symbols for multipart content, incompatible types, parameter ordering, and void as a success type \ 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 79208a4ecd1..22a01ed3450 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,175 @@ 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("orders request model call arguments to match the business interface", async () => { + const { updatePet } = await runner.compile(t.code` + model UpdatePetRequest { + optionalTag?: string; + age: int32; + } + + interface PetStore { + @route("/pets/{petId}") @post ${t.op("updatePet")}( + @path petId: string, + ...UpdatePetRequest, + @query apiVersion: string, + ): void; + } + `); + + const canonOp = canonicalizeOp(updatePet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpPost] + [Route("/pets/{petId}")] + [ProducesResponseType((int)HttpStatusCode.NoContent, Type = typeof(void))] + public virtual async Task UpdatePet( + string petId, + PetStoreUpdatePetRequest body, + [FromQuery(Name="apiVersion")] + string apiVersion + ) + { + await PetStoreImpl.UpdatePetAsync(petId, body.Age, apiVersion, body.OptionalTag); + return NoContent(); + } + } + `); +}); + +it("orders protocol parameter call arguments to match the business interface", async () => { + const { getPet, businessGetPet } = await runner.compile(t.code` + interface PetStore { + @route("/pets/{petId}") @get ${t.op("getPet")}( + @path petId: string, + @header feature: string, + @query apiVersion: string, + ): string; + + ${t.op("businessGetPet")}( + feature: string, + petId: string, + apiVersion: string, + ): string; + } + `); + + const canonOp = canonicalizeOp(getPet); + + expect( + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + class TestController + { + [HttpGet] + [Route("/pets/{petId}")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task GetPet( + string petId, + [FromHeader(Name="feature")] + string feature, + [FromQuery(Name="apiVersion")] + string apiVersion + ) + { + var result = await PetStoreImpl.GetPetAsync(feature, petId, apiVersion); + return Ok(result); + } + } + `); +}); 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 6588a190d1e..ade69aae2ef 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,7 +1,7 @@ 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 { isErrorModel, isVoidType, type Operation } from "@typespec/compiler"; import { useTsp } from "@typespec/emitter-framework"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { AspNetMvc } from "../../utils/csharp-libs.jsx"; @@ -15,6 +15,8 @@ import { getSuccessStatusCode } from "./response-analysis.js"; export interface ControllerActionProps { /** The canonicalized HTTP operation to generate an action method for. */ operation: OperationHttpCanonicalization; + /** The operation used to generate the matching business interface method. */ + businessOperation?: Operation; /** The name of the business logic implementation field (e.g., "petStoreImpl"). */ implFieldName: string; /** Request model info if this operation uses a synthetic request model. */ @@ -45,6 +47,7 @@ export function ControllerAction(props: ControllerActionProps): Children { // Map all HTTP parameters (path, query, header) to C# method parameters const pathParams: ParamInfo[] = []; const queryHeaderParams: ParamInfo[] = []; + const callArgBySourceName = new Map(); for (const p of props.operation.requestParameters.properties) { if (p.property.isContentTypeProperty) continue; const isOptional = p.property.sourceType.optional; @@ -52,6 +55,7 @@ export function ControllerAction(props: ControllerActionProps): Children { if (p.kind === "path") { const paramName = namePolicy.getName(p.property.sourceType.name, "parameter"); const attr = getBindingAttribute(p, paramName); + callArgBySourceName.set(p.property.sourceType.name, paramName); pathParams.push({ name: paramName, type: , @@ -61,8 +65,10 @@ export function ControllerAction(props: ControllerActionProps): Children { }); } else if (p.kind === "query" || p.kind === "header") { const attr = getBindingAttribute(p); + const paramName = namePolicy.getName(p.property.sourceType.name, "parameter"); + callArgBySourceName.set(p.property.sourceType.name, paramName); queryHeaderParams.push({ - name: namePolicy.getName(p.property.sourceType.name, "parameter"), + name: paramName, type: , attributes: attr ? [attr] : undefined, optional: isOptional, @@ -79,6 +85,15 @@ export function ControllerAction(props: ControllerActionProps): Children { }; // Default: path params, then query/header params (sorted by default presence) let parameters: ParamInfo[] = [...pathParams, ...queryHeaderParams.sort(sortByDefault)]; + const getOrderedCallArgs = () => + Array.from( + (props.businessOperation ?? props.operation.sourceType).parameters.properties.entries(), + ) + .filter(([_, prop]) => !isVoidType(prop.type)) + .map(([name, prop]) => ({ arg: callArgBySourceName.get(name), optional: prop.optional })) + .filter((item): item is { arg: string; optional: boolean } => item.arg !== undefined) + .sort((a, b) => (a.optional === b.optional ? 0 : a.optional ? 1 : -1)) + .map((item) => item.arg); // Add body parameter if present (but NOT for GET requests) const body = props.operation.requestParameters.body; @@ -98,10 +113,10 @@ export function ControllerAction(props: ControllerActionProps): Children { if (isGet) { // GET requests suppress body parameters entirely - callArgs = parameters.map((p) => p.name).join(", "); + callArgs = getOrderedCallArgs().join(", "); } else if (isMultipart) { // Multipart body: don't add body as parameter — we'll create a MultipartReader in the method body - callArgs = [...parameters.map((p) => p.name), "reader"].join(", "); + callArgs = [...getOrderedCallArgs(), "reader"].join(", "); } else if (isBodyRoot) { // @bodyRoot — the whole model is the body, no other HTTP params extracted parameters = [ @@ -115,15 +130,13 @@ export function ControllerAction(props: ControllerActionProps): Children { // Call args: path params, then body property accesses, then query/header params const bodyType = body.bodies[0].type.sourceType; if (bodyType.kind === "Model") { - const bodyArgs = Array.from(bodyType.properties.values()).map((p) => { + for (const p of bodyType.properties.values()) { const propName = namePolicy.getName(p.name, "class-property"); - return `body.${propName}`; - }); - const pathArgNames = pathParams.map((p) => p.name); - const queryArgNames = queryHeaderParams.map((p) => p.name); - callArgs = [...pathArgNames, ...bodyArgs, ...queryArgNames].join(", "); + callArgBySourceName.set(p.name, `body.${propName}`); + } + callArgs = getOrderedCallArgs().join(", "); } else { - callArgs = parameters.map((p) => p.name).join(", "); + callArgs = getOrderedCallArgs().join(", "); } } else if (hasExplicitBody) { parameters.push({ @@ -131,20 +144,28 @@ export function ControllerAction(props: ControllerActionProps): Children { type: , attributes: [{ name: AspNetMvc.FromBodyAttribute }], }); - callArgs = parameters.map((p) => p.name).join(", "); + const sourceProperty = body.bodies[0].property?.sourceType; + if (sourceProperty) callArgBySourceName.set(sourceProperty.name, "body"); + callArgs = sourceProperty + ? getOrderedCallArgs().join(", ") + : parameters.map((p) => p.name).join(", "); } else if (body?.bodyKind === "single" && body.bodies.length > 0) { parameters.push({ name: "body", type: , attributes: [{ name: AspNetMvc.FromBodyAttribute }], }); - callArgs = parameters.map((p) => p.name).join(", "); + const sourceProperty = body.bodies[0].property?.sourceType; + if (sourceProperty) callArgBySourceName.set(sourceProperty.name, "body"); + callArgs = sourceProperty + ? getOrderedCallArgs().join(", ") + : parameters.map((p) => p.name).join(", "); } else { - callArgs = parameters.map((p) => p.name).join(", "); + callArgs = getOrderedCallArgs().join(", "); } // 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; 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..ae233e42d9c 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 } 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; } { @@ -18,15 +21,24 @@ export function getSuccessStatusCode(operation: OperationHttpCanonicalization): // Check union responses - find the first non-error success response if (returnType.kind === "Union") { + let hasVoidSuccess = false; for (const variant of returnType.variants.values()) { const vt = variant.type; - if (isVoidType(vt)) continue; + if (isVoidType(vt)) { + hasVoidSuccess = true; + continue; + } if (vt.kind === "Model") { + if (isErrorModel(program, vt)) continue; // Skip models with @error decorator or error-range status codes const result = analyzeResponseModel(vt); if (result.statusCode !== undefined && result.statusCode >= 400) continue; return result; } + return { statusCode: 200, hasBody: true }; + } + if (hasVoidSuccess) { + return { statusCode: 204, hasBody: false }; } } diff --git a/packages/http-server-csharp/src/components/controllers/controllers.test.tsx b/packages/http-server-csharp/src/components/controllers/controllers.test.tsx index dfd99a24b49..c51c7e2e791 100644 --- a/packages/http-server-csharp/src/components/controllers/controllers.test.tsx +++ b/packages/http-server-csharp/src/components/controllers/controllers.test.tsx @@ -9,6 +9,7 @@ import { type OperationHttpCanonicalization, } from "@typespec/http-canonicalization"; import { beforeEach, describe, expect, it } from "vitest"; +import { OperationSources } from "../../context/operation-source-context.js"; import { BusinessLogicInterface } from "../interfaces/interfaces.jsx"; import { Controller } from "./controllers.jsx"; @@ -83,6 +84,68 @@ it("renders a controller class with an action method", async () => { `); }); +it("uses the exact source operation to preserve positional argument order", async () => { + const { PetStore, getPet, businessGetPet } = await runner.compile(t.code` + interface ${t.interface("PetStore")} { + @route("/pets/{petId}") @get ${t.op("getPet")}( + @path petId: string, + @header feature: string, + @query apiVersion: string, + ): string; + + ${t.op("businessGetPet")}( + feature: string, + petId: string, + apiVersion: string, + ): string; + } + `); + const canonOp = canonicalizeOp(getPet); + + expect( + + + + {"\n"} + + + , + ).toRenderTo(` + using Microsoft.AspNetCore.Mvc; + + public interface IPetStore + { + Task GetPetAsync(string petId, string feature, string apiVersion); + + Task BusinessGetPetAsync(string feature, string petId, string apiVersion); + } + [ApiController] + public partial class PetStoreController : ControllerBase + { + internal virtual IPetStore PetStoreImpl { get; } + public PetStoreController(IPetStore operations) + { + PetStoreImpl = operations; + } + + [HttpGet] + [Route("/pets/{petId}")] + [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(string))] + public virtual async Task GetPet( + string petId, + [FromHeader(Name="feature")] + string feature, + [FromQuery(Name="apiVersion")] + string apiVersion + ) + { + var result = await PetStoreImpl.GetPetAsync(feature, petId, apiVersion); + return Ok(result); + } + } + `); +}); + // Regression tests for https://github.com/microsoft/typespec/issues/11445. // `[ApiController]`, `ControllerBase` and `IActionResult` come from // `Microsoft.AspNetCore.Mvc`. They are emitted as library references so they diff --git a/packages/http-server-csharp/src/components/controllers/controllers.tsx b/packages/http-server-csharp/src/components/controllers/controllers.tsx index b4b8aaf93be..f671f60708a 100644 --- a/packages/http-server-csharp/src/components/controllers/controllers.tsx +++ b/packages/http-server-csharp/src/components/controllers/controllers.tsx @@ -3,6 +3,7 @@ import * as cs from "@alloy-js/csharp"; import { Attribute, Reference } from "@alloy-js/csharp"; import type { Interface } from "@typespec/compiler"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; +import { useOperationSources } from "../../context/operation-source-context.js"; import { AspNetMvc } from "../../utils/csharp-libs.jsx"; import { ControllerAction } from "../controller-action/controller-action.jsx"; import { businessLogicInterfaceRefkey } from "../interfaces/interfaces.jsx"; @@ -26,6 +27,7 @@ export function Controller(props: ControllerProps): Children { const baseName = namePolicy.getName(props.type.name, "class"); const controllerName = `${baseName}Controller`; const implPropName = `${baseName}Impl`; + const operationSources = useOperationSources(); const interfaceRef = ; @@ -49,7 +51,14 @@ export function Controller(props: ControllerProps): Children { {(op) => { const rm = props.requestModels?.find((r) => r.op === op); - return ; + return ( + + ); }} 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..548adc8bc17 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,66 @@ 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); + } + `); +}); + +it("falls back to multipart decorators when canonicalization is unavailable", async () => { + const { PetStore } = await runner.compile(t.code` + model MultipartParts { + metadata: HttpPart; + code: HttpPart; + } + + model DerivedMultipartParts { + ...MultipartParts; + } + + interface ${t.interface("PetStore")} { + @post upload( + @header contentType: "multipart/form-data", + @header checksum: string, + @multipartBody content: DerivedMultipartParts, + ): void; + } + `); + + expect( + + + , + ).toRenderTo(` + public interface IPetStore + { + Task UploadAsync(string checksum, MultipartReader reader); + } + `); +}); diff --git a/packages/http-server-csharp/src/components/interfaces/interfaces.tsx b/packages/http-server-csharp/src/components/interfaces/interfaces.tsx index c49fc290cbc..9f856b640a2 100644 --- a/packages/http-server-csharp/src/components/interfaces/interfaces.tsx +++ b/packages/http-server-csharp/src/components/interfaces/interfaces.tsx @@ -1,16 +1,47 @@ import { refkey as ayRefkey, code, For, type Children, type Refkey } from "@alloy-js/core"; import * as cs from "@alloy-js/csharp"; -import type { Interface, Operation } from "@typespec/compiler"; +import type { Interface, ModelProperty, Operation, Program } from "@typespec/compiler"; import { isTemplateDeclaration, isVoidType } from "@typespec/compiler"; import { useTsp } from "@typespec/emitter-framework"; +import { getHeaderFieldName, isMultipartBodyProperty } from "@typespec/http"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { getUniqueItems } from "@typespec/json-schema"; import { getDocComments } from "../../utils/doc-comments.jsx"; 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"); +/** Detects multipart operations without requiring HTTP canonicalization to succeed. */ +export function operationHasMultipartBody(program: Program, operation: Operation): boolean { + return Array.from(operation.parameters.properties.values()).some((prop) => + isMultipartBodyProperty(program, prop), + ); +} + +function isContentTypeHeader(program: Program, property: ModelProperty) { + return getHeaderFieldName(program, property)?.toLowerCase() === "content-type"; +} + +/** Gets raw multipart protocol parameters when canonical HTTP metadata is unavailable. */ +export function getMultipartProtocolParameterNames( + program: Program, + operation: Operation, +): Set { + if (!operationHasMultipartBody(program, operation)) return new Set(); + + const names = new Set(); + for (const [name, prop] of operation.parameters.properties) { + if (isMultipartBodyProperty(program, prop) || isContentTypeHeader(program, prop)) { + names.add(name); + } + } + return names; +} + /** Creates a stable refkey for a business logic interface from its TypeSpec Interface type. */ export function businessLogicInterfaceRefkey(type: Interface): Refkey { return ayRefkey(interfaceRefKeyPrefix, type); @@ -80,7 +111,9 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children { : code`Task`; // Check if this is a multipart request - const isMultipart = props.canonicalOp?.requestParameters.body?.bodyKind === "multipart"; + const isMultipart = + props.canonicalOp?.requestParameters.body?.bodyKind === "multipart" || + operationHasMultipartBody($.program, props.operation); // For GET operations, suppress body parameters entirely const isGet = props.canonicalOp?.method === "get"; @@ -104,7 +137,7 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children { // For multipart requests, suppress all body-related params // For all requests, suppress content-type params - const filteredPropNames = new Set(); + const filteredPropNames = getMultipartProtocolParameterNames($.program, props.operation); if (props.canonicalOp) { for (const p of props.canonicalOp.requestParameters.properties) { if ( @@ -129,6 +162,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 +175,7 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children { ); } else { - typeExpr = ; + typeExpr = ; } return { name: namePolicy.getName(pName, "parameter"), 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 new file mode 100644 index 00000000000..bf0df79739c --- /dev/null +++ b/packages/http-server-csharp/src/components/models/error-models.test.tsx @@ -0,0 +1,121 @@ +import { Tester } from "#test/tester.js"; +import { render, type Children } from "@alloy-js/core"; +import * as cs 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 { EmitterOptions } from "../../context/emitter-options-context.js"; +import { efRefkey } from "../type-expression/type-expression.jsx"; +import { getErrorConstructor } from "./error-models.jsx"; +import { Models } from "./models.jsx"; + +let runner: TesterInstance; + +beforeEach(async () => { + runner = await Tester.createInstance(); +}); + +function Wrapper(props: { children: Children }) { + return ( + + + {props.children} + + + ); +} + +function findFileContent(output: any, pathSuffix: string): string | undefined { + function search(dir: any): string | undefined { + for (const item of dir.contents) { + if ( + "contents" in item && + typeof item.contents === "string" && + (item.path === pathSuffix || item.path.endsWith("/" + pathSuffix)) + ) { + return item.contents; + } + if ("contents" in item && Array.isArray(item.contents)) { + const found = search(item); + if (found) return found; + } + } + return undefined; + } + return search(output); +} + +it("maps structured error constructor parameters to their property types", async () => { + const { ApiError } = await runner.compile(t.code` + @error + model ${t.model("ApiError")} { + message: string; + param?: string; + details?: ApiError[]; + additionalInfo?: Record; + counts?: Record; + pair?: [string, string]; + retryAfter?: int32 | null; + } + `); + + expect( + + + {getErrorConstructor(runner.program, ApiError, "ApiError")} + + , + ).toRenderTo(` + class ApiError + { + public ApiError( + string message, + string param = default, + ApiError[] details = default, + JsonObject additionalInfo = default, + IDictionary counts = default, + string[] pair = default, + int? retryAfter = default + ) : base( + 400, + value: new { message = message, param = param, details = details, additionalInfo = additionalInfo, counts = counts, pair = pair, retryAfter = retryAfter } + ) + { + MessageProp = message; + Param = param; + Details = details; + AdditionalInfo = additionalInfo; + Counts = counts; + Pair = pair; + RetryAfter = retryAfter; + } + } + `); +}); + +it("adds the JsonObject using for inherited record error constructor parameters", async () => { + const { Base, ApiError } = await runner.compile(t.code` + model ${t.model("Base")} { + data?: Record[]; + nestedData?: Record>; + } + + @error + model ${t.model("ApiError")} extends Base { + message: string; + } + `); + + const output = render( + + + + + , + ); + const apiErrorFile = findFileContent(output, "ApiError.cs"); + + expect(apiErrorFile).toBeDefined(); + expect(apiErrorFile).toContain("using System.Text.Json.Nodes;"); + expect(apiErrorFile).toContain("IDictionary nestedData = default"); +}); 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 c0b7b0e3cc4..def857dd92d 100644 --- a/packages/http-server-csharp/src/components/models/error-models.tsx +++ b/packages/http-server-csharp/src/components/models/error-models.tsx @@ -2,7 +2,9 @@ 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 { $ } from "@typespec/compiler/typekit"; import { getHeaderFieldName, isHeader, isStatusCode } from "@typespec/http"; +import { TypeExpression } from "../type-expression/type-expression.jsx"; import { getAllProperties, getCSharpTypeString, @@ -14,6 +16,7 @@ import { /** Generates the constructor for an error model. */ export function getErrorConstructor(program: Program, model: Model, className: string): Children { + const tk = $(program); const statusCode = getErrorStatusCode(program, model); const isChild = model.baseModel && isErrorModel(program, model.baseModel); const namePolicy = cs.createCSharpNamePolicy(); @@ -53,9 +56,21 @@ export function getErrorConstructor(program: Program, model: Model, className: s propName = propName === "Value" ? "ValueName" : `${propName}Prop`; } - const csharpType = getCSharpTypeString(program, prop.type); + const usesTypeExpression = + prop.type.kind === "Union" || + prop.type.kind === "Tuple" || + (prop.type.kind === "Model" && (tk.record.is(prop.type) || tk.array.is(prop.type))); + const csharpType = usesTypeExpression ? ( + + ) : ( + getCSharpTypeString(program, prop.type) + ); const defaultStr = defaultValue ? defaultValue : prop.optional ? "default" : undefined; - parameters.push({ name: prop.name, type: csharpType, default: defaultStr }); + parameters.push({ + name: prop.name, + type: csharpType, + default: defaultStr, + }); bodyParts.push(`${propName} = ${prop.name};`); if (isHeader(program, prop)) { diff --git a/packages/http-server-csharp/src/components/models/model-helpers.ts b/packages/http-server-csharp/src/components/models/model-helpers.ts index 094d5189b84..c18d64872b6 100644 --- a/packages/http-server-csharp/src/components/models/model-helpers.ts +++ b/packages/http-server-csharp/src/components/models/model-helpers.ts @@ -182,14 +182,34 @@ export function isValueType($: ReturnType["$"], type: Type): bool return false; } -/** Returns true if any property of the model uses Record (mapped to JsonObject). */ -export function modelNeedsJsonNodes($: ReturnType["$"], model: Model): boolean { - for (const prop of model.properties.values()) { - if (prop.type.kind === "Model" && $.record.is(prop.type)) { - // Only need JsonNodes for Record (maps to JsonObject) - const valueType = prop.type.indexer?.value; - if (valueType?.kind === "Intrinsic" && valueType.name === "unknown") return true; +/** Returns true if a model property uses Record (mapped to JsonObject). */ +export function modelNeedsJsonNodes( + $: ReturnType["$"], + model: Model, + includeInherited = false, +): boolean { + const typeNeedsJsonNodes = (type: Type): boolean => { + if (type.kind === "Tuple") return type.values.some(typeNeedsJsonNodes); + if (type.kind !== "Model") return false; + if ($.record.is(type)) { + const valueType = type.indexer?.value; + return ( + (valueType?.kind === "Intrinsic" && valueType.name === "unknown") || + (valueType !== undefined && typeNeedsJsonNodes(valueType)) + ); + } + if ($.array.is(type) && type.indexer?.value) { + return typeNeedsJsonNodes(type.indexer.value); + } + return false; + }; + + let current: Model | undefined = model; + while (current) { + for (const prop of current.properties.values()) { + if (typeNeedsJsonNodes(prop.type)) return true; } + current = includeInherited ? current.baseModel : undefined; } return false; } diff --git a/packages/http-server-csharp/src/components/models/models.tsx b/packages/http-server-csharp/src/components/models/models.tsx index 4e8bdffe46c..2f5f7f7d3df 100644 --- a/packages/http-server-csharp/src/components/models/models.tsx +++ b/packages/http-server-csharp/src/components/models/models.tsx @@ -61,7 +61,10 @@ export function Models(props: ModelsProps): Children { return ( {(model) => { - const needsJsonNodes = modelNeedsJsonNodes($, model); + const isRootError = + isErrorModel($.program, model) && + !(model.baseModel && isErrorModel($.program, model.baseModel)); + const needsJsonNodes = modelNeedsJsonNodes($, model, isRootError); const usings = needsJsonNodes ? [...modelUsings, "System.Text.Json.Nodes"] : modelUsings; const modelName = getModelEmitName($.program, model); const subNsParts = getSubNamespaceParts(model.namespace, props.serviceNamespace); diff --git a/packages/http-server-csharp/src/components/multipart-fallback.test.tsx b/packages/http-server-csharp/src/components/multipart-fallback.test.tsx new file mode 100644 index 00000000000..f0c0a550a8c --- /dev/null +++ b/packages/http-server-csharp/src/components/multipart-fallback.test.tsx @@ -0,0 +1,82 @@ +import { Tester } from "#test/tester.js"; +import { SourceDirectory, render } from "@alloy-js/core"; +import { Namespace, createCSharpNamePolicy } 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 { EmitterOptions } from "../context/emitter-options-context.js"; +import { ControllersAndInterfaces } from "./render-root.jsx"; +import { MockImplementations } from "./scaffolding/mock-implementations.jsx"; + +let runner: TesterInstance; + +beforeEach(async () => { + runner = await Tester.createInstance(); +}); + +function findFileContent(output: any, pathSuffix: string): string | undefined { + function search(dir: any): string | undefined { + for (const item of dir.contents) { + if ( + "contents" in item && + typeof item.contents === "string" && + (item.path === pathSuffix || item.path.endsWith("/" + pathSuffix)) + ) { + return item.contents; + } + if ("contents" in item && Array.isArray(item.contents)) { + const found = search(item); + if (found) return found; + } + } + return undefined; + } + return search(output); +} + +it("keeps multipart interfaces and mocks aligned without canonical metadata", async () => { + const { PetStore } = await runner.compile(t.code` + model MultipartParts { + metadata: HttpPart; + code: HttpPart; + } + + model DerivedMultipartParts { + ...MultipartParts; + } + + interface ${t.interface("PetStore")} { + @post upload( + @header contentType: "multipart/form-data", + @header checksum: string, + @multipartBody content: DerivedMultipartParts, + ): void; + } + `); + + const canonicalOpsMap = new Map(); + const output = render( + + + + + + + + + + + + , + ); + + const interfaceContent = findFileContent(output, "operations/IPetStore.cs"); + const mockContent = findFileContent(output, "mocks/PetStore.cs"); + + expect(interfaceContent).toContain("using Microsoft.AspNetCore.WebUtilities;"); + expect(interfaceContent).toContain("UploadAsync(string checksum, MultipartReader reader)"); + expect(mockContent).toContain("using Microsoft.AspNetCore.WebUtilities;"); + expect(mockContent).toContain("UploadAsync(string checksum, MultipartReader reader)"); + expect(interfaceContent).not.toContain("Unresolved Symbol"); + expect(mockContent).not.toContain("Unresolved Symbol"); +}); diff --git a/packages/http-server-csharp/src/components/render-root.tsx b/packages/http-server-csharp/src/components/render-root.tsx index e2405ba25a2..659824904e0 100644 --- a/packages/http-server-csharp/src/components/render-root.tsx +++ b/packages/http-server-csharp/src/components/render-root.tsx @@ -2,11 +2,12 @@ import { For, SourceDirectory, type Children } from "@alloy-js/core"; import * as cs from "@alloy-js/csharp"; import { Namespace } from "@alloy-js/csharp"; import type { Interface } from "@typespec/compiler"; +import { useTsp } from "@typespec/emitter-framework"; import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; import { useEmitterOptions } from "../context/emitter-options-context.js"; import { Controller } from "./controllers/controllers.jsx"; import { CSharpFile } from "./csharp-file.jsx"; -import { BusinessLogicInterface } from "./interfaces/interfaces.jsx"; +import { BusinessLogicInterface, operationHasMultipartBody } from "./interfaces/interfaces.jsx"; import { RequestModels, type RequestModelInfo } from "./request-models.jsx"; export interface ControllersAndInterfacesProps { @@ -21,6 +22,7 @@ export interface ControllersAndInterfacesProps { */ export function ControllersAndInterfaces(props: ControllersAndInterfacesProps): Children { const namePolicy = cs.useCSharpNamePolicy(); + const { $ } = useTsp(); const { serviceNamespace: parentNamespace } = useEmitterOptions(); const interfaceOps = props.interfaces.map((iface) => ({ @@ -61,9 +63,11 @@ export function ControllersAndInterfaces(props: ControllersAndInterfacesProps): {({ iface, ops }) => { - const hasMultipart = ops.some( - (op) => op.requestParameters.body?.bodyKind === "multipart", - ); + const hasMultipart = + ops.some((op) => op.requestParameters.body?.bodyKind === "multipart") || + Array.from(iface.operations.values()).some((op) => + operationHasMultipartBody($.program, op), + ); return ( op.requestParameters.body?.bodyKind === "multipart") || + operations.some(([, op]) => operationHasMultipartBody($.program, op)); return ( {code` @@ -112,6 +123,7 @@ interface MockMethodsProps { function MockMethods(props: MockMethodsProps): Children { const namePolicy = cs.useCSharpNamePolicy(); + const { $ } = useTsp(); return ( {([name, op]) => { @@ -126,8 +138,10 @@ function MockMethods(props: MockMethodsProps): Children { // Check if this is a multipart operation const canonicalOp = props.canonicalMap?.get(name); - const isMultipart = canonicalOp?.requestParameters.body?.bodyKind === "multipart"; - const multipartBodyPropNames = new Set(); + const isMultipart = + canonicalOp?.requestParameters.body?.bodyKind === "multipart" || + operationHasMultipartBody(props.program, op); + const multipartBodyPropNames = getMultipartProtocolParameterNames(props.program, op); if (isMultipart && canonicalOp) { for (const p of canonicalOp.requestParameters.properties) { if ( @@ -147,11 +161,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/src/context/operation-source-context.ts b/packages/http-server-csharp/src/context/operation-source-context.ts new file mode 100644 index 00000000000..cf69170b984 --- /dev/null +++ b/packages/http-server-csharp/src/context/operation-source-context.ts @@ -0,0 +1,13 @@ +import { createNamedContext, useContext } from "@alloy-js/core"; +import type { Operation } from "@typespec/compiler"; +import type { OperationHttpCanonicalization } from "@typespec/http-canonicalization"; + +export type OperationSourceMap = Map; + +export const OperationSources = createNamedContext("OperationSources"); + +const emptyOperationSources: OperationSourceMap = new Map(); + +export function useOperationSources(): OperationSourceMap { + return useContext(OperationSources) ?? emptyOperationSources; +} diff --git a/packages/http-server-csharp/src/emitter.tsx b/packages/http-server-csharp/src/emitter.tsx index 972b28a6bb3..bb00dced322 100644 --- a/packages/http-server-csharp/src/emitter.tsx +++ b/packages/http-server-csharp/src/emitter.tsx @@ -15,6 +15,7 @@ import { MockHelpers, MockImplementations } from "./components/scaffolding/mock- import { JsonConverters } from "./components/serialization/json-converters.jsx"; import { createServerScalarOverrides } from "./components/type-expression/type-expression.jsx"; import { EmitterOptions } from "./context/emitter-options-context.js"; +import { OperationSources } from "./context/operation-source-context.js"; import { reportEmitterDiagnostics } from "./diagnostics.js"; import type { CSharpServiceEmitterOptions } from "./lib.js"; import { resolveOpenApiPath, writeOutputWithOverwrite } from "./output-writer.js"; @@ -68,53 +69,55 @@ export async function $onEmit(context: EmitContext) - - - - - - + + + + + + + + - + + + + + + + + + + + + - - - - - - - + - - - - - - - - + diff --git a/packages/http-server-csharp/src/service-resolution.test.ts b/packages/http-server-csharp/src/service-resolution.test.ts index f8566dd9716..3832a034fc4 100644 --- a/packages/http-server-csharp/src/service-resolution.test.ts +++ b/packages/http-server-csharp/src/service-resolution.test.ts @@ -1,5 +1,5 @@ import { Tester } from "#test/tester.js"; -import type { TesterInstance } from "@typespec/compiler/testing"; +import { t, type TesterInstance } from "@typespec/compiler/testing"; import { $ } from "@typespec/compiler/typekit"; import { HttpCanonicalizer } from "@typespec/http-canonicalization"; import { beforeEach, expect, it } from "vitest"; @@ -110,6 +110,29 @@ it("does not emit template arguments that the instantiation never exposes", asyn expect(resolution.models.map((m) => m.name).sort()).toEqual(["Envelope", "Widget"]); }); +it("tracks the exact source operation for each canonical operation", async () => { + const { read } = await runner.compile(t.code` + @service + namespace Contoso { + interface ${t.interface("PetStore")} { + @route("/pets/{id}") @get ${t.op("read")}( + @path id: string, + @query apiVersion?: string, + ): string; + } + } + `); + const tk = $(runner.program); + const resolution = resolveServiceTypes(runner.program, tk, new HttpCanonicalizer(tk)); + const canonicalOperation = [...resolution.canonicalOperationSourceMap].find( + ([, sourceOperation]) => sourceOperation === read, + )?.[0]; + + expect(canonicalOperation).toBeDefined(); + expect(resolution.canonicalOperationSourceMap.get(canonicalOperation!)).toBe(read); + expect([...resolution.canonicalOpsMap.values()].flat()).toContain(canonicalOperation); +}); + it("discovers the payload type of an HttpPart", async () => { const resolution = await resolve(` namespace Other { diff --git a/packages/http-server-csharp/src/service-resolution.ts b/packages/http-server-csharp/src/service-resolution.ts index 2ccd68c21cd..cbf4d2dd97a 100644 --- a/packages/http-server-csharp/src/service-resolution.ts +++ b/packages/http-server-csharp/src/service-resolution.ts @@ -5,6 +5,7 @@ import { type Enum, type Interface, type Model, + type Operation, type Program, type Namespace as TspNamespace, type Type, @@ -44,6 +45,8 @@ export interface ServiceTypeResolution { unionEnums: Union[]; /** Canonicalized HTTP operations per interface. */ canonicalOpsMap: Map; + /** Original business operation for each canonicalized HTTP operation. */ + canonicalOperationSourceMap: Map; /** Namespaces whose declarations are emitted without being referenced. */ declarationNamespaces: Set; } @@ -96,7 +99,10 @@ export function resolveServiceTypes( ); // Phase 5: Canonicalize all HTTP operations - const canonicalOpsMap = canonicalizeAllInterfaces(canonicalizer, interfaces); + const { canonicalOpsMap, canonicalOperationSourceMap } = canonicalizeAllInterfaces( + canonicalizer, + interfaces, + ); return { serviceNamespace, @@ -106,6 +112,7 @@ export function resolveServiceTypes( enums, unionEnums, canonicalOpsMap, + canonicalOperationSourceMap, declarationNamespaces, }; } @@ -116,20 +123,26 @@ export function resolveServiceTypes( function canonicalizeAllInterfaces( canonicalizer: HttpCanonicalizer, interfaces: Interface[], -): Map { - const result = new Map(); +): { + canonicalOpsMap: Map; + canonicalOperationSourceMap: Map; +} { + const canonicalOpsMap = new Map(); + const canonicalOperationSourceMap = new Map(); for (const iface of interfaces) { const ops: OperationHttpCanonicalization[] = []; for (const [, op] of iface.operations) { try { - ops.push(canonicalizer.canonicalize(op) as OperationHttpCanonicalization); + const canonicalOp = canonicalizer.canonicalize(op) as OperationHttpCanonicalization; + ops.push(canonicalOp); + canonicalOperationSourceMap.set(canonicalOp, op); } catch { // Skip operations that can't be canonicalized } } - result.set(iface.name, ops); + canonicalOpsMap.set(iface.name, ops); } - return result; + return { canonicalOpsMap, canonicalOperationSourceMap }; } // ── Type discovery ────────────────────────────────────────────────────── 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+/); +}); 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(); } }