java:S1130 reports on overriden method outside of source control

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

java:S1130 reports on overriden method when super method outside of source control has superfluous Exception declarations

  • What language is this for?
    Java
  • Which rule?
    Java:S1130
  • Why do you believe it’s a false-positive/false-negative?
  • Are you using
    • SonarQube Cloud?
    • SonarQube Server / Community Build - which version?
    • SonarQube for IDE - which IDE/version?
      • in connected mode with SonarQube Server / Community Build or SonarQube Cloud?
        SonarLint for IntelliJ 10.12.0.79769
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)

When extending a class from a library, it is (at least in my view) good practice to keep the throws declarations equal to those of the super method. However, when the super method has superfluous throws declarations (like StdDeserializer.deserialize from Jackson), this leads to java:S1130 reporting:

public class SanitizedStringDeserializer extends StdDeserializer<String> {

    public SanitizedStringDeserializer() {
        super(String.class);
    }

    @Override
    public String deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException, JacksonException {
        var originalText = jsonParser.getText();
        var sanitized = stripHtml(originalText);
        return trimToEmpty(sanitized);
    }

}

Removing the superfluous throws declaration of JacksonException (which directly extends IOException) is acceptable for the compiler, but in my opinion not desirable because the visible signature of the method is different from that of the super method. When the super method is outside of source control (in this case a library) in my opinion this rule should not trigger.

I’ll admit I’m not 100% sure whether this should be a false positive or it should be in Discussions. Please move this report if it’s better suited else where.

4

Hi Jeff,
In my opinion, a method that will never throw a specific exception should not have it in its signature.
Let’s say that I have the following 3 classes:

class Stick {
  public void applyPressure(float pressure) throws BreaksException { /*...*/ }
}
class AdamantiumStick extends Stick{
  @Override
  public void applyPressure(float pressure) {/* No matter what, this cant break*/}
}

public class Dog {
  public void biteStick(Stick stick){
    try{
      stick.applyPressure(10);
    } catch (BreaksException e) {
      System.out.println("The stick broke");
    }
  }
  
  public void biteStick(AdamantiumStick stick){
    stick.applyPressure(10);
  }
}

In this example, I do not want to pollute the AdamantiumStick method with an unthrowable exception, as it is not necessary and also misleading for whoever will have to deal with that object. So, I would not say this is an FP, as to me, the exceptions in a signature should tell the user of that method what could go wrong when invoking the method itself.

5

I don’t disagree with that, but…

I also don’t think any overridden method should deviate from its super method. The compiler agrees to an extent (IIRC, the compiler finds the throws declaration has to be compatible with that of the super method). In my opinion sticking to the exact same visual signature (so exact throws declaration) of the super method has added value.

In my example, the super method is from a library - It’s outside of the source control of the project under analysis. If it were under source control, I would happily remove the superfluous throws declaration from the super method. In the given example, I use StdDeserializer from Jackson; which is not under my source control.

I could of course go the open source way and have the issue fixed in the used library, but that is not always feasible obviously.

So my proposition here is to not trigger this rule, when overriding (directly or indirectly) a method outside of source control, when the super method has the superfluous throws declaration.