Daily Archives: 06/04/2010

Best practices for FindBugs in new projects

I have been testing FindBugs effectiveness and ease of use in an existing project, DirectJNgine, a small 7000 lines project. With this experience in hand, I have arrived to a small set of preliminary best practices and recommendations for new projects.

Turn all settings to the max

  • Set Analysis effort to Maximal.
  • Check all detectors, except those with the “TEST” pattern.
  • Set Minimum priority to report to Low.
  • Check all reported bug categories, except Bogus random noise.

Tip: only deactivate FindBugs real-time analysis if it noticeably slows down your development (i.e., if it breaks your train of thought, it is bad; if it is just a little nuisance you will probably survive). If you do, make sure you run the analysis manually often.

Never commit source code if there is any warning

This is so important I’ll repeat it:

  • Do not commit source code if there is any outstanding FindBugs warning.

Always have an automated check process for FindBugs

  • Make running FindBugs part of the “pre commit to version control system” process.
  • Make running FindBugs part of your Continuous Integration system. Make it fail if there is any warning.

Use the Nullability Annotations extensively

  • @NonNull: mark parameters, method return values and fields that must not be null (for fields, we refer to being null after construction).
  • @CheckForNull: mark parameters, method return values and fields that might be null.
  • Set the following default annotations for packages (use package-info.java):
  • @DefaultAnnotationForParameters(value=CheckForNull.class)
  • @DefaultAnnotationForFields(value=CheckForNull.class)
  • @DefaultAnnotationForMethods(value=NonNull.class)

Use the Resource Management Annotations

  • @CleanupObligation: mark resource manager classes.
  • @CreateObligation: mark the constructor(s) that acquire the resource(s).
  • @DischargesObligation: mark the method(s) that release the resource(s).

Use the @CheckReturnValue annotation

Annotate those methods that:

  • Return diagnostic info or other data that should never be ignored by the caller.
  • Belong to immutable classes, and return a new object that is the result of applying an operation that would otherwise end up mutating the original object.

Handle warning supression with care

  • Warnings are your best friends: do not disable them unless you really must.
  • If you have to disable a certain kind of warning, do that globally: never use the @SupressWarnings annotation.
  • If you have to use the @SuppressWarnings annotation, check twice that you are 100% sure that’s what you really need.
  • If you use SuppressWarnings, always write the reason in the justification attribute. If you can’t write it in a way that makes you feel good, then you should probably not supress the warning.

Recommended warning supression

Warnings to supress globally on a per-project basis:

  • InefficientMemberAccess.

Other issues

  • Use the FindBugs plugin for your current IDE.
  • If you use the Eclipse plugin, take into account that it will not issue some kinds of warning just because you save a file: you will want to perform global checks periodically.