support inner classes in inline-java
Closed this issue · 7 comments
Currently inline-java fails with a runtime error when trying to marshal an object of an inner class.
[java| Thread.State.NEW |] :: IO (J ('Class "java.lang.Thread.State"))produces
Exception in thread "main" java.lang.NoSuchMethodError: inline__method_25
There are two problems with this behavior.
- Marshaling these objects should be possible.
- Ideally, inline-java should not produce these runtime errors when the user types an incorrect type.
To fix (1) we can have the user be explicit in that Thread.State is an inner class. He could do so by writing instead:
[java| Thread.State.NEW |] :: IO (J ('Class "java.lang.Thread:State"))using : as a separator of the inner class. We can't use $ as a separator because $ is a valid character in Java identifiers.
To fix (2), the plugin could call javap on the generated bytecode and check that the signature of the methods inline__method_i are as expected. Thus, typing . instead of : would be caught at build time.
SGTM.
I do wonder though, is it really such a cost to just assume that dollar signs always refer to inner classes? I know that technically the user is allowed to call his/her classes Test$, but it sounds fair to ban that in inline-java. The flip side is that in doing so we avoiding inventing new bespoke and non-standard syntax that the user has to learn.
Furthermore, the Java Language Specification says this:
The "Java letters" include uppercase and lowercase ASCII Latin letters A-Z (\u0041-\u005a), and a-z (\u0061-\u007a), and, for historical reasons, the ASCII underscore (_, or \u005f) and dollar sign ($, or \u0024). The $ sign should be used only in mechanically generated source code or, rarely, to access pre-existing names on legacy systems.
https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.8
We can ask the user to not use $, but he still might have to use classes whose names he hasn't chosen.
We could consider adding some escape syntax instead. So a.b$c is interpreted as a.b.c, but a.b\$c, a.b$$c or a.b:c is interpreted as a.b$c.
but he still might have to use classes whose names he hasn't chosen.
That's easy enough to work around (with class aliases), and if we are to take the JLS on face value, entirely a theoretical problem (i.e. only applicable for very old legacy code). And yes, we could introduce an escaping mechanism for legacy names, but I'd wait to see whether someone needs that. At least this way, the default (i.e. when we're not dealing with legacy names) is the unsurprising case, and the user only needs to do something special when dealing with legacy things (that may well mean never in this day age, but I'm not a Java old timer).
Regarding (2), we can do better than checking javap. In java we can test if the a class name has the name we expected
A.B.C.class.getName().equals("A.B$C")We write A.B.C.class using only dots, and "A.B$C" comes from the type of the java references as written by the user. If this test fails, we can report an error.
This test can be performed at runtime by inserting it in static blocks in the generated classes, or it can be done at build time in a few ways. The simplest way I found to do it at build time is to call javac on a generated program which runs the test. It takes near (80 ms). This time could be reduced if we hook javac to run the tests for us. javac offerst a couple of mechanisms for this (the Annotation Processor API and plugins).
I propose that we start with the simple approach to check it at build time. If 80 ms turns out to be too expensive, we either do it at runtime or consider javac hooks.
The main advantage of this approach is that we get better error messages. The user does not have to spend time figuring out the cause of a NoSuchMethodError message.
Regarding (2), we can do better than checking javap.
This turned out to complicate the plugin more than what a sparingly-used feature would deserve.
We can revisit this if inner classes are used more than we anticipate. The patches that we tried before are here.