Lombok put annotation on builder method parameter instead method itself

[AngryGami]

Lombok version is 1.18.4

My Test.java file is this:

import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Builder;

@Builder
public class Test {
    @JsonProperty("kebab-case-prop")
    public final String kebabCaseProp;
}

lombok.config file is like this:

lombok.copyableAnnotations+=com.fasterxml.jackson.annotation.JsonProperty

I.e. I want @JsonProperty annotation copied to builder method so jackson can actually use builder properly.
When I do "delombok" like this: java -jar lombok-1.18.4.jar delombok -p Test.java > d_Test.java
resulting file contains following:

...
        public TestBuilder kebabCaseProp(@JsonProperty("kebab-case-prop") final String kebabCaseProp) {
            this.kebabCaseProp = kebabCaseProp;
            return this;
        }
...

Note that @JsonProperty is attached to parameter of the method, not to a method itself, which confuses jackson and it won't do proper deserialization. If I manually move annotation to method, everything works.

Is this some sort of a bug or this is intended behavior?

[janrieke]

It's intended, because annotations like @NonNull can only be placed on the parameter, not on the method. Lombok cannot automatically determine which annotations to put on the parameter and which on the method. Thus, it puts them all on the parameter, which is the safer choice.

In this case, this does not work with Jackson, but I don't know a good way to solve it. I'm not a lombok maintainer, but to me it seems that adding more config options will push the complexity of this feature too much towards incomprehensibility.

So my suggestion is to request a feature with Jackson so that @JsonProperty is also recognized if present on the method's parameter. This may seem a bit far-fetched, but remember that the annotation has a similar effect if present on a constructor's parameter.

[Maaartinus]

I'm not a lombok maintainer, but to me it seems that adding more config options will push the complexity of this feature too much towards incomprehensibility.

@janrieke I agree that it's (mostly) a Jackson issue, but otherwise disagree. Now, we have copyableAnnotations, which sounds good, but then the questions comes what it exactly means. Lombok's choice makes sense, but it's not obvious. And, as we can see, it's not always what's needed.

Copying annotations to the builder opens even more choices. While I can't imagine configuring all possible target places individually, I'm afraid that it's the best solution since otherwise, there'll be always someone missing an option. Let's hope, I'm wrong.

[AngryGami]

Thus, it puts them all on the parameter, which is the safer choice.

I disagree that this is "safer choice" regardless of my usecase here.

import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Builder;
import java.lang.annotation.Target;
import java.lang.annotation.Retention;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.CONSTRUCTOR;
import static java.lang.annotation.ElementType.FIELD;

@Builder
public class Test {
    @JsonProperty("kebab-case-prop")
    @TestAnn
    public final String kebabCaseProp;

    @Target({ FIELD, METHOD })
    @Retention(RUNTIME)
    public static @interface TestAnn {}
}

Note that TestAnn annotation cannot be used on method parameter by target restriction, though, when I do delombok I'm getting this:

...
        @java.lang.SuppressWarnings("all")
        public TestBuilder kebabCaseProp(@JsonProperty("kebab-case-prop") @TestAnn final String kebabCaseProp) {
            this.kebabCaseProp = kebabCaseProp;
            return this;
        }
...

This is plain wrong usage.

[janrieke]

That's why you have to explicitly switch on copying for your annotation, because lombok cannot know if the annotation is also for ElementType.PARAMETER. However, for annotations that are for ElementType.FIELD, it is more likely they are also for ElementType.PARAMETER than ElementType.METHOD.

But of course that's not safe, it's just more likely to be valid. If we want to be able to support all cases, lombok would need separate config options annotationsCopyableToParameter and annotationsCopyableToMethod. Furthermore, we'd need to be able to configure that for builders differently (because builders may have different semantics for their parameters/methods/variables). But that's what I meant with increased complexity.

[AngryGami]

Well, yes... covering all possibilities might be unfeasible. Reasonable defaults are also good, but I believe that for cases like this there must be some sort of escape hatch. Global configuration is way too general to handle this. Maybe additional annotation(s) that control where other annotations should be copied would be a better approach (at least more granular).
My main issue with current behavior is that I have to repeat JsonProperty annotation at both field and builder method/field level which is boilerplate and error prone approach.

[AngryGami]

    @Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
    @Retention(RetentionPolicy.RUNTIME)
    @Repeatable(CopyToBuilderAsRepeat.class)
    public @interface CopyToBuilderAs {
        Class<? extends Annotation> value();
        ElementType[] copyTo();
    }

    @Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
    @Retention(RetentionPolicy.RUNTIME)
    public @interface CopyToBuilderAsRepeat {
        CopyToBuilderAs[] value();
    }

    @Builder
    public class Test {
        @CopyToBuilderAs(value = JsonProperty.class,copyTo = ElementType.METHOD)
        @JsonProperty("kebab-case-prop")
        @CopyToBuilderAs(value = NotNull.class,copyTo = ElementType.PARAMETER) // this is default behavior, so I put it here for example sake
        @NotNull
        public final String kebabCaseProp;
    }    

[Maaartinus]

@AngryGami

Global configuration is way too general to handle this.

Are you sure? My idea would be to allow to configure exactly where an annotation should go, but only globally. This makes it much more complicated than the current state ("copy to where it makes sense most of the time"), but still very simple when compared to your proposal.

It's also much less verbose, as you don't need to repeat the information for every field. Are there case when you want e.g., @JsonProperty to be handled differently than in your last example?

[AngryGami]

I don't have any usecases where JsonProperty should go not to a method but I assume someone might have. My proposal is not a replacement for global configuration, but merely an escape hatch for situations when someone want to override behavior that in all other cases is fine. Situation with NotNull annotation above is an example where you might want that. Verbosity could be mitigated by making @CopyToBuilderAs annotation attachable at class level and therefore work for all fields in the class.