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

Unable to compile @NotNull when applied to a field #116

Closed
lombokissues opened this Issue Jul 14, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@lombokissues
Collaborator

lombokissues commented Jul 14, 2015

Migrated from Google Code (issue 43)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 voidstar   🕗 Sep 02, 2009 at 01:35 UTC

What steps will reproduce the problem?

  1. Put an @ NotNull on a field and watch javac choke.

Please provide any additional information below.
In my case I'm using an older version of hibernate validator with it's
@ org.hibernate.validator.NotNull annotation. The newest version uses @ javax.validation.NotNull
which from what I can tell will have the same issue.

See this thread:
http://groups.google.com/group/project-lombok/browse_thread/thread/572eb913e1a86eea

Collaborator

lombokissues commented Jul 14, 2015

👤 voidstar   🕗 Sep 02, 2009 at 01:35 UTC

What steps will reproduce the problem?

  1. Put an @ NotNull on a field and watch javac choke.

Please provide any additional information below.
In my case I'm using an older version of hibernate validator with it's
@ org.hibernate.validator.NotNull annotation. The newest version uses @ javax.validation.NotNull
which from what I can tell will have the same issue.

See this thread:
http://groups.google.com/group/project-lombok/browse_thread/thread/572eb913e1a86eea

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Sep 02, 2009 at 02:04 UTC

Well, it'll only choke on that particular variant of @ NotNull (javax.validation.NotNull). But, show-stopper. We'll fix
this ASAP.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Sep 02, 2009 at 02:04 UTC

Well, it'll only choke on that particular variant of @ NotNull (javax.validation.NotNull). But, show-stopper. We'll fix
this ASAP.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Sep 02, 2009 at 08:27 UTC

To solve it right, we need to implement the bindings so we can tell what exact
NotNull is being used and maybe even check where it can be applied.

For now, I'm considering to NOT copy the NotNull annotation to the parameter because,
for as far as I know, the only other NotNull annotation in the wild is the one from
IDEA, and currently we don't support IntelliJ anyway. And not having the annotation
copied won't break anything.

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Sep 02, 2009 at 08:27 UTC

To solve it right, we need to implement the bindings so we can tell what exact
NotNull is being used and maybe even check where it can be applied.

For now, I'm considering to NOT copy the NotNull annotation to the parameter because,
for as far as I know, the only other NotNull annotation in the wild is the one from
IDEA, and currently we don't support IntelliJ anyway. And not having the annotation
copied won't break anything.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Sep 02, 2009 at 08:31 UTC

Yup. For safety's and consistency's sake, we should probably stop copying the annotations everywhere (so, also
for the getter), and not just for the parameters of the setter and the constructor.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Sep 02, 2009 at 08:31 UTC

Yup. For safety's and consistency's sake, we should probably stop copying the annotations everywhere (so, also
for the getter), and not just for the parameters of the setter and the constructor.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 jacek99   🕗 Sep 02, 2009 at 08:59 UTC

Wouldn't it be better to have some generic logic that would look at how an annotation
is defined and where it can be applied and use that in a common fashion to decide if
annotations should be copied around?

The same issue may pop up on other annotations that Lombok may have to deal with in
the future.

Collaborator

lombokissues commented Jul 14, 2015

👤 jacek99   🕗 Sep 02, 2009 at 08:59 UTC

Wouldn't it be better to have some generic logic that would look at how an annotation
is defined and where it can be applied and use that in a common fashion to decide if
annotations should be copied around?

The same issue may pop up on other annotations that Lombok may have to deal with in
the future.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Sep 02, 2009 at 09:18 UTC

jacek99: Well, what kind of 'generic' logic are you referring to? If you're referring to: Check the target
ElementType and copy only if its legal - well, lombok can't do that - no type info. That's what Roel meant in
comment ﹟2 with "we need to implement the bindings so we can tell". It's planned - the suggested 'let's not
copy anything' is a short-term workaround.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Sep 02, 2009 at 09:18 UTC

jacek99: Well, what kind of 'generic' logic are you referring to? If you're referring to: Check the target
ElementType and copy only if its legal - well, lombok can't do that - no type info. That's what Roel meant in
comment ﹟2 with "we need to implement the bindings so we can tell". It's planned - the suggested 'let's not
copy anything' is a short-term workaround.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Sep 02, 2009 at 23:41 UTC

Fixed in c46bf85 which will be rolled out in v0.8.5.

The "fix" simply reduces the feature to scan only for annotations named 'nonnull'. All annotations named nonnull
work the way we want them to (e.g. legal on parameters). The only commonly used notnull that ISNT a variant of
javax.validation.NotNull is IDEA's, but lombok doesn't support IDEA.

This is a workaround - once lombok supports type introspection we'll fix this properly.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Sep 02, 2009 at 23:41 UTC

Fixed in c46bf85 which will be rolled out in v0.8.5.

The "fix" simply reduces the feature to scan only for annotations named 'nonnull'. All annotations named nonnull
work the way we want them to (e.g. legal on parameters). The only commonly used notnull that ISNT a variant of
javax.validation.NotNull is IDEA's, but lombok doesn't support IDEA.

This is a workaround - once lombok supports type introspection we'll fix this properly.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 matt.deboer   🕗 Oct 17, 2011 at 19:48 UTC

This doesn't seem to be resolved (or has regressed?); using Lombok 0.10.1 with a maven 3.0.3 build, having a simple class with a single member which has @ javax.validation.constraints.NotNull applied causes compiler plugin to bomb with the aforementioned "annotation type not applicable to this kind of declaration".. this works correctly within Eclipse environment, but fails during Maven build.
(Compiler plugin is configured to use compilerVersion,source,target all = 1.6)

Collaborator

lombokissues commented Jul 14, 2015

👤 matt.deboer   🕗 Oct 17, 2011 at 19:48 UTC

This doesn't seem to be resolved (or has regressed?); using Lombok 0.10.1 with a maven 3.0.3 build, having a simple class with a single member which has @ javax.validation.constraints.NotNull applied causes compiler plugin to bomb with the aforementioned "annotation type not applicable to this kind of declaration".. this works correctly within Eclipse environment, but fails during Maven build.
(Compiler plugin is configured to use compilerVersion,source,target all = 1.6)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

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

It's a regression introduced in 0.10.1. See issue #360 for more up to date discussion.

Collaborator

lombokissues commented Jul 14, 2015

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

It's a regression introduced in 0.10.1. See issue #360 for more up to date discussion.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

End of migration

Collaborator

lombokissues commented Jul 14, 2015

End of migration

Tradunsky pushed a commit to Tradunsky/lombok that referenced this issue Aug 19, 2015

Michail Plushnikov
Issue #116
added handling of @builder annotation on the constructor/method and not on the class file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment