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

In latest version (0.10.1) @Data is incompatible with Spring's @NotNull #360

Closed
lombokissues opened this issue Jul 14, 2015 · 21 comments
Closed
Labels
Milestone

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 287)

@lombokissues
Copy link
Author

👤 francois.marot   🕗 Oct 13, 2011 at 08:21 UTC

What steps will reproduce the problem?

  1. anotate a class with @ Data
  2. add Spring's @ NotNull annotation on a private member ( import org.springmodules.validation.bean.conf.loader.annotation.handler.NotNull )
  3. Eclipse will complain (java error), underlining the @ Data in red with the error: "The annotation @ NotNull is disallowed for this location"

What is the expected output? What do you see instead?
everything should be fine

What version of the product are you using? On what operating system?
Eclipse indigo up to date
Lombok 0.10.1
spring-modules-validation-0.8-sources.jar

Please provide any additional information below.
Lombok 0.10.0 does not have this error so it was introduced in Lombok 0.10.1

I imagine that Lombok expects all @ NotNull annotations to be its own and that a more precise check on the whole @ NotNull annotation class+package would solve the problem but I may be wrong as I didn't look at the sources.

@lombokissues
Copy link
Author

👤 steven.dick.1972   🕗 Oct 14, 2011 at 07:59 UTC

I'm using the javax validation API (with hibernate validator 4.2.0 final) and Eclipse is fine, but javac running from ANT complains about the @ NotNull when using Lombok 0.10.1 - downgrading to Lombok 0.10.0 resolves the problem.

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 17, 2011 at 09:40 UTC

Any update to the use of javax validation API ? Bit of a stopper from us to upgrade this.

Thanks

@lombokissues
Copy link
Author

👤 r.spilker   🕗 Oct 17, 2011 at 09:57 UTC

We will discuss this tonight during our weekly Lombok development meeting

@lombokissues
Copy link
Author

👤 reinierz   🕗 Oct 17, 2011 at 17:20 UTC

We've looked at version 0.8 of spring modules, here:

http://java.net/projects/springmodules/sources/svn/show/tags/release-0_8/projects/validation/src/java/org/springmodules/validation/bean/conf/loader?rev=2110

And as you can see, there's no 'annotation' package. We have no idea where its from, in fact; googling around doesn't give us much.

As far as we know, for years and years now, the right annotation for this stuff is javax.validation.constraints.NotNull and not spring's own variant.

Spring's own variant seems to only be legal on fields, whereas a recent version of javax.validation.constraints.NotNull is valid on fields, methods, and parameters - everything we need it to be legal on.

We suggest you switch versions. Otherwise, can you point us at the sources of this NotNull annotation so we can learn a little more about it? We could try to hardcode an exception for that annotation, but without any trace of that annotation other than this bugreport we're a bit hesitant to do so.

@lombokissues
Copy link
Author

👤 reinierz   🕗 Oct 17, 2011 at 17:20 UTC

Stephen, Graham:

When we look at the javadoc for javax.validation.constraints.NotNull here:

http://download.oracle.com/javaee/6/api/javax/validation/constraints/NotNull.html

It looks like there should be no problem; it's legal everywhere we need it to be legal. When I check the javaEE 5 docs, there is no javax.validation at all, so it seems to be new in javaEE 6.

What error message are you getting? What version are you on? The same caveat applies: We can hardcode an exception but before we do so we need to know what's going on.

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 18, 2011 at 07:33 UTC

Attached a small example, error can be seen with

mvn compile

gives

Test.java:[8,1] annotation type not applicable to this kind of declaration

Appears to me that Lombok should not be getting involved but some error in processing is causing @ NotNull to be processed.

Thanks

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 18, 2011 at 07:33 UTC

🔗 Lombok-Bug.zip

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 19, 2011 at 07:11 UTC

As i'm not including Lombok's @ NotNull via an import statement I would assume it shoouldn't be processing the @ NotNull.

That assumption correct ?

@lombokissues
Copy link
Author

👤 pe.fips   🕗 Oct 19, 2011 at 20:55 UTC

Any annotation that matches the pattern ^(?:notnull|nonnull)$ will be processed by @ Data.

@lombokissues
Copy link
Author

👤 pe.fips   🕗 Oct 19, 2011 at 21:22 UTC

Issue #364 has been merged into this issue.

@lombokissues
Copy link
Author

👤 pe.fips   🕗 Oct 19, 2011 at 21:29 UTC

Only affects javac and works if the @ NotNull annotation is applicable to local variable declarations(so ElementType.LOCAL_VARIABLE), which I find particular strange.

@lombokissues lombokissues added accepted The issue/enhancement is valid, sensible, and explained in sufficient detail javac labels Jul 14, 2015
@lombokissues
Copy link
Author

👤 pe.fips   🕗 Oct 19, 2011 at 21:47 UTC

As for the Spring @ NotNull annotation, it is only allowed on fields and methods(so ElementType.METHOD, ElementType.FIELD). This means it shouldn't qualify for an annotation @ Data should process.

The question is how should we handle these cases?

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 20, 2011 at 07:45 UTC

To me it would seem dangerous to process annotations without an explicit call to use it. ie. an import statement.

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 21, 2011 at 07:35 UTC

Perhaps the change should be backed out until a solution is available. We are having numerous issues with eclipse and would like to narrow down any issues and get the change to remove the memory leak with @ Delegate. Though I don't really know if this is causing us any real impact.

@lombokissues
Copy link
Author

👤 dsuepke   🕗 Oct 24, 2011 at 08:22 UTC

Getting the same problem with javax.validation.constraints.NotNull, making it necessary to fall back to the previous version of Lombok. A quick fix would be greatly appreciated.

@lombokissues
Copy link
Author

👤 reinierz   🕗 Oct 25, 2011 at 13:02 UTC

We've stumbled on this issue before, and at that time I 'fixed' it by just not treating @ NotNull (Any kind of @ NotNull, from any package) special. I forgot to update the documentation and I also didn't document the code. Philipp recently noticed the discrepancy in the docs, and added it back in.

Seems like this is still the best option as we've never actually heard any complaints about not having @ NotNull trigger lombok's usually non-null processing.

We'll push a new version out the door with the fix soon. That would be version 0.10.2

Fixed in commit 782daa4 (and this time I did update the docs and add a comment explaining why NotNull isn't in the list).

@lombokissues lombokissues removed the accepted The issue/enhancement is valid, sensible, and explained in sufficient detail label Jul 14, 2015
@lombokissues lombokissues added this to the 0.10.2 milestone Jul 14, 2015
@lombokissues
Copy link
Author

👤 dsuepke   🕗 Oct 25, 2011 at 17:32 UTC

Great, many thanks :)

@lombokissues
Copy link
Author

👤 graham.berks   🕗 Oct 25, 2011 at 17:59 UTC

Fab, thanks

@lombokissues
Copy link
Author

👤 pe.fips   🕗 Oct 26, 2011 at 06:48 UTC

Issue #365 has been merged into this issue.

@lombokissues
Copy link
Author

End of migration

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

No branches or pull requests

1 participant