Skip to content

[jni] Add deep conversion utils - #3635

Merged
liamappelbe merged 7 commits into
dart-lang:mainfrom
Siya-05:jni-deep-conversion-utils
Sep 22, 2026
Merged

liamappelbe merged 7 commits into
dart-lang:mainfrom
Siya-05:jni-deep-conversion-utils

Conversation

@Siya-05

@Siya-05 Siya-05 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Description

Adds deep conversion utilities to package:jni, following the conversion API pattern in package:objective_c.

This adds conversions between Dart values and JNI objects for primitive values, strings, lists, sets, and maps, with recursive conversion of nested collections and nullable values. It also adds conversion support for JArray and the primitive array variants.

Java-to-Dart collection conversions return collections of Object?, allowing callers to cast them to the expected types when needed. Unsupported types can be handled through the convertOther callback in either conversion direction.

Related Issues

Fixes #3143

PR Checklist

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've run dart tool/ci.dart --all locally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevel pubspec.yaml workspace.
  • All existing and new tests are passing. I added new tests to check the change I am making.
  • The PR is actually solving the issue. PRs that don't solve the issue will be closed. Please be respectful of the maintainers' time. If it's not clear what the issue is, feel free to ask questions on the GitHub issue before submitting a PR.
  • I have updated CHANGELOG.md for the relevant packages. (Not needed for small changes such as doc typos).
  • I have updated the pubspec package version if necessary.

@github-actions github-actions Bot added type-infra A repository infrastructure change or enhancement package:jnigen package:jni labels Sep 14, 2026
@Siya-05
Siya-05 force-pushed the jni-deep-conversion-utils branch from cefc389 to 90bb7ec Compare September 14, 2026 07:36
@github-actions github-actions Bot removed type-infra A repository infrastructure change or enhancement package:jnigen labels Sep 14, 2026
@liamappelbe
liamappelbe self-requested a review September 15, 2026 00:14
@Siya-05

Siya-05 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the directives_ordering analyzer failure. I'm not sure about the runtime_test.dart failure.

@liamappelbe

Copy link
Copy Markdown
Contributor

Fixed the directives_ordering analyzer failure. I'm not sure about the runtime_test.dart failure.

It's a known flake. I really need to deflake all these tests, but I haven't been able to repro them locally.

@Siya-05

Siya-05 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Added the required jni 1.0.3 changelog entry.

@Siya-05

Siya-05 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Do you see any logical/API changes needed here? Also, should we support DateTime conversion like package:objective_c?

@liamappelbe liamappelbe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we support DateTime conversion like package:objective_c?

Not in this PR.

Comment thread pkgs/jni/lib/src/converter.dart Outdated

extension DartListToJList on List<Object?> {
/// Deep converts this Dart [List] to a [JList].
JList<JObject?> toJListDeep({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you delete toJListDeep and instead do this deep conversion inside toJList? It doesn't really make sense to have a distinction between deep conversion and shallow conversion, because whether or not the conversion is deep is determined by the type of the element:

  • If the element is a JObject, then toJObject is a no-op, so toJListDeep is a shallow conversion.
  • If the element is not a JObject then it's impossible to do a shallow conversion.

So ToJavaList could check the type of the element and either shallow convert or not. But even that is just an optimisation (avoiding an unnecessary .map() call if the element is a JObject). I think even if you ignored that distinction, and always do .map(toNullableJObject) in toJList, it would be behaviorally identical.

Same for the other collections.

Comment thread pkgs/jni/CHANGELOG.md Outdated
## 1.0.3

- Fix JNI build errors on Linux arm64.
- Add deep conversion utilities for converting between Dart values and Java

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.0.3 is already published, so start a new version. This is adding new APIs, so it'll need to be a minor version bump to 1.1.0-wip. Update here and the pubspec.

@Siya-05

Siya-05 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Updated the collection conversion APIs to integrate deep conversion into toJList, toJSet, and toJMap, and bumped the package version to 1.1.0-wip.

@Siya-05

Siya-05 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@liamappelbe, can you run the CI tests when you get the chance?

@Siya-05

Siya-05 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor Author

I hit the version_test.dart failure after bumping jni to 1.1.0-wip. dart_generator.dart still had jniMinorVersion = '0'.

Updating it to 1 fixes the version test, but since that value is emitted as JniVersionCheck(1, 1), running regenerate_all_bindings.dart updates the version check in some generated bindings too.

Should I include those regenerated bindings here, or is there a better way to handle the JNI version bump?

@liamappelbe

Copy link
Copy Markdown
Contributor

I hit the version_test.dart failure after bumping jni to 1.1.0-wip. dart_generator.dart still had jniMinorVersion = '0'.

Updating it to 1 fixes the version test, but since that value is emitted as JniVersionCheck(1, 1), running regenerate_all_bindings.dart updates the version check in some generated bindings too.

Should I include those regenerated bindings here, or is there a better way to handle the JNI version bump?

To avoid the churn, maybe just don't run regenerate_all_bindings.dart? The CI test will catch any real diffs. The point of weakeneing the test was that you could just not updating the bindings if the only change was a version bump.

@Siya-05

Siya-05 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Fixed it by updating jniMinorVersion to 1 and leaving the generated bindings unchanged. The version test passes locally now. Could you rerun the CI tests?

@Siya-05

Siya-05 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Looks like CI is now failing because the freshly generated bindings expect package:jni ^1.1.0, but the test environment is still importing 1.0.x. Should I update anything else for the 1.1.0-wip bump?

@liamappelbe

liamappelbe commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

We can use this PR as a test case to fix the remaining version test relaxation issues. Ideally a version bump like this (one that doesn't affect the generated code) should just require bumping a few constants in monorepo, not regenerating dozens of bindings files.

For example, this failure looks legit. It's another case that could be relaxed to not check the JNI version.

This failure also looks legit. There's probably some hard coded version constants that need to be bumped.

You should be able to repro both failures locally.

@Siya-05

Siya-05 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Sounds good, I'll reproduce both failures locally and work on fixing them in this PR.

@github-actions github-actions Bot added the type-infra A repository infrastructure change or enhancement label Sep 21, 2026
@Siya-05

Siya-05 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Quick update on the failures:

  • Version consistency: bumped JniVersionCheck.actualMinorVersion to 1.
  • Generated binding comparisons: ignore version-only JniVersionCheck diffs in comparePaths.
  • Example regeneration: applied the same relaxation to its direct git diff.
  • jni_flutter CI: applied the same relaxation to its workflow diff.

The previously failing tests now pass locally without regenerating the bindings.

Architectural question: the JNI major/minor version is hardcoded in a few places and manually kept in sync with jni/pubspec.yaml. Would it make sense to derive these from a single source of truth, or is keeping them explicit intentional?

@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.87629% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@f2b4942). Learn more about missing BASE report.

Files with missing lines Patch % Lines
pkgs/jni/lib/src/converter.dart 95.87% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3635   +/-   ##
=======================================
  Coverage        ?   77.29%           
=======================================
  Files           ?       53           
  Lines           ?     3718           
  Branches        ?        0           
=======================================
  Hits            ?     2874           
  Misses          ?      844           
  Partials        ?        0           
Flag Coverage Δ
jni 66.34% <95.87%> (?)
jnigen 82.77% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
code_assets ∅ <0.00%> (?)
data_assets ∅ <0.00%> (?)
ffi ∅ <0.00%> (?)
ffigen ∅ <0.00%> (?)
hooks ∅ <0.00%> (?)
hooks_runner ∅ <0.00%> (?)
jni 66.34% <0.00%> (?)
jni_flutter ∅ <0.00%> (?)
jni_util ∅ <0.00%> (?)
jnigen 82.77% <0.00%> (?)
json_syntax_generator ∅ <0.00%> (?)
native_test_helpers ∅ <0.00%> (?)
native_toolchain_c ∅ <0.00%> (?)
objective_c ∅ <0.00%> (?)
pub_formats ∅ <0.00%> (?)
record_use ∅ <0.00%> (?)
swift2objc ∅ <0.00%> (?)
swiftgen ∅ <0.00%> (?)
test_case_selector ∅ <0.00%> (?)
Files with missing lines Coverage Δ
pkgs/jni/lib/src/version_check.dart 100.00% <ø> (ø)
pkgs/jnigen/lib/src/bindings/dart_generator.dart 89.77% <ø> (ø)
pkgs/jni/lib/src/converter.dart 95.87% <95.87%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

);
}

extension DartIterableToJList on Iterable<Object?> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need the converter in jlist.dart? Seems redundant.

Same for the ones in jmap.dart and jset.dart.

@Siya-05 Siya-05 Sep 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing converters preserve the element types (Iterable.toJList() returns JList, and similarly for sets/maps), while the new converters accept Dart values and return JList<JObject?>/JSet<JObject?>/JMap<JObject?, JObject?>. I kept both to preserve the existing typed API.

@liamappelbe
liamappelbe merged commit ed7605f into dart-lang:main Sep 22, 2026
67 checks passed
Hassnaa9 pushed a commit to Hassnaa9/native that referenced this pull request Sep 27, 2026
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 6, 2026
Revisions updated by `dart tools/rev_sdk_deps.dart`.

native (https://github.com/dart-lang/native/compare/f628b26..27f7678):
  27f767827  Mon Sep 28 14:51:28 2026 +0200  Daco Harkes  [hooks_runner] Remove `KernelAssets` and `Target` (dart-lang/native#3703)
  ddc7d6deb  Mon Sep 28 05:40:03 2026 +0200  Jakob Kordež  [ffigen] Skip typedefs to function pointers with unsupported parameter types (dart-lang/native#3694)
  177d283d9  Mon Sep 28 04:30:28 2026 +0200  Jakob Kordež  [ffigen] Generate bindings for C++ enums, structs and unions nested in namespaces and records (dart-lang/native#3644)
  5ee8f67d8  Mon Sep 28 11:39:14 2026 +1000  Liam Appelbe  [objective_c] Clarify block isolate scoping (dart-lang/native#3692)
  25f090116  Fri Sep 25 12:09:12 2026 +1000  Liam Appelbe  [ffigen][jnigen] Deprecate YAML config (dart-lang/native#3654)
  8364413eb  Fri Sep 25 10:09:50 2026 +1000  Liam Appelbe  [jnigen] Document nullability (dart-lang/native#3690)
  e8d518574  Thu Sep 24 18:02:46 2026 +1000  Liam Appelbe  [ffigen] Code snippet-ify ffigen and objective_c (dart-lang/native#3685)
  663385955  Thu Sep 24 03:21:53 2026 +0300  Manar Elhabbal  [ffigen] prevent setting both cpp and objectiveC in ffigenerator (dart-lang/native#3687)
  1cf40952a  Thu Sep 24 09:27:09 2026 +1000  Liam Appelbe  [objective_c] Remove arm64e build hack (dart-lang/native#3680)
  c44550b6e  Wed Sep 23 13:01:33 2026 +0200  Simon Binder  [web_assets] Scaffold package (dart-lang/native#3610)
  57b4bad69  Wed Sep 23 16:21:27 2026 +1000  Liam Appelbe  [swift2objc] Code snippet-ify swift2objc (dart-lang/native#3684)
  5cee1d0be  Wed Sep 23 10:16:29 2026 +0530  Rachit Vikram Singh  [ffigen] Add Input.appendCompilerOptions to append clang args to defaults (`#3630`) (dart-lang/native#3676)
  225a648df  Wed Sep 23 14:37:13 2026 +1000  Liam Appelbe  [jnigen] Code snippet-ify jnigen and co (dart-lang/native#3641)
  2030decf9  Wed Sep 23 13:51:24 2026 +1000  Liam Appelbe  [infra] Move snippets_tool to its own package (dart-lang/native#3683)
  6fa2beea2  Wed Sep 23 12:14:28 2026 +1000  Liam Appelbe  [objective_c] Revert `#3664` (dart-lang/native#3681)
  ed7605f8a  Tue Sep 22 12:03:22 2026 +0530  Sivapriya  [jni] Add deep conversion utils (dart-lang/native#3635)
  8574b7422  Tue Sep 22 08:31:11 2026 +0200  Lukas Klingsbo  [ffigen] Allow package:cli_util 0.5.x and 0.6.x (dart-lang/native#3673)

test (https://github.com/dart-lang/test/compare/9946c95..4f92d53):
  4f92d530  Fri Sep 25 16:46:22 2026 -0700  Nate Bosch  Unskip a test on windows (dart-lang/test#2772)
  7fb84866  Fri Sep 25 14:52:39 2026 -0700  Nate Bosch  Bundle skips at the end of the GitHub reporter (dart-lang/test#2768)
  603e5ec5  Thu Sep 24 14:00:41 2026 -0700  Nate Bosch  Clean up the runner's temporary directories (dart-lang/test#2763)
  4bf04d90  Thu Sep 24 12:41:15 2026 -0700  Nate Bosch  Refactor signal handling in executable.dart (dart-lang/test#2765)
  2c660a84  Tue Sep 22 15:33:34 2026 -0700  Nate Bosch  Regenerate mono_repo output (dart-lang/test#2767)

tools (https://github.com/dart-lang/tools/compare/3614ce0..d87eaf7):
  d87eaf79  Thu Sep 24 08:31:33 2026 -0700  Kevin Moore  fix(benchmark_harness): migrate subprocess spawning to package:cli_util (dart-lang/tools#2585)
  8ccf8ccf  Wed Sep 23 05:09:10 2026 +0530  Aman Maurya  [string_scanner] Extract a mixin for LineScanner and deduplicate eager scanning logic (dart-lang/tools#2569)
  afcdd6d9  Tue Sep 22 15:38:07 2026 +0200  Morgan :)  Document that watch events converge on filesystem state. (dart-lang/tools#2607)
  f2157cbc  Tue Sep 22 04:52:58 2026 +0300  Hasan Armoush  [code_builder] Add null-aware cascades, cascade index, and fluent cascade helpers (dart-lang/tools#2540)

web (https://github.com/dart-lang/web/compare/e5c6c02..ec71a4c):
  ec71a4c  Mon Sep 28 10:07:01 2026 -0700  Kevin Moore  [js_interop_gen] Require `code_builder: ^4.12.0` and update golden headers (dart-lang/web#576)

[email protected]

Change-Id: I766bc5b2b1297e3251990821bb73387ba685e39b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/556320
Commit-Queue: Nate Bosch <[email protected]>
Reviewed-by: Brian Quinlan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package:jni package:jnigen type-infra A repository infrastructure change or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[jni] Add deep conversion utils

2 participants