SONAR-2038 LCOM4 value is incorrect when mixing getter/setter and direct field access

5 files changed, 52 insertions, 33 deletions

diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/asm/AsmMethod.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/asm/AsmMethod.java
index bce75c3bd76..5104162d148 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/asm/AsmMethod.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/asm/AsmMethod.java
@@ -31,7 +31,7 @@ public class AsmMethod extends AsmResource {
   private boolean inherited = false;
   private boolean empty = false;
   private boolean bodyLoaded = true;
-  private boolean accessor = false;
+  private AsmField accessedField = null;
   private String signature;
   private AsmMethod implementationLinkage = null;
 
@@ -140,11 +140,15 @@ public class AsmMethod extends AsmResource {
   }
 
   public boolean isAccessor() {
-    return accessor;
+    return accessedField != null;
+  }
+  
+  public AsmField getAccessedField() {
+    return accessedField;
   }
 
-  public void setAccessor(boolean accessor) {
-    this.accessor = accessor;
+  public void setAccessedField(AsmField accessedField) {
+    this.accessedField = accessedField;
   }
 
   @Override
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/AccessorVisitor.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/AccessorVisitor.java
index 46b52d1b023..4dcbccc898f 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/AccessorVisitor.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/AccessorVisitor.java
@@ -20,6 +20,7 @@
 package org.sonar.java.bytecode.visitor;
 
 import org.sonar.java.bytecode.asm.AsmClass;
+import org.sonar.java.bytecode.asm.AsmField;
 import org.sonar.java.bytecode.asm.AsmMethod;
 
 public class AccessorVisitor extends BytecodeVisitor {
@@ -32,8 +33,9 @@ public class AccessorVisitor extends BytecodeVisitor {
 
   public void visitMethod(AsmMethod asmMethod) {
     String propertyName = extractPropertyNameFromMethodName(asmMethod);
-    if (propertyName != null && asmClass.getField(propertyName) != null) {
-      asmMethod.setAccessor(true);
+    AsmField accessedField = asmClass.getField(propertyName);
+    if (propertyName != null && accessedField != null) {
+      asmMethod.setAccessedField(accessedField);
     }
   }
 
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/LCOM4Visitor.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/LCOM4Visitor.java
index df49ca4e500..a3598b3da12 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/LCOM4Visitor.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/visitor/LCOM4Visitor.java
@@ -50,13 +50,15 @@ public class LCOM4Visitor extends BytecodeVisitor {
 
   public void visitMethod(AsmMethod asmMethod) {
     if (isMethodElligibleForLCOM4Computation(asmMethod)) {
-      Set<AsmResource> block = getResourceBlockOrCreateIt(asmMethod);
+      ensureBlockIsCreated(asmMethod);
       for (AsmEdge edge : asmMethod.getOutgoingEdges()) {
         if (isCallToInternalFieldOrMethod(edge) && isNotCallToExcludedFieldFromLcom4Calculation(edge.getTo())) {
           AsmResource toResource = edge.getTo();
-          mergeAsmResourceToBlock(block, toResource);
+          linkAsmResources(asmMethod, toResource);
         }
       }
+    } else if (asmMethod.isAccessor()) {
+      linkAsmResources(asmMethod, asmMethod.getAccessedField());
     }
   }
 
@@ -87,19 +89,21 @@ public class LCOM4Visitor extends BytecodeVisitor {
       getSourceFile(asmClass).addData(Metric.LCOM4_BLOCKS, unrelatedBlocks);
     }
   }
-
-  private void mergeAsmResourceToBlock(Set<AsmResource> block, AsmResource toResource) {
-    if (block.contains(toResource)) {
+  
+  private void ensureBlockIsCreated(AsmResource resource) {
+    getOrCreateResourceBlock(resource);
+  }
+  
+  private void linkAsmResources(AsmResource resourceA, AsmResource resourceB) {
+    Set<AsmResource> blockA = getOrCreateResourceBlock(resourceA);
+    Set<AsmResource> blockB = getOrCreateResourceBlock(resourceB);
+    
+    if (blockA == blockB) {
       return;
     }
-    Set<AsmResource> otherBlock = getResourceBlock(toResource);
-    if (otherBlock == null) {
-      block.add(toResource);
-
-    } else {
-      block.addAll(otherBlock);
-      unrelatedBlocks.remove(otherBlock);
-    }
+    
+    blockA.addAll(blockB);
+    unrelatedBlocks.remove(blockB);
   }
 
   private boolean isCallToInternalFieldOrMethod(AsmEdge edge) {
@@ -107,23 +111,16 @@ public class LCOM4Visitor extends BytecodeVisitor {
         && (edge.getUsage() == SourceCodeEdgeUsage.CALLS_FIELD || edge.getUsage() == SourceCodeEdgeUsage.CALLS_METHOD);
   }
 
-  private Set<AsmResource> getResourceBlockOrCreateIt(AsmResource fromResource) {
-    Set<AsmResource> block = getResourceBlock(fromResource);
-    if (block != null) {
-      return block;
-    }
-    block = new HashSet<AsmResource>();
-    block.add(fromResource);
-    unrelatedBlocks.add(block);
-    return block;
-  }
-
-  private Set<AsmResource> getResourceBlock(AsmResource fromResource) {
+  private Set<AsmResource> getOrCreateResourceBlock(AsmResource fromResource) {
     for (Set<AsmResource> block : unrelatedBlocks) {
       if (block.contains(fromResource)) {
         return block;
       }
     }
-    return null;
+    
+    Set<AsmResource> block = new HashSet<AsmResource>();
+    block.add(fromResource);
+    unrelatedBlocks.add(block);
+    return block;
   }
 }
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/AccessorVisitorTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/AccessorVisitorTest.java
index 152589f1ee3..5ede45e3029 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/AccessorVisitorTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/AccessorVisitorTest.java
@@ -28,6 +28,7 @@ import org.sonar.java.bytecode.asm.AsmClassProvider;
 import org.sonar.java.bytecode.asm.AsmClassProviderImpl;
 import org.sonar.java.bytecode.asm.AsmMethod;
 
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
 public class AccessorVisitorTest {
@@ -47,11 +48,21 @@ public class AccessorVisitorTest {
   }
 
   @Test
-  public void testAccessorMethods() {
+  public void testIsAccessor() {
     assertTrue(javaBean.getMethod("getName()Ljava/lang/String;").isAccessor());
     assertTrue(javaBean.getMethod("setName(Ljava/lang/String;)V").isAccessor());
     assertTrue(javaBean.getMethod("setFrench(Z)V").isAccessor());
     assertTrue(javaBean.getMethod("isFrench()Z").isAccessor());
     assertFalse(javaBean.getMethod("anotherMethod()V").isAccessor());
   }
+  
+  @Test
+  public void testAccessedField() {
+    assertThat(javaBean.getMethod("getName()Ljava/lang/String;").getAccessedField().getName(), is("name"));
+    assertThat(javaBean.getMethod("setName(Ljava/lang/String;)V").getAccessedField().getName(), is("name"));
+    assertThat(javaBean.getMethod("setFrench(Z)V").getAccessedField().getName(), is("french"));
+    assertThat(javaBean.getMethod("isFrench()Z").getAccessedField().getName(), is("french"));
+    assertNull(javaBean.getMethod("anotherMethod()V").getAccessedField());
+  }
+  
 }
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/LCOM4VisitorTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/LCOM4VisitorTest.java
index 4a9d2af770f..adfa4ab7d24 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/LCOM4VisitorTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/visitor/LCOM4VisitorTest.java
@@ -54,5 +54,10 @@ public class LCOM4VisitorTest {
   public void testLCOM4Visitor() {
     assertThat(squid.search("LCOM4Exclusions").getInt(Metric.LCOM4), is(2));
   }
+  
+  @Test
+  public void testLCOM4AccessorMethodAndField() {
+    assertThat(squid.search("LCOM4AccessorMethodAndField").getInt(Metric.LCOM4), is(1));
+  }
 
 }