Consider Lombok val as valid type for S2159 and S2175

java
lombok

(Vmurthy) #1

Hi

Currently SillyEqulasCheck and CollectionInappropriateCallsCheck are not considering lombok.val as inclusive/inferred type and throws an rule violation whenever the following is executed.

String x ="Hello World";
lombok.val y = "Hello World";
boolean z = x.equals(y);  // Should be No Issue but S2159 (SillyEqualsCheck) throws up

My question is How can we include lombok.val as inferred type for either inclusion in collection or equality check as demonstrated above.

Could we just add lombok.val as another Type and if so what javaType does it belong to ; should we create a LombokType just so Type system can be used for all the rules.

Please share thoughts on how to include these

thanks
venkat.


(Vmurthy) #2

In the context of SillyEqualsCheck and CollectionInappropriateCallsCheck; i am looking at including an additional Predicate to deal with inferred types. These predicates could deal with additional type equivalence checks.

For eg; In SillyEqualsCheck.java

private static boolean areNotRelated(Type type1, Type type2) {
    return ignoreInferredTypes.test(type1, type2) && !type1.isSubtypeOf(type2) && !type2.isSubtypeOf(type1);
  }

In the above snippet; the ignoreInferredTypes is a BiPredicate that is checking whether type1 and type2 can be inferred automatically and ignored (such as lombok.val).

Similarly for CollectionInppropriateCallsCheck.javaor eg: lombok.val)

private static boolean isArgumentCompatible(Type argumentType, Type collectionParameterType) {
    return ignoreInferredType.test(argumentType)
      || isSubtypeOf(argumentType, collectionParameterType)
      || isSubtypeOf(collectionParameterType, argumentType)
      || autoboxing(argumentType, collectionParameterType);
  }

Here ignoreInferredType.test(argumentType) is a Predicate checking if the type passed is of inferred type .

The inferred type at this moment iam thinking it to be lombok.val; however; can be expanded to other inferred types too.

Please let me know what u think

Given that these above mentioneds are static methods the equivalence predicate is difficult to inject and thus have to be static final constants.

Appreciate your opinion


(Michael Gumowski) #3

Hello,

SonarJava is not supporting Lombok, and does not plan to support it. Lombok features such as val are currently only going to create noise with our rules. Handling it properly would add extreme complexity to their implementation, which we want to avoid at all cost.

Now, handling type inference with the approach you are proposing is a bit too extreme for us, and would have as consequences to introduce False Negatives for cases which are NOT relying on Lombok. We are however already filtering the issues generated by a few rules to avoid making too much noise, when we detect Lombok cases.

I consequently created the following ticket to adapt our filter to also consider S2159 and S2175: SONARJAVA-3089.
It won’t solve everything, and will potentially introduce FN when using Lombok, but it will reduce the noise.

Regards,
Michael


FP using Lombok @Cleanup