Fix filter blocks with shared fields and derived data variables (#2088)

Fixes #2087

Author: Vampire

spock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.java

@@ -905,7 +905,7 @@ private void createFilterMethod() {
 
     MethodNode filterMethod = new MethodNode(
       InternalIdentifiers.getFilterName(filterBlock.getParent().getAst().getName()),
-      Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC,
+      Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC,
       ClassHelper.VOID_TYPE,
       dataProcessorVars
         .stream()

spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java

@@ -29,7 +29,9 @@ public IDataIterator createFeatureDataIterator(SpockExecutionContext context) {
     if (context.getCurrentFeature().getDataProcessorMethod() == null) {
       return new SingleEmptyIterationDataIterator();
     }
-    return new DataProcessorIterator(supervisor, context, new FeatureDataProviderIterator(supervisor, context));
+    return new IterationFilterIterator(supervisor, context,
+      new DataProcessorIterator(supervisor, context,
+        new FeatureDataProviderIterator(supervisor, context)));
   }
 
   private abstract static class BaseDataIterator implements IDataIterator {
@@ -201,6 +203,88 @@ public void close() throws Exception {
     }
   }
 
+  private static class IterationFilterIterator extends BaseDataIterator {
+    private final IDataIterator delegate;
+    private final List<String> dataVariableNames;
+    private final boolean logFilteredIterations;
+    private IStackTraceFilter stackTraceFilter;
+
+    private IterationFilterIterator(IRunSupervisor supervisor, SpockExecutionContext context, IDataIterator delegate) {
+      super(supervisor, context);
+      this.delegate = delegate;
+      dataVariableNames = delegate.getDataVariableNames();
+
+      RunnerConfiguration runnerConfiguration = context
+        .getRunContext()
+        .getConfiguration(RunnerConfiguration.class);
+      logFilteredIterations = runnerConfiguration.logFilteredIterations;
+      if (logFilteredIterations) {
+        stackTraceFilter = runnerConfiguration.filterStackTrace ? new StackTraceFilter(context.getSpec()) : new DummyStackTraceFilter();
+      }
+    }
+
+    @Override
+    public int getEstimatedNumIterations() {
+      return delegate.getEstimatedNumIterations();
+    }
+
+    @Override
+    public List<String> getDataVariableNames() {
+      return dataVariableNames;
+    }
+
+    @Override
+    public void close() throws Exception {
+      delegate.close();
+    }
+
+    @Override
+    public boolean hasNext() {
+      return delegate.hasNext();
+    }
+
+    @Override
+    public Object[] next() {
+      while (true) {
+        Object[] next = delegate.next();
+
+        // delegate.next() will return null if an error occurred
+        if (next == null) {
+          return null;
+        }
+
+        MethodInfo filterMethod = context.getCurrentFeature().getFilterMethod();
+        if (filterMethod == null) {
+          return next;
+        }
+
+        try {
+          // do not use invokeRaw here, as that would report Assertion Error to the supervisor
+          filterMethod.invoke(context.getSharedInstance(), next);
+          return next;
+        } catch (AssertionError ae) {
+          if (logFilteredIterations) {
+            StringJoiner stringJoiner = new StringJoiner(", ", "Filtered iteration [", "]:\n");
+            for (int i = 0; i < dataVariableNames.size(); i++) {
+              stringJoiner.add(dataVariableNames.get(i) + ": " + next[i]);
+            }
+            StringWriter sw = new StringWriter();
+            sw.write(stringJoiner.toString());
+            stackTraceFilter.filter(ae);
+            try (PrintWriter pw = new PrintWriter(sw)) {
+              ae.printStackTrace(pw);
+            }
+            System.err.println(sw);
+          }
+          // filter block does not like these values, try next ones if available
+        } catch (Throwable t) {
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(filterMethod, t, getErrorContext()));
+          return null;
+        }
+      }
+    }
+  }
+
   private static class DataProcessorIterator extends BaseDataIterator {
     private final IDataIterator delegate;
     private final List<String> dataVariableNames;
@@ -262,8 +346,6 @@ private static class FeatureDataProviderIterator extends BaseDataIterator {
     private final int estimatedNumIterations;
     private final List<String> dataVariableNames;
     private boolean firstIteration = true;
-    private boolean logFilteredIterations = true;
-    private IStackTraceFilter stackTraceFilter;
 
     public FeatureDataProviderIterator(IRunSupervisor supervisor, SpockExecutionContext context) {
       super(supervisor, context);
@@ -272,14 +354,6 @@ public FeatureDataProviderIterator(IRunSupervisor supervisor, SpockExecutionCont
       dataProviders = createDataProviders();
       dataProviderIterators = createDataProviderIterators();
       estimatedNumIterations = estimateNumIterations(dataProviderIterators);
-
-      RunnerConfiguration runnerConfiguration = context
-        .getRunContext()
-        .getConfiguration(RunnerConfiguration.class);
-      logFilteredIterations = runnerConfiguration.logFilteredIterations;
-      if (logFilteredIterations) {
-        stackTraceFilter = runnerConfiguration.filterStackTrace ? new StackTraceFilter(context.getSpec()) : new DummyStackTraceFilter();
-      }
     }
 
     @Override
@@ -308,60 +382,29 @@ public Object[] next() {
       }
       firstIteration = false;
 
-      while (true) {
-        // advances iterators and computes args
-        Object[] next = new Object[dataProviders.length];
-        for (int i = 0; i < dataProviders.length; ) {
-          try {
-            // if the filter block excluded an iteration
-            // this might be called after the last iteration
-            // so just return null if no further data is available
-            // to just cause the iteration to be skipped
-            if (!dataProviderIterators[i].hasNext()) {
-              return null;
-            }
-            Object[] nextValues = dataProviderIterators[i].next();
-            if (nextValues == null) {
-              return null;
-            }
-            System.arraycopy(nextValues, 0, next, i, nextValues.length);
-            i += nextValues.length;
-          } catch (Throwable t) {
-            supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t, getErrorContext()));
+      // advances iterators and computes args
+      Object[] next = new Object[dataProviders.length];
+      for (int i = 0; i < dataProviders.length; ) {
+        try {
+          // if the filter block excluded an iteration
+          // this might be called after the last iteration
+          // so just return null if no further data is available
+          // to just cause the iteration to be skipped
+          if (!dataProviderIterators[i].hasNext()) {
             return null;
           }
-        }
-
-        MethodInfo filterMethod = context.getCurrentFeature().getFilterMethod();
-
-        if (filterMethod == null) {
-          return next;
-        }
-
-        try {
-          // do not use invokeRaw here, as that would report Assertion Error to the supervisor
-          filterMethod.invoke(null, next);
-          return next;
-        } catch (AssertionError ae) {
-          if (logFilteredIterations) {
-            StringJoiner stringJoiner = new StringJoiner(", ", "Filtered iteration [", "]:\n");
-            for (int i = 0; i < dataVariableNames.size(); i++) {
-              stringJoiner.add(dataVariableNames.get(i) + ": " + next[i]);
-            }
-            StringWriter sw = new StringWriter();
-            sw.write(stringJoiner.toString());
-            stackTraceFilter.filter(ae);
-            try (PrintWriter pw = new PrintWriter(sw)) {
-              ae.printStackTrace(pw);
-            }
-            System.err.println(sw);
+          Object[] nextValues = dataProviderIterators[i].next();
+          if (nextValues == null) {
+            return null;
           }
-          // filter block does not like these values, try next ones if available
+          System.arraycopy(nextValues, 0, next, i, nextValues.length);
+          i += nextValues.length;
         } catch (Throwable t) {
-          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(filterMethod, t, getErrorContext()));
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t, getErrorContext()));
           return null;
         }
       }
+      return next;
     }
 
     @Override

spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataTables.groovy

@@ -400,6 +400,84 @@ i != 6
     result.testsSucceededCount == 6
   }
 
+  @Issue("https://github.com/spockframework/spock/issues/2087")
+  def "filter block supports derived data variables"() {
+    when:
+    def result = runner.runFeatureBody '''
+expect:
+i != 2
+
+where:
+i << (1..2)
+b = 42
+
+filter:
+i != 2
+b == 42
+    '''
+
+    then:
+    result.testsAbortedCount == 0
+    result.testsFailedCount == 0
+    result.testsSkippedCount == 0
+    result.testsStartedCount == 2
+    result.testsSucceededCount == 2
+  }
+
+  @Issue("https://github.com/spockframework/spock/issues/2087")
+  def "filter block supports shared fields"() {
+    when:
+    def result = runner.runSpecBody '''
+@Shared
+def b = 42
+
+def "a feature"() {
+  expect:
+  i != 2
+
+  where:
+  i << (1..2)
+
+  filter:
+  i != 2
+  b == 42
+}
+    '''
+
+    then:
+    result.testsAbortedCount == 0
+    result.testsFailedCount == 0
+    result.testsSkippedCount == 0
+    result.testsStartedCount == 2
+    result.testsSucceededCount == 2
+  }
+
+  def "filter block supports static fields"() {
+    when:
+    def result = runner.runSpecBody '''
+static b = 42
+
+def "a feature"() {
+  expect:
+  i != 2
+
+  where:
+  i << (1..2)
+
+  filter:
+  i != 2
+  b == 42
+}
+    '''
+
+    then:
+    result.testsAbortedCount == 0
+    result.testsFailedCount == 0
+    result.testsSkippedCount == 0
+    result.testsStartedCount == 2
+    result.testsSucceededCount == 2
+  }
+
   def "filter block can not exclude all iterations"() {
     when:
     runner.runFeatureBody '''

spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/InvalidWhereBlocks.groovy

@@ -205,25 +205,6 @@ def foo() {
     e.message.contains("@Shared")
   }
 
-  def "filter block may not reference local variables"() {
-    when:
-    compiler.compileFeatureBody '''
-def local = 1
-
-expect:
-x == 1
-
-where:
-x = 1
-
-filter:
-local
-    '''
-
-    then:
-    thrown(SyntaxException)
-  }
-
   def 'referencing a data variable in a data provider is not allowed'() {
     when:
     runner.runFeatureBody """

spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/DataTablesAstSpec/filter_block_becomes_its_own_method.groovy

@@ -37,7 +37,7 @@ public static org.spockframework.runtime.model.DataVariableMultiplication[] $spo
     return new org.spockframework.runtime.model.DataVariableMultiplication[]{new org.spockframework.runtime.model.DataVariableMultiplication(new java.lang.String[]{'a', 'b'}, new org.spockframework.runtime.model.DataVariableMultiplicationFactor(new java.lang.String[]{'a'}), new org.spockframework.runtime.model.DataVariableMultiplicationFactor(new java.lang.String[]{'b'}))}
 }
 
-public static void $spock_feature_0_0filter(java.lang.Object a, java.lang.Object b) {
+public void $spock_feature_0_0filter(java.lang.Object a, java.lang.Object b) {
     org.spockframework.runtime.ErrorCollector $spock_errorCollector = org.spockframework.runtime.ErrorRethrower.INSTANCE
     org.spockframework.runtime.ValueRecorder $spock_valueRecorder = new org.spockframework.runtime.ValueRecorder()
     try {