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

Regression: forbidden-apis 1.5 fails on non-runtime annotations (e.g. java.lang.Synthetic) which are not in classpath #27

Closed
GoogleCodeExporter opened this issue Mar 14, 2015 · 9 comments

Comments

@GoogleCodeExporter
Copy link

Simon Willnauer tried to use forbidden 1.5 with ElasticSearch. Sometimes the 
compiler generates "virtual" annotations (java.lang.Synthetic) or annotations 
that are not visible at runtime. Those annotations should not be checked, 
because they might not exist on classpath (e.g. Guava's @NotNull).

This has to be fixed and 1.5.1 needs to be released ASAP.

Original issue reported on code.google.com by uwe.h.schindler on 17 Apr 2014 at 1:02

@GoogleCodeExporter
Copy link
Author

Original comment by uwe.h.schindler on 17 Apr 2014 at 1:03

  • Added labels: Priority-Critical
  • Removed labels: Priority-Medium

@GoogleCodeExporter
Copy link
Author

As a quick fix, we should ignore all RetentionPolicy.CLASS annotations... It 
also makes not much sense to look at those.

This would limit Forbidden APIS to only check for RetentionPolicy.RUNTIME 
annotations.

Original comment by uwe.h.schindler on 17 Apr 2014 at 1:17

@GoogleCodeExporter
Copy link
Author

Patch that disables non-runtime visible annotations.

Original comment by uwe.h.schindler on 17 Apr 2014 at 1:25

Attachments:

@GoogleCodeExporter
Copy link
Author

this is a good approach. For a build, i'd rather it be totally consistent here 
with this simple fix for now.

Original comment by [email protected] on 17 Apr 2014 at 1:59

@GoogleCodeExporter
Copy link
Author

A good fix that inspects RetentionPolicy.CLASS annotations, which ignores 
automatically non-lookup-able annotations needs more work, because you have to 
pass the type down through checkDescriptor, which is not only used for 
Annotations. So its needs more refactoring.

I think, Iwill delay this for version 1.6 of forbidden as a new "feature".

Original comment by uwe.h.schindler on 17 Apr 2014 at 2:05

@GoogleCodeExporter
Copy link
Author

Original comment by uwe.h.schindler on 17 Apr 2014 at 2:06

  • Changed title: Regression: forbidden-apis 1.5 fails on non-runtime annotations (e.g. java.lang.Synthetic) which are not in classpath

@GoogleCodeExporter
Copy link
Author

Improved patch with better reporting for line-number less code parts.

This also adds a test class that had the problem with java.lang.Synthetic:
The ctor of the non-static inner class gets an additional parameter to pass in 
the outer class' instance to have access to OuterClass.this. This parameter 
gets the magic annotation if there are other annotated parameters.

Original comment by uwe.h.schindler on 17 Apr 2014 at 6:55

Attachments:

@GoogleCodeExporter
Copy link
Author

Committed to trunk, will backport now.

Original comment by uwe.h.schindler on 17 Apr 2014 at 8:38

  • Changed state: Committed

@GoogleCodeExporter
Copy link
Author

Backported to 1.5 branch and released hotfix.

Original comment by uwe.h.schindler on 17 Apr 2014 at 9:08

  • Changed state: Fixed

uschindler added a commit that referenced this issue Mar 14, 2015
….g. java.lang.Synthetic) which are not in classpath
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant