ReviewOS

also looking at this

craft-native/craft

fix(android): jsQuote takes Any?, because three call sites are not strings

#164
Merged glennmichael123 wants to merge fix/android-jsquote-types into main
2 files +18 -5
9 days ago

A bug I shipped in #156, and the reason nothing caught it.

The bug

#156 rewrote 58 evaluateJavascript call sites to route through a new helper typed String?. Three of them do not pass a String:

siteexpressionactual type
addContactContentUris.parseId(results[0].uri!!)Long
downloadFiledownloadManager.enqueue(request)Long
authenticateonAuthenticationError(code, errString)CharSequence

String interpolation accepted all three. A String? parameter does not, so CraftBridge.kt would not compile in a generated app.

Why every check was green

Nothing in this repository compiles Kotlin. The only workflow that touches Android is mobile-e2e.yml, which is workflow_dispatch-only and whose Android job is guarded by if [ -x "packages/android/gradlew" ] — a file that does not exist; packages/android has no Gradle project at all. Filed as #163 with what it would take to change that, because adding an Android SDK and a Gradle build to CI is a cost the project should choose deliberately.

The fix

Any? is what interpolation always was, and Any?.toString() renders null as the string "null" exactly as '$x' did. So the three sites work again and the other 55 are unchanged — same bytes emitted, same escaping.

The interim guard

android_reply_escaping_test.zig asserted only that private fun jsQuote( existed. It now asserts the full signature and says why, so narrowing the parameter back fails a test rather than a user's build.

That covers exactly one regression. It is a note beside #163, not a substitute for compiling the language.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.