When generated value class is not public, factory methods should have package visibility too.

[jbgi]

And not be public, as of now. Probably.

[sviperll]

Yes, but... I see no benefit... Method access is already limited by class access level. Methods will not become less accessible if this is implemented. I haven't encounted any static analysis tools that require manual limiting of access level.

There seems to be benefits for API design in manually written code, but they don't apply to mechanically generated code.

[jbgi]

That's not completely true (unfortunately). If A extends B, B being the generated value class, and if A is public, I can access the public static methods of B through A (if not shadowed in A). This would be bad practice, of course, and of no-use at all. BUT the methods show up in the auto-completion suggestions in my IDE... I agree that this is not high priority.

[talios]

Really? Static methods are not inherited tho - even if your IDE auto completes ( Eclipse maybe? ) - what does Javac say to it?

[jbgi]

Yep really! (this is Java we are talking about, after all).

package a;
class A {
    public static void printTest() {
        System.out.println("test");
    }
}

package a;
public class ExtendsA extends A {}

package b;
import a.ExtendsA;
public class B {
    public static void main(String[] args) {
        ExtendsA.printTest();
    }
}

$ javac a/* b/* 
$ java -cp . b.B
test

[sviperll]

I'm thinking about adding access level customization. And I'm stumble with the question: do we need "protected" access level?

Currently all methods are public. If we want to extend generated class we may want to make constructor methods and access/match method package-private (aka no-modifier) or protected. Do we need both? Or can we settle on protected always. Or package-private?

Should we provide enum AccessLevel { PRIVATE, PROTECTED, PACKAGE, PUBLIC } ? Or is there some analog in JDK or common libraries?

[jbgi]

I think always package-private (for all currently public methods) would be fine for when valueClassIsPublic = false. This would minimize API exposed accidentally. But this would break current code. I see no use for protected level. (since value class should not use subtyping).

[sviperll]

Ok, I'm leaning towards your position :) Should we break current code, now?

[jbgi]

I have currently very few code code that use value-class inheritance so I'm OK with the breaking change.

[sviperll]

I'm going to implement "always package-private (for all currently public methods) would be fine for when valueClassIsPublic = false" proposal.

@talios, This is breaking change, are you OK with it?

[talios]

As long as public is still public I'm down for it. We generate ADT4j classes in a separate API artifact that is shared/exposed via OSGi and used by other artifacts - so everything is public anyway. If I wanted public but not published I'd change my OSGi metadata.

[sviperll]

In 0.14 release constructor methods are package-private when generated class is package-private. Access restrictions for all other methods can be individually customized.

[sviperll]

@jbgi I'd like to get some feedback on access restriction.

Currently there are access parameters with value of AccessLevel type in most annotations. Maybe It's better now (sooner than later) to switch to simple isPublic boolean flag?

AccessLevel is an enumeration with 4 values: "private", "package", "protected" and "public". These complex 4 level access system is designed for culture where inheritance is promoted. Value-types like ones generated by adt4j will probably never encounter with the need for such a complex system.

isPublic false will result in package-private access level for getter, updater, etc.

[jbgi]

I do not use AccessLevel, isPublic and customization of accept method name is enough for me.