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"
---

Prevent optional nullable value parameters from emitting duplicate nullable suffixes in generated C# interfaces and mocks.
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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: <TypeExpression type={p.property.sourceType.type} />,
type: <TypeExpression type={nullableValueType ?? p.property.sourceType.type} />,
attributes: attr ? [attr] : undefined,
optional: isOptional,
default: literalDefault,
Expand All @@ -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: <TypeExpression type={p.property.sourceType.type} />,
type: <TypeExpression type={nullableValueType ?? p.property.sourceType.type} />,
attributes: attr ? [attr] : undefined,
optional: isOptional,
default: literalDefault,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(
<Wrapper>
<EnumDeclaration name="Choice" refkey={efRefkey(Choice)}>
One
</EnumDeclaration>
<hbr />
<BusinessLogicInterface type={PetStore} />
</Wrapper>,
).toRenderTo(`
enum Choice
{
One
}
public interface IPetStore
{
Task UpdateAsync(int? value, Choice? choice);
}
`);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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 = (
Expand All @@ -139,7 +145,7 @@ function BusinessLogicMethod(props: BusinessLogicMethodProps): Children {
</>
);
} else {
typeExpr = <TypeExpression type={prop.type} />;
typeExpr = <TypeExpression type={nullableValueType ?? prop.type} />;
}
return {
name: namePolicy.getName(pName, "parameter"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -112,6 +115,7 @@ interface MockMethodsProps {

function MockMethods(props: MockMethodsProps): Children {
const namePolicy = cs.useCSharpNamePolicy();
const { $ } = useTsp();
return (
<For each={props.operations} doubleHardline>
{([name, op]) => {
Expand Down Expand Up @@ -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: <TypeExpression type={prop.type} />,
optional: prop.optional,
}))
.map(([pName, prop]) => {
const nullableValueType = prop.optional
? getNullableValueTypeUnionInnerType($, prop.type)
: undefined;
return {
name: namePolicy.getName(pName, "parameter"),
type: <TypeExpression type={nullableValueType ?? prop.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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,23 @@ 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;
let current: Type = type;
const visited = new Set<Type>();

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;
}

/**
* Wrapper around emitter-framework's TypeExpression that handles
* additional type kinds the server emitter encounters.
Expand Down Expand Up @@ -199,10 +216,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 (
<>
<TypeExpression type={innerType} />?
<TypeExpression type={nullableValueType} />?
</>
);
}
Expand Down
52 changes: 52 additions & 0 deletions packages/http-server-csharp/test/nullable-parameters.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { expect, it } from "vitest";
import { EmitterTester, getStandardService } from "./test-host.js";

it("emits one nullable suffix for optional nullable value parameters", async () => {
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;
}
`),
{
compilerOptions: {
options: {
"@typespec/http-server-csharp": {
"emit-mocks": "mocks-only",
"skip-format": true,
},
},
},
},
);

const interfaceContent = outputs["generated/operations/INullableParameters.cs"];
const mockContent = outputs["mocks/NullableParameters.cs"];
const controllerContent = outputs["generated/controllers/NullableParametersController.cs"];

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+/);
});
Loading