Repository navigation
[jnigen] Code snippet-ify jnigen and co - #3641
Conversation
liamappelbe
changed the title
github-actions
Bot
added
package:hooks
package:jnigen
package:jni
package:jni_flutter
labels
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3641 +/- ##
=======================================
Coverage 87.96% 87.96%
=======================================
Files 307 307
Lines 19658 19658
=======================================
Hits 17292 17292
Misses 2366 2366
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
dcharkes
left a comment
dcharkes
left a comment
There was a problem hiding this comment.
This is great!
We should think about which snippets are checked in which CI runs.
The current PR checks the snippets on the native.yaml workflow not in the JNIgen/FFIgen workflows.
If we want the tool to be usable from different workflows we could have a snippet_tool package with a Dart API or a bin/snippet_tool.dart [paths].
Or, if we tackle moving FFIgen and JNIgen into the pub workspace and into native.yml as github workflow.
WDYT?
liamappelbe
changed the title
github-actions
Bot
added
type-infra
github-actions
Bot
removed
package:hooks
package:data_assets
package:code_assets
labels
dcharkes
left a comment
dcharkes
left a comment
There was a problem hiding this comment.
LGTM with comments addressed 🚀
| run: dart analyze --fatal-infos | ||
| - name: Check snippets | ||
| run: dart run ../snippet_tool/bin/snippet_tool.dart --set-exit-if-changed . | ||
| if: always() && steps.install.outcome == 'success' |
There was a problem hiding this comment.
why this if? if pub get fails, analyzing will fail too for sure?
and don't we stop executing steps if one step fails?
(is this hallucinated bloat from an agent?)
There was a problem hiding this comment.
It was from the agent, but my eyes glaze over this sort of boilerplate when I'm reviewing. TBH I never really thought about what that line did. It basically overrides the default behaviour (when one step fails, skip the rest) to only check that the install step succeeds. It's useful if you have a series of independent steps after the install step.
There are other places where that's a good idea, but here it's not so useful. If the analysis step fails, there's not much point running the snippet tool.
| The files in this directory are used as snippets in the Dartdoc documentation. | ||
|
|
||
| They are updated by the script at | ||
| [`hooks/tool/update_snippets.dart`](../../../../hooks/tool/update_snippets.dart). |
There was a problem hiding this comment.
Should the example/api dir be pub ignored? e.g. they are not useful as examples. and this readme will have a dead link on github. Oh wait we don't see this readme on github, the top level readme points to github already?
Should we consider having the snippets in tool/snippets/.. instead of examples? E.g we can use examples in the api docs and readmes, and that is great, especially if we run those examples. But maybe having simple snippets that don't run by themselves in example/ is not great. (Yeah, I know, past @dcharkes introduced this pattern! 😄 )
There was a problem hiding this comment.
moved the snippets to tool/ and .pubignored it
|
|
||
| <!-- no-source-file --> | ||
| ```dart | ||
| // Dart Bindings - Boilerplate omitted for clarity. |
There was a problem hiding this comment.
It would be cool if we could have snippets where we can omit things
// snippet-start#dangerous
Future<void> dangerousCall() async {
// Do x-y-z.
// snippet-omit-start
// 1. Get the Activity (e.g., Activity "A")
final activity = androidActivity(engineId);
// 2. An `await` occurs. The main thread is freed.
// While waiting, Android might destroy Activity "A" and create "B".
await someOtherFuture();
// 3. CRASH: The code resumes, but `activity` is now a stale
// reference to the destroyed Activity "A".
if (activity != null) {
someGeneratedApi.doSomething(activity); // This will crash
activity.release();
}
// snippet-omit-end
}
// snippet-end#dangerousWhich would then become
Future<void> dangerousCall() async {
// Do x-y-z.
// [...]
}Maybe file an issue on the new package to add this as a feature.
But, since this example is also a generated file we can't really get that into the generated file. 🙈 (No idea what to do with generated files and snippets, we can't easily keep those in sync unless you add some kind of generateSnippets option to the Dart API of JniGen 🙈
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]>
Apply snippets_tool to
jnigen,jni,jni_flutter, andjni_util, and fixed all the errors it found.There are a bunch of examples in java_differences.md that say "Boilerplate omitted for clarity". Not sure what to do about them, since they can't easily be converted to snippets without that boilerplate. Added
<!-- no-source-file -->for now.There are also a bunch of cases in
dart_generate.dart, which I didn't turn into snippets because these classes are not exposed publicly.This change is stacked on #3683.