Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New features for @FieldNameConstants #1774

Closed
vyemialyanchyk opened this issue Jul 11, 2018 · 3 comments
Closed

New features for @FieldNameConstants #1774

vyemialyanchyk opened this issue Jul 11, 2018 · 3 comments
Assignees
Labels
accepted The issue/enhancement is valid, sensible, and explained in sufficient detail enhancement

Comments

@vyemialyanchyk
Copy link

vyemialyanchyk commented Jul 11, 2018

is it possible to have this configurable so:

@FieldNameConstants(toEnum = true)
public class FieldNameConstantsExample {
  private final String iAmAField;
  private final int andSoAmI;
}

will turn to

public class FieldNameConstantsExample {
  public enum Fields {
    iAmAField,
    andSoAmI,
  }
  private final String iAmAField;
  private final int andSoAmI;
}

?

and enum Fields, "Fields" make this configurable...

@rzwitserloot
Copy link
Collaborator

There's a pretty serious problem we can't solve in lombok, which is the following:

javac will abort early with an error, before it even runs any annotation processors, if you have a NAMED static import which doesn't exist yet because lombok would generate it. So, given:

@FieldNameConstants
public class FieldnameTest {
    int test;
}

if you then, in another source file you compile at the same time as above, try: import static pkg.FieldnameTest.FIELD_TEST;, you get an error, and there's not a lot we can do about it because that error occurs before lombok even runs. The solution is to go with import static pkg.FieldnameTest.*; and then it does work.

Now back to this issue: I've made a proof of concept for this setup and you can then write import pkg.FieldnameTest.Fields.test; and now it all works fine.

This is awesome. Therefore, 2 things:

[1] We shall be adding this to lombok soon, and
[2] We're going to debate removing the old way entirely, because we can't solve the above problem and it's an obvious way to try to use the generated constants. Possibly we will not remove this functionality but we make the enum style as described in this issue the default.

@rzwitserloot rzwitserloot self-assigned this Jul 28, 2018
@rzwitserloot rzwitserloot added enhancement accepted The issue/enhancement is valid, sensible, and explained in sufficient detail labels Jul 28, 2018
@Maaartinus
Copy link
Contributor

That's a good news! Maybe it'd make sense to name it @FieldNameEnum instead, as prefix and suffix are rather useless with an enum (the old @FieldNameConstants could stay untouched for some time).

The only usage problem, I can see, is that pkg.FieldnameTest.Fields.test.name() is not a compile-time constant, so there's no string usable in annotations like e.g., Hibernate's uniqueConstraints (though here, the "logical" column names get used which may differ from the field names, so I should have chosen a different example).

@rzwitserloot Above, you wrote pkg.FieldnameTest.Fields.test, which violates the conventions, though I could imagine myself wanting to violate them, too. I guess, by default, it should behave like before ("same name as the field it represents, except with all uppercase letters, with underscores in front of the uppercase letters in the original field")?

  • configuration (maybe lombok.fieldNameEnum.uppercased)?
  • member final String pkg.FieldnameTest.Fields.TEST.fieldName = "test"?

@rzwitserloot
Copy link
Collaborator

Implementation is now on master branch: 3d432c3

along with the docs: 0038b48

Because the feature is in experimental I decided (for now, we can revisit this decision prior to releasing v1.18.4) to just remove the old way entirely and replace it with this new way.

You can now choose whether you want an enum or an inner type with psf String constants; by default you get the string constants, you have to explicitly choose to go with enum instead. Only configurable aspect right now is the name of the inner type.

I can see adding 'uppercased' as an option via lombok.config, but piling it into the annotation itself feels like a bridge too far (too many finicky settings, and it's purely a style thing; I don't think any framework demands that these are capitalized).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The issue/enhancement is valid, sensible, and explained in sufficient detail enhancement
Projects
None yet
Development

No branches or pull requests

3 participants