From 40c539bb504c64632d464ba8bc8d13ff7fb92aa2 Mon Sep 17 00:00:00 2001 From: Hossein Yousefi Date: Fri, 10 Jan 2025 16:37:38 +0100 Subject: [PATCH 1/3] [jnigen] Fix crash on Kotlin wildcards --- pkgs/jni/CHANGELOG.md | 4 +++ pkgs/jni/pubspec.yaml | 2 +- .../elements/KotlinTypeProjection.java | 3 +++ .../lib/src/bindings/kotlin_processor.dart | 9 ++++--- pkgs/jnigen/lib/src/elements/elements.dart | 7 +++++- pkgs/jnigen/lib/src/elements/elements.g.dart | 5 ++-- pkgs/jnigen/pubspec.yaml | 2 +- .../test/kotlin_test/bindings/kotlin.dart | 25 +++++++++++++++++++ .../github/dart_lang/jnigen/Nullability.kt | 4 +++ .../kotlin_test/runtime_test_registrant.dart | 8 ++++++ 10 files changed, 61 insertions(+), 8 deletions(-) diff --git a/pkgs/jni/CHANGELOG.md b/pkgs/jni/CHANGELOG.md index db31af1ea0..f36f3bb652 100644 --- a/pkgs/jni/CHANGELOG.md +++ b/pkgs/jni/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.13.1 + +- Fixed a bug where Kotlin wildcards would crash the code generation. + ## 0.13.0 - **Breaking Change**: Separated primitive arrays from object arrays. diff --git a/pkgs/jni/pubspec.yaml b/pkgs/jni/pubspec.yaml index dfabbfd1d9..a4ee75f28a 100644 --- a/pkgs/jni/pubspec.yaml +++ b/pkgs/jni/pubspec.yaml @@ -4,7 +4,7 @@ name: jni description: A library to access JNI from Dart and Flutter that acts as a support library for package:jnigen. -version: 0.13.0 +version: 0.13.1-wip repository: https://github.com/dart-lang/native/tree/main/pkgs/jni issue_tracker: https://github.com/dart-lang/native/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Ajni diff --git a/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java b/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java index 364634c2e9..41f1f8cf7c 100644 --- a/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java +++ b/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java @@ -15,6 +15,9 @@ public static KotlinTypeProjection fromKmTypeProjection(KmTypeProjection t) { var proj = new KotlinTypeProjection(); proj.type = KotlinType.fromKmType(t.getType()); proj.variance = t.getVariance(); + if (proj.type == null) { + return null; + } return proj; } } diff --git a/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart b/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart index b1539fa94b..260d8301ad 100644 --- a/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart +++ b/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart @@ -254,7 +254,9 @@ class _KotlinTypeProcessor extends TypeVisitor { @override void visitDeclaredType(DeclaredType node) { for (var i = 0; i < node.params.length; ++i) { - node.params[i].accept(_KotlinTypeProcessor(kotlinType.arguments[i].type)); + if (kotlinType.arguments[i]?.type case final type?) { + node.params[i].accept(_KotlinTypeProcessor(type)); + } } super.visitDeclaredType(node); } @@ -262,8 +264,9 @@ class _KotlinTypeProcessor extends TypeVisitor { @override void visitArrayType(ArrayType node) { if (kotlinType.arguments.isNotEmpty) { - node.elementType - .accept(_KotlinTypeProcessor(kotlinType.arguments.first.type)); + if (kotlinType.arguments.first?.type case final type?) { + node.elementType.accept(_KotlinTypeProcessor(type)); + } } super.visitArrayType(node); } diff --git a/pkgs/jnigen/lib/src/elements/elements.dart b/pkgs/jnigen/lib/src/elements/elements.dart index d1bcaaadd5..b55cba30aa 100644 --- a/pkgs/jnigen/lib/src/elements/elements.dart +++ b/pkgs/jnigen/lib/src/elements/elements.dart @@ -78,6 +78,7 @@ class ClassDecl with ClassMember, Annotated implements Element { this.kotlinPackage, }); + @JsonKey(includeFromJson: false) bool isExcluded; @override @@ -625,6 +626,7 @@ class Method with ClassMember, Annotated implements Element { required this.returnType, }); + @JsonKey(includeFromJson: false) bool isExcluded; @override @@ -728,6 +730,7 @@ class Field with ClassMember, Annotated implements Element { this.defaultValue, }); + @JsonKey(includeFromJson: false) bool isExcluded; @override @@ -1120,7 +1123,9 @@ class KotlinType implements Element { final String kind; final String? name; final int id; - final List arguments; + + /// `null` represents a wildcard. + final List arguments; final bool isNullable; factory KotlinType.fromJson(Map json) => diff --git a/pkgs/jnigen/lib/src/elements/elements.g.dart b/pkgs/jnigen/lib/src/elements/elements.g.dart index 180741d658..5882c73802 100644 --- a/pkgs/jnigen/lib/src/elements/elements.g.dart +++ b/pkgs/jnigen/lib/src/elements/elements.g.dart @@ -335,8 +335,9 @@ KotlinType _$KotlinTypeFromJson(Map json) => KotlinType( id: (json['id'] as num).toInt(), isNullable: json['isNullable'] as bool, arguments: (json['arguments'] as List?) - ?.map((e) => - KotlinTypeProjection.fromJson(e as Map)) + ?.map((e) => e == null + ? null + : KotlinTypeProjection.fromJson(e as Map)) .toList() ?? const [], ); diff --git a/pkgs/jnigen/pubspec.yaml b/pkgs/jnigen/pubspec.yaml index 00accf350a..5b57e16f53 100644 --- a/pkgs/jnigen/pubspec.yaml +++ b/pkgs/jnigen/pubspec.yaml @@ -4,7 +4,7 @@ name: jnigen description: A Dart bindings generator for Java and Kotlin that uses JNI under the hood to interop with Java virtual machine. -version: 0.13.0 +version: 0.13.1-wip repository: https://github.com/dart-lang/native/tree/main/pkgs/jnigen issue_tracker: https://github.com/dart-lang/native/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Ajnigen diff --git a/pkgs/jnigen/test/kotlin_test/bindings/kotlin.dart b/pkgs/jnigen/test/kotlin_test/bindings/kotlin.dart index 6b1ab86c64..22bbe3c959 100644 --- a/pkgs/jnigen/test/kotlin_test/bindings/kotlin.dart +++ b/pkgs/jnigen/test/kotlin_test/bindings/kotlin.dart @@ -994,6 +994,31 @@ class Nullability<$T extends jni$_.JObject?, $U extends jni$_.JObject> .object(const jni$_.JStringNullableType()); } + static final _id_list = _class.instanceMethodId( + r'list', + r'()Ljava/util/List;', + ); + + static final _list = jni$_.ProtectedJniExtensions.lookup< + jni$_.NativeFunction< + jni$_.JniResult Function( + jni$_.Pointer, + jni$_.JMethodIDPtr, + )>>('globalEnv_CallObjectMethod') + .asFunction< + jni$_.JniResult Function( + jni$_.Pointer, + jni$_.JMethodIDPtr, + )>(); + + /// from: `public final java.util.List list()` + /// The returned object must be released after use, by calling the [release] method. + jni$_.JList list() { + return _list(reference.pointer, _id_list as jni$_.JMethodIDPtr) + .object>( + const jni$_.JListType(jni$_.JObjectNullableType())); + } + static final _id_methodGenericEcho = _class.instanceMethodId( r'methodGenericEcho', r'(Ljava/lang/Object;)Ljava/lang/Object;', diff --git a/pkgs/jnigen/test/kotlin_test/kotlin/src/main/kotlin/com/github/dart_lang/jnigen/Nullability.kt b/pkgs/jnigen/test/kotlin_test/kotlin/src/main/kotlin/com/github/dart_lang/jnigen/Nullability.kt index 159be46baf..25e3c7042f 100644 --- a/pkgs/jnigen/test/kotlin_test/kotlin/src/main/kotlin/com/github/dart_lang/jnigen/Nullability.kt +++ b/pkgs/jnigen/test/kotlin_test/kotlin/src/main/kotlin/com/github/dart_lang/jnigen/Nullability.kt @@ -14,6 +14,10 @@ public class Nullability(val t: T, val u: U, var nullableU: U?) { return if (returnNull) null else "hello" } + public fun list(): List<*> { + return listOf("hello", 42) + } + public fun methodGenericEcho(v: V): V { return v } diff --git a/pkgs/jnigen/test/kotlin_test/runtime_test_registrant.dart b/pkgs/jnigen/test/kotlin_test/runtime_test_registrant.dart index 827579ec53..81a1c47f70 100644 --- a/pkgs/jnigen/test/kotlin_test/runtime_test_registrant.dart +++ b/pkgs/jnigen/test/kotlin_test/runtime_test_registrant.dart @@ -81,6 +81,14 @@ void registerTests(String groupName, TestRunnerCallback test) { test('Methods', () { using((arena) { final obj = testObject(arena); + expect( + obj + .list() + .first! + .as(JString.type, releaseOriginal: true) + .toDartString(releaseOriginal: true), + 'hello', + ); expect(obj.hello().toDartString(releaseOriginal: true), 'hello'); expect( obj.nullableHello(false)!.toDartString(releaseOriginal: true), From a1d2b539d45bfc33b3bba4f7b8104ebf22a23017 Mon Sep 17 00:00:00 2001 From: Hossein Yousefi Date: Fri, 10 Jan 2025 16:42:15 +0100 Subject: [PATCH 2/3] Fix versioning --- pkgs/jni/CHANGELOG.md | 4 ---- pkgs/jni/pubspec.yaml | 2 +- pkgs/jnigen/CHANGELOG.md | 4 ++++ 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/jni/CHANGELOG.md b/pkgs/jni/CHANGELOG.md index f36f3bb652..db31af1ea0 100644 --- a/pkgs/jni/CHANGELOG.md +++ b/pkgs/jni/CHANGELOG.md @@ -1,7 +1,3 @@ -## 0.13.1 - -- Fixed a bug where Kotlin wildcards would crash the code generation. - ## 0.13.0 - **Breaking Change**: Separated primitive arrays from object arrays. diff --git a/pkgs/jni/pubspec.yaml b/pkgs/jni/pubspec.yaml index a4ee75f28a..dfabbfd1d9 100644 --- a/pkgs/jni/pubspec.yaml +++ b/pkgs/jni/pubspec.yaml @@ -4,7 +4,7 @@ name: jni description: A library to access JNI from Dart and Flutter that acts as a support library for package:jnigen. -version: 0.13.1-wip +version: 0.13.0 repository: https://github.com/dart-lang/native/tree/main/pkgs/jni issue_tracker: https://github.com/dart-lang/native/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Ajni diff --git a/pkgs/jnigen/CHANGELOG.md b/pkgs/jnigen/CHANGELOG.md index e187a40d37..ad0d3fa8dc 100644 --- a/pkgs/jnigen/CHANGELOG.md +++ b/pkgs/jnigen/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.13.1-wip + +- Fixed a bug where Kotlin wildcards would crash the code generation. + ## 0.13.0 - **Breaking Change**([#1516](https://github.com/dart-lang/native/issues/1516)): From 6f5a2429ac89e1451ef6d2a4651d1155314c25ad Mon Sep 17 00:00:00 2001 From: Hossein Yousefi Date: Mon, 13 Jan 2025 13:14:37 +0100 Subject: [PATCH 3/3] Address comments --- .../elements/KotlinTypeProjection.java | 3 -- .../lib/src/bindings/kotlin_processor.dart | 9 ++--- pkgs/jnigen/lib/src/elements/elements.dart | 34 ++++++++++++------- pkgs/jnigen/lib/src/elements/elements.g.dart | 12 ++----- 4 files changed, 28 insertions(+), 30 deletions(-) diff --git a/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java b/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java index 41f1f8cf7c..364634c2e9 100644 --- a/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java +++ b/pkgs/jnigen/java/src/main/java/com/github/dart_lang/jnigen/apisummarizer/elements/KotlinTypeProjection.java @@ -15,9 +15,6 @@ public static KotlinTypeProjection fromKmTypeProjection(KmTypeProjection t) { var proj = new KotlinTypeProjection(); proj.type = KotlinType.fromKmType(t.getType()); proj.variance = t.getVariance(); - if (proj.type == null) { - return null; - } return proj; } } diff --git a/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart b/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart index 260d8301ad..b59ddf166f 100644 --- a/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart +++ b/pkgs/jnigen/lib/src/bindings/kotlin_processor.dart @@ -254,8 +254,8 @@ class _KotlinTypeProcessor extends TypeVisitor { @override void visitDeclaredType(DeclaredType node) { for (var i = 0; i < node.params.length; ++i) { - if (kotlinType.arguments[i]?.type case final type?) { - node.params[i].accept(_KotlinTypeProcessor(type)); + if (kotlinType.arguments[i] case final KotlinTypeProjection projection) { + node.params[i].accept(_KotlinTypeProcessor(projection.type)); } } super.visitDeclaredType(node); @@ -264,8 +264,9 @@ class _KotlinTypeProcessor extends TypeVisitor { @override void visitArrayType(ArrayType node) { if (kotlinType.arguments.isNotEmpty) { - if (kotlinType.arguments.first?.type case final type?) { - node.elementType.accept(_KotlinTypeProcessor(type)); + if (kotlinType.arguments.first + case final KotlinTypeProjection projection) { + node.elementType.accept(_KotlinTypeProcessor(projection.type)); } } super.visitArrayType(node); diff --git a/pkgs/jnigen/lib/src/elements/elements.dart b/pkgs/jnigen/lib/src/elements/elements.dart index b55cba30aa..a082c9756e 100644 --- a/pkgs/jnigen/lib/src/elements/elements.dart +++ b/pkgs/jnigen/lib/src/elements/elements.dart @@ -1123,9 +1123,7 @@ class KotlinType implements Element { final String kind; final String? name; final int id; - - /// `null` represents a wildcard. - final List arguments; + final List arguments; final bool isNullable; factory KotlinType.fromJson(Map json) => @@ -1195,8 +1193,26 @@ class KotlinValueParameter implements Element { } } -@JsonSerializable(createToJson: false) -class KotlinTypeProjection implements Element { +sealed class KotlinTypeArgument implements Element { + KotlinTypeArgument(); + + factory KotlinTypeArgument.fromJson(Map json) => + json['type'] == null + ? KotlinWildcard() + : KotlinTypeProjection( + type: KotlinType.fromJson(json['type'] as Map), + variance: $enumDecode(_$KmVarianceEnumMap, json['variance']), + ); + + @override + R accept(Visitor v) { + return v.visit(this); + } +} + +class KotlinWildcard extends KotlinTypeArgument {} + +class KotlinTypeProjection extends KotlinTypeArgument { KotlinTypeProjection({ required this.type, required this.variance, @@ -1204,12 +1220,4 @@ class KotlinTypeProjection implements Element { final KotlinType type; final KmVariance variance; - - factory KotlinTypeProjection.fromJson(Map json) => - _$KotlinTypeProjectionFromJson(json); - - @override - R accept(Visitor v) { - return v.visit(this); - } } diff --git a/pkgs/jnigen/lib/src/elements/elements.g.dart b/pkgs/jnigen/lib/src/elements/elements.g.dart index 5882c73802..19b4339a7d 100644 --- a/pkgs/jnigen/lib/src/elements/elements.g.dart +++ b/pkgs/jnigen/lib/src/elements/elements.g.dart @@ -335,9 +335,8 @@ KotlinType _$KotlinTypeFromJson(Map json) => KotlinType( id: (json['id'] as num).toInt(), isNullable: json['isNullable'] as bool, arguments: (json['arguments'] as List?) - ?.map((e) => e == null - ? null - : KotlinTypeProjection.fromJson(e as Map)) + ?.map( + (e) => KotlinTypeArgument.fromJson(e as Map)) .toList() ?? const [], ); @@ -371,10 +370,3 @@ KotlinValueParameter _$KotlinValueParameterFromJson( : KotlinType.fromJson( json['varargElementType'] as Map), ); - -KotlinTypeProjection _$KotlinTypeProjectionFromJson( - Map json) => - KotlinTypeProjection( - type: KotlinType.fromJson(json['type'] as Map), - variance: $enumDecode(_$KmVarianceEnumMap, json['variance']), - );