[FEATURE] Generate JSF component properties

[Rawi01]

Describe the feature
A JSF component usually uses an enum to list all its properties. For each of the enumerated properties there is a getter and a setter which refers to the state helper to store the value.

public class Component extends UIComponent {
	public enum PropertyKeys {
		property1,
		...
	}
	
	public String getProperty1() {
		return (String) getStateHelper().eval(PropertyKeys.property1, "default");
	}
	
	public void setProperty1(String property1) {
		getStateHelper().put(PropertyKeys.property1, property1);
	}
	...
}

Real world example: https://github.com/primefaces/primefaces/blob/9f8f972a1a549759859f684b4c221436c0ed93e2/primefaces/src/main/java/org/primefaces/component/selectonemenu/SelectOneMenuBase.java

Writing all that by hand is quite annoying and error prone (It took me more than an hour to figure out that I missed to adjust one property key in my copy pasted setter).

I propose to add an annotation (e.g. @JSFComponentProperties) that can be added to inner classes. It transforms the whole class into an enum using the field names, uses the field types for getter and setter types and moves the initializer to the default value. Some special handling for primitive types without initializers is required.

Example to generate the code above:

public class Component extends UIComponent {
	@JSFComponentProperties
	public class PropertyKeys {
		String property1 = "default";
		...
	}
}

Describe the target audience
JSF developers

Additional context
Most of this should be quite easy, converting a class into an enum might be a little bit harder.

[rzwitserloot]

Seems like we eliminate a ton of easily messed up boilerplate in trade for a low maintenance burden. Sounds like a good idea.

As for the name:

• @lombok.extern.faces.JsfComponentProperties
• @lombok.extern.jsf.JSFComponentProperties

Or some other combination sounds right. I'm guessing the first one is the better option (fits the DvdPlayer style conventions of writing acronyms in java's CamelCase format, and you import from package javax.faces, I think, so presumably faces is more natural than jsf on this one. But, you're clearly more familiar with the domain :)

Some questions:

• Is it to be expected that these properties can have generics, e.g. public List<String> getProperty1() { .. return (List<String>) getStateHelper()....? If it is, how do we deal with the ugly-cast, do we also add the @SuppressWarnings? Given that it's all runtime glue, the generics do nothing, I assume. Should we leave the warning (but then, how would one get rid of it? @SuppressWarnings on the field? @lombok.extern.faces.JsfComponentProperties.SuppressUnchecked on the field?
• Are lowercase enum names the common pattern? It matches what @FieldNameConstants does, so that'd be nice if it is :)
• If the initializing expression contains side-effects and/or isn't stable, what would you expect? Let's say its long property5 = System.currentTimeMillis() - would you expect the get command, when the statehelper doesn't have anything (so the default kicks in) to return the time as it is was when the class was loaded, or would you expect the get command to execute the expression (thus, that getProperty5() returns the same thing System.currentTimeMillis() would.
• Which spec controls the translation from property name to accessor name (fCell to getFCell() or does JSF expect getfCell())? See issue Change lombok's property capitalization code. Field xName -> getxName(). #2693 - if JSF explicitly fails unless we generate getfCell(), or something in the chain fails no matter which way we go, we should probably make an exception here and generate what JSF needs even if it isn't compatible with with lombok's @Getter would do (or if happiness is impossible with a field named fCell, we should error out on the field and explain the problem).
• I assume getStateHelper() is coming from javax.faces.component.UIComponent - Looks like any usage of this feature would necessarily require that you extend something. What should we do if the outer class doesn't extend anything - should we just silently add extends UIComponent, or should we error out?
• Should we also generate something for getFamily() and that constructor that invokes setRendererType?
• How big is the problem here that in your example (SelectOneMenuBase), lombok can make on getTitle and setTitle for you, but isn't going to be able to figure out that it should stick an @Override on these?
• I assume this is the source of HtmlSelectOneMenu. That very file, even though somebody checked it into github, has // *** GENERATED CODE - DO NOT EDIT *** at the top of it, so somebody out there already wrote some code gen system for this problem already. Is whatever made this commonly used (in which case the need for this feature goes down, or at least the focus on marketing this feature goes up), or is it something internal (in which case the fact that JBoss saw the need to write a code generator for this stuff only highlights that lombok should add this feature)?
• That code (HtmlSelectOneMenu as above) calls handleAttribute in addition to getStateHelper().put, which I assume is some internal JBoss thing and not relevant for this code generation. But it does open the question: If you want additional behaviour to e.g. a set method, do we just say: Hey, then write the whole thing yourself; feel free to use the PropertyKeys.propName enum value we made for yo uthough. Or do we say: No problem, we'll inject that put line for you / you can tag some method as 'please run this method, supplying the enum and the property, as part of a set call? e.g. that this JBoss code could be replaced with:

@lombok.extern.faces.JsfComponentProperties.SetterHelper
public setAccesskeyHelper(PropertyKeys key, String accesskey) {
    handleAttribute(key.name(), accessKey);
}

Where lombok figures out what to do, per argument (you can have up to one argument of type PropertyKeys, and up to one argument of the key type, with any name and in any order).

• Let's say someone tags a top-level class with @JsfComponentProperties, I assume the best idea there is to just error out and refer to the feature page on projectlombok.org in the error message showing how to use it. Or should we just spell it out (make an inner class and annotate it. The fields of that class are turned into properties, the initializers on those fields are considered the default-generating expression). I guess there's no nice way to just make this feature 'tag your top level with @JsfComponent, and we'll take care of the rest. Seems a bit crass to scan for an inner class whose name matches the pattern xKeys. Then again, tagging in inner class with an annotation which then results in lombok generating a ton of stuff in its enclosing class is also slightly weird. As this is an extern feature, weirdness is fine (We presume that the user knows the library and its associated patterns well, for extern features).

I think satisfactory answers are available for all these questions, and armed with those, the feature seems good to go!

[Rawi01]

Is there really a convention to use CamelCase for acronyms? What about URLConnection?

faces is most likely the better name, there is already is a set of @Faces... annotations so I think @lombok.extern.faces.FacesComponentProperties is a better name.

• Yes, generics are possible. I think we can add a @SuppressWarning as soon as we detect the generic.
• Yes, lowercase is common in this case.
• Great question, I never used something else than a constant value. To keep it simple I would simply copy the initializer to the method parameter, if the user needs something else it is still possible to implement a custom getter.
• IIRC it is getfName because it uses java.beans.Introspector.decapitalize
• Adding the extends seems to be a little bit to much magic especially because we add the annotation to the inner class and not to the component itself. An error is fine I guess.
• I think we should keep the first version as simple as possible.
• As @Override is only a marker I think we can simply ignore that.
• Interesing, there also was a code generator in mojarra (JSF implementation) some time ago (eclipse-ee4j/mojarra@a4bac4a). I also checked a bunch of other projects and found two related issues (No longer use ComponentMojo to generate classes primefaces/maven-jsf-plugin#4, Declaring component attributes via annotations jakartaee/faces#1193).
• As mentioned above I think we should allow the user to implement custom methods.

[rzwitserloot]

Is there really a convention to use CamelCase for acronyms?

Yes, adopted by amongst others Oracle/team OpenJDK and google.

What about URLConnection?

Predates the convention. Same for URL and URI.

See:

• Google Style Guide on this topic
• I think 'Effective Java', but I don't have a copy of the book handy. Links I found on the web suggest page 237.
• For a real spin, think of HttpURLConnection which mixes conventions. It partly has to (HTTPURLConnection looks real bizarre), but it also serves as a nice objective example: DVDPlayer is not just subjectively the wrong choice (as in, 'it is not (any longer) the official convention). It is in fact objectively the wrong choice. No style guide needed. Auto-complete systems based on first-letter-of-each-camelcased-word, such as most IDE's 'open type' dialogs, also don't really know what to do with DVDPlayer (it's not found if you type DP), whereas DvdPlayer is handled just fine.
• O'Reilly java pocket guide on naming conventions (section 'Acronyms', explicitly says GpsVersion, not GPSVersion).

lombok.extern.faces.FacesComponentProperties

Works for me!

Yes, generics are possible. I think we can add a @SuppressWarning as soon as we detect the generic.

Yup, let's just flat out generate it if needed and not make it configurable. I don't foresee any point in wanting to e.g. not generate it. @SuppressWarnings is a bit of a mess (the actual keys you can pass to it are severely underspecced, which has resulted in various tools interpreting @SW annotations in different ways, and generally means we have to usually add "all" to the pile, but, one way or another, we just need to figure out what @SuppressWarnings line works for the maximal amount of common environments and generate it. But, yes, only if we detect generics.

Great question, I never used something else than a constant value.

My primary concern is that users of this feature will make an assumption about how it works and will consider their assumption so obvious that they'd be highly surprised if it worked differently than what they think it does. Except, there are non-trivial amounts of programmers with that idea making different assumptions. I'm not sure there's any solution there other than to just accept that some folks will get confused and a little angry that 'lombok made such a dumb choice', and all we get to do is pick which group gets angry. One way out is to only allow literally constant literals, and not even refs to constants (because we need resolution to know), but that feels needlessly restrictive.

Copying the initializer though... not so sure we can do that. Expressions can contain almost all syntax nodes, as new Object() { whatever you want here }.foo(); is a legal expression. Copying all that is impossible. We can move it, though. With some minor risk (we're moving nodes and therefore changing contexts, which may break stuff), but there are other places we already do this. We mostly operate on the assumption nobody would be silly enough to actually make that the default. I guess the call is: Just.. do something, whatever. Some folks won't like it, but the vast majority of the team its a constant value and its a moot point.

IIRC it is getfName because it uses java.beans.Introspector.decapitalize

Okay, let's do exactly that and not make it configurable.

Adding the extends seems to be a little bit to much magic

Okay, that means the feature will error out and link to the docs if you put the annotation on a top-level typedef.