Hi there, I’ve hit something I think is a false positive, involving lombok’s “SuperBuilder” annotation. The reported code smell is S3252
We are using SonarCloud
Sample Code (this is not code I have run through it, but a summary of the code I have that triggers it) -
@SuperBuilder
@NoArgsConstructor
public class BaseClass<T, R> {
@Getter
private String itemId;
@Getter
private R item1;
@Getter
private T item2;
}
@NoArgsConstructor
public UtilityClass {
@SuperBuilder
protected static class SubClass extends BaseClass<String, String> {
@Getter
String anotherId;
@Getter
String andAnotherId;
}
public void doSomething() {
SubClass subClass = SubClass.builder().anotherId("a").andAnotherId("b").itemId("c").item1("d").item2("e").build();
}
}
The SuperBuilder annotation gives the subclass a method - builder() - which can also be used to set attributes in the superclass as well, avoiding the need for constructor boilerplate. Sonarcloud raises issue S3252 against the call to SubClass.builder(), and appears to be telling me I ought to use the BaseClass.builder(), but that’s wrong here.
Hello @Starnose, thanks for reporting this false positive.
First, I just want to remind everyone that tools modifying source code and static analysis are usually not best friends… We try our best to make the experience as pleasant as possible by filtering out issues when we can, but you should be aware that you have a higher chance to see rules misbehaving when using Lombok.
Your example seems indeed a FP. The analyzer gets confused by the fact that it sees an inconsistent state between the source code and what is actually in the bytecode.
Solving this problem is probably possible but seems at first glance quite costly compared to the noise it aims to kill. Especially knowing that “SuperBuilder” is still considered as an experimental feature.
What I suggest at this point is to wait for more feedback, if it appears that many people share the same pain, it will justify going further. In the meantime, you can close the issue as false positive and eventually remove this rule from the quality profile if it generates unmanageable noise.
Thanks for the response. Yes I figured this was probably something to do with the code generation that happens behind the scenes with SuperBuilder.
We will either ignore these or close as FP for the forseeable. I think it’s probably useful to keep the rule in our profile as it seems like a decent rule to have in general.