Repository navigation
fix: Kotlin typings (supersedes #86) - #89
Merged
Merged
Conversation
…tion Only a malformed command line is meant to print a short usage hint, but the handler wrapped the whole run, so any IllegalArgumentException thrown while generating - BCEL's "Bad method signature" among them - was reported as a command line mistake, exited with 2 and lost its stack trace. The handler now covers parsing the arguments only.
A class declared as `class Foo<E extends Throwable & Copyable<E>>` records both bounds after the label. The regex that split the formal type parameters read them as a single type, which BCEL rejects, so generating typings for kotlinx-coroutines-core aborted outright. The parameters are now read with a small parser that walks each bound, keeping the first as the label's type. getTypeSuffix computed an extends clause from that type that it then never used, and which cast the bound to ObjectType - wrong for a bound that is a type variable - so that dead code is removed.
Namespaces were opened and closed by comparing the current class with the previous one, counting separators. A class anywhere in a nesting chain can be skipped - Kotlin internal, obfuscated, synthetic, a FooKt facade - and then the previous class no longer describes what is open: a nested class whose outer class was skipped opened a namespace with an empty name, and moving from Outer$A$X to Outer$B$Y, with Outer$B skipped, left A open around B. The generator now keeps the list of namespaces it has opened and closes and opens down to the common prefix with the path the next class needs. Classes whose binary name has an empty segment, such as kotlinx.serialization's Foo$$serializer, cannot be spelled as a namespace path; they are skipped and references to them become any. Based on the approach in #86. Co-authored-by: Martin Guillon <dev@akylas.fr>
Parameter names were taken from the LocalVariableTable by array position, assuming the table lists `this` and the parameters first. It does not have to: Kotlin inlining puts locals such as $i$a$-apply-... ahead of them, and javac can order entries differently too. So names came out shifted by one or borrowed from unrelated locals - subscribe(this_: string, parentId: Bundle, ...) for subscribe(parentId, options, callback), enable($i$a$-also-...: ...), and duplicate names in one signature. Each parameter now takes the entry that occupies its slot at pc 0, counting two slots for long and double and skipping the synthetic leading parameters (an inner class's outer instance, an enum's name and ordinal) that the generic signature leaves out. Kotlin names a setter's value parameter <set-?>; it now takes the property's name, so setCurrentRunnable(<set-?>) becomes setCurrentRunnable(currentRunnable). An extension receiver ($this$foo) becomes receiver, any other name that is not a valid identifier falls back to paramN, and a name already used in the same signature gets a numeric suffix. Co-authored-by: Martin Guillon <dev@akylas.fr>
Types from the ignored namespaces (kotlin, org.jetbrains, java.util.function, and so on) were replaced with any by a regex over the finished text. The regex had no left boundary and did not accept a comma inside generic arguments, so it left the arguments behind - detectDarkMode: any<any,java.lang.Boolean> - and it rewrote string constants as well, turning MediaBrowserCompat.EXTRA_PAGE's "android.media.browse.extra.PAGE" into "any". The replacement now happens where a Java type is converted, on the class name itself, so the whole type including its arguments becomes any and nothing that merely contains the name is touched. A supertype from an ignored namespace, or one whose name cannot be spelled, is left out of implements, and extends falls back to java.lang.Object as the text pass did for `extends any`. Classes whose name has a segment that is not an identifier are now skipped as well, not only those with an empty one: Kotlin names some file facades -Base64 to hide them from Java.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
A constant field was written as `public static PX: number = 1;`, but a declaration file cannot initialize a property that way: every one is TS1039, nearly 28,000 of them across the platform jar, hidden only by skipLibCheck. Char constants were also written as their int code, so `MENU_KEY: string = 115` did not even type-check against its own type. A static final constant is now declared `public static readonly PX = 1;`, the form tsc itself emits. Its type is the literal, so the value shows on hover and narrows comparisons and switches, while `let v = View.VISIBLE` still widens to number. Writing `PX: 1` instead would pin `v` to 1. Values are written as JavaScript reads them: booleans and chars, which the class file stores as ints, become true/false and a one-character string, and a float becomes the double it widens to (0.1f reads 0.10000000149011612). NaN and the infinities have no literal type, and the runtime hands out longs beyond 2^53 as NativeScriptLongNumber rather than as numbers, so those keep their declared type and carry the value in the doc comment, as do instance fields. Co-authored-by: Martin Guillon <dev@akylas.fr>
TypeScript 6 rejects `module Foo {}` for a namespace with TS1540, in
declaration files too, and TypeScript 7 drops the form entirely. The published
typings produce thousands of these as soon as skipLibCheck is off. The
generated definitions, android-declarations.d.ts and _helpers.d.ts now use
`namespace`, which means the same thing to every TypeScript version.
Co-authored-by: Martin Guillon <dev@akylas.fr>
From Java, and so from NativeScript, a companion object's methods are called
on the instance held in the outer class's static Companion field:
okhttp3.MediaType.Companion.parse("text/plain"). The runtime gives that static
field precedence over the nested class of the same name. The typings, though,
skip the field (TypeScript cannot give one name both a nested class and a
static field), so Outer.Companion was typed as the class constructor and every
such call failed with TS2339.
A companion object, recognised by its Kotlin metadata rather than by its name,
now also declares each instance method as static. Outer.Companion keeps naming
the class as a type, and calls on it type-check. Methods named after a
constructor's own properties (name, length, ...) are left instance-only, since
TypeScript rejects those as statics.
Co-authored-by: Martin Guillon <dev@akylas.fr>
Projects that lint their .d.ts files get reports such as unified-signatures, adjacent-overload-signatures and no-redeclare on every generated overload and namespace merge, none of which can be fixed in a generated file. Writing the header now also starts the output file afresh when --skip-declarations is given; before, that mode appended to whatever an earlier run had left in it. Co-authored-by: Martin Guillon <dev@akylas.fr>
…bject
The constructor that takes an implementation object listed every method of
the interface, <clinit> included, which an interface has as soon as it holds a
companion object or a non-constant field. Kotlin fun interfaces with a
companion are common, so `new okhttp3.Interceptor({ intercept(chain) {...} })`
failed to type-check with "Property '<clinit>' is missing". Synthetic methods
are left out for the same reason.
edusperoni
force-pushed
the
fix/kotlin-typings
branch
from
November 8, 2026 20:24
812728f to
e77d538
Compare
The expected-output test only covered Java AndroidX jars, so nothing guarded the Kotlin handling. test/kotlin-fixture is a small Kotlin and Java library, built from source with a pinned Kotlin version, in which each declaration exercises one case: companion objects (named and unnamed, with a quoted JVM name), a fun interface with a companion, a property setter, an extension receiver, inlined lambda locals, kotlin.* function types, a type parameter with two bounds, inner class constructors, an internal class in the middle of a nesting chain, a facade whose name is not an identifier, constants with and without a literal, and long/double parameter slots. `make test-kotlin-fixture` builds it, generates its typings and compares them with test/expected-output/kotlin-fixture; `make test` runs both tests. test-compare-output now removes its previous output first, since android-declarations.d.ts is merged into whatever an earlier run left.
The runtime matches a constructor call against the JVM descriptor, in which an inner class's constructor takes the outer instance first. The generic signature leaves that parameter out, and kotlinc writes one for every inner class constructor, so the typings declared `new DiskLruCache.Editor(entry)`, which fails at runtime with "Failed resolving constructor ... provide reference to the outer `this` as the first argument", while the call that works, `new DiskLruCache.Editor(cache, entry)`, did not type-check. Java inner classes were only affected when the constructor had generic parameter types. The parameters the signature leaves out are now declared from the descriptor ahead of the signature's, and named `outer` rather than javac's `this$0`. Nullability is looked up by descriptor index: Kotlin's value parameters and javac's parameter annotations both leave the outer instance out, so they are lined up past it.
…iver The JVM passes an extension function's receiver as its first parameter, but Kotlin metadata keeps it apart from the value parameters. Nullability was read from the value parameters by JVM index, so every parameter of an extension function took the nullability of the one after it: `fun String.tagged(label: String?)` was declared `tagged(receiver: string | null, label: ...)`. The receiver's nullability now comes from its own type, and the value parameters are read one index on.
Nothing ran the generator in CI, so a change that broke the output only showed up when someone ran the tests by hand. A workflow now builds the generator, runs `make test` (the AndroidX and Kotlin fixture comparisons), and runs `make test-typecheck`. test-typecheck compiles the expected output against the published @nativescript/types-android platform typings with TypeScript 6, which also rejects `module` as a namespace keyword. The Kotlin fixture must have no errors at all. The AndroidX output must have no syntax or grammar errors (TS1xxx, which includes initializers in an ambient context); it still has a few type errors where a nullable return type narrows a base class's, so its types are not checked yet. The diagnostics come from the TypeScript API per file, since tsc stops reporting type errors once any file fails to parse. The fixture's android-declarations.d.ts is now committed and compared too, as the type check needs it next to android.d.ts.
edusperoni
marked this pull request as ready for review
November 8, 2026 20:42
This was referenced Nov 8, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #86 (which built on #85). This applies the typings fixes from #86 on top of current
main, which has since gained Kotlin metadata support (nullable unions, hiding Kotlininternal, API-level tags). It also fixes the bugs #86 introduced. #86 conflicts withmain, so this re-implements its fixes at the root cause instead of rebasing it. Thanks to Martin Guillon for #86 (co-author on the commits derived from it) and to Daniel Grima for #85 and the testing on it.What changes
Each commit is self-contained and refreshes
test/expected-output, so it can be reviewed one at a time.<E extends Throwable & CopyableThrowable<E>>) was split with a regex that merged both bounds into one type. BCEL rejects that type, so generation aborted. Mapbox pulls in coroutines, so it hit this too. The bounds are now parsed properly. Neithermainnor improved PR #85 #86 handled this.IllegalArgumentExceptionduring generation was reported as a command-line mistake (exit 2, "Run with --help") and its stack trace was lost. Only argument parsing gets that message now.FooKt) used to produceexport module {and unbalanced braces. Classes whose name can't be written as a namespace path (Foo$$serializer,-Base64facades) are skipped, and references to them becomeany. (improved PR #85 #86 also appended missing braces at the end of the file; that isn't needed now.)subscribe(this_: string, parentId: Bundle, options: ...)becomessubscribe(parentId: string, options: Bundle, callback: ...). Kotlin's<set-?>becomes the property name,$this$foobecomesreceiver, other invalid names fall back toparamN, and duplicates get a numeric suffix. Unlike improved PR #85 #86, names are not truncated or hashed.kotlin,java.util.function, …). These types becomeanywhen each type is converted, not via a regex over the finished file. The regex leftany<any,java.lang.Boolean>behind and even rewrote string constants ("android.media.browse.extra.PAGE"became"any"). improved PR #85 #86's version turnedkotlinx.intoanyx.throughout; this approach can't.public static PX: number = 1;is TS1039 in a declaration file (about 28,000 errors on the platform jar withskipLibCheckoff). Static final constants are now declaredpublic static readonly PX = 1;, the formtscitself emits. The type is the literal, so it shows on hover and narrows comparisons andswitches.let v = View.VISIBLEstill widens tonumber, whichPX: 1would not allow. Booleans and chars becometrue/falseand one-character strings, and floats become the double they widen to. NaN/Infinity and longs beyond 2^53 (which the runtime hands out asNativeScriptLongNumber) keep their declared type, with the value in the doc comment. On the android-37 platform jar this gives 27,459 literal constants and no new type errors.namespaceinstead ofmodule. TypeScript 6 rejectsmodule Foo {}with TS1540, in declaration files too.okhttp3.MediaType.Companion.parse(...)now type-checks: the runtime resolvesOuter.Companionto the companion instance, so its methods are also declaredstatic.Outer.Companionstays usable as a type, and quoted names like"-deprecated_get"still work. (improved PR #85 #86 usedfunctiondeclarations inside a namespace instead, which can't take quoted names.)<clinit>in implementation objects. It's no longer listed as a member you have to implement, sonew okhttp3.Interceptor({ intercept(chain) {...} })type-checks./* eslint-disable */header on the generated file.inner class, sonew okhttp3.internal.cache.DiskLruCache.Editor(entry)type-checked but failed at runtime. That parameter is now declared and namedouter.The
-per-libraryoption from #86 is left for a separate PR.Output changes worth knowing
public static FOO: number = 1;becomespublic static readonly FOO = 1;. Assigning to a constant is now a type error; Java inlines compile-time constants, so such an assignment never had an effect.any | nullbecomesany, aswithNullalready intends.--skip-declarationsnow startsandroid.d.tsfrom scratch. Before, it appended to whatever an earlier run left in the output directory.android-declarations.d.tsis still merged across runs, as before.Testing
test/kotlin-fixture, built from source with Kotlin 2.2.20): each declaration exercises one of the cases above.maincrashes on it; with the crash removed, every case shows its bug.make testruns it together with the AndroidX comparison..github/workflows/test.yml): builds the generator and runsmake testandmake test-typecheck. The type check compiles the expected output against@nativescript/types-androidwith TypeScript 6. The fixture must have no errors; the AndroidX output must have no syntax/grammar errors. It still has a few type errors where a nullable return type clashes with a base class, so its types aren't checked yet.tsc5.9,strict,skipLibCheck: false, against the platform typings:maincom.mapbox.maps:android:11.15.3+ its transitive dependenciesThe remaining errors are references to libraries outside the corpus, plus the pre-existing issues below.
Not addressed here (pre-existing on
main)internaltypes are hidden, but public declarations still reference them. Examples: Mapbox'sLayerProperty, coroutines'JobNode, andSpecialEffectsController, whose nested classes are still emitted.AlertDialog.onCreatePanelViewvsDialog). One Java method is emitted twice with different nullability: once from the class's own declaration and once from an interface it implements.