projectlombok / lombok

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

[FEATURE] onstruct create variables from getters #3517

Open glelouet opened 1 year ago

glelouet commented 1 year ago

Describe the feature

https://groups.google.com/g/project-lombok/c/AWx5uDsX6mQ

I'm trying to code it.

This annotation would replace

@Onstruct
Object author, name, date, editor, purchasable = mybook;

by

var author=mybook.getAuthor();
var name = mybook.getName();
var date = mybook.getDate();
var editor=mybook.getEditor();
var purchasable=mybook.isPurchasable();

with prefix/suffix options to avoid name clashes.

Describe the target audience

Anyone with a lot of getter access on the same object.

Additional context

Making a fork, not sure how to debug/test, need a ticket id to link in the commits. If you have dev doc please provide :)

glelouet commented 1 year ago

I really need dev doc, I imported the project in eclipse since there is no root pom.xml, but it can't find the eclipse imports.

The import org.eclipse.jdt.internal cannot be resolved

rspilker commented 1 year ago

Running ant eclipse (use ant -p to find all documented tasks) will generate the proper eclipse project and classpath files.

glelouet commented 1 year ago

Thanks. I did it and then was able to import the eclipse project.

How can I start the tests, including the dockerized ones ?

ant test

failed.

ant test.broad

failed later (on my own tests), is that the correct ? Seems like placing my code in test/transform/resource/before/OnstructBook.java does not work.

Once I removed them, it failed with

test.javac17:

-find.vm.property:

-find.vm.java_home:

-find.vm.reg:

-find.vm.ask: [echo] A JVM17 is required to run the request tests. [echo] this script can automatically find VMs on mac and windows but did not find a suitable VM. [input] Enter the full path to JVM17:

while java --version returns

java --version openjdk 21-ea 2023-09-19 OpenJDK Runtime Environment (build 21-ea+14-Ubuntu-0ubuntu1) OpenJDK 64-Bit Server VM (build 21-ea+14-Ubuntu-0ubuntu1, mixed mode, sharing)

glelouet commented 1 year ago

Also I tried to add a simple annotation handler that prints the AST (copied from printAST) but doesn't seem to be called.

I noticed handlers are present in javac and eclipse folders, so I copied it in both, but seems still not to work. How am I supposed to register my handler ?

my handler is

@Provides
@DeferUntilPostDiet
@HandlerPriority(65536) // same as HandleValue // TODO
public class HandleOnstruct extends EclipseAnnotationHandler<Onstruct> {

    public static final HandleOnstruct INSTANCE = new HandleOnstruct();

    @Override public void handle(AnnotationValues<Onstruct> annotation, Annotation ast, EclipseNode annotationNode) {
        PrintStream stream = System.out;
        annotationNode.up().traverse(new EclipseASTVisitor.Printer(true, stream, true));
    }

}

my test contains

        @PrintAST
        @Onstruct
        Object author, name, editiondate, purchasable = mybook;

which results only in

[junit] [PASS] javac-WitherLegacyStar.java(lombok.transform.TestWithDelombok)
[junit] <LOCAL Object author> 
[junit]   <ANNOTATION: @PrintAST() />
[junit]   <ANNOTATION: @Onstruct() />
[junit]   <TYPE class com.sun.tools.javac.tree.JCTree$JCIdent>
[junit]     Object
[junit]   </TYPE class com.sun.tools.javac.tree.JCTree$JCIdent>
[junit] </LOCAL Object author>
[junit] ***** OnstructBook.java *****
[junit] Missing / empty expected file: OnstructBook.java

so it seems only @printast is processing the annotation

edit : found it : the javac handler needs to implement JavacAnnotationHandler -.-

seems to work now.

glelouet commented 1 year ago

ok so I have a JCVariableDecl variable that is a declaration of the variable on which there is the annotation, a JCIdent that is the sole identifier assigned to the list of variables annotated, the varName name of the variable we want to use, methName name fo the method we want to use.

How can I replace the elements of the JVariableDecl ? I want to have var varName=ident.methName(); (so a change of type to var, of name to varName, of init to ident.methName() ) So far

variable.name = varNode.toName(varName); effectively changes the name

            JCExpression methCall = maker.Select(ident, varNode.toName(methName));
            variable.init = maker.Apply(com.sun.tools.javac.util.List.<JCExpression>nil(),
                methCall, 
                com.sun.tools.javac.util.List.<JCExpression>nil());

effectively changes the assignment (This is not the List I am looking for …)

variable.type = (JavacHandlerUtil.chainDotsString(varNode, "java.lang.String")).type; does nothing. also tried to use null instead, no diff. How can I change the type ? can I use var, or do I need to find the actual type using reflexion? Can reflexion even work at this stage of the compiling ? I guess I do need to check both, that is lookup is class exists by reflexion and find the method with given name to extract its return type, or find in the compilation unit the class declaration (not compiled yet) and search the AST for the corresponding method …

glelouet commented 1 year ago

so far the following code

    void test() {
        Book mybook = new Book("author0", "bookname0", new Date(), true);
        @Onstruct
        Object author, editionDate = mybook;
        @Onstruct(methodPre = "")
        Object name = mybook;
        assert Objects.equals(mybook.getAuthor(), author);
        assert Objects.equals(mybook.name(), name);
        assert Objects.equals(mybook.getEditionDate(), editionDate);
        @Onstruct(methodPre = "is")
        Object purchasable = mybook;
        assert Objects.equals(mybook.isPurchasable(), purchasable);
    }

    void testPrefix() {
        Book mybook = new Book("author0", "bookname0", new Date(), true);
        @Onstruct(pre = "b_")
        Object author, editionDate = mybook;
        assert Objects.equals(mybook.getAuthor(), b_author);
        assert Objects.equals(mybook.getEditionDate(), b_editionDate);
        @Onstruct(pre="b_", methodPre = "is")
        Object purchasable = mybook;
        assert Objects.equals(mybook.isPurchasable(), b_purchasable);
    }

is correctly delomboked into

    [junit]     void test() {
    [junit]         Book mybook = new Book("author0", "bookname0", new Date(), true);
    [junit]         Object author = mybook.getAuthor();
    [junit]         Object editionDate = mybook.getEditionDate();
    [junit]         Object name = mybook.name();
    [junit]         assert Objects.equals(mybook.getAuthor(), author);
    [junit]         assert Objects.equals(mybook.name(), name);
    [junit]         assert Objects.equals(mybook.getEditionDate(), editionDate);
    [junit]         Object purchasable = mybook.isPurchasable();
    [junit]         assert Objects.equals(mybook.isPurchasable(), purchasable);
    [junit]     }
    [junit] 
    [junit]     void testPrefix() {
    [junit]         Book mybook = new Book("author0", "bookname0", new Date(), true);
    [junit]         Object b_author = mybook.getAuthor();
    [junit]         Object b_editionDate = mybook.getEditionDate();
    [junit]         assert Objects.equals(mybook.getAuthor(), b_author);
    [junit]         assert Objects.equals(mybook.getEditionDate(), b_editionDate);
    [junit]         Object b_purchasable = mybook.isPurchasable();
    [junit]         assert Objects.equals(mybook.isPurchasable(), b_purchasable);
    [junit]     }

, except the type remains object.

victorwss commented 1 year ago

Is Onstruct really a good name? On a first look, it looks like as a mistyped Construct. Also, the proper capitalization would be OnStruct. Further, other lombok annotations don't have the prefix On in the annotation itself, but only in fields that inject annotations themselves like @Setter(onParam_=@Max(10000)).

Not a big deal, but I think it deserves a last thought.

glelouet commented 1 year ago

It's not a good name. People on the feature discussion proposed Destruct but it looks to me like finalize-related. On- annotations looks to me lilke an event-trigger annotation : this one is also bad. Other names could work, like : Split(but already exists for strings) , Spread(but javascript operator "..." to concatenate iterable), From(but too close to for), Expand . This will be an important issue when the feature works … IF it works. cause I still can't change the class of the field so if nobody can help, then I can't do it.

But yes, naming and fine tweaking the use cases is very important.

rzwitserloot commented 1 year ago

Java itself is working on deconstruction. Which is what this feature is doing.

I rate the odds that this is ever going to make it into lombok as very low as a consequence (backporting existing java feaures to older java releases with different syntax is not lombok's purpose).

glelouet commented 1 year ago

Source, please.