False-positive report of "Optional value should only be accessed after calling isPresent()" in logical "&&" clause

Clean Code Report False-positive / False-negative...
, ,
#1

In the following code, the warning “Optional value should only be accessed after calling isPresent()” is shown on the statement “pde.getResource().get()”, however “pde.getResource().isPresent()” was already called in the left-hand clause of the “&&”.

private static String prohibited(AuthException e, String alreadyDisplayedResource) {
String msg = e.getMessage();
if (e instanceof PermissionDeniedException) {
  PermissionDeniedException pde = (PermissionDeniedException) e;
  if (pde.getResource().isPresent()
      && pde.getResource().get().equals(alreadyDisplayedResource)) {
    // Avoid repeating resource name if exactly the given name was already displayed by the
    // generic git push machinery.
    msg = PermissionDeniedException.MESSAGE_PREFIX + pde.describePermission();
  }
}
return "prohibited by Gerrit: " + msg;

}

Link to the code on googlesource.com

#2

Hi,

The analyzer can’t be sure pde.getResource() will always return the same object (it may be the result of a computation, or modified concurrently by another thread). To avoid the issue, you should extract the method call in a local variable:

private static String prohibited(AuthException e, String alreadyDisplayedResource) {
String msg = e.getMessage();
if (e instanceof PermissionDeniedException) {
  PermissionDeniedException pde = (PermissionDeniedException) e;
  ResourceType resourceOpt = pde.getResource();
  if (resourceOpt.isPresent()
      && resourceOpt.get().equals(alreadyDisplayedResource)) {
    // Avoid repeating resource name if exactly the given name was already displayed by the
    // generic git push machinery.
    msg = PermissionDeniedException.MESSAGE_PREFIX + pde.describePermission();
  }
}
return "prohibited by Gerrit: " + msg;
#3

Thanks. That makes sense.

I’ll adjust the code as suggested.

1 Like