diff --git a/lib/codegen/fromcto/csharp/csharpvisitor.js b/lib/codegen/fromcto/csharp/csharpvisitor.js index ad27915..3e2c857 100644 --- a/lib/codegen/fromcto/csharp/csharpvisitor.js +++ b/lib/codegen/fromcto/csharp/csharpvisitor.js @@ -130,6 +130,11 @@ class CSharpVisitor { .filter(namespace => namespace !== modelFile.getNamespace()) // Skip own namespace. .filter((v, i, a) => a.indexOf(v) === i) // Remove any duplicates from direct imports .forEach(namespace => { + // concerto.decorator types are provided by the .NET runtime package. + if (ModelUtil.parseNamespace(namespace).name === 'concerto.decorator') { + parameters.fileWriter.writeLine(0, 'using AccordProject.Concerto.Metamodel;'); + return; + } const otherModelFile = modelFile.getModelManager()?.getModelFile(namespace); if (!otherModelFile) { // Couldn't resolve the other model file. @@ -250,8 +255,13 @@ class CSharpVisitor { */ getDotNetNamespaceOfType(type, classDeclaration, parameters) { let dotnetNs = ''; + const parsedNamespace = type ? ModelUtil.parseNamespace(ModelUtil.getNamespace(type)).name : null; + // concerto.decorator base types are provided by the .NET runtime package. + if (parsedNamespace === 'concerto.decorator') { + return 'AccordProject.Concerto.Metamodel.'; + } // Resolve to dotnet namespace only if its a non system type - if (type && ModelUtil.parseNamespace(ModelUtil.getNamespace(type)).name !== 'concerto') { + if (type && parsedNamespace !== 'concerto') { const mm = classDeclaration.getModelFile()?.getModelManager(); let typeNamespace = mm?.getType(type)?.getNamespace(); if (typeNamespace && typeNamespace !== classDeclaration.getNamespace()) { @@ -282,6 +292,9 @@ class CSharpVisitor { const csharpType = fqn === 'concerto.scalar.UUID' ? 'System.Guid' : this.toCSharpType(scalarDeclaration.getType()); + // If the scalar type itself is named Value, using Value as the generated + // record parameter/member causes a C# CS0542 collision. + const scalarMemberName = identifier === 'Value' ? 'RawValue' : 'Value'; const validatorLines = this.buildScalarValidatorLines(scalarDeclaration); const converterName = `${identifier}JsonConverter`; const useNewtonsoft = !!parameters.useNewtonsoftJson; @@ -300,42 +313,42 @@ class CSharpVisitor { : `public override void Write(System.Text.Json.Utf8JsonWriter w, ${identifier} v, System.Text.Json.JsonSerializerOptions o)`; parameters.fileWriter.writeLine(0, converterAttr); - parameters.fileWriter.writeLine(0, `public readonly record struct ${identifier}(${csharpType} Value)`); + parameters.fileWriter.writeLine(0, `public readonly record struct ${identifier}(${csharpType} ${scalarMemberName})`); parameters.fileWriter.writeLine(0, '{'); if (validatorLines.length > 0) { validatorLines.forEach(line => parameters.fileWriter.writeLine(1, line)); - parameters.fileWriter.writeLine(1, `public ${csharpType} Value { get; init; } = Value;`); + parameters.fileWriter.writeLine(1, `public ${csharpType} ${scalarMemberName} { get; init; } = ${scalarMemberName};`); } - parameters.fileWriter.writeLine(1, `public static implicit operator ${csharpType}(${identifier} s) => s.Value;`); + parameters.fileWriter.writeLine(1, `public static implicit operator ${csharpType}(${identifier} s) => s.${scalarMemberName};`); parameters.fileWriter.writeLine(1, `public static implicit operator ${identifier}(${csharpType} v) => new(v);`); - parameters.fileWriter.writeLine(1, 'public override string ToString() => Value.ToString();'); + parameters.fileWriter.writeLine(1, `public override string ToString() => ${scalarMemberName}.ToString();`); parameters.fileWriter.writeLine(0, '}'); // Companion converter — one per scalar, flavoured by the active serializer let readExpr, writeExpr; if (csharpType === 'System.Guid') { readExpr = useNewtonsoft ? 'System.Guid.Parse((string)r.Value!)' : 'r.GetGuid()'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value.ToString())' : 'w.WriteStringValue(v.Value.ToString())'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName}.ToString())` : `w.WriteStringValue(v.${scalarMemberName}.ToString())`; } else if (csharpType === 'string') { readExpr = useNewtonsoft ? '(string)r.Value!' : 'r.GetString()!'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value)' : 'w.WriteStringValue(v.Value)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName})` : `w.WriteStringValue(v.${scalarMemberName})`; } else if (csharpType === 'bool') { readExpr = useNewtonsoft ? '(bool)r.Value!' : 'r.GetBoolean()'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value)' : 'w.WriteBooleanValue(v.Value)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName})` : `w.WriteBooleanValue(v.${scalarMemberName})`; } else if (csharpType === 'int') { readExpr = useNewtonsoft ? 'System.Convert.ToInt32(r.Value)' : 'r.GetInt32()'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value)' : 'w.WriteNumberValue(v.Value)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName})` : `w.WriteNumberValue(v.${scalarMemberName})`; } else if (csharpType === 'long') { readExpr = useNewtonsoft ? 'System.Convert.ToInt64(r.Value)' : 'r.GetInt64()'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value)' : 'w.WriteNumberValue(v.Value)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName})` : `w.WriteNumberValue(v.${scalarMemberName})`; } else if (csharpType === 'double') { readExpr = useNewtonsoft ? 'System.Convert.ToDouble(r.Value)' : 'r.GetDouble()'; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value)' : 'w.WriteNumberValue(v.Value)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName})` : `w.WriteNumberValue(v.${scalarMemberName})`; } else { readExpr = useNewtonsoft ? `(${csharpType})System.Convert.ChangeType((string)r.Value!, typeof(${csharpType}))` : `(${csharpType})System.Convert.ChangeType(r.GetString()!, typeof(${csharpType}))`; - writeExpr = useNewtonsoft ? 'w.WriteValue(v.Value.ToString()!)' : 'w.WriteStringValue(v.Value.ToString()!)'; + writeExpr = useNewtonsoft ? `w.WriteValue(v.${scalarMemberName}.ToString()!)` : `w.WriteStringValue(v.${scalarMemberName}.ToString()!)`; } parameters.fileWriter.writeLine(0, `public class ${converterName} : ${converterBase}`); @@ -492,7 +505,8 @@ class CSharpVisitor { const { keyType, valueType } = this.resolveMapTypes(mapDeclaration, parameters); const nullable = field.isOptional() ? '?' : ''; const resolvedType = `Dictionary<${keyType}, ${valueType}>`; - const lines = this.toCSharpProperty('public', field.getParent()?.getName(), field.getName(), null, '', nullable, '{ get; set; }', parameters, resolvedType); + const emitRequired = !!parameters.useRequiredForNonOptionalReferenceTypes && !field.isOptional(); + const lines = this.toCSharpProperty('public', field.getParent()?.getName(), field.getName(), null, '', nullable, '{ get; set; }', parameters, resolvedType, emitRequired); lines.forEach(line => parameters.fileWriter.writeLine(1, line)); return null; } @@ -571,6 +585,22 @@ class CSharpVisitor { : (externalFieldType !== undefined ? scalarDefaultValue : field.getDefaultValue()); const csDefault = this.formatDefaultLiteral(rawDefault, rawFieldType, !!externalFieldType, field, fieldType); const getset = csDefault != null ? `{ get; set; } = ${csDefault};` : '{ get; set; }'; + const csharpType = this.toCSharpType(fieldType, parameters); + let isEnum = false; + try { + isEnum = field.getModelFile().getType(field.getType())?.isEnum?.() || false; + } catch (e) { + // Keep false when declaration cannot be resolved. + } + const emitRequired = this.shouldEmitRequired(parameters, { + nullableType, + hasDefault: csDefault != null, + isArray: field.isArray(), + isScalarAlias: externalFieldType !== undefined, + isPrimitive: field.isPrimitive(), + isEnum, + csharpType + }); const lines = this.toCSharpProperty( 'public', @@ -580,12 +610,40 @@ class CSharpVisitor { array, nullableType, getset, - parameters + parameters, + undefined, + emitRequired ); lines.forEach(line => parameters.fileWriter.writeLine(1, line)); return null; } + /** + * Determines if a property should emit the C# `required` modifier. + * This centralizes required-emission decisions for fields and relationships. + * @param {Object} parameters - visitor parameters + * @param {Object} options - decision options + * @param {string} [options.nullableType] - nullable marker (`?`) when present + * @param {boolean} [options.hasDefault] - true when a default initializer is emitted + * @param {boolean} [options.isArray] - true when property is an array type + * @param {boolean} [options.isScalarAlias] - true for scalar alias value-type wrappers + * @param {boolean} [options.isPrimitive] - true for Concerto primitive fields + * @param {boolean} [options.isEnum] - true for enum fields + * @param {string} [options.csharpType] - resolved C# type string + * @returns {boolean} true if `required` should be emitted + * @private + */ + shouldEmitRequired(parameters, options = {}) { + if (!parameters.useRequiredForNonOptionalReferenceTypes) {return false;} + if (options.nullableType) {return false;} + if (options.hasDefault) {return false;} + // Scalar aliases are generated as record structs (value types), not reference types. + if (options.isScalarAlias) {return false;} + if (options.isEnum) {return false;} + if (!options.csharpType) {return false;} + return this.isCSharpReferenceType(options.csharpType, !!options.isArray); + } + /** * Visitor design pattern * @param {EnumValueDeclaration} enumValueDeclaration - the object being visited @@ -652,6 +710,17 @@ class CSharpVisitor { type = `${fqn}${ModelUtil.getShortName(relationship.getFullyQualifiedTypeName())}`; } + const csharpType = this.toCSharpType(type, parameters); + const emitRequired = this.shouldEmitRequired(parameters, { + nullableType: optional, + hasDefault: false, + isArray: !!array, + isScalarAlias: false, + isPrimitive: false, + isEnum: false, + csharpType + }); + // we export all relationships const lines = this.toCSharpProperty( 'public', @@ -661,12 +730,28 @@ class CSharpVisitor { array, optional, '{ get; set; }', - parameters + parameters, + undefined, + emitRequired ); lines.forEach(line => parameters.fileWriter.writeLine(1, line)); return null; } + /** + * Determines whether a generated C# property type is a reference type. + * @param {string} csharpType - resolved C# type name + * @param {boolean} isArray - whether the property is an array + * @returns {boolean} true if reference type + * @private + */ + isCSharpReferenceType(csharpType, isArray) { + if (isArray) {return true;} + if (csharpType === 'string') {return true;} + if (csharpType === 'System.Guid' || csharpType === 'System.DateTime') {return false;} + return !csharpBuiltInTypes.includes(csharpType); + } + /** * Format a Concerto default value as a C# literal suitable for a property initializer. * String values are quoted; scalar-typed fields wrap the literal in `new(...)`. @@ -714,9 +799,10 @@ class CSharpVisitor { * @param {string} getset the getter and setter declaration * @param {Object} [parameters] - the parameter * @param {string} [resolvedType] - pre-built C# type string; when provided, skips toCSharpType + * @param {boolean} [emitRequired] - true to emit the C# `required` modifier * @returns {string} the property declaration */ - toCSharpProperty(access, parentName, propertyName, propertyType, array, nullableType, getset, parameters, resolvedType = undefined) { + toCSharpProperty(access, parentName, propertyName, propertyType, array, nullableType, getset, parameters, resolvedType = undefined, emitRequired = false) { const identifier = this.toCSharpIdentifier(parentName, propertyName, parameters); const type = resolvedType ?? this.toCSharpType(propertyType, parameters); @@ -731,7 +817,7 @@ class CSharpVisitor { } } - lines.push(`${access} ${type}${array}${nullableType} ${identifier} ${getset}`); + lines.push(`${access} ${emitRequired ? 'required ' : ''}${type}${array}${nullableType} ${identifier} ${getset}`); return lines; } diff --git a/test/codegen/__snapshots__/codegen.js.snap b/test/codegen/__snapshots__/codegen.js.snap index 5587f3a..c73842a 100644 --- a/test/codegen/__snapshots__/codegen.js.snap +++ b/test/codegen/__snapshots__/codegen.js.snap @@ -632,7 +632,7 @@ public abstract class Decorator : Concept { } [AccordProject.Concerto.Type(Namespace = "concerto.decorator", Version = "1.0.0", Name = "DotNetNamespace")] [System.Text.Json.Serialization.JsonConverter(typeof(AccordProject.Concerto.ConcertoConverterFactorySystem))] -public class DotNetNamespace : Decorator { +public class DotNetNamespace : AccordProject.Concerto.Metamodel.Decorator { [System.Text.Json.Serialization.JsonPropertyName("$class")] public override string _class { get; } = "concerto.decorator@1.0.0.DotNetNamespace"; [System.Text.Json.Serialization.JsonPropertyName("namespace")] @@ -648,7 +648,7 @@ exports[`codegen #formats check we can convert all formats from namespace versio "value": "namespace AccordProject.Concerto; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; -using concerto.decorator; +using AccordProject.Concerto.Metamodel; [AccordProject.Concerto.Type(Namespace = "concerto", Version = "1.0.0", Name = "Concept")] [System.Text.Json.Serialization.JsonConverter(typeof(AccordProject.Concerto.ConcertoConverterFactorySystem))] public abstract class Concept { @@ -785,11 +785,11 @@ exports[`codegen #formats check we can convert all formats from namespace versio using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using org.acme.hr.base; -using concerto.decorator; +using AccordProject.Concerto.Metamodel; using AccordProject.Concerto; [AccordProject.Concerto.Type(Namespace = "org.acme.hr", Version = "1.0.0", Name = "pii")] [System.Text.Json.Serialization.JsonConverter(typeof(AccordProject.Concerto.ConcertoConverterFactorySystem))] -public class pii : concerto.decorator.Decorator { +public class pii : AccordProject.Concerto.Metamodel.Decorator { [System.Text.Json.Serialization.JsonPropertyName("$class")] public override string _class { get; } = "org.acme.hr@1.0.0.pii"; public bool isPii { get; set; } diff --git a/test/codegen/fromcto/csharp/csharpvisitor.js b/test/codegen/fromcto/csharp/csharpvisitor.js index f9faf53..8575918 100644 --- a/test/codegen/fromcto/csharp/csharpvisitor.js +++ b/test/codegen/fromcto/csharp/csharpvisitor.js @@ -605,6 +605,27 @@ describe('CSharpVisitor', function () { file.should.match(/w\.WriteNumberValue\(v\.Value\)/); }); + it('should avoid CS0542 name collision when scalar type is named Value', () => { + const modelManager = new ModelManager({ strict: true }); + modelManager.addCTOModel(` + namespace org.acme@1.0.0 + + scalar Value extends Double + + concept Sample { + o Value amount + } + `); + csharpVisitor.visit(modelManager, { fileWriter }); + const file = fileWriter.getFilesInMemory().get('org.acme@1.0.0.cs'); + + file.should.match(/public readonly record struct Value\(double RawValue\)/); + file.should.match(/public static implicit operator double\(Value s\) => s\.RawValue;/); + file.should.match(/public override string ToString\(\) => RawValue\.ToString\(\);/); + file.should.match(/w\.WriteNumberValue\(v\.RawValue\)/); + file.should.not.match(/public readonly record struct Value\(double Value\)/); + }); + it('should emit a JsonConverter for a Boolean-backed scalar (bool round-trip)', () => { const modelManager = new ModelManager({ strict: true }); modelManager.addCTOModel(` @@ -880,6 +901,71 @@ public class SampleModel : Concept { file1.should.match(/public Status status \{ get; set; \} = Status.Active;/); }); + it('should emit required for non-optional reference fields when flag is enabled', () => { + const modelManager = new ModelManager({ strict: true }); + modelManager.addCTOModel(` + namespace org.acme@1.2.3 + + scalar SSN extends String + + enum Status { + o ACTIVE + o INACTIVE + } + + concept Child { + o String id + } + + concept Parent { + o String name + o Integer count + o String nick optional + o SSN ssn + o Status status default="ACTIVE" + o Status state + o Child child + o Child[] children + } + `); + csharpVisitor.visit(modelManager, { fileWriter, useRequiredForNonOptionalReferenceTypes: true }); + const files = fileWriter.getFilesInMemory(); + const file1 = files.get('org.acme@1.2.3.cs'); + file1.should.match(/public required string name \{ get; set; \}/); + file1.should.match(/public int count \{ get; set; \}/); + file1.should.match(/public string\? nick \{ get; set; \}/); + file1.should.match(/public SSN ssn \{ get; set; \}/); + file1.should.match(/public Status status \{ get; set; \} = Status.Active;/); + file1.should.match(/public Status state \{ get; set; \}/); + file1.should.match(/public required Child child \{ get; set; \}/); + file1.should.match(/public required Child\[\] children \{ get; set; \}/); + file1.should.not.match(/public required SSN ssn \{ get; set; \}/); + file1.should.not.match(/public required Status state \{ get; set; \}/); + }); + + it('should not emit required when flag is disabled', () => { + const modelManager = new ModelManager({ strict: true }); + modelManager.addCTOModel(` + namespace org.acme@1.2.3 + + concept Child { + o String id + } + + concept Parent { + o String name + o Child child + } + `); + csharpVisitor.visit(modelManager, { fileWriter, useRequiredForNonOptionalReferenceTypes: false }); + const files = fileWriter.getFilesInMemory(); + const file1 = files.get('org.acme@1.2.3.cs'); + file1.should.match(/public string name \{ get; set; \}/); + file1.should.match(/public Child child \{ get; set; \}/); + file1.should.not.match(/public required string name \{ get; set; \}/); + file1.should.not.match(/public required Child child \{ get; set; \}/); + }); + it('should use UUID alias for scalar type UUID with different namespace than concerto.scalar', () => { const modelManager = new ModelManager({ strict: true }); modelManager.addCTOModel(` @@ -2019,6 +2105,35 @@ public class SampleModel : Concept { csharpVisitor.visitField(mockField, param); param.fileWriter.writeLine.withArgs(1, 'public Dictionary Map1 { get; set; }').calledOnce.should.be.ok; }); + + it('should write required for non-optional map fields when required flag is enabled', () => { + const mockField = sinon.createStubInstance(Field); + const getType = sinon.stub(); + + mockField.getModelFile.returns({ getType: getType }); + + sandbox.restore(); + sandbox.stub(ModelUtil, 'isMap').callsFake(() => { + return true; + }); + + const mockMapDeclaration = sinon.createStubInstance(MapDeclaration); + const getKeyType = sinon.stub(); + const getValueType = sinon.stub(); + + getType.returns(mockMapDeclaration); + getKeyType.returns('String'); + getValueType.returns('String'); + mockField.getName.returns('Map1'); + mockField.isOptional.returns(false); + mockMapDeclaration.getName.returns('Map1'); + mockMapDeclaration.isMapDeclaration.returns(true); + mockMapDeclaration.getKey.returns({ getType: getKeyType }); + mockMapDeclaration.getValue.returns({ getType: getValueType }); + + csharpVisitor.visitField(mockField, { ...param, useRequiredForNonOptionalReferenceTypes: true }); + param.fileWriter.writeLine.withArgs(1, 'public required Dictionary Map1 { get; set; }').calledOnce.should.be.ok; + }); }); describe('visitEnumValueDeclaration', () => {