search

projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.74k stars 2.36k forks source link

[FEATURE] Generate JSF component properties #2986

Open Rawi01 opened 2 years ago

Rawi01 commented 2 years ago

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 commented 2 years ago

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:

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:

@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).

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

Rawi01 commented 2 years ago

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.

rzwitserloot commented 2 years ago

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:

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.