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

This commit is contained in:
Dinesh Bolkensteyn 2011-10-25 12:44:15 +02:00
parent 053e6b05a2
commit 4cade605cd
8 changed files with 86 additions and 29 deletions

View File

@ -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();
}
}

View File

@ -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) {

View File

@ -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));
}

View File

@ -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"));
}
}

View File

@ -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;
}
}