diff --git a/src/typechat.schema/TypeEx.cs b/src/typechat.schema/TypeEx.cs index c5fe980..5785077 100644 --- a/src/typechat.schema/TypeEx.cs +++ b/src/typechat.schema/TypeEx.cs @@ -40,7 +40,21 @@ public static bool IsBoolean(this Type type) public static bool IsDateTime(this Type type) { - return type == typeof(DateTime) || type == typeof(TimeSpan); + // These are value types that System.Text.Json serializes to/from a JSON *string* rather than + // to an object. They must be treated as the 'string' primitive by the exporter (see + // Typescript.Types.ToPrimitive); otherwise they would be exported as (mostly empty) interfaces. + if (type == typeof(DateTime) || type == typeof(TimeSpan) || type == typeof(DateTimeOffset)) + { + return true; + } +#if NET6_0_OR_GREATER + // DateOnly/TimeOnly only exist on net6.0+, but Json serializes them as strings too. + if (type == typeof(DateOnly) || type == typeof(TimeOnly)) + { + return true; + } +#endif + return false; } public static bool IsNumber(this Type type) @@ -149,6 +163,113 @@ internal static bool IsTask(this Type type) return (args.IsNullOrEmpty()) ? null : args[0]; } + /// + /// Is the given type one that serializes to a JSON array? This includes arrays and any type + /// that implements IEnumerable{T} (List{T}, IList{T}, ICollection{T}, HashSet{T}...). + /// Strings (which are IEnumerable{char}) and dictionaries are deliberately excluded: they do + /// not serialize to JSON arrays. + /// + /// type to inspect + /// the type of the items in the array + /// true if the type is array-like + internal static bool IsArrayLike(this Type type, out Type elementType) + { + elementType = null; + + // Strings are IEnumerable but must be exported as scalar strings, not char arrays + if (type.IsString()) + { + return false; + } + // Dictionaries are IEnumerable> but must be exported as maps, not arrays. + // Use a structural check so that even a self-mapping dictionary is excluded here. + if (type.IsDictionaryShaped()) + { + return false; + } + if (type.IsArray) + { + elementType = type.GetElementType(); + return true; + } + + Type? enumerableType = FindGenericInterface(type, typeof(IEnumerable<>)); + if (enumerableType is not null) + { + Type genericArg = enumerableType.GetGenericArguments()[0]; + // Guard against a type that enumerates itself (e.g. class Node : IEnumerable): + // there is no finite element type to unwrap to, so treat it as a plain object instead. + if (genericArg != type) + { + elementType = genericArg; + return true; + } + return false; + } + // Non-generic IEnumerable (e.g. ArrayList): the element type is unknown, so treat it as 'any' + if (typeof(IEnumerable).IsAssignableFrom(type)) + { + elementType = typeof(object); + return true; + } + return false; + } + + /// + /// Is the given type one that serializes to a JSON object/map? This includes any type that + /// implements IDictionary{K,V} or IReadOnlyDictionary{K,V}, as well as the non-generic IDictionary. + /// + /// type to inspect + /// the type of the map's keys + /// the type of the map's values + /// true if the type is a dictionary + internal static bool IsDictionary(this Type type, out Type keyType, out Type valueType) + { + keyType = typeof(string); + valueType = typeof(object); + + Type? dictionaryType = FindGenericInterface(type, typeof(IDictionary<,>)) ?? + FindGenericInterface(type, typeof(IReadOnlyDictionary<,>)); + if (dictionaryType is not null) + { + Type[] args = dictionaryType.GetGenericArguments(); + // Guard against a type that maps to itself (e.g. class D : IDictionary): + // there is no finite value type to unwrap to, so treat it as a plain object instead. + if (args[1] != type) + { + keyType = args[0]; + valueType = args[1]; + return true; + } + return false; + } + // Non-generic IDictionary (e.g. Hashtable): keys and values are untyped + return typeof(IDictionary).IsAssignableFrom(type); + } + + private static Type? FindGenericInterface(Type type, Type genericInterface) + { + if (type.IsGenericType && type.GetGenericTypeDefinition() == genericInterface) + { + return type; + } + foreach (Type iface in type.GetInterfaces()) + { + if (iface.IsGenericType && iface.GetGenericTypeDefinition() == genericInterface) + { + return iface; + } + } + return null; + } + + private static bool IsDictionaryShaped(this Type type) + { + return FindGenericInterface(type, typeof(IDictionary<,>)) is not null || + FindGenericInterface(type, typeof(IReadOnlyDictionary<,>)) is not null || + typeof(IDictionary).IsAssignableFrom(type); + } + internal static IEnumerable Subclasses(this Type type) { if (!type.IsClass) diff --git a/src/typechat.schema/Typescript.cs b/src/typechat.schema/Typescript.cs index 57b6cbd..dc8df58 100644 --- a/src/typechat.schema/Typescript.cs +++ b/src/typechat.schema/Typescript.cs @@ -70,7 +70,15 @@ public static class Types } else if (type.IsDateTime()) { - return String; // Json does not have a primitive DateTime + return String; // Json has no DateTime primitive: these value types serialize as strings + } + else if (type == typeof(Guid)) + { + // Guid is a value type, but System.Text.Json writes it as a JSON string (e.g. + // "3f2504e0-4f89-11d3-9a0c-0305e82c3301") and it exposes no public fields/properties. + // Without this mapping it would fall through to the object path and be exported as an + // empty "interface Guid {}", which a model could never populate correctly. + return String; } else if (type.IsObject()) { diff --git a/src/typechat.schema/TypescriptExporter.cs b/src/typechat.schema/TypescriptExporter.cs index 2d06173..3295374 100644 --- a/src/typechat.schema/TypescriptExporter.cs +++ b/src/typechat.schema/TypescriptExporter.cs @@ -175,29 +175,36 @@ public override void ExportPending() public override void ExportType(Type type) { - if (type.IsArray) + if (type.IsDictionary(out _, out Type valueType)) { - AddPending(type.GetElementType()); + AddPending(valueType); + } + else if (type.IsArrayLike(out Type elementType)) + { + AddPending(elementType); + } + else if (type.IsEnum) + { + ExportEnum(type); } else { - if (type.IsEnum) - { - ExportEnum(type); - } - else - { - ExportClass(type); - } + ExportClass(type); } } public TypescriptExporter ExportClass(Type type) { ArgumentVerify.ThrowIfNull(type, nameof(type)); - if (!type.IsClass) + // Export both reference types (classes) and non-enum value types (structs) as interfaces: + // System.Text.Json serializes a struct's public fields/properties into a JSON object exactly + // like a class (e.g. KeyValuePair => { Key, Value }, a custom Money => { Amount, Currency }). + // Enums are routed to ExportEnum before they reach here, and scalar value types that Json writes + // as a single value (Guid, DateTime, DateTimeOffset, ...) are mapped to primitives in + // Typescript.Types.ToPrimitive, so only genuine object-like structs arrive at this point. + if (!type.IsClass && (!type.IsValueType || type.IsPrimitive || type.IsEnum)) { - ArgumentVerify.Throw($"{type.Name} must be a class"); + ArgumentVerify.Throw($"{type.Name} must be a class or struct"); } if (IsExported(type)) @@ -505,7 +512,7 @@ private TypescriptExporter ExportVariable(MemberInfo member, Type type) _writer.Variable( member.PropertyName(), DataType(actualType), - type.IsArray, + actualType.IsArrayLike(out _), isNullable ); } @@ -534,7 +541,7 @@ private TypescriptExporter ExportParameter(ParameterInfo parameter, int i, int c DataType(type), i, count, - type.IsArray, + type.IsArrayLike(out _), isNullable ); AddPending(type); @@ -658,9 +665,20 @@ private string DataType(Type type) type = type.GetGenericType() ?? typeof(void); } - if (type.IsArray) + Type? nullableValueType = type.GetNullableValueType(); + if (nullableValueType is not null) + { + type = nullableValueType; + } + + if (type.IsDictionary(out Type keyType, out Type valueType)) + { + return DataTypeMap(keyType, valueType); + } + + if (type.IsArrayLike(out Type elementType)) { - return DataType(type.GetElementType()); + return DataType(elementType); } string? typeName = null; @@ -680,6 +698,23 @@ private string DataType(Type type) return typeName; } + private string DataTypeMap(Type keyType, Type valueType) + { + string valueDataType = DataType(valueType); + if (valueType.IsArrayLike(out _)) + { + valueDataType += Typescript.Punctuation.Array; + } + return $"Record<{MapKeyType(keyType)}, {valueDataType}>"; + } + + private static string MapKeyType(Type keyType) + { + // JSON object keys are always strings; only numeric keys are represented as 'number' + string? primitive = Typescript.Types.ToPrimitive(keyType); + return primitive == Typescript.Types.Number ? Typescript.Types.Number : Typescript.Types.String; + } + private string InterfaceName(Type type, bool useMapper = true) { string? typeName = null; @@ -756,12 +791,26 @@ protected override bool ShouldExport(Type type, out Type typeToExport) return false; } - if (type.IsArray) + // Unwrap arrays, collections and dictionaries down to the underlying element/value type + while (true) { - type = type.GetElementType(); + if (type.IsDictionary(out _, out Type valueType)) + { + type = valueType; + } + else if (type.IsArrayLike(out Type elementType)) + { + type = elementType; + } + else + { + break; + } } typeToExport = type; - return !type.IsPrimitive && !_nonExportTypes.Contains(type); + return !type.IsPrimitive && + !type.IsNullableValueType() && + !_nonExportTypes.Contains(type); } } diff --git a/src/typechat/OpenAIConfig.cs b/src/typechat/OpenAIConfig.cs index bde1f88..6c4128b 100644 --- a/src/typechat/OpenAIConfig.cs +++ b/src/typechat/OpenAIConfig.cs @@ -39,11 +39,6 @@ public static class VariableNames /// public const string OPENAI_EMBEDDINGMODEL = "OPENAI_EMBEDDINGMODEL"; - /// - /// The embedding endpoint to use - /// - public const string OPENAI_EMBEDDING_ENDPOINT = "OPENAI_EMBEDDING_ENDPOINT"; - /// /// Api key to use for Azure OpenAI service /// @@ -149,11 +144,6 @@ public static OpenAIConfig FromEnvironment(bool isEmbedding = false) } if (isEmbedding) { - if (Environment.GetEnvironmentVariable(VariableNames.OPENAI_EMBEDDING_ENDPOINT) != null) - { - config.Endpoint = Environment.GetEnvironmentVariable(VariableNames.OPENAI_EMBEDDING_ENDPOINT); - } - config.Model = Environment.GetEnvironmentVariable(VariableNames.OPENAI_EMBEDDINGMODEL); } else diff --git a/tests/TypeChat.TestLib/Schemas.cs b/tests/TypeChat.TestLib/Schemas.cs index f49236e..d50c366 100644 --- a/tests/TypeChat.TestLib/Schemas.cs +++ b/tests/TypeChat.TestLib/Schemas.cs @@ -336,6 +336,69 @@ public class FriendsOfPerson public Name[] FriendNames { get; set; } } +// For testing that IEnumerable exports as a JSON array (string[]) and not as a +// leaky List_String interface. See https://github.com/microsoft/typechat.net/issues/218 +public sealed class Pizza +{ + public IEnumerable Toppings { get; set; } + public string Size { get; set; } +} + +// For testing that the various collection and dictionary types all export as JSON arrays/maps +public class CollectionsObj +{ + public IEnumerable Tags { get; set; } + public List Names { get; set; } + public IList Scores { get; set; } + public ICollection Locations { get; set; } + public IReadOnlyList Ratings { get; set; } + public HashSet UniqueTags { get; set; } + public string[] Aliases { get; set; } + public Dictionary Counts { get; set; } + public IDictionary LocationsByCity { get; set; } + public Dictionary NamesById { get; set; } + public Dictionary> Buckets { get; set; } +} + +// A type that enumerates itself (composite pattern): it implements IEnumerable, +// so it has no finite "array of..." representation. Schema generation must still terminate. +public class Composite : IEnumerable +{ + public string Name { get; set; } + public IEnumerator GetEnumerator() => Enumerable.Empty().GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); +} + +public class CompositeHolder +{ + public List Nodes { get; set; } + public Composite Root { get; set; } +} + +// For testing that structs export as interfaces: Json serializes a struct's public members as an +// object, just like a class. +public struct Money +{ + public decimal Amount { get; set; } + public string Currency { get; set; } +} + +public struct Coordinates +{ + public double Latitude { get; set; } + public double Longitude { get; set; } +} + +public class StructHolder +{ + public Money Price { get; set; } + public Coordinates Location { get; set; } + public KeyValuePair Pair { get; set; } + // Scalar value types that Json writes as a single string - must NOT become empty interfaces + public Guid Id { get; set; } + public DateTimeOffset When { get; set; } +} + // For testing Generics public class Child { diff --git a/tests/TypeChat.UnitTests/TestSchema.cs b/tests/TypeChat.UnitTests/TestSchema.cs index ecd36d4..db81aed 100644 --- a/tests/TypeChat.UnitTests/TestSchema.cs +++ b/tests/TypeChat.UnitTests/TestSchema.cs @@ -95,5 +95,100 @@ public void ExportGenerics() Assert.True(lines.ContainsSubstring("Child_Name[]")); Assert.True(lines.ContainsSubstring("Child_Location[]")); } + + [Fact] + public void ExportEnumerable_Issue218() + { + var schema = TypescriptExporter.GenerateSchema(typeof(Pizza)); + var lines = schema.Schema.Text.Lines(); + + // IEnumerable must be exported as a JSON array of strings... + Assert.True(lines.ContainsSubstring("Toppings", "string[]")); + Assert.True(lines.ContainsSubstring("Size", "string")); + // ...and NOT as a leaky List_String interface exposing Capacity/Count/Item + Assert.False(lines.ContainsSubstring("List_String")); + Assert.False(lines.ContainsSubstring("Capacity")); + } + + [Fact] + public void ExportCollections() + { + var schema = TypescriptExporter.GenerateSchema(typeof(CollectionsObj)); + var lines = schema.Schema.Text.Lines(); + + // IEnumerable, List, IList, ICollection, IReadOnlyList, HashSet and arrays => T[] + Assert.True(lines.ContainsSubstring("Tags", "string[]")); + Assert.True(lines.ContainsSubstring("Names", "Name[]")); + Assert.True(lines.ContainsSubstring("Scores", "number[]")); + Assert.True(lines.ContainsSubstring("Locations", "Location[]")); + Assert.True(lines.ContainsSubstring("Ratings", "number[]")); + Assert.True(lines.ContainsSubstring("UniqueTags", "string[]")); + Assert.True(lines.ContainsSubstring("Aliases", "string[]")); + + // Dictionaries => Record, including a dictionary whose value is itself a collection + Assert.True(lines.ContainsSubstring("Counts", "Record")); + Assert.True(lines.ContainsSubstring("LocationsByCity", "Record")); + Assert.True(lines.ContainsSubstring("NamesById", "Record")); + Assert.True(lines.ContainsSubstring("Buckets", "Record")); + + // Element/value types are still exported as interfaces + Assert.True(lines.ContainsSubstring("interface", "Name")); + Assert.True(lines.ContainsSubstring("interface", "Location")); + + // The internals of List/Dictionary<> must NOT leak into the schema + Assert.False(lines.ContainsSubstring("Capacity")); + Assert.False(lines.ContainsSubstring("KeyValuePair")); + Assert.False(lines.ContainsSubstring("interface", "List_")); + } + + [Fact] + public void SelfReferentialEnumerable_Terminates() + { + // Composite : IEnumerable has no finite "array of..." representation. + // Schema generation must terminate rather than loop forever while unwrapping it. + TypescriptSchema schema = null; + var worker = new System.Threading.Thread( + () => schema = TypescriptExporter.GenerateSchema(typeof(CompositeHolder))) + { + IsBackground = true + }; + worker.Start(); + bool finished = worker.Join(System.TimeSpan.FromSeconds(5)); + + Assert.True(finished, "Schema generation did not terminate for a self-referential IEnumerable type"); + + var lines = schema.Schema.Text.Lines(); + // A List is still a normal array... + Assert.True(lines.ContainsSubstring("Nodes", "Composite[]")); + // ...but the self-enumerating Composite itself falls back to a plain object interface + Assert.True(lines.ContainsSubstring("interface", "Composite")); + Assert.True(lines.ContainsSubstring("Name", "string")); + } + + [Fact] + public void ExportStructs() + { + var schema = TypescriptExporter.GenerateSchema(typeof(StructHolder)); + var lines = schema.Schema.Text.Lines(); + + // Structs export as interfaces, because Json serializes their public members as an object + Assert.True(lines.ContainsSubstring("Price", "Money")); + Assert.True(lines.ContainsSubstring("interface", "Money")); + Assert.True(lines.ContainsSubstring("Amount", "number")); + Assert.True(lines.ContainsSubstring("Currency", "string")); + + Assert.True(lines.ContainsSubstring("Location", "Coordinates")); + Assert.True(lines.ContainsSubstring("interface", "Coordinates")); + + Assert.True(lines.ContainsSubstring("Pair", "KeyValuePair")); + Assert.True(lines.ContainsSubstring("Key", "string")); + Assert.True(lines.ContainsSubstring("Value", "number")); + + // Scalar value types map to string, NOT to empty interfaces + Assert.True(lines.ContainsSubstring("Id", "string")); + Assert.True(lines.ContainsSubstring("When", "string")); + Assert.False(lines.ContainsSubstring("interface", "Guid")); + Assert.False(lines.ContainsSubstring("interface", "DateTimeOffset")); + } }