Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix subclass typedefs #359

Merged
merged 2 commits into from
Aug 16, 2024
Merged
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
2 changes: 1 addition & 1 deletion examples/semantic-kernel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
},
"devDependencies": {
"@types/node": "^20.8.10",
"typescript": "^5.3.3"
"typescript": "~5.5.4"
}
}
4 changes: 2 additions & 2 deletions src/NodeApi.DotNetHost/JSMarshaller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/NodeApi.Generator/SourceGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public enum DiagnosticId
UnsupportedMethodReturnType,
UnsupportedOverloads,
UnsupportedIndexer,
UnsupportedExtensionMethod,
ReferenedTypeNotExported,
ESModulePropertiesAreConst,
DocLoadError,
Expand Down
186 changes: 124 additions & 62 deletions src/NodeApi.Generator/TypeDefinitionsGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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<void>.
implements = $" extends {FormatTSType(type.BaseType, null)}" + implements;
}

string exportPrefix = "export ";
Expand All @@ -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")
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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 ||
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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!);
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -1878,6 +1892,54 @@ private string GetTSType(
return tsType;
}

private string FormatTSType(
Type type,
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 :
nsPrefix + 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)
Expand Down Expand Up @@ -2333,7 +2395,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
Expand Down
3 changes: 3 additions & 0 deletions src/NodeApi/Interop/JSAbortSignal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion test/TestCases/projects/js-cjs-dynamic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/js-cjs-module/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/js-esm-dynamic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/js-esm-module/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/ts-cjs-dynamic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/ts-cjs-module/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/ts-esm-dynamic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
2 changes: 1 addition & 1 deletion test/TestCases/projects/ts-esm-module/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
},
"devDependencies": {
"@types/node": "^18.16.14",
"typescript": "^5.0.4"
"typescript": "~5.5.4"
}
}
Loading
Loading