Community
Participate
Working Groups
The method works in the following, inconsistent way: * java.lang.Object -> Ljava/lang/Object * java.lang.Object[] -> [Ljava/lang/Object; The former is a valid VmTypeName, the latter isn't. In my opinion this should be at least consistent.
Andreas, can you have a look on this?
This is something I talked about with Marcel already: Yes, it is inconsistent that there's no semicolon for scalar types and that "Ljava/lang/Object" is not a valid type descriptor as per JVM spec but that "[Ljava/lang/Object;". However, changing this would affect *lots* of code and also the models, as they refer to types also using the inconsistent syntax. So, this is really a change for Code Recommenders 2.0, unless you can *demonstrate* that some code breaks because of this inconsistency.
I figured as much... I think just removing the ; if its returned is an easy enough workaround for now. Would be nice to add a little javadoc comment that prevents clients to learn about the inconsistency the hard way ;)
https://git.eclipse.org/r/#/c/19185/ contains a proposed fix. Please review. I'm not sure which parts in recommenders this affects since most methods are used by test cases only. We'll have to figure out some time soon.
If we change this, I would prefer to have the semicolon both for arrays and non-arrays, rather than having no semicolon in both cases. That's consistent with the JVM spec at least. Things that are affected are at the very least Zips.path(..) and things like VmMethodName.getParameterTypes(). This change would cause lots of code to be changed. I would consider it for 2.1, not earlier.
I was just bitten again by this: Contrary to its name, src2VmType does not produce a valid VmTypeName, in the sense that its return value can be used a parameter to VmTypeName.get(String). This is very surprising behavior.
If we ever decide to make this change and related ones like Bug 393119, it won't be before 2.3.0; too much code relies on the knowledge of the string representation of I*Names at the moment. FWIW, I think the way forward on this one would be a richer interface for IMethodName and friends; the fact that one often has to resort to name.getIdentifier and then some String operations makes it hard to change the internal string representation without breaking things.
Postponing to 2.3.0.
