We’re excited to introduce advanced class cycle detection for Java projects with two new rules:
S7091 – Circular dependencies between classes across packages
S7027 – Circular dependencies between classes in the same package
These rules will highlight the most problematic class, while also displaying other involved classes as secondary locations. Multiple cycles in your classes are also reported, as shown in the screenshot below:
I like the feature, but we also have false positive detections when using Jackson’s @JsonSubTypes.Type annotation (for an example, see Jackson Polymorphic Type Handling Annotations). Circular dependencies between base and child classes exist there “by design”.
Thanks for the feature, it is a good thing to consider. However, I also found other false positive detection (in addition to JPA entities) in the custom validation annotations creation: https://www.baeldung.com/spring-mvc-custom-validator
It marks the interface of the annotation and the usage in the ConstraintValidator as a cycle too. Unless there is a another way to create a custom validator, this also could be a false positive detection.
Thanks Orlando for sharing, and welcome to the sonar Community!
We are tracking all of these to find a good solution to reduce the noise.
Keep them coming!
public sealed class BaseRestController permits ChargingSessionRestController, SiteRestController, VehicleRestController, VehicleTypeRestController {}
and
public non-sealed class VehicleRestController extends BaseRestController {}
Sonar concludes that:
“This class is part of 4 cycles containing 5 classes”
I’d say this is a false positive. The permits clause doesn’t really generate a dependency, it generates a restriction. The only way to “fix” the cycle is to make BaseRestController not sealed.
The work I was doing got flagged by this new rule … it was somewhat of an effort to refactor to avoid cycles: in particular I ended up splitting some classes up, in the ugliest case into a superclass/subclass pair. The end result I think is cleaner and more maintainable.
In the ugly case, the class to be split up was conceptual very long, with many inner classes responsible for the implementation. I had already factored these inner classes out into separate files to reduce the file length. These caused multiple cycles (which would have been excluded by the inner class exclusion on the rule). My refactor, allowed the (design-wise inner) implementation classes to refer to the outer class by a base class, rather than by the subclass that
public class MyCoreClass implements ILargeInterface {
int packageScopedInternals;
// ...
public String query() {
new Query(this).query();
}
}
class Query {
Query(MyCoreClass outer) {
// ...
}
String query() {
// ...
}
}
to
abstract class MyCoreClassBase implements ILargeInterface {
int packageScopedInternals;
// ...
}
public class MyCoreClass extends MyCoreClassBase {
public String query() {
new Query(this).query();
}
}
class Query {
Query(MyCoreClassBase outer) {
// ...
}
String query() {
// ...
}
}
The proposal in the rule description to use an interface was unattractive, since I think this would have made some of the package scoped internals publicly accessible. Using a package scoped superclass avoid such spillage.
