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

RFC: Remove Lombok. #644

Open
jjjasper opened this issue Sep 19, 2023 · 2 comments
Open

RFC: Remove Lombok. #644

jjjasper opened this issue Sep 19, 2023 · 2 comments

Comments

@jjjasper
Copy link
Contributor

Using lombok in the generated sources does not make sense. Lombok value is to keep source code readable and compact by avoiding boiler-plate getters and setters. Generated code is not meant to be looked at, it is not aimed to be pretty and nobody cares if it is boiler-plate because nobody maintains it.

Adding lombok support complicates solutions (having to take into account both with and without lombok)

@walaniam
Copy link
Contributor

@jjjasper I'm wondering.. maybe it would be easier to always use lombok in generated code? Looks like less code to maintain in mustache templates.
Have a look at this code which is generated when not using lombok

{{^useLombokAnnotations}}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}{{#hasVars}}
{{classname}} {{classVarName}} = ({{classname}}) o;
return {{#vars}}{{#vendorExtensions.x-is-jackson-optional-nullable}}equalsNullable(this.{{name}}, {{classVarName}}.{{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#isByteArray}}Arrays{{/isByteArray}}{{^isByteArray}}Objects{{/isByteArray}}.equals(this.{{name}}, {{classVarName}}.{{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^-last}} &&
{{/-last}}{{/vars}}{{#parent}} &&
super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
}
{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
private static <T> boolean equalsNullable(JsonNullable<T> a, JsonNullable<T> b) {
return a == b || (a != null && b != null && a.isPresent() && b.isPresent() && Objects.deepEquals(a.get(), b.get()));
}
{{/vendorExtensions.x-jackson-optional-nullable-helpers}}
@Override
public int hashCode() {
return Objects.hash({{#vars}}{{#vendorExtensions.x-is-jackson-optional-nullable}}hashCodeNullable({{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{^isByteArray}}{{name}}{{/isByteArray}}{{#isByteArray}}Arrays.hashCode({{name}}){{/isByteArray}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}});
}
{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
private static <T> int hashCodeNullable(JsonNullable<T> a) {
if (a == null) {
return 1;
}
return a.isPresent() ? Arrays.deepHashCode(new Object[]{a.get()}) : 31;
}
{{/vendorExtensions.x-jackson-optional-nullable-helpers}}
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("class {{classname}} {\n");
{{#parent}}
sb.append(" ").append(toIndentedString(super.toString())).append("\n");
{{/parent}}
{{#vars}}sb.append(" {{name}}: ").append(toIndentedString({{name}})).append("\n");
{{/vars}}sb.append("}");
return sb.toString();
}
/**
* Convert the given object to string with each line indented by 4 spaces
* (except the first line).
*/
private String toIndentedString(Object o) {
if (o == null) {
return "null";
}
return o.toString().replace("\n", "\n ");
}
{{/useLombokAnnotations}}
in comparison to lombok approach
{{#useLombokAnnotations}}
@lombok.EqualsAndHashCode(onlyExplicitlyIncluded = true, doNotUseGetters = true{{#parent}}, callSuper = true{{/parent}})
@lombok.ToString(onlyExplicitlyIncluded = true, doNotUseGetters = true{{#parent}}, callSuper = true{{/parent}})
{{/useLombokAnnotations}}

Maybe we need to do some more analysis which approach to choose (I'm definitely for keeping only one of them)

@walaniam
Copy link
Contributor

@jjjasper
And other thing that I spotted:
Code generated with lombok:

  @lombok.Getter
  @lombok.Setter
  @lombok.EqualsAndHashCode.Include
  @lombok.ToString.Include

    @Valid
    @NotNull
    
    private List<@Valid PaymentRequestLine> lines = new ArrayList<>();

code generated without lombok

    @Valid
    @NotNull
    
    private List<@Valid PaymentRequestLine> lines = new ArrayList<>();

  /**
    * Payment request details
  * @return lines
  */
    @NotNull @Valid 
    @Schema(name = "lines", description = "Payment request details", requiredMode = Schema.RequiredMode.REQUIRED)
    @JsonProperty("lines")
  public List<@Valid PaymentRequestLine> getLines() {
  return lines;
  }

  public void setLines(List<@Valid PaymentRequestLine> lines) {
  this.lines = lines;
  }

The sample without lombok contains some additional annotations (e.g. JsonProperty, Schema). The one which uses lombok seems not to have these annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants