SONARJAVA-4837 Fix FP on records with no components (#4721)

SonarSource · Mar 18, 2024 · a9ffbd8 · a9ffbd8
1 parent 7aa9c21
commit a9ffbd8
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/...st-sources/default/src/main/java/checks/RecordPatternInsteadOfFieldAccessCheckSample.java b/...st-sources/default/src/main/java/checks/RecordPatternInsteadOfFieldAccessCheckSample.java
@@ -4,6 +4,15 @@
 
 public class RecordPatternInsteadOfFieldAccessCheckSample {
 
+  record Box() { }
+
+  static void switchOnSealedClass(Object shape) {
+    switch (shape) {
+      case Box unused -> { } // Compliant, record has no components
+      default -> {}
+    }
+  }
+
   int sameComponentAccessTwice(Object obj){
     if (obj instanceof Point p) { // Compliant; not all record components are used
       return p.x() + p.x();

diff --git a/java-checks/src/main/java/org/sonar/java/checks/RecordPatternInsteadOfFieldAccessCheck.java b/java-checks/src/main/java/org/sonar/java/checks/RecordPatternInsteadOfFieldAccessCheck.java
@@ -80,8 +80,7 @@ && isRecordPattern(typePattern)) {
 
   private void checkTypePatternVariableUsage(VariableTree patternVariable) {
     var secondaryLocationsTrees = new HashSet<MemberSelectExpressionTree>();
-    var type = patternVariable.symbol().type().symbol();
-    var comps = recordComponentNames(type);
+    var recordSymbol = patternVariable.symbol().type().symbol();
     for (Tree usage : patternVariable.symbol().usages()) {
       if (usage.parent() instanceof MemberSelectExpressionTree mse && isNotRecordGetter(mse)) {
         secondaryLocationsTrees.add(mse);
@@ -90,12 +89,17 @@ private void checkTypePatternVariableUsage(VariableTree patternVariable) {
       }
     }
     // only if all the records components are used we report an issue
-    if (secondaryLocationsTrees.stream().map(mse -> mse.identifier().name()).toList().containsAll(comps)) {
+    if (isEveryRecordComponentUsed(secondaryLocationsTrees, recordSymbol)) {
       reportIssue(patternVariable, "Use the record pattern instead of this pattern match variable.",
         getSecondaryLocations(secondaryLocationsTrees), null);
     }
   }
 
+  private static boolean isEveryRecordComponentUsed(Set<MemberSelectExpressionTree> secondaryLocationsTrees, Symbol.TypeSymbol recordSymbol) {
+    var recordComponentNames = recordComponentNames(recordSymbol);
+    return !recordComponentNames.isEmpty() && secondaryLocationsTrees.stream().map(mse -> mse.identifier().name()).toList().containsAll(recordComponentNames);
+  }
+
   private static boolean isNotRecordGetter(MemberSelectExpressionTree mse) {
     return !ALLOWED_METHODS.contains(mse.identifier().name());
   }