Add java-class-prefix option to djinni

[izacus]

--java-class-prefix will prefix all Java class names with passed prefix when generating output. This is helpful in cases where Djinni is used in a library to prevent naming collision with possible public APIs.

[smarx]

Automated message from Dropbox CLA bot

@izacus, it looks like you've already signed the Dropbox CLA. Thanks!

[mknejp]

There's already a --java-package option. I think the more sensible solution is to just always fully qualify all class names rather than adding yet another option.

[4brunu]

This would be a good option to add consistency, especially for those like me that are working in iOS and Android, because when I switch from on platform to another, I have to remember that one has a prefix and the other don't.

[mknejp]

@4brunu I'd call that the inherent difference between the two platforms. Objective-C prefixes are really just there because of lacking language functionality. Java doesn't have that problem so prefixes just donn't make sense. They're not part of Android coding standards. When it comes to the author's intention I think it would be enough to just always fully qualify class references in Djinni generated code to make sure there are no collisions.

[izacus]

Ok, to be clear about the usecase. The point of prefixing is to avoid confusing users of the library by having duplicated named classes:

Since Java doesn't have concept of exported symbols, we cannot hide the Djinni classes to public users. We also do not want the library users to invoke those classes directly, because

1. due to limitations of Djinni and JNI they do not have a nice idiomatic API
2. having a clear split means that changes in common code due to platform limitations (or architecture changes) do not affect the public API

The class names also cannot be obfuscated by Proguard, because Djinni finds them by name.

To avoid having users import the wrong class (and then be confused why other method calls do not accept them - see issues with java.sql.Date vs. java.util.Date and similar), prefixing the name with JNI or Native is the simplest and most elegant way of disambiguating objects meant for public use and objects that are part of native bridge. Having Djinni prefix Java classes also means that the C++ code underneath still retains simple and readable naming.

Fully qualifying the name will not solve the issue.

[mknejp]

Right, so if I understand this correctly the actual problem is that Djinni generated classes are all public by default? So wouldn't specifying the access keyword for generated classes be the actual feature required? Presumably this would be a project-wide setting like --java-access-modifier=[public|package]?

[izacus]

That would demand that all our code that interacts with these framework-only classes is stored inside a single package (due to limited Java granularity). A significantly worse maintenance and structure headache as opposed to a simple prefix.

[mknejp]

Wait, isn't this option actually redundant? Have you tried specifying the Java ident style as MyPrefixFooBar? I'm pretty sure that already works

[artwyman]

Note that there is no ident-java-type argument for this to be redundant with, though it could easily be added. The Java ident options are pretty minimal. I think the original author left it that way because Java (unlike C++) has such a strong default style which everyone follows for most things, making more options unnecessary. The author of the ObjC generator didn't take that shortcut, apparently, even though it also has a pretty strong default style.

I don't object to adding this argument, for clarity of intent and similarity to ObjC, even if it could theoretically be accomplished through an ident style alone (as could ObjC's prefix). I would prefer the argument be called --java-type-prefix, though, so that it's clear that its intent is the same as --objc-type-prefix. Also because it should apply, now or in future, to Java types which aren't declared as "class", but instead "enum" or "interface".

In summary: @izacus consider whether you'd prefer this argument vs. adding an ident option to do the same thing. If you still prefer this option, and change its name to --java-type-prefix, I'll merge.

[artwyman]

Invoking code review process now that it's available, and assigning to the author for next steps. See my prior comment for requested changes.

[izacus]

For consistency with other parameters, I think using a ident parameter makes more sense. I'll implement it and notify here.

[izacus]

There, I have rewritten the code to use ident-java-class parameter and apply it like other ident options. This should improve consistency.

[artwyman]

Thanks. Can you call the parameter ident-java-type for consistency with Objc, and with the name of the JavaIdentStyle field?
Looks like JNI is unfortunately already inconsistently using "class", but it's also already a special-case in the implementations (being an "intermediate" language rather than a target).

[izacus]

Yup, of course.

[izacus]

Updated to the more consistent version now.