nhatminhle / cofoja

Contracts for Java
GNU Lesser General Public License v3.0
151 stars 18 forks source link

Annotation declarations break compilation #12

Closed nhatminhle closed 9 years ago

nhatminhle commented 9 years ago

From davidmor...@google.com on November 12, 2010 05:56:37

What steps will reproduce the problem? 1. Add an annotation declaration to a contracted class

  1. Compile with contract compilation
  2. Observe NullPointerException at compile time

This declaration should be enough:

@Retention(RetentionPolicy.RUNTIME) @Target({ElementType.PARAMETER, ElementType.METHOD}) @BindingAnnotation public @interface MyAnnotation { }

And the resulting stacktrace at compile time:

java.lang.NullPointerException at com.google.java.contract.core.model.ElementModel.fixEnclosedElement(ElementModel.java:170) at com.google.java.contract.core.model.ElementModel.addEnclosedElement(ElementModel.java:165) at com.google.java.contract.core.apt.TypeFactory$TypeBuilder.visitType(TypeFactory.java:416) at com.google.java.contract.core.apt.TypeFactory$TypeBuilder.visitType(TypeFactory.java:310)

Original issue: http://code.google.com/p/cofoja/issues/detail?id=5

nhatminhle commented 9 years ago

From nhat.min...@gmail.com on November 12, 2010 17:06:56

Diagnostic:

The contracted stacktrace gives away that this is because TypeFactory does not support reflection of annotation types correctly:

com.google.java.contract.PostconditionError: type != null at com.google.java.contract.core.apt.TypeFactory$TypeBuilder.com$google$java$contract$QH$com$google$java$contract$core$apt$TypeFactory$TypeBuilder$visitType(TypeFactory.java) at com.google.java.contract.core.apt.TypeFactory$TypeBuilder.visitType(TypeFactory.java:443) at com.google.java.contract.core.apt.TypeFactory$TypeBuilder.visitType(TypeFactory.java:313) at com.sun.tools.javac.code.Symbol$ClassSymbol.accept(Symbol.java:862) at com.google.java.contract.core.apt.TypeFactory.createType(TypeFactory.java:770) at com.google.java.contract.core.apt.AnnotationProcessor.createTypes(AnnotationProcessor.java:502) at com.google.java.contract.core.apt.AnnotationProcessor.process(AnnotationProcessor.java:163)

The fact that annotation types cannot be contracted is well known, but the current code also breaks if there are nested annotation types as well.

Suggested fix:

I suggest that we warn the user and ignore the contracts, if contracts are put on annotation types.

Rationale:

Support for annotation types would be, as is already the case for enums, another ad hoc hack, because annotation interfaces are "special" interfaces (what that means is that the reflection given by the API is kind of misleading and can't be output as is using the generic printing code). And the added value is close to zero because these contracts are not easily (if at all) enforcable: the only case where any of these methods gets called is when used reflectively to access annotations on some class (which is uncommon to say the least), yet it is the JVM that manufactures the actual classes implementing these pseudo interfaces. And AFAIK, it may or may not actually compile classes for these and remember that unless we instrument the actual class of the object, no contract is ever enforced. Plus, a contract error at that point would not mean an error in the implementation (which is provided by the JVM and not alterable by any means) but in the class being reflected upon, which is probably not something we want to throw an exception for... All in all, run-time contracts on annotations are not easy to implement AND not something we actually want.

Status: Accepted
Cc: andreasl...@google.com

nhatminhle commented 9 years ago

From andreasl...@google.com on February 06, 2011 09:20:30

LGTM

nhatminhle commented 9 years ago

From chat...@google.com on April 11, 2011 04:11:28

Fixed in r108

Status: Fixed