False Positive in Java S1193 - "Don't use instanceof in catch blocks"

Summary: The rule should have an exception to prevent it triggering if the type on the RHS of the instanceof operator is an interface.

  • Plugin for IntelliJ: SonarLint 4.10.0.19739.
  • IntelliJ itself:
    IntelliJ IDEA 2020.2 (Community Edition)
    Build #IC-202.6397.94, built on July 27, 2020
    Runtime version: 11.0.7+10-b944.20 amd64
    VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.

It is legal for exceptions to implement interfaces in Java. It is not legal to have an interface extend java.lang.Throwable because it is a class.

If one has multiple unrelated exceptions that all implement the same interface (for whatever purpose - I discovered this whilst adding an interface for mapping from exceptions to a framework’s error reporting system) then you have no choice but to either enumerate every implementer of the interface that is also a throwable in the catch clause (ie: catch (ExceptionWithInterface1 | ExceptionWithInterface2 | ...) which is awful, or write

catch (SuitableExceptionType e) {
  if (e instanceof MyInterface) {
    ((MyInterface) e).someMethod();
  }
}

which triggers Java S1193. The rule should have an exception if the type on the RHS of the instanceof operator is an interface.

Here’s some code that will trigger it:

interface FrameworkErrorProvider {
  Collection<String> getFrameworkErrorStrings();
}

class SpecialisedIAE extends IllegalArgumentException implements FrameworkErrorProvider {
   @Override
   public Collection<String> getFrameworkErrorStrings() {
     // imagine an actually useful implementation here
     return Collections.emptyList();
  }
}

class BusinessException extends RuntimeException implements FrameworkErrorProvider {
   @Override
   public Collection<String> getFrameworkErrorStrings() {
     return Collections.emptyList();
  }
}

class AClass {
  public static void aMethod() {
     try {
       // ... some code that could throw some kind of exception
     } catch (RuntimeException e) {
       // false positive on the next code line - there is no sane way to write catch blocks to handle this
        if (e instanceof FrameworkErrorProvider) {
          ((FrameworkErrorProvider) e).getFrameworkErrorStrings();
        }
     }
  }
}

Hi S. Haywood,
I agree the rule should only target sub-types of Throwable.
Ticket created: SONARJAVA-3498

Thanks for your feedback,
Alban