piątek, 24 marca 2023

Eliminate dreadful NPEs with NullAway

What is NullAway

Static code analysis has always been a little controversial in the Java development community. As with almost each and every tool, it can be used for good (to improve code quality) or bad (infuriate engineers with picky rules). 

Null Away (https://github.com/uber/NullAway) is however different. There are no rules to configure, no code styles to discuss. Just one plugin that will help eliminate NPEs in your code by reviewing all code execution paths.


Configuration


For Gradle

https://github.com/uber/NullAway#gradle


For Maven: 



<plugin>

        <groupId>org.apache.maven.plugins</groupId>

        <artifactId>maven-compiler-plugin</artifactId>

        <configuration>

          <source>${maven.compiler.source}</source>

          <target>${maven.compiler.target}</target>

          <fork>true</fork>

          <compilerArgs>

            <arg>-XDcompilePolicy=simple</arg>

            <arg>-Xplugin:ErrorProne -XepAllErrorsAsWarnings -XepExcludedPaths:.*/src/test/java/.* -Xep:NullAway:ERROR -XepOpt:NullAway:TreatGeneratedAsUnannotated=true -XepOpt:NullAway:ExcludedClasses=sf.cloudmetricsyncer.CloudProvider -XepOpt:NullAway:AnnotatedPackages=sf.cloudmetricsyncer,sf.externalmonitor</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>

            <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>

            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>

            <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>

          </compilerArgs>

          <annotationProcessorPaths>

            <path>

              <groupId>com.google.errorprone</groupId>

              <artifactId>error_prone_core</artifactId>

              <version>2.16</version>

            </path>

            <path>

              <groupId>com.uber.nullaway</groupId>

              <artifactId>nullaway</artifactId>

              <version>0.10.5</version>

            </path>

          </annotationProcessorPaths>

        </configuration>

      </plugin>


 

How to


For a new project it’s easy - just enable the plugin and fix the build each time it is required. For old code, especially with a large codebase, it might be trickier. NullAway plugin is not reporting all of the violations on the first run - only a subset, making the whole iterative process longer and more painful. 


Here’s a proposed approach I’ve successfully applied to a number of projects:


  1. Start with service boundaries - API, DB data model. Review which fields definitely need to be nullable. If possible implement appropriate validation to reject illegal NULLs or apply defaults.

  2. If you have a good modularization of the code - split work by sub-package, using configuration options “-XepOpt:NullAway:AnnotatedPackages=XYZ” and “-XepOpt:NullAway:UnannotatedSubPackages=ABC”

  3. Run the build and review  “returning @Nullable expression from method with @NonNull return type” messages. Consider if you really need to return NULLs in such cases. Perhaps a default value would be better.

  4. Review “initializer method does not guarantee @NonNull field XXX is initialized along all control-flow paths (remember to check for exceptions or early returns).” messages. Perhaps an instance of the class is initialized in a different way (guaranteed to be called after construction but before the first use)? If so, add a pre-configured (plugin configuration) initialization annotation to the method. This is also a great moment to think about making the data model (or parts of the model) immutable without any NULLs.

  5. Review messages pertaining the class hierarchy (super / subclass) 

    1. parameter X is @NonNull, but parameter in superclass method ABC#abc) is @Nullable"

    2. method returns @Nullable, but superclass method ABC#abc returns @NonNull

  6. Review “passing @Nullable parameter 'null' where @NonNull is required” - do you need to pass an explicit null? Perhaps a default (empty) value would be better?

  7. Review “passing @Nullable parameter XXX where @NonNull is required”. This often means that a data model class properties are nullable. Perhaps the model entity can be turned into immutable one, getting rid of NULLs there entirely?




Tips


Initialisation annotation

In some cases instance is initialized before the first use in some other way than via a constructor. This may happen in a typical framework implementing some kind of an instance lifecycle. In order to notify NullAway that fields initialization will happen in such method, one can use the initialization annotation:  https://github.com/uber/NullAway/wiki/Supported-Annotations#initialization 


NullSafeMap

Map get() is treated as inherently unsafe (nullable). In many cases however a programmer knows that map will contain mapping for a particular key. In order to use NullAway efficiently in such cases it’s a good idea to have an utility (eg NullSafeMap) with static nullSafeGet(Map, key) method annotated with @SuppressWarnings("NullAway") 


Use standards 

Out of possible annotations, I recommend: javax.annotation.Nullable. Keep it… standardized ;-)


Local variables

It’s not possible to suppress a check for a local variable - in such cases it’s best to extract a separate method (one liner) out of expression where the variable is used and annotate the method.


Generated code

One can easily ignore generated code (since they can’t do much about it) using: -XepOpt:NullAway:TreatGeneratedAsUnannotated=true 


Last resort - ignoring particular classes

Exclude some classes (only if really needed) using:  -XepOpt:NullAway:ExcludedClasses=XYZ



Now go out there, have fun coding and hunt down some nasty NullPointerExceptions! ;-)


Brak komentarzy: