diff --git a/.chronus/changes/nullable-2024-11-6-1-23-27.md b/.chronus/changes/nullable-2024-11-6-1-23-27.md new file mode 100644 index 0000000000..5aa6b8945c --- /dev/null +++ b/.chronus/changes/nullable-2024-11-6-1-23-27.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Fix nullable types, anonymous types, and safeInt diff --git a/packages/http-server-csharp/src/attributes.ts b/packages/http-server-csharp/src/attributes.ts index de792e7402..cc4531d31f 100644 --- a/packages/http-server-csharp/src/attributes.ts +++ b/packages/http-server-csharp/src/attributes.ts @@ -419,13 +419,45 @@ export function getNumericConstraintAttribute( export function getSafeIntAttribute(type: Scalar): Attribute | undefined { if (type.name.toLowerCase() !== "safeint") return undefined; - return new Attribute( + const attr: Attribute = new Attribute( new AttributeType({ - name: "SafeInt", + name: `NumericConstraint`, namespace: HelperNamespace, }), [], ); + + attr.parameters.push( + new Parameter({ + name: "MinValue", + value: new NumericValue(-9007199254740991), + optional: true, + type: new CSharpType({ + name: "long", + namespace: "System", + isBuiltIn: true, + isValueType: true, + isNullable: false, + }), + }), + ); + + attr.parameters.push( + new Parameter({ + name: "MaxValue", + value: new NumericValue(9007199254740991), + optional: true, + type: new CSharpType({ + name: "long", + namespace: "System", + isBuiltIn: true, + isValueType: true, + isNullable: false, + }), + }), + ); + + return attr; } function getEnumAttribute(type: Enum, cSharpName?: string): Attribute { diff --git a/packages/http-server-csharp/src/interfaces.ts b/packages/http-server-csharp/src/interfaces.ts index de6f87a985..b7142c48a0 100644 --- a/packages/http-server-csharp/src/interfaces.ts +++ b/packages/http-server-csharp/src/interfaces.ts @@ -20,17 +20,20 @@ export class CSharpType implements CSharpTypeMetadata { namespace: string; isBuiltIn: boolean; isValueType: boolean; + isNullable: boolean; public constructor(input: { name: string; namespace: string; isBuiltIn?: boolean; isValueType?: boolean; + isNullable?: boolean; }) { this.name = input.name; this.namespace = input.namespace; this.isBuiltIn = input.isBuiltIn !== undefined ? input.isBuiltIn : input.namespace === "System"; this.isValueType = input.isValueType !== undefined ? input.isValueType : false; + this.isNullable = input.isNullable !== undefined ? input.isNullable : false; } isNamespaceInScope(scope?: Scope, visited?: Set>): boolean { diff --git a/packages/http-server-csharp/src/service.ts b/packages/http-server-csharp/src/service.ts index a64e9bf690..2eefa32289 100644 --- a/packages/http-server-csharp/src/service.ts +++ b/packages/http-server-csharp/src/service.ts @@ -346,7 +346,8 @@ export async function $onEmit(context: EmitContext) property, property.name, ); - const [typeName, typeDefault] = this.#findPropertyType(property); + + const [typeName, typeDefault, nullable] = this.#findPropertyType(property); const doc = getDoc(this.emitter.getProgram(), property); const attributes = getModelAttributes(this.emitter.getProgram(), property, propertyName); // eslint-disable-next-line @typescript-eslint/no-deprecated @@ -356,7 +357,9 @@ export async function $onEmit(context: EmitContext) : typeDefault; return this.emitter.result .rawCode(code`${doc ? `${formatComment(doc)}\n` : ""}${`${attributes.map((attribute) => attribute.getApplicationString(this.emitter.getContext().scope)).join("\n")}${attributes?.length > 0 ? "\n" : ""}`}public ${this.#isInheritedProperty(property) ? "new " : ""}${typeName}${ - property.optional && isValueType(this.emitter.getProgram(), property.type) ? "?" : "" + isValueType(this.emitter.getProgram(), property.type) && (property.optional || nullable) + ? "?" + : "" } ${propertyName} { get; ${typeDefault ? "}" : "set; }"}${ defaultValue ? ` = ${defaultValue};\n` : "\n" } @@ -365,14 +368,27 @@ export async function $onEmit(context: EmitContext) #findPropertyType( property: ModelProperty, - ): [EmitterOutput, string | boolean | undefined] { + ): [EmitterOutput, string | boolean | undefined, boolean] { return this.#getTypeInfoForTsType(property.type); } + #getTypeInfoForUnion( + union: Union, + ): [EmitterOutput, string | boolean | undefined, boolean] { + const propResult = this.#getNonNullableTsType(union); + if (propResult === undefined) { + return [ + code`${emitter.emitTypeReference(union)}`, + undefined, + [...union.variants.values()].filter((v) => isNullType(v.type)).length > 0, + ]; + } + const [typeName, typeDefault, _] = this.#getTypeInfoForTsType(propResult.type); + return [typeName, typeDefault, propResult.nullable]; + } #getTypeInfoForTsType( - this: any, tsType: Type, - ): [EmitterOutput, string | boolean | undefined] { + ): [EmitterOutput, string | boolean | undefined, boolean] { function extractStringValue(type: Type, span: StringTemplateSpan): string { switch (type.kind) { case "String": @@ -403,54 +419,62 @@ export async function $onEmit(context: EmitContext) } switch (tsType.kind) { case "String": - return [code`string`, `"${tsType.value}"`]; + return [code`string`, `"${tsType.value}"`, false]; case "StringTemplate": const template = tsType; if (template.stringValue !== undefined) - return [code`string`, `"${template.stringValue}"`]; + return [code`string`, `"${template.stringValue}"`, false]; const spanResults: string[] = []; for (const span of template.spans) { spanResults.push(extractStringValue(span, span)); } - return [code`string`, `"${spanResults.join("")}"`]; + return [code`string`, `"${spanResults.join("")}"`, false]; case "Boolean": - return [code`bool`, `${tsType.value === true ? true : false}`]; + return [code`bool`, `${tsType.value === true ? true : false}`, false]; case "Number": const [type, value] = this.#findNumericType(tsType); - return [code`${type}`, `${value}`]; + return [code`${type}`, `${value}`, false]; case "Tuple": const defaults = []; const [csharpType, isObject] = this.#coalesceTypes(tsType.values); - if (isObject) return ["object[]", undefined]; + if (isObject) return ["object[]", undefined, false]; for (const value of tsType.values) { const [_, itemDefault] = this.#getTypeInfoForTsType(value); defaults.push(itemDefault); } - return [code`${csharpType.getTypeReference()}[]`, `[${defaults.join(", ")}]`]; + return [ + code`${csharpType.getTypeReference()}[]`, + `[${defaults.join(", ")}]`, + csharpType.isNullable, + ]; case "Object": - return [code`object`, undefined]; + return [code`object`, undefined, false]; case "Model": if (this.#isRecord(tsType)) { - return [code`JsonObject`, undefined]; + return [code`JsonObject`, undefined, false]; } - return [code`${emitter.emitTypeReference(tsType)}`, undefined]; + return [code`${emitter.emitTypeReference(tsType)}`, undefined, false]; + case "ModelProperty": + return this.#getTypeInfoForTsType(tsType.type); case "Enum": - return [code`${emitter.emitTypeReference(tsType)}`, undefined]; + return [code`${emitter.emitTypeReference(tsType)}`, undefined, false]; case "EnumMember": if (typeof tsType.value === "number") { const stringValue = tsType.value.toString(); if (stringValue.includes(".") || stringValue.includes("e")) - return ["double", stringValue]; - return ["int", stringValue]; + return ["double", stringValue, false]; + return ["int", stringValue, false]; } if (typeof tsType.value === "string") { - return ["string", tsType.value]; + return ["string", tsType.value, false]; } - return [code`object`, undefined]; + return [code`object`, undefined, false]; case "Union": - return [code`${emitter.emitTypeReference(tsType)}`, undefined]; + return this.#getTypeInfoForUnion(tsType); + case "UnionVariant": + return this.#getTypeInfoForTsType(tsType.type); default: - return [code`${emitter.emitTypeReference(tsType)}`, undefined]; + return [code`${emitter.emitTypeReference(tsType)}`, undefined, false]; } } @@ -753,13 +777,13 @@ export async function $onEmit(context: EmitContext) } let i = 1; for (const requiredParam of requiredParams) { - const [paramType, _] = this.#findPropertyType(requiredParam); + const [paramType, _, __] = this.#findPropertyType(requiredParam); signature.push( code`${paramType} ${ensureCSharpIdentifier(this.emitter.getProgram(), requiredParam, requiredParam.name, NameCasingType.Parameter)}${i++ < totalParams ? ", " : ""}`, ); } for (const optionalParam of optionalParams) { - const [paramType, _] = this.#findPropertyType(optionalParam); + const [paramType, _, __] = this.#findPropertyType(optionalParam); signature.push( code`${paramType}? ${ensureCSharpIdentifier(this.emitter.getProgram(), optionalParam, optionalParam.name, NameCasingType.Parameter)}${i++ < totalParams ? ", " : ""}`, ); @@ -896,7 +920,7 @@ export async function $onEmit(context: EmitContext) name, NameCasingType.Parameter, ); - let [emittedType, emittedDefault] = this.#findPropertyType(parameter); + let [emittedType, emittedDefault, _] = this.#findPropertyType(parameter); if (emittedType.toString().endsWith("[]")) emittedDefault = undefined; // eslint-disable-next-line @typescript-eslint/no-deprecated const defaultValue = parameter.default @@ -907,11 +931,18 @@ export async function $onEmit(context: EmitContext) code`${httpParam.type !== "path" ? this.#emitParameterAttribute(httpParam) : ""}${emittedType} ${emittedName}${defaultValue === undefined ? "" : ` = ${defaultValue}`}`, ); } + #getBodyParameters(operation: HttpOperation): ModelProperty[] | undefined { + const bodyParam = operation.parameters.body; + if (bodyParam === undefined) return undefined; + if (bodyParam.property !== undefined) return [bodyParam.property]; + if (bodyParam.type.kind !== "Model" || bodyParam.type.properties.size < 1) return undefined; + return [...bodyParam.type.properties.values()]; + } #emitOperationCallParameters(operation: HttpOperation): EmitterOutput { const signature = new StringBuilder(); - const bodyParam = operation.parameters.body; let i = 0; + const bodyParameters = this.#getBodyParameters(operation); //const pathParameters = operation.parameters.parameters.filter((p) => p.type === "path"); for (const parameter of operation.parameters.parameters) { i++; @@ -922,13 +953,27 @@ export async function $onEmit(context: EmitContext) ) { signature.push( code`${this.#emitOperationCallParameter(operation, parameter)}${ - i < operation.parameters.parameters.length || bodyParam !== undefined ? ", " : "" + i < operation.parameters.parameters.length || bodyParameters !== undefined ? ", " : "" }`, ); } } - if (bodyParam !== undefined) { - signature.push(code`body`); + if (bodyParameters !== undefined) { + if (bodyParameters.length === 1) { + signature.push(code`body`); + } else { + let j = 0; + for (const parameter of bodyParameters) { + j++; + const propertyName = ensureCSharpIdentifier( + this.emitter.getProgram(), + parameter, + parameter.name, + NameCasingType.Property, + ); + signature.push(code`body?.${propertyName}${j < bodyParameters.length ? ", " : ""}`); + } + } } return signature.reduce(); @@ -1148,6 +1193,14 @@ export async function $onEmit(context: EmitContext) return result; } + #getNonNullableTsType(union: Union): { type: Type; nullable: boolean } | undefined { + const types = [...union.variants.values()]; + const nulls = types.flatMap((v) => v.type).filter((t) => isNullType(t)); + const nonNulls = types.flatMap((v) => v.type).filter((t) => !isNullType(t)); + if (nonNulls.length === 1) return { type: nonNulls[0], nullable: nulls.length > 0 }; + return undefined; + } + #coalesceTypes(types: Type[]): [CSharpType, boolean] { const defaultValue: [CSharpType, boolean] = [ new CSharpType({ @@ -1158,8 +1211,9 @@ export async function $onEmit(context: EmitContext) true, ]; let current: CSharpType | undefined = undefined; + let nullable: boolean = false; for (const type of types) { - let candidate: CSharpType; + let candidate: CSharpType | undefined = undefined; switch (type.kind) { case "Boolean": candidate = new CSharpType({ name: "bool", namespace: "System", isValueType: true }); @@ -1186,14 +1240,24 @@ export async function $onEmit(context: EmitContext) case "Scalar": candidate = getCSharpTypeForScalar(this.emitter.getProgram(), type); break; + case "Intrinsic": + if (isNullType(type)) { + nullable = true; + candidate = current; + } else { + return defaultValue; + } + break; default: return defaultValue; } current = current ?? candidate; - if (current === undefined || !candidate.equals(current)) return defaultValue; + if (current === undefined || (candidate !== undefined && !candidate.equals(current))) + return defaultValue; } + if (current !== undefined && nullable) current.isNullable = true; return current === undefined ? defaultValue : [current, false]; } diff --git a/packages/http-server-csharp/src/utils.ts b/packages/http-server-csharp/src/utils.ts index c345973b7c..ff667b0ca0 100644 --- a/packages/http-server-csharp/src/utils.ts +++ b/packages/http-server-csharp/src/utils.ts @@ -41,10 +41,13 @@ import { } from "./interfaces.js"; import { reportDiagnostic } from "./lib.js"; +const _scalars: Map = new Map(); export function getCSharpTypeForScalar(program: Program, scalar: Scalar): CSharpType { + if (_scalars.has(scalar)) return _scalars.get(scalar)!; if (program.checker.isStdType(scalar)) { return getCSharpTypeForStdScalars(program, scalar); } + if (scalar.baseScalar) { return getCSharpTypeForScalar(program, scalar.baseScalar); } @@ -54,12 +57,16 @@ export function getCSharpTypeForScalar(program: Program, scalar: Scalar): CSharp format: { typeName: scalar.name }, target: scalar, }); - return new CSharpType({ + + const result = new CSharpType({ name: "Object", namespace: "System", isBuiltIn: true, isValueType: false, }); + + _scalars.set(scalar, result); + return result; } export const UnknownType: CSharpType = new CSharpType({ @@ -71,7 +78,7 @@ export const UnknownType: CSharpType = new CSharpType({ export function getCSharpType( program: Program, type: Type, - namespace: string, + namespace?: string, ): { type: CSharpType; value?: CSharpValue } | undefined { const known = getKnownType(program, type); if (known !== undefined) return { type: known }; @@ -118,7 +125,7 @@ export function getCSharpType( return { type: new CSharpType({ name: ensureCSharpIdentifier(program, type, type.name, NameCasingType.Class), - namespace: namespace, + namespace: namespace || "Models", isBuiltIn: false, isValueType: false, }), @@ -167,24 +174,26 @@ export function getCSharpType( export function coalesceTypes( program: Program, types: Type[], - namespace: string, + namespace?: string, ): { type: CSharpType; value?: CSharpValue } { const visited = new Map(); let candidateType: CSharpType | undefined = undefined; let candidateValue: CSharpValue | undefined = undefined; for (const type of types) { - if (!visited.has(type)) { - const resolvedType = getCSharpType(program, type, namespace); - if (resolvedType === undefined) return { type: UnknownType }; - if (resolvedType.type === UnknownType) return resolvedType; - if (candidateType === undefined) { - candidateType = resolvedType.type; - candidateValue = resolvedType.value; - } else { - if (candidateValue !== resolvedType.value) candidateValue = undefined; - if (candidateType !== resolvedType.type) return { type: UnknownType }; + if (!isNullType(type)) { + if (!visited.has(type)) { + const resolvedType = getCSharpType(program, type, namespace); + if (resolvedType === undefined) return { type: UnknownType }; + if (resolvedType.type === UnknownType) return resolvedType; + if (candidateType === undefined) { + candidateType = resolvedType.type; + candidateValue = resolvedType.value; + } else { + if (candidateValue !== resolvedType.value) candidateValue = undefined; + if (candidateType !== resolvedType.type) return { type: UnknownType }; + } + visited.set(type, resolvedType); } - visited.set(type, resolvedType); } } @@ -364,8 +373,11 @@ export function getCSharpTypeForStdScalars( program: Program, scalar: Scalar & { name: ExtendedIntrinsicScalarName }, ): CSharpType { + const cached: CSharpType | undefined = _scalars.get(scalar); + if (cached !== undefined) return cached; const builtIn: CSharpType | undefined = standardScalars.get(scalar.name); if (builtIn !== undefined) { + _scalars.set(scalar, builtIn); if (scalar.name === "numeric" || scalar.name === "integer" || scalar.name === "float") { reportDiagnostic(program, { code: "no-numeric", @@ -390,10 +402,18 @@ export function getCSharpTypeForStdScalars( } export function isValueType(program: Program, type: Type): boolean { - if (type.kind === "Boolean" || type.kind === "Number" || type.kind === "Enum") return true; - if (type.kind !== "Scalar") return false; - const scalarType = getCSharpTypeForScalar(program, type); - return scalarType.isValueType; + if ( + type.kind === "Boolean" || + type.kind === "Number" || + type.kind === "Enum" || + type.kind === "EnumMember" + ) + return true; + if (type.kind === "Scalar") return getCSharpTypeForScalar(program, type).isValueType; + if (type.kind !== "Union") return false; + return [...type.variants.values()] + .flatMap((v) => v.type) + .every((t) => isNullType(t) || isValueType(program, t)); } export function formatComment( diff --git a/packages/http-server-csharp/test/generation.test.ts b/packages/http-server-csharp/test/generation.test.ts index cc1f17441b..78328daf5b 100644 --- a/packages/http-server-csharp/test/generation.test.ts +++ b/packages/http-server-csharp/test/generation.test.ts @@ -1024,3 +1024,77 @@ it("generates valid code for anonymous models", async () => { ], ); }); + +it("handles nullable types correctly", async () => { + await compileAndValidateMultiple( + runner, + ` + /** A simple test model*/ + model Foo { + /** Nullable numeric property */ + intProp: int32 | null; + /** Nullable reference type */ + stringProp: string | null; + #suppress "@typespec/http-server-csharp/anonymous-model" "This is a test" + /** A complex property */ + modelProp: { + bar: string; + } | null; + #suppress "@typespec/http-server-csharp/anonymous-model" "This is a test" + anotherModelProp: { + baz: string; + }; + + yetAnother: Foo.modelProp | null; + + } + + @route("/foo") op foo(): void; + `, + [ + ["Model0.cs", ["public partial class Model0", "public string Bar { get; set; }"]], + ["Model1.cs", ["public partial class Model1", "public string Baz { get; set; }"]], + [ + "Foo.cs", + [ + "public partial class Foo", + "public int? IntProp { get; set; }", + "public string StringProp { get; set; }", + "public Model0 ModelProp { get; set; }", + "public Model1 AnotherModelProp { get; set; }", + "public Model0 YetAnother { get; set; }", + ], + ], + ["ContosoOperationsControllerBase.cs", [`public virtual async Task Foo()`]], + ["IContosoOperations.cs", [`Task FooAsync( );`]], + ], + ); +}); + +it("handles implicit request body models correctly", async () => { + await compileAndValidateMultiple( + runner, + ` + #suppress "@typespec/http-server-csharp/anonymous-model" "Test" + @route("/foo") @post op foo(intProp?: int32, arrayProp?: string[]): void; + `, + [ + [ + "Model0.cs", + [ + "public partial class Model0", + "public int? IntProp { get; set; }", + "public string[] ArrayProp { get; set; }", + ], + ], + [ + "ContosoOperationsControllerBase.cs", + [ + `public virtual async Task Foo(Model0 body)`, + ".FooAsync(body?.IntProp, body?.ArrayProp)", + ], + ], + ["IContosoOperations.cs", [`Task FooAsync( int? intProp, string[]? arrayProp);`]], + ], + ); +});