2

I have a method which is removing the trim box from a pdf file. Given a path argument to the method I annotate it with a JetBrains' "@NotNull" annotation:

Path removeTrimBox(@NotNull Path pdfPath) {
 if (!Files.exists(pdfPath)) {
  throw new IllegalArgumentException(myErrorMessage);
 }
 // code removing the trim box follows
}

I also have a unit test for this method which expects an IllegalArgumentException:

@Test(expected = IllegalArgumentException.class)
public void removeTrimboxNull() throws Exception {
    PdfTools.removeTrimbox(null);
}

Though my checkstyle complains about passing null argument to paramater annotated with NotNull the test runs successfully. The problem is after I push the code to the bamboo server the build runs red with the message: How should I 'teach' my bamboo server to understand this JetBrains annotation?

java.lang.Exception: Unexpected exception, expected "java.lang.IllegalArgumentException" but was "java.lang.NullPointerException".

Should I 'teach' my bamboo server to understand this JetBrains annotation?

EDIT

Here is the pom:

<dependency>
  <groupId>org.jetbrains</groupId>
  <artifactId>annotations</artifactId>
  <version>13.0</version>
</dependency>

EDIT2

Using Maven's 'test' cycle to run the test causes no NPE, while bamboo is still complaining. I use Intellij IDEA 14.1.4 and maven 3.2.3.

Arthur Eirich
  • 3,368
  • 9
  • 31
  • 63
  • Have you specified dependency for jar that contains this annotation into your pom? – mvb13 Aug 25 '15 at 08:19
  • [This link](http://stackoverflow.com/questions/29497755/intellij-idea-complains-about-null-check-for-notnull-parameter/29500102#29500102) says that runtime assertions are added to the compiled code. Does it then mean that Bamboo does not add these assertions and therefore can't deal with this annotation? – Arthur Eirich Aug 27 '15 at 11:35

2 Answers2

1

From the API doc of @NotNull:

Apart from documentation purposes this annotation is intended to be used by static analysis tools to validate against probable runtime errors and element contract violations.

This looks like @NotNull is not intended to throw an IllegalArgumentException at runtime. That would mean you have to check yourself if the argument is null and throw an IllegalArgumentException if this is the case.

In which environment do you run Maven's 'test' cycle? Inside IntelliJ Idea? Maybe the @NotNull performs its "black magic" only within your IDE?

Maybe you could use AspectJ to create an aspect that checks for null, that also works in environment other than Idea? The following code is inspired by http://twest-log.blogspot.de/2011/07/merkzettel-notnull-check-mit-aspectj.html but since there the comments are in German, I amended it a bit. Go there for a more complete example:

@Aspect
public class CheckArgumentsAspect {

  /**
   * Pointcut for all methods with at least one parameter
   * with the {@link NotNull} annotation.
   */
  @Around("execution(* *(..,@NotNull (*),..))")
  public Object checkArgsForMethod(ProceedingJoinPoint joinPoint)
      throws Throwable {
    if (joinPoint.getSignature() instanceof MethodSignature) {
      MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature();
      Method method = methodSignature.getMethod();
      Class<?>[] parameterTypes = method.getParameterTypes();
      Annotation[][] parameterAnnotationArray = method.getParameterAnnotations();

      checkParameters(joinPoint.getArgs(), parameterTypes, parameterAnnotationArray, methodSignature.toLongString());
    }
    return joinPoint.proceed();
  }

  private void checkParameters(Object[] parameter, Class<?>[] parameterTypes, Annotation[][] parameterAnnotationArray, String signature) {
    for (int i = 0; i < parameterTypes.length; i++) {
      Annotation[] parameterAnnotations = parameterAnnotationArray[i];
      for (Annotation annotation : parameterAnnotations) {
        if (annotation instanceof NotNull) {
          checkNotNull(parameter[i], parameterTypes[i], i, ((NotNull) annotation).parameterName(), signature);
        }
      }
    }
  }

  private void checkNotNull(Object parameter, Class<?> parameterType, int parameterIndex, String parameterName, String signature) {
    if (parameter == null) {
      if (StringUtils.isBlank(parameterName)) {
        parameterName = "-";
      }
      String longMsg = MessageFormat.format(
          "Error: parameter no.{0} (name: {1}, type: {2}) of {3} is null", parameterIndex + 1, parameterName, parameterType.getName(), signature);
      throw new IllegalArgumentException(longMsg);
    }
  }
}
Lars Gendner
  • 1,816
  • 2
  • 14
  • 24
  • Well, if I have to check the argument for being null, I then do not see any benefit of this annotation. I actually thought that I wouldn't need to do the null check havin my argument annotated. Both annotation and null check is a bit redundancy for me. I actually put back the null check and removed the annotation as bamboo understands only this version. Thanks anyway for your answer. – Arthur Eirich Aug 28 '15 at 07:28
  • The benefit of the annotation is that it provides declarative information about whether an API expects `null` values or not. In large systems/frameworks this can save your... time. It can be totally reasonable that a method parameter annotated with `@NotNull` triggers a `NullPointerException`. But the information that null is not expected is really useful, to compilers and fellow programmers. I remember situations where I had to dig into the code just to figure out: is it acceptable that a certain parameter is null in some method call. – Lars Gendner Aug 28 '15 at 09:14
0

The @NotNull annotation is just a marker and it wouldn't reflect in the Runtime. Developers are supposed to handle the NullPointerException and provide necessary null checks.

It would be really cumbersome and time consuming for you to write any AspectJ code for this purpose, which of course doesn't help you unless you again write a logic to check if the variable is null.

Many people confuse the @NotNull annotation to it's non-existing run time behavior. As every other developer, handle the NPE manually :)

Karthik R
  • 5,523
  • 2
  • 18
  • 30