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

[BUG] Lombok and ECJ/Eclipse null checking annotations/compiler settings #2318

Closed
Gaibhne opened this issue Dec 20, 2019 · 3 comments
Closed

Comments

@Gaibhne
Copy link

Gaibhne commented Dec 20, 2019

Describe the bug
Lombok does not play nice with the ECJ/Eclipse specific null annotations. The core issue is the method signature of the generated equals(Object).

To Reproduce
I have provided what I hope is a reproducible SSCCE of the problem at https://github.com/Gaibhne/lombok-onparam that includes details about the various errors and warnings mentioned above as comments in the source code.

Expected behavior
The expected behavior is code that still compiles. Instead, one gets the following error:

Illegal redefinition of parameter o, inherited method from Object does not constrain this parameter

Explanation of the bug:

The ECJ annotation org.eclipse.jdt.annotation.@NonNullByDefault implicitly annotates all unannotated parameters (amongst others) with org.eclipse.jdt.annotation.@NonNull. This poses a problem with methods inherited from unannotated classes such as java.lang.Object, as this introduces a new narrowing to the method that is not present in the super class. The result is, correctly, an error message.

The solution, when using just ECJ, is to annotate the method signatures correctly; in the case of equals(Object), the correct signature would be:

@Override
public boolean equals(final java.lang.@Nullable Object o)

The problem with Lombok arises when @EquashAndHashCode or one of the annotations implicitly containing it (@Data, @Value, possibly others) is used to generate the method, because the generated method is missing the @Nullable parameter annotation. @EqualsAndHashCode already contains a mechanism for specifying annotations on the parameter, in the form of onParam (or onParam_ for Java 8+ respectively), but this option does not work correctly in this case.

Problem 1

With the Lombok Eclipse plugin or ECJ, the error Illegal redefinition of parameter o, inherited method from Object does not constrain this parameter appears on @EqualsAndHashCode, even when the onParam option is used. This suggests that in this case, the onParam option is simply ignored.

Problem 2

When Delombok is used on the annotated code, code with the correct annotation is generated, but the generated code has a syntax error. The type annotation is not in its correct place. Generated code:

public boolean equals(@Nullable final java.lang.Object o)

Correct code:

public boolean equals(final java.lang.@Nullable Object o)

Unlike running Lombok directly, in this case, the onParam option seems to be used, but applied slightly incorrectly.

Version info (please complete the following information):

  • lombok:1.18.10
  • org.eclipse.jdt.annotation:2.2.300
  • ecj:3.19.0
  • plexus-compiler-eclipse:2.8.5

I do not believe that this problem is related to version issues.

@Gaibhne
Copy link
Author

Gaibhne commented Dec 20, 2019

I believe the following issues are relevant, but don't completely overlap:

#1248
#788

@Gaibhne
Copy link
Author

Gaibhne commented Dec 20, 2019

If google brings you here looking for a solution, changing the default @NonNullByDefault annotation to @NonNullByDefault({ RETURN_TYPE, FIELD, TYPE_BOUND, TYPE_ARGUMENT }) instead can mitigate this problem, although of course you lose the NonNull-default for parameters. Still, some null checking is better than no null checking.

@rzwitserloot
Copy link
Collaborator

No, what lombok does is correct: You are asking lombok to put @Nullable on the parameter and it does just does that.

Unfortunately, the authors of eclipse's nullity code, in their infinite wisdom, decided that the @Nullable annotation as a concept can only apply to types and thus 'put that annotation on the parameter' is the wrong move. I guess lombok should have an onTypeOfParam in addition to onParam except.. really, let's not go there.

I see nothing new here; we're still on 788, which is the only (but a real) problem.

So, closed as duplicate of #788.

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