From a14114eb8c416ab8d6495155ecfd1f6f214d078f Mon Sep 17 00:00:00 2001 From: Jason Ginchereau Date: Wed, 14 Aug 2024 21:07:29 -1000 Subject: [PATCH 1/2] Fix subclass typedefs --- examples/semantic-kernel/package.json | 2 +- src/NodeApi.DotNetHost/JSMarshaller.cs | 4 +- src/NodeApi.Generator/SourceGenerator.cs | 1 + .../TypeDefinitionsGenerator.cs | 183 ++++++++++++------ src/NodeApi/Interop/JSAbortSignal.cs | 3 + .../projects/js-cjs-dynamic/package.json | 2 +- .../projects/js-cjs-module/package.json | 2 +- .../projects/js-esm-dynamic/package.json | 2 +- .../projects/js-esm-module/package.json | 2 +- .../projects/ts-cjs-dynamic/package.json | 2 +- .../projects/ts-cjs-module/package.json | 2 +- .../projects/ts-esm-dynamic/package.json | 2 +- .../projects/ts-esm-module/package.json | 2 +- 13 files changed, 136 insertions(+), 73 deletions(-) diff --git a/examples/semantic-kernel/package.json b/examples/semantic-kernel/package.json index ffadc283..39f014c7 100644 --- a/examples/semantic-kernel/package.json +++ b/examples/semantic-kernel/package.json @@ -9,6 +9,6 @@ }, "devDependencies": { "@types/node": "^20.8.10", - "typescript": "^5.3.3" + "typescript": "~5.5.4" } } diff --git a/src/NodeApi.DotNetHost/JSMarshaller.cs b/src/NodeApi.DotNetHost/JSMarshaller.cs index 62a4dbf0..babe1059 100644 --- a/src/NodeApi.DotNetHost/JSMarshaller.cs +++ b/src/NodeApi.DotNetHost/JSMarshaller.cs @@ -2181,9 +2181,9 @@ Expression TupleItem(int index) => InlineOrInvoke( else if (toType == typeof(CancellationToken)) { MethodInfo toAbortSignal = typeof(JSAbortSignal).GetExplicitConversion( - typeof(JSValue), typeof(JSAbortSignal)); + typeof(JSValue), typeof(JSAbortSignal?)); MethodInfo toCancellationToken = typeof(JSAbortSignal).GetExplicitConversion( - typeof(JSAbortSignal), typeof(CancellationToken)); + typeof(JSAbortSignal?), typeof(CancellationToken)); statements = new[] { Expression.Call( diff --git a/src/NodeApi.Generator/SourceGenerator.cs b/src/NodeApi.Generator/SourceGenerator.cs index ab156b78..4995435f 100644 --- a/src/NodeApi.Generator/SourceGenerator.cs +++ b/src/NodeApi.Generator/SourceGenerator.cs @@ -42,6 +42,7 @@ public enum DiagnosticId UnsupportedMethodReturnType, UnsupportedOverloads, UnsupportedIndexer, + UnsupportedExtensionMethod, ReferenedTypeNotExported, ESModulePropertiesAreConst, DocLoadError, diff --git a/src/NodeApi.Generator/TypeDefinitionsGenerator.cs b/src/NodeApi.Generator/TypeDefinitionsGenerator.cs index a16fd6d3..c48e9d42 100644 --- a/src/NodeApi.Generator/TypeDefinitionsGenerator.cs +++ b/src/NodeApi.Generator/TypeDefinitionsGenerator.cs @@ -822,7 +822,8 @@ private void GenerateClassDefinition(ref SourceBuilder s, Type type) GenerateDocComments(ref s, type); bool isStaticClass = type.IsAbstract && type.IsSealed && !type.IsGenericTypeDefinition; - bool isStreamSubclass = type.BaseType?.FullName == typeof(Stream).FullName; + bool isStreamSubclass = type.BaseType?.FullName == typeof(Stream).FullName || + type.BaseType?.BaseType?.FullName == typeof(Stream).FullName; string classKind = type.IsInterface ? "interface" : isStaticClass ? "namespace" : "class"; string implementsKind = type.IsInterface ? "extends" : "implements"; @@ -863,10 +864,14 @@ private void GenerateClassDefinition(ref SourceBuilder s, Type type) } } - if (isStreamSubclass) + if (type.BaseType?.FullName == typeof(Stream).FullName) { implements = " extends Duplex" + implements; - _emitDuplex = true; + } + else if (type.BaseType != null && type.BaseType.FullName != typeof(object).FullName) + { + // Don't use GetTSType() here: Avoid converting to JS types like Task => Promise. + implements = $" extends {FormatTSType(type.BaseType, null)}" + implements; } string exportPrefix = "export "; @@ -892,31 +897,43 @@ private void GenerateClassDefinition(ref SourceBuilder s, Type type) if (!isStreamSubclass) { - foreach (PropertyInfo property in type.GetProperties( - BindingFlags.Public | BindingFlags.Instance | + BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Instance | (isStaticClass ? BindingFlags.DeclaredOnly : default) | - (type.IsInterface ? default : BindingFlags.Static)).Where(IsExported)) + (type.IsInterface ? default : BindingFlags.Static); + + // Include only properties declared by the current type. + foreach (PropertyInfo property in + type.GetProperties(bindingFlags | BindingFlags.DeclaredOnly) + .Where((p) => IsExported(p) && !IsExcluded(p))) { // Indexed properties are not implemented. - if (!IsExcluded(property) && property.GetIndexParameters().Length == 0) + if (property.GetIndexParameters().Length == 0) { if (isFirstMember) isFirstMember = false; else s++; ExportTypeMember(ref s, property); } } - foreach (MethodInfo method in type.GetMethods( - BindingFlags.Public | BindingFlags.Instance | - (isStaticClass ? BindingFlags.DeclaredOnly : default) | - (type.IsInterface ? default : BindingFlags.Static)).Where(IsExported)) + // Include methods that are declared on the current type or the same name as a method + // declared on the current type. This ensures the full set of overloads is included. + string[] declaredMethodNames = + type.GetMethods(bindingFlags | BindingFlags.DeclaredOnly) + .Where(IsExported) + .Select((m) => m.Name).ToArray(); + foreach (MethodInfo method in type.GetMethods(bindingFlags) + .Where((m) => IsExported(m) && !IsExcluded(m))) { - if (!IsExcluded(method)) + if (method.DeclaringType == type || declaredMethodNames.Contains(method.Name)) { if (isFirstMember) isFirstMember = false; else s++; ExportTypeMember(ref s, method); } } } + else + { + _emitDuplex = true; + } s += "}"; if (exportName == "__default") @@ -1120,18 +1137,7 @@ private void ExportTypeMember(ref SourceBuilder s, MemberInfo member, bool asExt { GenerateDocComments(ref s, constructor); string parameters = GetTSParameters(constructor.GetParameters()); - - if (declaringType.IsGenericTypeDefinition) - { - string exportName = GetExportName(declaringType); - Type[] typeArgs = declaringType.GetGenericArguments(); - string typeParams = string.Join(", ", typeArgs.Select((t) => t.Name)); - s += $"new({parameters}): {exportName}${typeArgs.Length}<{typeParams}>;"; - } - else - { - s += $"constructor({parameters});"; - } + s += $"constructor({parameters});"; } else if (member is PropertyInfo property) { @@ -1246,6 +1252,11 @@ private static bool IsExcluded(MemberInfo member) Type type = member as Type ?? member.DeclaringType!; + if (type.BaseType != null && IsExcluded(type.BaseType)) + { + return true; + } + // While most types in InteropServices are excluded, safe handle classes are useful // to include because they are extended by handle classes in other assemblies. if (type.FullName == typeof(System.Runtime.InteropServices.SafeHandle).FullName || @@ -1259,6 +1270,7 @@ private static bool IsExcluded(MemberInfo member) return type.Namespace switch { "System.Runtime.CompilerServices" or + "System.Runtime.Hosting" or "System.Runtime.InteropServices" or "System.Runtime.Remoting.Messaging" or "System.Runtime.Serialization" or @@ -1283,7 +1295,7 @@ private static bool IsExcluded(PropertyInfo property) return false; } - private static bool IsExcluded(MethodBase method) + private bool IsExcluded(MethodBase method) { // Exclude "special" methods like property get/set and event add/remove. if (method is MethodInfo && method.IsSpecialName) @@ -1320,13 +1332,44 @@ private static bool IsExcluded(MethodBase method) return true; } - if (method.GetParameters().Any((p) => IsExcluded(p.ParameterType)) || - (method is MethodInfo methodWithReturn && IsExcluded(methodWithReturn.ReturnType))) + bool isExcluded = false; + foreach (ParameterInfo parameter in method.GetParameters()) { - return true; + if (IsExcluded(parameter.ParameterType)) + { + ReportWarning( + DiagnosticId.UnsupportedMethodParameterType, + $"Method {method.DeclaringType!.Name}.{method.Name}() has unsupported " + + $"parameter type: {parameter.ParameterType.Name}"); + isExcluded = true; + } } - return false; + if (method is MethodInfo methodWithReturn && IsExcluded(methodWithReturn.ReturnType)) + { + ReportWarning( + DiagnosticId.UnsupportedMethodReturnType, + $"Method {method.DeclaringType!.Name}.{method.Name}() has unsupported " + + $"return type: {methodWithReturn.ReturnType.Name}"); + isExcluded = true; + } + + if (IsExtensionMember(method)) + { + Type extensionTargetType = method.GetParameters()[0].ParameterType; + if (extensionTargetType.GetMethods(BindingFlags.Public | BindingFlags.Instance) + .Any((m) => m.Name == method.Name)) + { + ReportWarning( + DiagnosticId.UnsupportedExtensionMethod, + $"Extension method {method.DeclaringType!.Name}.{method.Name}() with target " + + $"type {extensionTargetType.Name} is not supported because it overloads a " + + "method on the target type."); + isExcluded = true; + } + } + + return isExcluded; } private void GenerateEnumDefinition(ref SourceBuilder s, Type type) @@ -1632,11 +1675,7 @@ private string GetTSType( } else if (IsExported(type)) { - // Types exported from a module are not namespaced. - string nsPrefix = !_isModule && type.Namespace != null ? type.Namespace + '.' : ""; - - tsType = (type.IsNested ? GetTSType(type.DeclaringType!, null) + '.' : nsPrefix) + - (_isModule ? GetExportName(type) : type.Name); + tsType = FormatTSType(type, nullability, allowTypeParams); } } else if (type.FullName == typeof(ValueTuple).FullName) @@ -1682,9 +1721,7 @@ private string GetTSType( } else if (_referenceAssemblies.ContainsKey(type.Assembly.GetName().Name!)) { - tsType = type.IsNested ? - GetTSType(type.DeclaringType!, null, allowTypeParams) + '.' + type.Name : - (type.Namespace != null ? type.Namespace + '.' + type.Name : type.Name); + tsType = FormatTSType(type, nullability, allowTypeParams); _imports.Add(type.Assembly.GetName().Name!); } @@ -1839,30 +1876,7 @@ private string GetTSType( } else { - int typeNameEnd = tsType.IndexOf('`'); - if (typeNameEnd > 0) - { - tsType = tsType.Substring(0, typeNameEnd); - - string typeParams = string.Join(", ", typeArgs.Select( - (t, i) => GetTSType(t, typeArgsNullability?[i], allowTypeParams))); - tsType = $"{tsType}${typeArgs.Length}<{typeParams}>"; - } - else if (type.IsNested && type.DeclaringType!.IsGenericTypeDefinition) - { - int genericParamsStart = tsType.IndexOf('$'); - int genericParamsEnd = tsType.IndexOf('>'); - if (genericParamsStart > 0 && genericParamsEnd > 0) - { - // TS doesn't support nested types (static properties) on a generic class. - // For now, move the generic type parameters onto the nested class. - string declaringType = tsType.Substring(0, genericParamsStart); - string genericParams = tsType.Substring( - genericParamsStart, genericParamsEnd + 1 - genericParamsStart); - string nestedType = tsType.Substring(genericParamsEnd + 1); - tsType = $"{declaringType}{nestedType}{genericParams}"; - } - } + tsType = FormatTSType(type, nullability, allowTypeParams); } } @@ -1878,6 +1892,51 @@ private string GetTSType( return tsType; } + private string FormatTSType( + Type type, + NullabilityInfo? nullability, + bool allowTypeParams = true) + { + string tsType = type.IsNested ? + GetTSType(type.DeclaringType!, null, allowTypeParams) + '.' + type.Name : + (type.Namespace != null ? type.Namespace + '.' + type.Name : type.Name); + + int typeNameEnd = tsType.IndexOf('`'); + if (typeNameEnd > 0) + { + Type[] typeArgs = type.GetGenericArguments(); + NullabilityInfo[]? typeArgsNullability = nullability?.GenericTypeArguments; + if (typeArgsNullability?.Length < typeArgs.Length) + { + // NullabilityContext doesn't handle generic type arguments of by-ref parameters. + typeArgsNullability = null; + } + + tsType = tsType.Substring(0, typeNameEnd); + + string typeParams = string.Join(", ", typeArgs.Select( + (t, i) => GetTSType(t, typeArgsNullability?[i], allowTypeParams))); + tsType = $"{tsType}${typeArgs.Length}<{typeParams}>"; + } + else if (type.IsNested && type.DeclaringType!.IsGenericTypeDefinition) + { + int genericParamsStart = tsType.IndexOf('$'); + int genericParamsEnd = tsType.IndexOf('>'); + if (genericParamsStart > 0 && genericParamsEnd > 0) + { + // TS doesn't support nested types (static properties) on a generic class. + // For now, move the generic type parameters onto the nested class. + string declaringType = tsType.Substring(0, genericParamsStart); + string genericParams = tsType.Substring( + genericParamsStart, genericParamsEnd + 1 - genericParamsStart); + string nestedType = tsType.Substring(genericParamsEnd + 1); + tsType = $"{declaringType}{nestedType}{genericParams}"; + } + } + + return tsType; + } + private string GetTSParameters(ParameterInfo[] parameters) { static string GetOptionalToken(ParameterInfo parameter, ref string parameterType) @@ -2333,7 +2392,7 @@ private static string FormatDocMemberParameterType( private static string TSIdentifier(string? identifier) { return string.IsNullOrEmpty(identifier) ? "_" : - s_tsReservedWords.Contains(identifier) ? "_" + identifier : identifier; + s_tsReservedWords.Contains(identifier!) ? "_" + identifier : identifier!; } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#reserved_words diff --git a/src/NodeApi/Interop/JSAbortSignal.cs b/src/NodeApi/Interop/JSAbortSignal.cs index 0373de24..456c902f 100644 --- a/src/NodeApi/Interop/JSAbortSignal.cs +++ b/src/NodeApi/Interop/JSAbortSignal.cs @@ -55,6 +55,9 @@ private JSAbortSignal(JSValue value) public static explicit operator CancellationToken(JSAbortSignal signal) => signal.ToCancellationToken(); + public static explicit operator CancellationToken(JSAbortSignal? signal) + => signal?.ToCancellationToken() ?? default; + public static explicit operator JSAbortSignal(CancellationToken cancellation) => FromCancellationToken(cancellation); diff --git a/test/TestCases/projects/js-cjs-dynamic/package.json b/test/TestCases/projects/js-cjs-dynamic/package.json index 5055532d..6a8f8a51 100644 --- a/test/TestCases/projects/js-cjs-dynamic/package.json +++ b/test/TestCases/projects/js-cjs-dynamic/package.json @@ -8,6 +8,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/js-cjs-module/package.json b/test/TestCases/projects/js-cjs-module/package.json index 59f0c866..17d1d199 100644 --- a/test/TestCases/projects/js-cjs-module/package.json +++ b/test/TestCases/projects/js-cjs-module/package.json @@ -8,6 +8,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/js-esm-dynamic/package.json b/test/TestCases/projects/js-esm-dynamic/package.json index 49516c61..0e05100e 100644 --- a/test/TestCases/projects/js-esm-dynamic/package.json +++ b/test/TestCases/projects/js-esm-dynamic/package.json @@ -9,6 +9,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/js-esm-module/package.json b/test/TestCases/projects/js-esm-module/package.json index 910f7d95..af7d9cd1 100644 --- a/test/TestCases/projects/js-esm-module/package.json +++ b/test/TestCases/projects/js-esm-module/package.json @@ -9,6 +9,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/ts-cjs-dynamic/package.json b/test/TestCases/projects/ts-cjs-dynamic/package.json index acfbb89b..dec2112a 100644 --- a/test/TestCases/projects/ts-cjs-dynamic/package.json +++ b/test/TestCases/projects/ts-cjs-dynamic/package.json @@ -8,6 +8,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/ts-cjs-module/package.json b/test/TestCases/projects/ts-cjs-module/package.json index 59f0c866..17d1d199 100644 --- a/test/TestCases/projects/ts-cjs-module/package.json +++ b/test/TestCases/projects/ts-cjs-module/package.json @@ -8,6 +8,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/ts-esm-dynamic/package.json b/test/TestCases/projects/ts-esm-dynamic/package.json index 6f285b89..31e600dc 100644 --- a/test/TestCases/projects/ts-esm-dynamic/package.json +++ b/test/TestCases/projects/ts-esm-dynamic/package.json @@ -9,6 +9,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } diff --git a/test/TestCases/projects/ts-esm-module/package.json b/test/TestCases/projects/ts-esm-module/package.json index 910f7d95..af7d9cd1 100644 --- a/test/TestCases/projects/ts-esm-module/package.json +++ b/test/TestCases/projects/ts-esm-module/package.json @@ -9,6 +9,6 @@ }, "devDependencies": { "@types/node": "^18.16.14", - "typescript": "^5.0.4" + "typescript": "~5.5.4" } } From dd499bedd3c2ef06a00702e8bfe0270383614b86 Mon Sep 17 00:00:00 2001 From: Jason Ginchereau Date: Wed, 14 Aug 2024 22:01:00 -1000 Subject: [PATCH 2/2] Fix base class TS namespace --- src/NodeApi.Generator/TypeDefinitionsGenerator.cs | 5 ++++- test/TypeDefsGeneratorTests.cs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/NodeApi.Generator/TypeDefinitionsGenerator.cs b/src/NodeApi.Generator/TypeDefinitionsGenerator.cs index c48e9d42..6a28d8dc 100644 --- a/src/NodeApi.Generator/TypeDefinitionsGenerator.cs +++ b/src/NodeApi.Generator/TypeDefinitionsGenerator.cs @@ -1897,9 +1897,12 @@ private string FormatTSType( NullabilityInfo? nullability, bool allowTypeParams = true) { + // Types exported from a module are not namespaced. + string nsPrefix = !_isModule && type.Namespace != null ? type.Namespace + '.' : ""; + string tsType = type.IsNested ? GetTSType(type.DeclaringType!, null, allowTypeParams) + '.' + type.Name : - (type.Namespace != null ? type.Namespace + '.' + type.Name : type.Name); + nsPrefix + type.Name; int typeNameEnd = tsType.IndexOf('`'); if (typeNameEnd > 0) diff --git a/test/TypeDefsGeneratorTests.cs b/test/TypeDefsGeneratorTests.cs index 14d57633..003571c5 100644 --- a/test/TypeDefsGeneratorTests.cs +++ b/test/TypeDefsGeneratorTests.cs @@ -198,7 +198,7 @@ public void GenerateGenericClass() /** generic-class */ export class GenericClass$1 implements GenericInterface$1 { /** constructor */ - new(value: T): GenericClass$1; + constructor(value: T); /** instance-property */ TestProperty: T;