From 44535b310d471e67ce33b02668f6f09d031d15f1 Mon Sep 17 00:00:00 2001 From: Dinesh Bolkensteyn Date: Tue, 25 Oct 2011 14:54:20 +0200 Subject: [PATCH] SONAR-2934 Accessors should be considered transitively --- .../sonar/java/bytecode/asm/AsmMethod.java | 18 +++++++++++++++--- .../java/bytecode/asm/AsmMethodTest.java | 4 ++++ .../bytecode/bin/properties/JavaBean.class | Bin 1824 -> 2009 bytes .../bytecode/src/properties/JavaBean.java | 10 +++++++++- 4 files changed, 28 insertions(+), 4 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 e68caf6e8a4..51e27b1d7ee 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 @@ -150,18 +150,30 @@ public class AsmMethod extends AsmResource { if (!isConstructor()) { for (AsmEdge edge: getOutgoingEdges()) { if (isCallToNonStaticInternalField(edge)) { - if (accessedField != null && accessedField != edge.getTo()) { + if (isFieldAccesingDifferentField((AsmField)edge.getTo())) { accessedField = null; break; } accessedField = (AsmField)edge.getTo(); } else if (isCallToNonStaticInternalMethod(edge)) { - accessedField = null; - break; + AsmMethod method = (AsmMethod)edge.getTo(); + if (isMethodNotAccessorOrAccessingDifferentField(method)) { + accessedField = null; + break; + } + accessedField = method.getAccessedField(); } } } } + + private boolean isMethodNotAccessorOrAccessingDifferentField(AsmMethod method) { + return !method.isAccessor() || (accessedField != null && accessedField != method.getAccessedField()); + } + + private boolean isFieldAccesingDifferentField(AsmField field) { + return accessedField != null && accessedField != field; + } private boolean isCallToNonStaticInternalField(AsmEdge edge) { return edge.getTargetAsmClass() == (AsmClass)getParent() && edge.getUsage() == SourceCodeEdgeUsage.CALLS_FIELD && !((AsmField)edge.getTo()).isStatic(); diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/asm/AsmMethodTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/asm/AsmMethodTest.java index 69b37bb94f4..631f0078b0a 100644 --- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/asm/AsmMethodTest.java +++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/asm/AsmMethodTest.java @@ -62,6 +62,8 @@ public class AsmMethodTest { @Test public void testIsAccessor() { assertTrue(javaBean.getMethod("getName()Ljava/lang/String;").isAccessor()); + assertTrue(javaBean.getMethod("getNameIndirect()Ljava/lang/String;").isAccessor()); + assertTrue(javaBean.getMethod("getNameOrEmpty()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()); @@ -76,6 +78,8 @@ public class AsmMethodTest { @Test public void testGetAccessedField() { assertThat(javaBean.getMethod("getName()Ljava/lang/String;").getAccessedField().getName(), is("name")); + assertThat(javaBean.getMethod("getNameIndirect()Ljava/lang/String;").getAccessedField().getName(), is("name")); + assertThat(javaBean.getMethod("getNameOrEmpty()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")); 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 5fbd5f8656fc20e5de7049a73f337916d9485a12..6c6d60868f08de04a3c909733e78b30e423532d4 100644 GIT binary patch literal 2009 zcmZ`&U31$+6g}%tSy8^+#E_;0(gJmCh$$^CZPHLXY3PTO6wI_k-!}FpR!5OCk_?l7 zz(3)I2N+1pzzna<@S{51yOI^fb?q7LYIX0~bI-Z=zt?~L1K=7uCg!kS#9b2^te0>B z8~pgnq{lr2O@3+dw@m|Ee0bl$wt=q=JmAxZ1|D(9V}7)Y*ePOHAm_>>B~Wg^kUz+V zBi+MBC-iOia800a;48P=7ZA?{%mdpG!d;3E1m>ggL1;UTrtiy>wjG3&QR>)-t_%l0 zL9UF8S%34M4a&2EP=L(dIUb*wq3P5;PCI{ zzN0Jfy3%UId*jN6J3j1D< z1d+X7N`z7(zQ9bvU`rjyfkUJ&3jS+oeO#e`bgMW@|uJ?j3AO$00=u zN7^!hO8cB_C&xqSDt4#u4V+#}H6z%L3PU!>qWe?S%#JtkyK38J6DEh^u5vqBcpo1S zJKb96d&R;6E?SsJ#lpw9DgX=b;9U!QctY$*onG(6!VS(Zw4_Uaf&3FCxgAaFQ28TS z?^DP(_?AK*D97=ZExgBoOQ_JTV=O1}G#731-u?^iXqKd=j!v5weOhC$3iP$= z(hiuZy@IIyjm-D2kUd52H=S{ro;=5VPYxwqp`8`Q<3oHzZ-S+au^O~8R=)NJ^1sq1 z0|qGQ!>LqqK1r^QNq&jKXmWXM@=B7t7T-KuqvU^Kl#4&3IF3an1vHzmsG&{(ksKLa zZlP8`MSfI-izLIMMyNzlF#^}fCUE(m;L{i@Er;>Th#X5PB4=`}f|Je@Sv-X?O5`#T zx$+hw+yzYJGYmbh)7|WgID?5ixtby&5;d+RYP6_IHs2&EbM==fJv&2)+M-5HQBM&u z6GcpX9xHNvj7`)aY?JvGn^sfAhBlRWGGlcXh!p>HMIzN@*D2tOBgLQ|n*Rg`TshVqX%@Z_MH}&Y< zqD&r>=;RXZ1R)H`-_Y;WGv%4zF*~a0J+ii$D!-8Uay!v6-C5<)S=mY@pLNz9qLjhf E|7Vj!#Q*>R 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%AF;|aj5$tQ0<=4+$ztCg?Dh3*y*`BI|vr8;4KStn76Qis)aA`j98H> zgTb+ddmLwUq(^^&;#W%YjF~ic@;mBhKrUaROD<29>-sk=EHlvCn5S$P^pQl<9CYV) z_gqrIb5gJ(8f!d;9?ql8}BEM!j)@;$(W8 z2+V1OYV4IDFglmdiECI%u(CvqA18=ZFC