SonarLint plugin gives advice that results in binary incompatibilities

Please provide

  • Operating system: Any
  • SonarLint plugin version: Any, currently using latest (and latest IntelliJ)
  • Programming language you’re coding in: Java
  • Is connected mode used:
    • Connected to SonarCloud or SonarQube (and which version): N/A

And a thorough description of the problem / question:

had a class with a nested builder that originally had no parent class:

public class SomeClass {

public static class Builder {

...

public Builder name(String name) {

return this;

}

...

}

}

I needed to add a parent class, so I used the curiously-recurrring template pattern (Curiously recurring template pattern - Wikipedia):

public class SomeClassBase {

public static class BuilderBase<B extends BuilderBase<B>> {

...

public B name(String name) {

...

return (B) this;

}

...

}

}

public class SomeClass {

public static class Builder extends BuilderBase<Builder> {

...

@Override

public Builder name(String name) {

return super.name(name);

}

...

}

}

Sonar said the “name” method in the subclass could be removed:

Remove this method to simply inherit it.

Overriding methods should do more than simply call the same method in the super class

java:S1185

Unfortunately, that’s really bad advice, because when the method was removed, code already compiled against the original SomeClass no longer worked at runtime, blowing up with a missing-method exception.

The runtime was trying to match the original signature, which was no longer there.

Sonar should warn when its advice could cause a binary incompatibility, or not give advice that can lead to a binary incompatibility, or offer a configuration option to disable advice that could lead to a binary incompatibility.

Hey there.

It looks like your code formatting isn’t quite right. Can you please edit your post and paste in your entire code snippets, then format using the preformatted text button in the editor? It’s in between the quotation and the upload button.

I don’t see a way to edit the original, so here it is again:

Please provide

  • Operating system: Any
  • SonarLint plugin version: Any, currently using latest (and latest IntelliJ)
  • Programming language you’re coding in: Java
  • Is connected mode used:
    • Connected to SonarCloud or SonarQube (and which version): N/A

And a thorough description of the problem / question:

We had a class with a nested builder that originally had no parent class:

public class SomeClass {

  public static class Builder {
    ...
    public Builder name(String name) {
      return this;
    }
    ...
  }
}

We needed to add a parent class, so I used the curiously-recurrring template pattern (Curiously recurring template pattern - Wikipedia):

public class SomeClassBase {

  public static class BuilderBase<B extends BuilderBase<B>> {
    ...
    public B name(String name) {
    ...
      return (B) this;
    }
    ...
  }
}

public class SomeClass {

  public static class Builder extends BuilderBase<Builder> {
    ...
    @Override
    public Builder name(String name) {
      return super.name(name);
    }
    ...
  }
}

Sonar said the “name” method in the subclass could be removed:

Remove this method to simply inherit it.
Overriding methods should do more than simply call the same method in the super class
java:S1185

Unfortunately, that’s really bad advice, because when the method was removed, code already compiled against the original SomeClass no longer worked at runtime, blowing up with a missing-method exception.

The runtime was trying to match the original signature, which was no longer there.

Sonar should warn when its advice could cause a binary incompatibility, or not give advice that can lead to a binary incompatibility, or offer a configuration option to disable advice that could lead to a binary incompatibility.

Hello Jim,

Thanks for describing your problem in detail.

Unfortunately, I don’t think we can help a lot with this problem. Changing a dependency’s code can often lead to incompatibilities if your dependent code is not recompiled. Hence, updating a project’s dependency without recompiling it against this updated dependency should be done with a lot of care and it is often probably easier to avoid.

Changing the class hierarchy is conflicting somewhat with having a stable API. I don’t know anything about your project, so it is hard to say for me what the best strategy here is. I don’t see a necessity to change this rule, however, as such behavior may happen due to many reasons not related to this rule. It is a broader topic than removing a redundant overriding method.