Lombok: [BUG] @FieldNameConstants doesn't take parent fields into account

Created on 21 Dec 2018 · 14Comments · Source: projectlombok/lombok

Describe the bug
When you have:

@Getter
@FieldNameConstants
public class A {
  private String fieldA;
}

and:

@Getter
@FieldNameConstants
public class B extends A {
  private String fieldB;
}

then you can access both fields like this:

B b = new B():
b.getFieldA();
b.getFieldB()

"Old" @FieldNameConstants also followed this rule (it inherited field names):

Form<B> f = new Form<B>();
f.getValue(B.FIELD_A);
f.getValue(B.FIELD_B);

Unfortunatelly with "new" @FieldNameConstants this is not possible anymore.

Expected behavior
It shoudl be possible to access field names of the parent, exactly like you can access fields of the parent.

Version info (please complete the following information):

Lombok version 1.18.4
Platform Eclipse Photon

Source

eximius313

Most helpful comment

@iherasymenko, the case is: in almost any application you have user input.
It's best when you organize user input as Forms:

public class BasicForm {
   private String username;
   private String email;
}

in order to DRY, you organize forms in hierarchies:

public class ExtendedForm extends BasicForm {
   private String password;
   private String repeatPassword;
}

Now "old" FieldNameConstants usecases was:

In Apache Wicket, when you create form, you must explicitly point the fieldName. Using whis pseudocode:

new UserInput(new InputText(ExtendedForm.FIELD_EMAIL), new InputText(ExtendedForm.FIELD_PASSWORD);

In Play Framework or Spring, you use automatic binding BUT when you want to use validation with this pseudocode:

public class ExtendedForm extends BasicForm {
   @NotNull
   private String password;

   @SameAs(ExtendedForm.FIELD_PASSWORD)
   private String repeatPassword;
}

or - you test your code, right?

final String username = "anystring";
Result result = controller.performRequest(username);
assertThat(result.getJson(ExtendedForm.FIELD_USERNAME)).isEqualTo(username);

there are many more usecases for this feature and "new" FieldNameConstants kills them completly

eximius313 on 19 Feb 2019

❤2 👍1

All 14 comments

This lost feature was unusable for many, anyway, as the expression B.FIELD_A produces a warning. With the asEnum=true option it's impossible to restore, with asEnum=false, it may be possible with B.Fields extends A.Fields, but seriously, is such a warning-producing feature working only in half the cases worth it?

Maaartinus on 21 Dec 2018

@Maaartinus I'm not talking about enums. Try @FieldNameConstants in 1.18.2 -> it doesn't produce any warning - it just works.

eximius313 on 22 Dec 2018

What you have is

class A {
     private String fieldA;
     public static final String FIELD_A = ...; // generated
}

class B {
   ...
}

and you use

B.FIELD_A

despite the fact that there's no FIELD_A in class B (not even inherited as statics don't get inherited; it's just that the compiler knows what you mean).

The warning is called "indirect access to static members" in my Eclipse. You may argue that the warning is irrelevant (see SO).

Maaartinus on 22 Dec 2018

We're not gonna go here, as it's very very complicated. See resolution.

rzwitserloot on 19 Feb 2019

@rzwitserloot - I'm a part of several projects (hundreds of thousands lines of code each).
We used "old" FieldNamesConstants in each of the projects with great success.
We tried to migrate to "new" version in each project and not a single one succeeded.

I'll be honest - "old" version was very simple but very handy tool, while "new" version is over engineered mess, which is completly unusable (and as you say - hard to mantain)!

We have no choice but stay with 1.18.2 version of Lombok in every project.
Maybe you'll consider going back to working version?

eximius313 on 19 Feb 2019

👍1

@eximius313 Did you consider method references for your need?

iherasymenko on 19 Feb 2019

@iherasymenko, the case is: in almost any application you have user input.
It's best when you organize user input as Forms:

public class BasicForm {
   private String username;
   private String email;
}

in order to DRY, you organize forms in hierarchies:

public class ExtendedForm extends BasicForm {
   private String password;
   private String repeatPassword;
}

Now "old" FieldNameConstants usecases was:

In Apache Wicket, when you create form, you must explicitly point the fieldName. Using whis pseudocode:

new UserInput(new InputText(ExtendedForm.FIELD_EMAIL), new InputText(ExtendedForm.FIELD_PASSWORD);

In Play Framework or Spring, you use automatic binding BUT when you want to use validation with this pseudocode:

public class ExtendedForm extends BasicForm {
   @NotNull
   private String password;

   @SameAs(ExtendedForm.FIELD_PASSWORD)
   private String repeatPassword;
}

or - you test your code, right?

final String username = "anystring";
Result result = controller.performRequest(username);
assertThat(result.getJson(ExtendedForm.FIELD_USERNAME)).isEqualTo(username);

there are many more usecases for this feature and "new" FieldNameConstants kills them completly

eximius313 on 19 Feb 2019

❤2 👍1

@eximius313 What I'm missing here?

Can't you simply replace all occurrences of the no more existent fields by the existent ones? I mean, something like ExtendedForm.FIELD_USERNAME ⇒ BasicForm.FIELD_USERNAME everywhere?

When you switch on the eclipse warnings I mentioned above, Eclipse will tell you exactly what it wants and IIRC even offer a corrective action. I could imagine to automate the whole conversion.

The other question is that if using the fields the way you do is not actually better than avoiding the warning. I indeed can imagine, that when dealing with ExtendedForm, I don't care about where the field comes from (the form itself or inherited) as it's an implementation detail.

Maaartinus on 19 Feb 2019

@eximius313 thanks for the detailed explanation. This definitely makes a lot of sense when it comes to the examples you provided.

iherasymenko on 19 Feb 2019

@Maaartinus I bet that you use inheritance in some (or many) places in your code.
Let's say:

public class Animal {
   public void feed(){
   }
}

public class Dog extends Animal {
   public void play(){
   }
}

and if you find just one place in your code that looks like this:

Dog dog = new Dog();
dog.play();
Animal animal = dog;
animal.feed();

then I surrender. But if no, then please consider my kind request to not create a solution which is completly unusable.

eximius313 on 19 Feb 2019

👍1

@eximius313 To make it clear: I'm not in charge of this project, I'm just trying to clarify things. And as you can see from the second part of my above posting, I may already agree with you.

However, your example is too unrealistic. I surely don't do such things, but I often do things like this:

Dog dog = new Dog();
dog.play(Dog.FUNNY_PLAYING);
dog.feed(Animal.BIG_PORTION);

I find this perfectly acceptable for me, as I rarely need such constants. I guess, you need them all the time and that's the game changer, right?

For what you need, I can imagine two possibilities:

The old way of spamming the current class with field constants.
An extension to the new way, which would either
- add the inherited fields (I'm afraid, rather impossible as stated by @rzwitserloot above)
- let the generated inner class optionally inherit from the corresponding class in the superclass

Maaartinus on 19 Feb 2019

@Maaartinus, I saw that you agreed with me on the second part. I just wanted to explain the first one:

Can't you simply replace all occurrences of the no more existent fields by the existent ones?

I just wanted to show how cumbersome can it be.

I guess, you need them all the time and that's the game changer, right?

Yes. It's the second-most often used feature of Lombok in our code. And for programmers in my team is better to use old Lombok (or not use Lombok at all) than doing such silly things as wondering "which class is the carrier of this field".