Fix regression with multi-assignment of method call result

caused by #1314 fixes #1332
spockframework · Jul 7, 2021 · 9da9608 · 9da9608
1 parent ac2afc8
commit 9da9608
Show file tree

Hide file tree

Showing 5 changed files with 143 additions and 14 deletions.
diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java
@@ -16,19 +16,17 @@
 
 package org.spockframework.compiler;
 
+import org.spockframework.compiler.model.*;
+import org.spockframework.util.*;
+
+import java.util.*;
+
 import org.codehaus.groovy.ast.*;
 import org.codehaus.groovy.ast.expr.*;
 import org.codehaus.groovy.ast.stmt.*;
 import org.codehaus.groovy.runtime.MetaClassHelper;
-import org.codehaus.groovy.syntax.Token;
-import org.codehaus.groovy.syntax.Types;
+import org.codehaus.groovy.syntax.*;
 import org.objectweb.asm.Opcodes;
-import org.spockframework.compiler.model.*;
-import org.spockframework.util.InternalIdentifiers;
-import org.spockframework.util.ObjectUtil;
-import org.spockframework.util.ReflectionUtil;
-
-import java.util.*;
 
 import static org.spockframework.compiler.AstUtil.createDirectMethodCall;
 
@@ -773,11 +771,20 @@ private Expression transformRhsExpressionIfNecessary(DeclarationExpression declE
     // as groovy would now interpret it as a `foo.call()` invocation on the local variable.
     if (rightExpression instanceof MethodCallExpression) {
       MethodCallExpression methodCallExpression = (MethodCallExpression)rightExpression;
-      if (methodCallExpression.isImplicitThis() &&
-        declExpr.getVariableExpression().getName().equals(methodCallExpression.getMethod().getText())) {
-        // change to explicit `this` to turn the expression to `foo = this.foo()`
-        methodCallExpression.setImplicitThis(false);
-      }
+      if (methodCallExpression.isImplicitThis())
+        if (declExpr.isMultipleAssignmentDeclaration()) {
+          ArgumentListExpression argumentListExpression = (ArgumentListExpression)declExpr.getLeftExpression();
+          argumentListExpression.getExpressions().stream()
+            .filter(VariableExpression.class::isInstance)
+            .map(VariableExpression.class::cast)
+            .map(VariableExpression::getName)
+            .filter(methodCallExpression.getMethod().getText()::equals)
+            .findAny()
+            .ifPresent(ignore-> methodCallExpression.setImplicitThis(false));
+        } else if (declExpr.getVariableExpression().getName().equals(methodCallExpression.getMethod().getText())) {
+          // change to explicit `this` to turn the expression to `foo = this.foo()`
+          methodCallExpression.setImplicitThis(false);
+        }
     }
     return rightExpression;
   }

diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/CleanupBlocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/CleanupBlocks.groovy
@@ -194,7 +194,19 @@ def feature() {
     foobar.size()
   }
 
+  @Issue("https://github.com/spockframework/spock/issues/1332")
+  def "cleanup blocks don't destroy method reference when invocation is assigned to a multi-assignment with the same name"() {
+    when:
+    def (foobar, b) = foobar()
+
+    then:
+    println(foobar)
+
+    cleanup:
+    foobar.size()
+  }
+
   def foobar() {
-    return "foo"
+    return ["foo", "bar"]
   }
 }
diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/ast/CleanupBlocksAstSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/ast/CleanupBlocksAstSpec.groovy
@@ -67,4 +67,66 @@ public void $spock_feature_0_0() {
 }'''
 
   }
+
+  @Issue("https://github.com/spockframework/spock/issues/1332")
+  def "cleanup rewrite keeps correct method reference for multi-assignments"() {
+    when:
+    def result = compiler.transpileSpecBody('''
+def "cleanup blocks don't destroy method reference when invocation is assigned to variable with the same name"() {
+  when:
+  def (foobar, b) = foobar()
+
+  then:
+  println(foobar)
+
+  cleanup:
+  foobar.size()
+}
+
+def foobar() {
+  return ["foo", "bar"]
+}''', EnumSet.of(Show.METHODS))
+    then:
+    result.source == '''\
+public java.lang.Object foobar() {
+    return ['foo', 'bar']
+}
+
+public void $spock_feature_0_0() {
+    org.spockframework.runtime.ErrorCollector $spock_errorCollector = org.spockframework.runtime.ErrorRethrower.INSTANCE
+    org.spockframework.runtime.ValueRecorder $spock_valueRecorder = new org.spockframework.runtime.ValueRecorder()
+    def (java.lang.Object foobar, java.lang.Object b) = [null, null]
+    java.lang.Throwable $spock_feature_throwable
+    try {
+        (foobar, b) = this.foobar()
+        try {
+            org.spockframework.runtime.SpockRuntime.verifyMethodCondition($spock_errorCollector, $spock_valueRecorder.reset(), 'println(foobar)', 6, 3, null, this, $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0), 'println'), new java.lang.Object[]{$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), foobar)}, $spock_valueRecorder.realizeNas(4, false), false, 3)
+        }
+        catch (java.lang.Throwable throwable) {
+            org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector, $spock_valueRecorder, 'println(foobar)', 6, 3, null, throwable)}
+        finally {
+        }
+    }
+    catch (java.lang.Throwable $spock_tmp_throwable) {
+        $spock_feature_throwable = $spock_tmp_throwable
+        throw $spock_tmp_throwable
+    }
+    finally {
+        try {
+            foobar.size()
+        }
+        catch (java.lang.Throwable $spock_tmp_throwable) {
+            if ( $spock_feature_throwable != null) {
+                $spock_feature_throwable.addSuppressed($spock_tmp_throwable)
+            } else {
+                throw $spock_tmp_throwable
+            }
+        }
+        finally {
+        }
+    }
+    this.getSpecificationContext().getMockController().leaveScope()
+}'''
+
+  }
 }
diff --git a/.../src/test/groovy/org/spockframework/smoke/ast/condition/ExceptionConditionsAstSpec.groovy b/.../src/test/groovy/org/spockframework/smoke/ast/condition/ExceptionConditionsAstSpec.groovy
@@ -43,4 +43,42 @@ public void $spock_feature_0_0() {
 }'''
 
   }
+
+  @Issue("https://github.com/spockframework/spock/issues/1332")
+  def "thrown rewrite keeps correct method reference for multi-assignments"() {
+    when:
+    def result = compiler.transpileSpecBody('''
+def "cleanup blocks don't destroy method reference when invocation is assigned to variable with the same name"() {
+  when:
+  def (foobar, b) = foobar()
+
+  then:
+  thrown(IllegalStateException)
+}
+
+def foobar() {
+  throw new IllegalStateException("foo")
+}''', EnumSet.of(Show.METHODS))
+    then:
+    result.source == '''\
+public java.lang.Object foobar() {
+    throw new java.lang.IllegalStateException('foo')
+}
+
+public void $spock_feature_0_0() {
+    def (java.lang.Object foobar, java.lang.Object b) = [null, null]
+    this.getSpecificationContext().setThrownException(null)
+    try {
+        (foobar, b) = this.foobar()
+    }
+    catch (java.lang.Throwable $spock_ex) {
+        this.getSpecificationContext().setThrownException($spock_ex)
+    }
+    finally {
+    }
+    this.thrownImpl(null, null, java.lang.IllegalStateException)
+    this.getSpecificationContext().getMockController().leaveScope()
+}'''
+
+  }
 }
diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ExceptionConditions.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/condition/ExceptionConditions.groovy
@@ -279,7 +279,17 @@ thrown() == e
     thrown(IllegalStateException)
   }
 
+  @Issue("https://github.com/spockframework/spock/issues/1332")
+  def "thrown conditions don't destroy method reference when invocation is assigned to a multi-assignment with the same name"() {
+    when:
+    def (foobar, b) = foobar()
+
+    then:
+    thrown(Exception)
+  }
+
   def foobar() {
     throw new IllegalStateException("foo")
   }
+
 }