From 4cade605cd7a3a7dc3cd035137cd88ffa69233ec Mon Sep 17 00:00:00 2001
From: Dinesh Bolkensteyn <dinesh@dinsoft.net>
Date: Tue, 25 Oct 2011 12:44:15 +0200
Subject: [PATCH] SONAR-2724 SONAR-2723 Improved getter and setter detection
 algorithm

---
 .../bytecode/visitor/AccessorVisitor.java     |  57 +++++++++++-------
 .../java/bytecode/visitor/LCOM4Visitor.java   |   2 +-
 .../java/bytecode/BytecodeVisitorsTest.java   |  10 +--
 .../bytecode/visitor/AccessorVisitorTest.java |  10 +++
 .../bytecode/bin/properties/JavaBean.class    | Bin 798 -> 1824 bytes
 .../bytecode/bin/tags/impl/FixMe$1.class      | Bin 821 -> 821 bytes
 .../bytecode/bin/tags/impl/Todo.class         | Bin 1577 -> 1573 bytes
 .../bytecode/src/properties/JavaBean.java     |  36 +++++++++++
 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 96067d246b952b93f1a83b7d23dd8bf9fe7c6001..5fbd5f8656fc20e5de7049a73f337916d9485a12 100644
GIT binary patch
literal 1824
zcmZ`&ZFAd15Pr@tvaKj>q9iU!ps!FIyN%nDLP~;*of1lM8fMJ2L%&_@ORSC}Wh5CU
z|A2qO7e2s1C<8NmWriQcuzQjf#R>Mzoo;XU*=L{K`{&<3{sM3xJ0@(bm+;6$4(k=n
z;$vPmc-dszCuOuuIy^4p(=ys+blAF8#uIk=jF;^aJ}=>^K*5uTO2F>Emfy;zE4_ne
zFA5y*U_-#z50uv*3W!$%=Drhz(R1<+1r}oWQRKMIb`Z$pt`kNSQRz7co{UBTL9Sfn
zvw1#Gi{h*>l9AKjQHOgfAeilOE+Vn%cuv$3$k$e05Z^6-K;W0Vj;EfF*!^p{=jsal
zzI0#6z~OUZEJQ;`(>)6OBNarB3Y*WU(2kNGr7ayOU6H_iEuni>fl@ejSg4(6&V0&F
zxSgm(1WBg~g^VqQ+AH>%mYyFCRj{L?p+BH%<zSGhu#ze(Fq1Ia4W6idIdUU`xlyQG
z6^6Y;q8MXQ_WNY?gO^S;Y<EV~r``Q6>AF;|aj5$tQ0<=4<oIZ;OV#NO{gFH9sCEq7
zQ&GgqTa-UT&FuN3ps%(aR(N`>+$ztCg?Dh3*y*`BI|vr8;4KStn76Qis)aA`j98H>
zgTb+ddmLwUq(^^&;#W%YjF~ic@;mBhKrUaROD<29>-sk=EHlvCn5S$P^pQl<9CYV)
z_g<?$iIyjcX-u<Ci?24gM+V)tT~}Z+anSs?eAlI5$M?tyH|Ukv^xlf}bf^;g%5gEY
zgUbU2Yuge`CaAxOBkdO5kv6nmqf-lc`T}O^Zy@S_BKOT3<WEreSw~!_l^-YPDWF1G
z<9PDNP28fBz`J-a!D`aZSjGCUDE>qrIb5gJ(8f!d;9?ql8}BE<E%N3>M!j)@;$(W8
z2+V1OYV4IDFglmdiECI%u(CvqA18=ZFC<b&;~bIY6Bv_3mWasm{}5sQnaG_~-3H}m
zMZ_sgtj4tr36ZLCH&vsPs9}<ng~o3vy*x#T8*-D{aVtZ_OcgP)nkdptTeG2w0khni
zzfrc!KcI9`n{H=_%_fi)eiH=1^TJcbKV6Z?^q1wkpSpdY@UQ=lN-|X4hHY{o4SOoy
zEZ&m(rgeRJjdv9#O0Y_U*~Ap3wh4C;59n@<I{YwGb3GwAMbsYXq5ps)dB9`KlCF9!
uWwoZyG&A<4UvPO+(RFI=qfGfm+Lwo^j@ikYnH=%WOz`Q5e@K*a_~<|Upe!N)

literal 798
zcmZ{iO;5r=5Qg7{LII1&r{7;UltkiL!$A`c2KAuv;BBcZEP>W6)nDa76N!lje}F&A
zIJ*{6sA)30Go5$l>6?ChzP$t3$94t@tfgULU55=Dn>Mx>ERPQaL!s5<54_&zUZ;K$
zDCu<$7?QV2cy5=0T~l%>JsC6@;?<oi(jEIZq|3FWC(iB%ZJ{oCyKg4>F7ID)C3PMa
zt)MH14CU6H^6!KSq!`vuXyK9I9xbvvA~-V~rRq);i9H-Ql&iDvHBU+on;|U$5^tI$
z<BC<i);(G7`9W8xQxSCi8-~n<f3IB8l$tv`=GoIuILKhmK@zru6w(aE2y%vniEHQW
zo^S(fler0UJVp<x&5Z`onFMXb97Tt+I-QKp#O^EDvuVqQO+Xy;ba%cRki!CH40#kN
z{uRF<_B16<M~aImh2n-`Bc9#b2(iiP4Ap@w(a)O+6_Y2!^_pLQkESl85&;r_oC3*5
wf-GWb24r=F_%ukFKq~)$XhSu~@{gT0;}E$t8KeCMR(?Uy?;2XUVo+DV0FE1SBme*a

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 95daff2fdbd009b6abb2607ebc3df6e523abd82d..7581dc25b8a10b5220d47ce581ab20636a4f2127 100644
GIT binary patch
delta 19
ZcmdnWwv}x|IuoPF<P4@3Ao-Tb0{}iE1_b~B

delta 19
ZcmdnWwv}x|IuoPl<P4@3Ao-Tb0{}iU1_uBD

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 1f443ef601899c29765997df74ed87e970bcc559..ca3774bfbea62a1cafac76004c33a9687eb9a431 100644
GIT binary patch
literal 1573
zcmZ`)+fEZv6kXHX(4plHf(QzVwneFmS6Wd-hz|{z&|vVz4DDEl3@vG=M*hhMO+*q&
z`~W}7xb~cBJ1y`qr~7u+Ui&iBzyE&!0q_9p29j9T@yI|3%W)*|SjQ6sQKdW;>6wn_
zeONINM@~mx#|s^+8X});$I{SWaLr01yGbRdA?nywtD#}MaA1BhvrX4_vTOCad0em?
zF8%QxyHYjXW}P?ZFN(=4xB_8vtGp1)+g00Tx^ODJ%cSeIGOrGbf2(=8Z`F(DzR(yc
z)JmqaYu0ViePhJ^Y?I88!n6;MoNTdHt`T;kJN)>lb!EIKHecNEc<2qwl^Qh+qyiNS
zC%w{gB~$5uj~=sJ7N(s`UR_=xj%W9~=b1Gmn+<EH)~uH-m1@jS)v@K$A2bZBBmt`i
zW8-VdI&$q=wV~ssBDG~6GWAf4RMD)6h1K_wv7}^gCh6^rYQ5$-R!LA8wQz~6cSPCz
z#)@K8FsqfOS>faqrb9iuy?;Q+oQfa^A{!m*TnQP;l5j|191|R_w@{ss1O_prVYEZD
zy-^7aV<drBSksX3uK7iBQ;OSfwgvGuyYIQ$^Jh;ytN=asq;;MBZHeuka{V|;u662s
zVKR{}`;J?;tCd`O7jt}{l3;$mTp_7~?_5YI(>ChKW6&wHY@SzW`G_zjU^;UOEpvv@
z-YLQ-h)|4DjGaLLL3bFFJO}tIXN16m1f8OmP}gx4)6}kYS!Sqdn8kHJ**lLG5X&qw
zMF&^D1t6&?X$j_pr^kFu#(Sk`0d(;x^cyT%m9oZPDJ7cuiO6>fige5~P6fzD&yf|2
zf}6)pAFk+=H3+9K{zBjTUcxi{K+OgS8*Rb{Zef8qopy*%Ww?zyM3&yaWrP$S3V-Wi
zI~PC=wNYiFsV-FU32K^8gq{qaMcm~n_?!N0jWfjeMvPPR_jY9=(3KYZi#XrIlFvEV
aWW5(I9e}&M$!Za^1XR*}{w>IwKKu{y1{dD|

literal 1577
zcmZ`&YflqF6g|_|mZg+ZXn`UKD%uvM;scdd6cOTw225x$_-$#|x@2idyEXEk{Gf?Q
zB8fl1A7wl<yQNEkG@YHfk2B|<d*|=J-+ursVO>K&Anq9TcELJ4vI~`Ft*Id-kT@{D
z7zNvC)C=4D2WHjL5D|!~$cj-H2z|0_Q(&;HOq)Cu1;Vz~FxvvDvP$nbmR(qDwT$Dk
z)pqC~*s<yj!|Ak`dF!f}k_%TROmUS7k&@N0oK=BfHn+=5*PAs{M-U@13?Z&zRA5*U
z*lBiJRa4T?k;KRV1ky2Rh-#P+7`=jyj&V0`Duw~*1d_LIbq@E<R>jzt#U;zls$uUM
zElZv~W61ezvBZSS^jtNPO<xQ@KI*2V`eI9!4ON}iHXTWsEy((Gl$C1ru4^cp^ZDp8
zYBkBUm!#Cn?aG33G32p8xfsMYWL=GYZ7k+KQbt?ld%aWX0z;j)*;7$q%%jw^sa$>I
zYt=k*tY)LF;gP^dcO=g3w7G<RnQlQ_#vy4Zx&?8Lq@}3gnLw<IBTZ4fAj``3+ie}o
zD9UD+`pVgHT2`Z8lv+ipl`Mi?nILwr*XvE&=ID|uyEA`{tAFJ59&HY+d=QHgQJ_%T
zSaAz0^I3F^I!E-fzHS6dD`JjA800^U0DqiCzPW&$HQEN%UQQ^LN1W?0Emai{Oyy6J
z$e$s(cZ#tSq_`dDcJc(79}ElP2G=A)T;p65Oim)h{WL8xf)N5UxXJJA1<M>ye0sR;
zUHD#610wkaUem+YX$MlIo>rJ?b&hzLj929dAG-9p=yx#hp|9~Ph!f8LMCv;?F7b#m
zPWs4(FOiiN7w#_Zd2khvtWLPh!Y^b$^b?+@x-&k)x`zvDejg8rGfNh(K+@;lu!sVY
zAJP+I#FFa7JMN?ef#5f;e%^CF{J=TB?C;|X_|oU%KfzOaWPdlk{_1B~-b+3|#Y(>_
r^FCF&>}Q$T&-o=*$<LjlR|?xNcBUUT=Yzdo>;PW#ssPqJ;5YvPS(q4c

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;
+  }
+  
 }
-- 
2.39.5