SONAR-2724 SONAR-2723 Improved getter and setter detection algorithm

8 files changed, 86 insertions, 29 deletions

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 4dcbccc898f..e022590177d 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
@@ -19,38 +19,49 @@
  */
 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;
+import org.sonar.java.bytecode.asm.*;
+import org.sonar.squid.api.SourceCodeEdgeUsage;
 
 public class AccessorVisitor extends BytecodeVisitor {
-
+  
   private AsmClass asmClass;
-
+  
   public void visitClass(AsmClass asmClass) {
     this.asmClass = asmClass;
   }
 
   public void visitMethod(AsmMethod asmMethod) {
-    String propertyName = extractPropertyNameFromMethodName(asmMethod);
-    AsmField accessedField = asmClass.getField(propertyName);
-    if (propertyName != null && accessedField != null) {
-      asmMethod.setAccessedField(accessedField);
-    }
+    if (asmMethod.isConstructor()) return;
+    
+    AsmField accessedField = getAccessedField(asmMethod);
+    asmMethod.setAccessedField(accessedField);
   }
-
-  private String extractPropertyNameFromMethodName(AsmMethod asmMethod) {
-    String propertyName;
-    String methodName = asmMethod.getName();
-    if (methodName.length() > 3 && (methodName.startsWith("get") || methodName.startsWith("set"))) {
-      propertyName = methodName.substring(3);
-    } else if (methodName.length() > 2 && methodName.startsWith("is")) {
-      propertyName = methodName.substring(2);
-    } else {
-      return null;
+  
+  private AsmField getAccessedField(AsmMethod asmMethod) {
+    AsmField accessedField = null;
+    
+    for (AsmEdge edge: asmMethod.getOutgoingEdges()) {
+      if (isCallToNonStaticInternalField(edge)) {
+        if (accessedField != null && accessedField != edge.getTo()) {
+          accessedField = null;
+          break;
+        }
+        accessedField = (AsmField)edge.getTo();
+      } else if (isCallToNonStaticInternalMethod(edge)) {
+        accessedField = null;
+        break;
+      }
     }
-    byte[] bytes = propertyName.getBytes();
-    bytes[0] = (byte) Character.toLowerCase((char) bytes[0]);
-    return new String(bytes);
+    
+    return accessedField;
+  }
+  
+  private boolean isCallToNonStaticInternalField(AsmEdge edge) {
+    return edge.getTargetAsmClass() == asmClass && edge.getUsage() == SourceCodeEdgeUsage.CALLS_FIELD && !((AsmField)edge.getTo()).isStatic();
+  }
+  
+  private boolean isCallToNonStaticInternalMethod(AsmEdge edge) {
+    return edge.getTargetAsmClass() == asmClass && edge.getUsage() == SourceCodeEdgeUsage.CALLS_METHOD && !((AsmMethod)edge.getTo()).isStatic();
   }
+  
 }
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 d1b411f078c..c75c264e8f3 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
@@ -118,7 +118,7 @@ public class LCOM4Visitor extends BytecodeVisitor {
   }
   
   private boolean isCallToInternalFieldOrMethod(AsmEdge edge) {
-    return edge.getTargetAsmClass() == asmClass && (edge.getUsage() == SourceCodeEdgeUsage.CALLS_FIELD || edge.getTargetAsmClass() == asmClass && edge.getUsage() == SourceCodeEdgeUsage.CALLS_METHOD);
+    return edge.getTargetAsmClass() == asmClass && (edge.getUsage() == SourceCodeEdgeUsage.CALLS_FIELD || edge.getUsage() == SourceCodeEdgeUsage.CALLS_METHOD);
   }
 
   private Set<AsmResource> getOrCreateResourceBlock(AsmResource resource) {
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/BytecodeVisitorsTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/BytecodeVisitorsTest.java
index 2ced1147d4b..c7f04fa841c 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/BytecodeVisitorsTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/BytecodeVisitorsTest.java
@@ -74,11 +74,11 @@ public class BytecodeVisitorsTest {
 
   @Test
   public void testLCOM4Visitor() {
-    assertEquals(3, squid.search("tags/impl/Todo").getInt(Metric.LCOM4));
-    assertEquals(3, squid.search("tags/impl/Todo.java").getInt(Metric.LCOM4));
+    assertEquals(2, squid.search("tags/impl/Todo").getInt(Metric.LCOM4));
+    assertEquals(2, squid.search("tags/impl/Todo.java").getInt(Metric.LCOM4));
 
     List<Set<AsmResource>> lcom4Blocks = (List<Set<AsmResource>>) squid.search("tags/impl/Todo.java").getData(Metric.LCOM4_BLOCKS);
-    assertEquals(3, lcom4Blocks.size());
+    assertEquals(2, lcom4Blocks.size());
 
     assertEquals(1, squid.search("tags/Tag").getInt(Metric.LCOM4));
     assertEquals(1, squid.search("tags/TagName").getInt(Metric.LCOM4));
@@ -86,8 +86,8 @@ public class BytecodeVisitorsTest {
 
   @Test
   public void testRFCVisitor() {
-    assertEquals(9, todo.getInt(Metric.RFC));
-    assertEquals(9, squid.search("tags/impl/Todo.java").getInt(Metric.RFC));
+    assertEquals(8, todo.getInt(Metric.RFC));
+    assertEquals(8, squid.search("tags/impl/Todo.java").getInt(Metric.RFC));
     assertEquals(5, sourceFile.getInt(Metric.RFC));
   }
 
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 5ede45e3029..7a2ded02245 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
@@ -54,6 +54,11 @@ public class AccessorVisitorTest {
     assertTrue(javaBean.getMethod("setFrench(Z)V").isAccessor());
     assertTrue(javaBean.getMethod("isFrench()Z").isAccessor());
     assertFalse(javaBean.getMethod("anotherMethod()V").isAccessor());
+    assertTrue(javaBean.getMethod("addFirstName(Ljava/lang/String;)V").isAccessor());
+    assertTrue(javaBean.getMethod("getNameOrDefault()Ljava/lang/String;").isAccessor());
+    assertTrue(javaBean.getMethod("accessorWithABunchOfCalls()V").isAccessor());
+    assertFalse(javaBean.getMethod("iShouldBeAStaticSetter()V").isAccessor());
+    assertTrue(javaBean.getMethod("getFirstName()Ljava/lang/String;").isAccessor());
   }
   
   @Test
@@ -63,6 +68,11 @@ public class AccessorVisitorTest {
     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());
+    assertThat(javaBean.getMethod("addFirstName(Ljava/lang/String;)V").getAccessedField().getName(), is("firstNames"));
+    assertThat(javaBean.getMethod("getNameOrDefault()Ljava/lang/String;").getAccessedField().getName(), is("name"));
+    assertThat(javaBean.getMethod("accessorWithABunchOfCalls()V").getAccessedField().getName(), is("firstNames"));
+    assertNull(javaBean.getMethod("iShouldBeAStaticSetter()V").getAccessedField());
+    assertThat(javaBean.getMethod("getFirstName()Ljava/lang/String;").getAccessedField().getName(), is("FirstName"));
   }
   
 }
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/properties/JavaBean.class b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/properties/JavaBean.class
index 96067d246b9..5fbd5f8656f 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/properties/JavaBean.class
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/properties/JavaBean.class
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/FixMe$1.class b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/FixMe$1.class
index 95daff2fdbd..7581dc25b8a 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/FixMe$1.class
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/FixMe$1.class
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/Todo.class b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/Todo.class
index 1f443ef6018..ca3774bfbea 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/Todo.class
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/bin/tags/impl/Todo.class
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/src/properties/JavaBean.java b/plugins/sonar-squid-java-plugin/test-resources/bytecode/src/properties/JavaBean.java
index be09a2b5b99..6e858627c24 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/src/properties/JavaBean.java
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/src/properties/JavaBean.java
@@ -1,9 +1,14 @@
 package properties;
 
+import java.util.ArrayList;
+
 public class JavaBean {
   
   private String name;
   private boolean french;
+  ArrayList<String> firstNames = new ArrayList<String>();
+  private static String staticMember;
+  private String FirstName;
   
   public String getName(){
     return name;
@@ -24,4 +29,35 @@ public class JavaBean {
   public void anotherMethod(){
     
   }
+  
+  public void addFirstName(String firstName) {
+    firstNames.add(firstName);
+  }
+  
+  public String getNameOrDefault() {
+    return (name == null) ? "Freddy" : name;
+  }
+  
+  public static void uselessStaticMethod() {
+    
+  }
+  
+  public void accessorWithABunchOfCalls() {
+    uselessStaticMethod();
+    ArrayList<String> myList = new ArrayList<String>();
+    myList.add("Banana");
+    myList.add("Peach");
+    myList.add("Strawberry");
+    
+    firstNames.addAll(myList);
+  }
+  
+  public void iShouldBeAStaticSetter() {
+    staticMember = "Hello!";
+  }
+  
+  public String getFirstName() {
+    return FirstName;
+  }
+  
 }