Skip to content

Commit

Permalink
refactors the filtering API
Browse files Browse the repository at this point in the history
1) uses runtime reflection instead of explicit and duplicate static typing
2) consequently, removes one of the oldest todos of Spoon
2) simplifies the API of filters (now only one method "matches")
3) adds test cases for the untested filters
4) cleans FieldAccesFilter, now a subtype of VariableAccessFilter
  • Loading branch information
monperrus committed Feb 16, 2015
1 parent 397983f commit 8489e68
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 53 deletions.
8 changes: 0 additions & 8 deletions src/main/java/spoon/reflect/visitor/Filter.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,4 @@ public interface Filter<T extends CtElement> {
*/
boolean matches(T element);

/**
* Gets the runtime type that corresponds to the <code>T</code> parameter
* (the type of the filtered elements). Any element assignable from this
* type is a potential match and is tested using the
* {@link #matches(CtElement)} method, while other elements are never a
* match.
*/
Class<?> getType();
}
12 changes: 8 additions & 4 deletions src/main/java/spoon/reflect/visitor/QueryVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,16 @@ public List<T> getResult() {
@SuppressWarnings("unchecked")
@Override
public void scan(CtElement element) {
if (element == null)
if (element == null)
return;
if (filter.getType().isAssignableFrom(element.getClass())) {
if (filter.matches((T) element)) {
result.add((T) element);
try {
{
if (filter.matches((T) element)) {
result.add((T) element);
}
}
} catch (ClassCastException e) {
// expected, some elements are not of type T
}
super.scan(element);
}
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/spoon/reflect/visitor/filter/AbstractFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,25 @@
import spoon.reflect.visitor.Filter;

/**
* This class defines an abstract filter that needs to be subclassed in order to
* define the matching criteria.
* Defines an abstract filter based on matching on the element types.
*
* @see spoon.reflect.visitor.Filter#matches(CtElement)
* Not necessary in simple cases thanks to the use of runtime reflection.
*/
public abstract class AbstractFilter<T extends CtElement> implements Filter<T> {

Class<T> type;
private Class<T> type;

/**
* Creates a filter with the type of the potentially matching elements.
*/
// TODO: INFER TYPE BY INTROSPECTION
@SuppressWarnings("unchecked")
public AbstractFilter(Class<?> type) {
this.type = (Class<T>) type;
}

public Class<T> getType() {
return type;
@Override
public boolean matches(T element) {
return type.isAssignableFrom(element.getClass());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
import java.lang.annotation.Annotation;

import spoon.reflect.declaration.CtElement;
import spoon.reflect.visitor.Filter;

/**
* This filter matches all the elements annotated with a given annotation type.
*/
public class AnnotationFilter<E extends CtElement> extends AbstractFilter<E> {
Class<? extends Annotation> annotationType;
public class AnnotationFilter<E extends CtElement> extends TypeFilter<E> {

private Class<? extends Annotation> annotationType;

/**
* Creates the filter.
Expand All @@ -47,7 +49,8 @@ public AnnotationFilter(Class<E> elementType,
this.annotationType = annotationType;
}

@Override
public boolean matches(E element) {
return element.getAnnotation(annotationType) != null;
return super.matches(element) && element.getAnnotation(annotationType) != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ public CompositeFilter(FilteringOperator operator, Filter<T>... filters) {
}

private boolean hasMatch(Filter<T> filter, T element) {
if (filter.getType().isAssignableFrom(element.getClass())) {
try {
return filter.matches(element);
} catch (ClassCastException e) {
return false;
}
return false;
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,16 @@
/**
* This simple filter matches all the accesses to a given field.
*/
public class FieldAccessFilter extends AbstractFilter<CtFieldAccess<?>> {
CtFieldReference<?> field;

public class FieldAccessFilter extends VariableAccessFilter<CtFieldAccess<?>> {

/**
* Creates a new field access filter.
*
* @param field
* the accessed field
*/
public FieldAccessFilter(CtFieldReference<?> field) {
super(CtFieldAccess.class);
this.field = field;
super(field);
}

public boolean matches(CtFieldAccess<?> fieldAccess) {
return fieldAccess.getVariable().equals(field);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@

import spoon.reflect.code.CtInvocation;
import spoon.reflect.reference.CtExecutableReference;
import spoon.reflect.visitor.Filter;

/**
* This simple filter matches all the accesses to a given executable or any
* executable that overrides it.
*/
public class InvocationFilter extends AbstractFilter<CtInvocation<?>> {
CtExecutableReference<?> executable;
public class InvocationFilter implements Filter<CtInvocation<?>> {

private CtExecutableReference<?> executable;

/**
* Creates a new invocation filter.
Expand All @@ -34,10 +36,10 @@ public class InvocationFilter extends AbstractFilter<CtInvocation<?>> {
* the executable to be tested for being invoked
*/
public InvocationFilter(CtExecutableReference<?> executable) {
super(CtInvocation.class);
this.executable = executable;
}

@Override
public boolean matches(CtInvocation<?> invocation) {
return invocation.getExecutable().isOverriding(executable);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,15 @@
import spoon.reflect.code.CtCFlowBreak;
import spoon.reflect.code.CtReturn;
import spoon.reflect.code.CtThrow;
import spoon.reflect.visitor.Filter;

/**
* This simple filter matches all the occurrences of a return or a throw
* statement (end of execution flow).
*/
public class ReturnOrThrowFilter extends AbstractFilter<CtCFlowBreak> {

/**
* Creates a filter.
*/
public ReturnOrThrowFilter() {
super(CtCFlowBreak.class);
}
public class ReturnOrThrowFilter implements Filter<CtCFlowBreak> {

@Override
public boolean matches(CtCFlowBreak cflowBreak) {
return (cflowBreak instanceof CtReturn)
|| (cflowBreak instanceof CtThrow);
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/spoon/reflect/visitor/filter/TypeFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

/**
* This simple filter matches all the elements of a given type.
*
*/
public class TypeFilter<T extends CtElement> extends AbstractFilter<T> {

Expand All @@ -34,7 +35,4 @@ public TypeFilter(Class<?> type) {
super(type);
}

public boolean matches(T element) {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@

import spoon.reflect.code.CtVariableAccess;
import spoon.reflect.reference.CtVariableReference;
import spoon.reflect.visitor.Filter;

/**
* This simple filter matches all the accesses to a given field.
*/
public class VariableAccessFilter extends AbstractFilter<CtVariableAccess<?>> {
public class VariableAccessFilter<T extends CtVariableAccess<?>> implements Filter<T> {
CtVariableReference<?> variable;

/**
Expand All @@ -33,11 +34,12 @@ public class VariableAccessFilter extends AbstractFilter<CtVariableAccess<?>> {
* the accessed variable
*/
public VariableAccessFilter(CtVariableReference<?> variable) {
super(CtVariableAccess.class);
this.variable = variable;
}

public boolean matches(CtVariableAccess<?> variableAccess) {
@Override
public boolean matches(T variableAccess) {
return variableAccess.getVariable().equals(variable);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void visitCtForEach(CtForEach foreach) {
for (Object element : value) {
CtStatement b = foreach.getFactory().Core().clone(body);
for (CtVariableAccess<?> va : Query.getElements(b,
new VariableAccessFilter(foreach.getVariable()
new VariableAccessFilter<CtVariableAccess<?>>(foreach.getVariable()
.getReference()))) {
va.replace((CtElement) element);
}
Expand Down
43 changes: 43 additions & 0 deletions src/test/java/spoon/test/filters/FilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,27 @@

import spoon.Launcher;
import spoon.compiler.SpoonResourceHelper;
import spoon.reflect.code.CtCFlowBreak;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtFieldAccess;
import spoon.reflect.code.CtNewClass;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtField;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtNamedElement;
import spoon.reflect.declaration.CtVariable;
import spoon.reflect.factory.Factory;
import spoon.reflect.reference.CtFieldReference;
import spoon.reflect.visitor.Query;
import spoon.reflect.visitor.filter.AnnotationFilter;
import spoon.reflect.visitor.filter.CompositeFilter;
import spoon.reflect.visitor.filter.FieldAccessFilter;
import spoon.reflect.visitor.filter.FilteringOperator;
import spoon.reflect.visitor.filter.InvocationFilter;
import spoon.reflect.visitor.filter.NameFilter;
import spoon.reflect.visitor.filter.RegexFilter;
import spoon.reflect.visitor.filter.ReturnOrThrowFilter;
import spoon.reflect.visitor.filter.TypeFilter;
import spoon.support.reflect.declaration.CtMethodImpl;
import spoon.test.TestUtils;
Expand All @@ -46,6 +55,40 @@ public void testFilters() throws Exception {
assertEquals(2, expressions.size());
}

@Test
public void testReturnOrThrowFilter() throws Exception {
CtClass<?> foo = factory.Package().get("spoon.test.filters").getType("Foo");
assertEquals("Foo", foo.getSimpleName());
List<CtCFlowBreak> expressions = foo.getElements(new ReturnOrThrowFilter());
assertEquals(2, expressions.size());
}


@Test
public void testFieldAccessFilter() throws Exception {
// also specifies VariableAccessFilter since FieldAccessFilter is only a VariableAccessFilter with additional static typing
CtClass<?> foo = factory.Package().get("spoon.test.filters").getType("Foo");
assertEquals("Foo", foo.getSimpleName());

List<CtNamedElement> elements = foo.getElements(new NameFilter<>("i"));
assertEquals(1, elements.size());

CtFieldReference ref = (CtFieldReference)(elements.get(0)).getReference();
List<CtFieldAccess<?>> expressions = foo.getElements(new FieldAccessFilter(ref));
assertEquals(2, expressions.size());
}


@Test
public void testAnnotationFilter() throws Exception {
CtClass<?> foo = factory.Package().get("spoon.test.filters").getType("Foo");
assertEquals("Foo", foo.getSimpleName());
List<CtElement> expressions = foo.getElements(new AnnotationFilter<>(SuppressWarnings.class));
assertEquals(2, expressions.size());
List<CtMethod> methods = foo.getElements(new AnnotationFilter<>(CtMethod.class, SuppressWarnings.class));
assertEquals(1, methods.size());
}

@SuppressWarnings("rawtypes")
@Test
public void filteredElementsAreOfTheCorrectType() throws Exception {
Expand Down
11 changes: 10 additions & 1 deletion src/test/java/spoon/test/filters/Foo.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
package spoon.test.filters;

@SuppressWarnings("bar")
class Foo {
int i;
void foo() {
int x = 3;
int z;
z= x+1;
z= x+i;
System.out.println(z);
}

@SuppressWarnings("foo")
int bar () {
if (0==1) {
throw new RuntimeException();
}
return i;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.factory.Factory;
import spoon.reflect.reference.CtExecutableReference;
import spoon.reflect.visitor.Filter;
import spoon.reflect.visitor.Query;
import spoon.reflect.visitor.filter.AbstractFilter;
import spoon.reflect.visitor.filter.AbstractReferenceFilter;
Expand Down Expand Up @@ -243,7 +244,7 @@ public boolean matches(CtExecutableReference<?> reference) {
}

private List<CtConstructor<?>> getConstructorsByClass(final String myClass) {
return Query.getElements(factory, new AbstractFilter<CtConstructor<?>>(CtConstructor.class) {
return Query.getElements(factory, new Filter<CtConstructor<?>>() {
@Override
public boolean matches(CtConstructor<?> element) {
return myClass.equals(((CtClass<?>) element.getParent()).getSimpleName());
Expand All @@ -261,7 +262,7 @@ public boolean matches(CtExecutableReference<?> reference) {
}

private CtClass<?> getCtClassByName(final String name) {
return Query.getElements(factory, new AbstractFilter<CtClass<?>>(CtClass.class) {
return Query.getElements(factory, new Filter<CtClass<?>>() {
@Override
public boolean matches(CtClass<?> element) {
return name.equals(element.getSimpleName());
Expand Down

0 comments on commit 8489e68

Please sign in to comment.