diff --git a/src/main/java/spoon/reflect/visitor/FilterPipe.java b/src/main/java/spoon/reflect/visitor/FilterPipe.java index ec5e1ac31fb..dd7dd14639c 100644 --- a/src/main/java/spoon/reflect/visitor/FilterPipe.java +++ b/src/main/java/spoon/reflect/visitor/FilterPipe.java @@ -3,22 +3,19 @@ import spoon.reflect.declaration.CtElement; import spoon.reflect.visitor.chain.ElementPipeImpl; import spoon.reflect.visitor.filter.AbstractFilter; -import spoon.support.util.RtHelper; import spoon.support.util.SafeInvoker; /** + * if the input element is matching the filter (or there is no filter), + * then element is sent to output consumer * - * @param type of element produced by this pipe + * @param type of element produced by this pipe - matched by the filter */ -public class FilterPipe extends ElementPipeImpl { +public class FilterPipe extends ElementPipeImpl { private Filter filter; - private SafeInvoker> invoke_matches = new SafeInvoker<>(CtElement.class, "matches"); - private SafeInvoker> invoke_setFilterInput = new SafeInvoker<>(CtElement.class, "setFilterInput"); - /* - * Type which is accepted by the Filter#matches() - */ - private Class filterType; + private SafeInvoker> invoke_matches = new SafeInvoker<>("matches"); + private SafeInvoker> invoke_setFilterInput = new SafeInvoker<>("setFilterInput"); public FilterPipe() { } @@ -28,27 +25,31 @@ public FilterPipe(Filter filter) { } @Override - public void accept(CtElement element) { - if (element == null) { - return; + public boolean matches(CtElement element) { + if (filter == null) { + return true; } //first check if filter matches - boolean matches = true; - if (filter != null) { - try { - if ((filterType == null || filterType.isAssignableFrom(element.getClass()))) { - matches = filter.matches(element); - } - } catch (ClassCastException e) { - // expected, some elements are not of type T - // Still need to protect from CCE, if users extend Filter (instead of AbstractFilter) directly, - // but with concrete type parameter - matches = false; + if (filter instanceof AbstractFilter) { + //Check if required type of AbstractFilter is matching + @SuppressWarnings("rawtypes") + Class filterType = ((AbstractFilter) filter).getType(); + if (filterType != null && filterType.isAssignableFrom(element.getClass()) == false) { + //required type is defined and it is not matching + return false; } } - if (matches) { - //it matches. Send the element to output - fireElement(element); + if (invoke_matches.isParameterTypeAssignableFrom(element) == false) { + //the element is not matching with parameter type of Filter#matches method + return false; + } + try { + return filter.matches(element); + } catch (ClassCastException e) { + // expected, some elements are not of type T + // Still need to protect from CCE, if users extend Filter (instead of AbstractFilter) directly, + // but with concrete type parameter + return false; } } @@ -59,38 +60,31 @@ public Filter getFilter() { @SuppressWarnings("unchecked") public void setFilter(Filter filter) { this.filter = filter; - if (filter != null) { - if (filter instanceof AbstractFilter) { - filterType = ((AbstractFilter) filter).getType(); - } else { - Class[] params = RtHelper.getMethodParameterTypes(filter.getClass(), "matches", 1); - filterType = (Class) params[0]; - } - } else { - filterType = null; - } + invoke_matches.setDelegate(filter); + invoke_setFilterInput.setDelegate(filter); } - public Class getFilterType() { - return filterType; - } - - public void runOnScanner(CtElement inputElement, QueryVisitor p_queryVisitor) { - if (filter != null) { - if (filter instanceof ChainableFilter && inputElement != null) { + public CtElement getSearchScopeOfElement(CtElement inputElement) { + if (filter == null) { + //there is no filter, all the elements starting from inputElement has to be visited + return inputElement; + } + if (invoke_setFilterInput.hasMethod()) { + //the filter has method setFilterInput + if (invoke_setFilterInput.isParameterTypeAssignableFrom(inputElement)) { + //the inputElement may be matching with setFilterInput parameter type try { - ((ChainableFilter) filter).setFilterInput(inputElement); + invoke_setFilterInput.invoke(inputElement); } catch (ClassCastException e) { - //the filter does not accepts this as input. Ignore this not matching element - return; + //the filter does not accepts this as input. Ignore this not matching element. + //Do not run scanner on it + return null; } } - if (filter instanceof ScopeAwareFilter) { - queryVisitor.scan(((ScopeAwareFilter) filter).getSearchScope()); - } else { - queryVisitor.scan(inputElement); - } } - scan(element); + if (filter instanceof ScopeAwareFilter) { + return ((ScopeAwareFilter) filter).getSearchScope(); + } + return inputElement; } } diff --git a/src/main/java/spoon/reflect/visitor/Query.java b/src/main/java/spoon/reflect/visitor/Query.java index 4771e10a63c..6f379de6818 100644 --- a/src/main/java/spoon/reflect/visitor/Query.java +++ b/src/main/java/spoon/reflect/visitor/Query.java @@ -66,7 +66,10 @@ public static List getElements(Factory factory, } /** - * Returns all the program elements that match the filter. + * Returns all the program elements that match the filter starting from defined rootElement. + * This method ignores search scope provided by {@link ScopeAwareFilter#getSearchScope()}, + * so normally it is better to call {@link #getElements(ScopeAwareFilter)} + * or {@link CtElement#query(Filter)} * * @param * the type of the sought program elements @@ -77,7 +80,7 @@ public static List getElements(Factory factory, */ public static List getElements( CtElement rootElement, Filter filter) { - List list = new ArrayList<>(); + final List list = new ArrayList<>(); QueryVisitor visitor = new QueryVisitor<>(filter, new ElementConsumer() { @Override public void accept(E element) { diff --git a/src/main/java/spoon/reflect/visitor/QueryBuilder.java b/src/main/java/spoon/reflect/visitor/QueryBuilder.java index 83f79fdb602..1680757405e 100644 --- a/src/main/java/spoon/reflect/visitor/QueryBuilder.java +++ b/src/main/java/spoon/reflect/visitor/QueryBuilder.java @@ -19,7 +19,6 @@ import java.util.ArrayList; import java.util.List; -import spoon.SpoonException; import spoon.reflect.declaration.CtElement; /** @@ -84,7 +83,8 @@ public QueryBuilder then(Filter filter) { * and it's output is registered to call next level of the chain * @return this instance, so it can be used for next query building. */ - public QueryBuilder then(ElementPipe pipe) { + @SuppressWarnings("unchecked") + public QueryBuilder then(ElementPipe pipe) { chainItems.add(pipe); return (QueryBuilder) this; } @@ -94,6 +94,7 @@ public QueryBuilder then(ElementPipe pipe) { * If the consumer does not implements {@link ElementSupplier}, then it is terminal consumer of this chain * @return this instance, so it can be used for next query building. */ + @SuppressWarnings("unchecked") public QueryBuilder then(ElementConsumer consumer) { chainItems.add(consumer); return (QueryBuilder) this; @@ -152,12 +153,8 @@ public void forEach(ElementConsumer consumer) { //there is no filter defined. Do not call consumer return; } - if (chainItems.get(0) instanceof ScopeAwareFilter) { - //first filter is ScopeAwareFilter, so it computes start element of AST traversal. Use null as start element - myConsumer.accept(null); - } else { - throw new SpoonException("The QueryBuilder is missing the input element. Call QueryBuilder.onInput(element)"); - } + //some filters can define starting point of AST traversal, so null makes sense for them + myConsumer.accept(null); } else { for (CtElement element : inputs) { myConsumer.accept(element); diff --git a/src/main/java/spoon/reflect/visitor/QueryVisitor.java b/src/main/java/spoon/reflect/visitor/QueryVisitor.java index 3f8216acc63..4364d432f17 100644 --- a/src/main/java/spoon/reflect/visitor/QueryVisitor.java +++ b/src/main/java/spoon/reflect/visitor/QueryVisitor.java @@ -17,7 +17,6 @@ package spoon.reflect.visitor; import spoon.reflect.declaration.CtElement; -import spoon.reflect.visitor.chain.SafeElementConsumerWrapper; /** * A simple visitor that takes a filter and returns all the elements that match @@ -46,26 +45,29 @@ public QueryVisitor(ElementConsumer visitedElementConsumer) { * Constructs a query visitor with a given filter. */ @SuppressWarnings("unchecked") - public QueryVisitor(Filter filter) { - filterPipe.setFilter(filter); + public QueryVisitor(Filter filter) { + filterPipe.setFilter((Filter) filter); } /** * Constructs a query visitor with a given filter and Consumer. */ @SuppressWarnings("unchecked") - public QueryVisitor(Filter filter, ElementConsumer outputConsumer) { + public QueryVisitor(Filter filter, ElementConsumer outputConsumer) { this(filter); setOutput(outputConsumer); } /** - * starts traversal of AST from element. Same like {@link #scan(CtElement))} + * starts traversal of AST from element, but comparing to {@link #scan(CtElement)} The configured filter may influence the starting point */ @Override public void accept(CtElement inputElement) { - filterPipe.runOnScanner(inputElement, this); + CtElement startElement = filterPipe.getSearchScopeOfElement(inputElement); + if (startElement != null) { + scan(startElement); + } } /** @@ -74,18 +76,12 @@ public void accept(CtElement inputElement) { @SuppressWarnings("unchecked") @Override public void setOutput(ElementConsumer visitedElementsConsumer) { - //use SafeElementConsumerWrapper to effectively call the consumer only with the elements which it can accept - //without unwanted CCE - if (visitedElementsConsumer instanceof SafeElementConsumerWrapper) { - this.visitedElementConsumer = (SafeElementConsumerWrapper) visitedElementsConsumer; - } else { - this.visitedElementConsumer = new SafeElementConsumerWrapper<>(visitedElementsConsumer); - } + filterPipe.setOutput(visitedElementsConsumer); } @Override public void scan(CtElement element) { - visitedElementConsumer.accept(element); + filterPipe.accept(element); super.scan(element); } } diff --git a/src/main/java/spoon/reflect/visitor/chain/ElementPipeImpl.java b/src/main/java/spoon/reflect/visitor/chain/ElementPipeImpl.java index 59b277fe58f..8000fb594be 100644 --- a/src/main/java/spoon/reflect/visitor/chain/ElementPipeImpl.java +++ b/src/main/java/spoon/reflect/visitor/chain/ElementPipeImpl.java @@ -29,11 +29,37 @@ public void setOutput(ElementConsumer outputConsumer) { @Override public void accept(I element) { + if (element == null) { + return; + } + onAccept(element); + boolean matches; + try { + matches = matches(element); + } catch (ClassCastException e) { + // expected, some elements are not of type T + // Still need to protect from CCE, if users extend Filter (instead of AbstractFilter) directly, + // but with concrete type parameter + matches = false; + } + if (matches) { + //it matches. Send the element to output + onMatches(element); + } + } + + protected void onAccept(I p_element) { + } + + protected void onMatches(I element) { fireElement(element); } + public boolean matches(I element) { + return true; + } + protected void fireElement(CtElement element) { outputConsumer.accept(element); } - } diff --git a/src/main/java/spoon/reflect/visitor/chain/SafeElementConsumerWrapper.java b/src/main/java/spoon/reflect/visitor/chain/SafeElementConsumerWrapper.java index 5d26b934aff..a50b8874c6a 100644 --- a/src/main/java/spoon/reflect/visitor/chain/SafeElementConsumerWrapper.java +++ b/src/main/java/spoon/reflect/visitor/chain/SafeElementConsumerWrapper.java @@ -1,20 +1,21 @@ package spoon.reflect.visitor.chain; +import spoon.Launcher; import spoon.reflect.declaration.CtElement; import spoon.reflect.visitor.ElementConsumer; import spoon.reflect.visitor.ElementPipe; import spoon.support.util.SafeInvoker; /** - * wraps another {@link ElementConsumer} and silently catches all ClassCastExceptions, - * which would be thrown by calling of ElementConsumer which accepts different class + * wraps another {@link ElementConsumer} and silently catches all ClassCastExceptions, + * which would be thrown by calling of ElementConsumer which accepts different class * * @param type of elements accepted by this ElementConsumer */ public class SafeElementConsumerWrapper implements ElementPipe { - - private SafeInvoker> safeInvoker = new SafeInvoker<>(CtElement.class, "accept"); - + + private SafeInvoker> invoke_accept = new SafeInvoker<>("accept"); + public SafeElementConsumerWrapper() { } @@ -25,21 +26,28 @@ public SafeElementConsumerWrapper(ElementConsumer delegate) { @Override public void accept(CtElement element) { - safeInvoker.invoke(element); + try { + invoke_accept.invoke(element); + } catch (ClassCastException e) { + // expected, some elements are not of type T + // Still need to protect from CCE, if client uses Lambda expression which does not provide real parameter type. + //log this exception with lower level, because it can be expected behavior that it fails here... + //but log it anyway to make it possible to debug cases when it really fails because of unexpected reasons + Launcher.LOGGER.trace("ElementConsumer does not accepts such elements. It may be expected behavior.", e); + } } @Override - public void setOutput(ElementConsumer delegate) - { - this.safeInvoker.setDelegate(delegate); + public void setOutput(ElementConsumer delegate) { + this.invoke_accept.setDelegate(delegate); } public ElementConsumer getOutput() { - return safeInvoker.getDelegate(); + return invoke_accept.getDelegate(); } @SuppressWarnings("unchecked") public Class getOutputType() { - return (Class) safeInvoker.getParamType(); + return (Class) invoke_accept.getParamType(); } } diff --git a/src/main/java/spoon/reflect/visitor/filter/AbstractFilter.java b/src/main/java/spoon/reflect/visitor/filter/AbstractFilter.java index a69c94fd200..0bd5b46d272 100644 --- a/src/main/java/spoon/reflect/visitor/filter/AbstractFilter.java +++ b/src/main/java/spoon/reflect/visitor/filter/AbstractFilter.java @@ -16,6 +16,8 @@ */ package spoon.reflect.visitor.filter; +import java.lang.reflect.Method; + import spoon.SpoonException; import spoon.reflect.declaration.CtElement; import spoon.reflect.visitor.Filter; @@ -43,11 +45,11 @@ public AbstractFilter(Class type) { */ @SuppressWarnings("unchecked") public AbstractFilter() { - Class[] params = RtHelper.getMethodParameterTypes(getClass(), "matches", 1); - if (params == null) { + Method method = RtHelper.getMethod(getClass(), "matches", 1); + if (method == null) { throw new SpoonException("The method matches with one parameter was not found on the class " + getClass().getName()); } - this.type = (Class) params[0]; + this.type = (Class) method.getParameterTypes()[0]; } public Class getType() { diff --git a/src/main/java/spoon/support/util/RtHelper.java b/src/main/java/spoon/support/util/RtHelper.java index c14f97ed968..8a77c5b198b 100644 --- a/src/main/java/spoon/support/util/RtHelper.java +++ b/src/main/java/spoon/support/util/RtHelper.java @@ -16,7 +16,6 @@ */ package spoon.support.util; -import spoon.SpoonException; import spoon.reflect.code.CtExpression; import spoon.reflect.code.CtInvocation; import spoon.reflect.code.CtLiteral; diff --git a/src/main/java/spoon/support/util/SafeInvoker.java b/src/main/java/spoon/support/util/SafeInvoker.java index 7e4254c7e0b..8803c32285a 100644 --- a/src/main/java/spoon/support/util/SafeInvoker.java +++ b/src/main/java/spoon/support/util/SafeInvoker.java @@ -3,36 +3,43 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import spoon.Launcher; import spoon.SpoonException; public class SafeInvoker { - final private Class genericType; - final private String methodName; - + private final String methodName; + private T delegate; private Class paramType; private Method method; - public SafeInvoker(Class genericType, String methodName) { - this.genericType = genericType; + /** + * + * @param methodName + */ + public SafeInvoker(String methodName) { this.methodName = methodName; } - + + /** + * @return type of first parameter of invoked method or null if delegate does not have this method + */ + public Class getParamType() { + return paramType; + } + + /** + * @return the object on which is the method invoked + */ public T getDelegate() { return delegate; } public void setDelegate(T delegate) { if (delegate != null) { - method = RtHelper.getMethod(delegate.getClass(), "methodName", 1); - if(method != null) { - Class[] params = method.getParameterTypes(); - paramType = params[0]; - if (paramType.isAssignableFrom(genericType)) { - //we are not able to detect type (e.g. in case of Lambda expression or it really accepts everything) - paramType = null; - } + method = RtHelper.getMethod(delegate.getClass(), methodName, 1); + if (method != null) { + paramType = method.getParameterTypes()[0]; + method.setAccessible(true); } else { paramType = null; } @@ -43,56 +50,36 @@ public void setDelegate(T delegate) { this.delegate = delegate; } - public Class getParamType() { - return paramType; - } - - public boolean canInvoke() { + /** + * @return true if delegate has a required method + */ + public boolean hasMethod() { return method != null; } - - public Object invoke(Object param) { - if (delegate != null && param != null) { - if (paramType==null) { - //we do not know the type. So we must catch ClassCastException - try { - return invokeInternal(param); - } catch (ClassCastException e) { - // expected, some elements are not of type T - // Still need to protect from CCE, if client uses Lambda expression which does not provide real parameter type. - return handleNotInvoked(param, e); - } - } else { - //we know the accepted type. Do not catch ClassCaseException. - //It is better because we do not want to hide ClassCastException thrown in body of ElementConsumer#accept(...) - if (paramType.isAssignableFrom(param.getClass())) { - return invokeInternal(param); - } - } - } - return handleNotInvoked(param, null); - } - - private Object invokeInternal(Object param) { - try { - return method.invoke(delegate, param); - } catch (IllegalAccessException e) { - throw new SpoonException(e); - } catch (IllegalArgumentException e) { - throw new SpoonException(e); - } catch (InvocationTargetException e) { - if (e.getTargetException() instanceof RuntimeException) { - throw (RuntimeException) e.getTargetException(); - } - throw new SpoonException(e.getTargetException()); - } + + /** + * + * @param parameter + * @return true if parameter might be accepted by the invoked method + */ + public boolean isParameterTypeAssignableFrom(Object parameter) { + return paramType != null && parameter != null && paramType.isAssignableFrom(parameter.getClass()); } - protected Object handleNotInvoked(Object param, ClassCastException e) { - if (e != null) { - //log this exception with lower level, because it can be expected behavior that it fails here... - //but log it anyway to make it possible to debug cases when it really fails because of unexpected reasons - Launcher.LOGGER.trace("Safe invoke failed", e); + public Object invoke(Object parameter) { + if (method != null && delegate != null && parameter != null) { + try { + return method.invoke(delegate, parameter); + } catch (IllegalAccessException e) { + throw new SpoonException(e); + } catch (IllegalArgumentException e) { + throw new SpoonException(e); + } catch (InvocationTargetException e) { + if (e.getTargetException() instanceof RuntimeException) { + throw (RuntimeException) e.getTargetException(); + } + throw new SpoonException(e.getTargetException()); + } } return null; } diff --git a/src/test/java/spoon/test/filters/FilterTest.java b/src/test/java/spoon/test/filters/FilterTest.java index 77374f278a1..9ccf37d5117 100644 --- a/src/test/java/spoon/test/filters/FilterTest.java +++ b/src/test/java/spoon/test/filters/FilterTest.java @@ -43,6 +43,7 @@ import spoon.reflect.visitor.Query; import spoon.reflect.visitor.QueryBuilder; import spoon.reflect.visitor.QueryVisitor; +import spoon.reflect.visitor.chain.ElementPipeImpl; import spoon.reflect.visitor.filter.AbstractFilter; import spoon.reflect.visitor.filter.AnnotationFilter; import spoon.reflect.visitor.filter.CompositeFilter; @@ -544,14 +545,26 @@ public void testQueryBuilderWithFilterChain() throws Exception { launcher.run(); class Context { - + CtMethod method; + CtMethod overridenMethod; } Context context = new Context(); launcher.getFactory().Package().getRootPackage().query(new TypeFilter(CtMethod.class)) - .then(CtMethod m->context.method=m) - .then(new OverriddenMethodFilter()). + .then(new ElementPipeImpl(){ + @Override + protected void onAccept(CtMethod element) { + context.method = element; + } + }).then(new OverriddenMethodFilter()) + .then(new ElementPipeImpl(){ + @Override + protected void onAccept(CtMethod element) { + context.overridenMethod = element; + //TODO test + } + }); }