Fix variable name conflict in JMockit to Mockito migration (#865)

* Fix variable name conflict in JMockit to Mockito migration

When migrating JMockit `Expectations` blocks to Mockito, variables declared inside the block that have the same names as variables later in the test method now stay accessible to the generated `when()` calls.

### Changes
- Modified `SetupStatementsRewriter` to skip setup statement extraction when there's a variable name conflict
- Updated `JMockitBlockRewriter` to track and preserve setup statements within mock invocation groups
- Added `wrapStatementsInBlock()` to wrap both setup statements and `when()` calls in the same block
- Updated `setupStatementVariableNameConflict` test to reflect the corrected behavior
- Added new test `setupStatementVariableUsedInResultAndTestCode` for issue #844

### Before (broken)
```java
{
    String res = "mockedValue";
}
when(config.getValue()).thenReturn(res);  // error: res not in scope
```

### After (fixed)
```java
{
    String res = "mockedValue";
    when(config.getValue()).thenReturn(res);
}
```

Fixes:
 - #844

* Polish

* Polish

Author: knutwannheden

src/main/java/org/openrewrite/java/testing/jmockit/JMockitBlockRewriter.java

@@ -21,13 +21,16 @@
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.Cursor;
 import org.openrewrite.ExecutionContext;
+import org.openrewrite.Tree;
 import org.openrewrite.java.JavaTemplate;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.tree.*;
+import org.openrewrite.marker.Markers;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static java.util.stream.Collectors.toList;
 import static org.openrewrite.java.testing.jmockit.JMockitBlockType.*;
 import static org.openrewrite.java.testing.jmockit.JMockitUtils.MOCKITO_ALL_IMPORT;
 import static org.openrewrite.java.testing.jmockit.JMockitUtils.getJavaParser;
@@ -70,6 +73,10 @@ boolean isRewriteFailed() {
     // used with bodyStatementIndex to obtain the coordinates of the next statement to be written
     private int numStatementsAdded = 0;
 
+    // Track setup statements that need to be preserved and wrapped in a block
+    private final List<Statement> setupStatementsBeforeFirstMock = new ArrayList<>();
+    private boolean hasSetupStatements = false;
+
     JMockitBlockRewriter(JavaVisitor<ExecutionContext> visitor, ExecutionContext ctx, J.Block methodBody,
                          J.NewClass newExpectations, int bodyStatementIndex, JMockitBlockType blockType, int verificationsInOrderIdx) {
         this.visitor = visitor;
@@ -102,6 +109,17 @@ J.Block rewriteMethodBody() {
         List<J.Identifier> uniqueMocks = new ArrayList<>();
         int methodInvocationIdx = -1;
         for (Statement jmockitBlockStatement : jmockitBlock.getStatements()) {
+            // Track setup statements (variable declarations) that appear before mock invocations
+            if (isSetupStatement(jmockitBlockStatement)) {
+                hasSetupStatements = true;
+                if (methodInvocationIdx == -1) {
+                    // Setup statement before the first mock invocation
+                    setupStatementsBeforeFirstMock.add(jmockitBlockStatement);
+                    continue;
+                }
+                // Setup statement after a mock invocation - will be handled by the group
+            }
+
             if (jmockitBlockStatement instanceof J.MethodInvocation) {
                 J.MethodInvocation invocation = (J.MethodInvocation) jmockitBlockStatement;
                 Expression select = invocation.getSelect();
@@ -130,6 +148,9 @@ J.Block rewriteMethodBody() {
             removeBlock();
         }
 
+        // Output setup statements that come before the first mock invocation
+        outputSetupStatements(setupStatementsBeforeFirstMock);
+
         List<Object> mocks = new ArrayList<>(uniqueMocks);
         if (isVerificationsInOrder()) {
             rewriteInOrderVerify(mocks);
@@ -141,9 +162,61 @@ J.Block rewriteMethodBody() {
         if (isFullVerifications()) {
             rewriteFullVerify(mocks);
         }
+
+        // If there were setup statements, wrap the generated statements in a block
+        // to avoid variable name conflicts with the rest of the method
+        if (hasSetupStatements) {
+            wrapStatementsInBlock();
+        }
+
         return methodBody;
     }
 
+    private void wrapStatementsInBlock() {
+        List<Statement> statements = methodBody.getStatements();
+        if (numStatementsAdded <= 0) {
+            return;
+        }
+
+        // Get the statements that were added (from bodyStatementIndex to bodyStatementIndex + numStatementsAdded - 1)
+        int startIdx = bodyStatementIndex;
+        int endIdx = bodyStatementIndex + numStatementsAdded;
+
+        if (startIdx < 0 || endIdx > statements.size()) {
+            return;
+        }
+
+        // Extract the statements to wrap
+        List<Statement> statementsToWrap = new ArrayList<>();
+        for (int i = startIdx; i < endIdx; i++) {
+            statementsToWrap.add(statements.get(i));
+        }
+
+        // Create a new block containing these statements
+        J.Block wrapperBlock = new J.Block(
+                Tree.randomId(),
+                Space.EMPTY,
+                Markers.EMPTY,
+                JRightPadded.build(false),
+                statementsToWrap.stream()
+                        .map(JRightPadded::build)
+                        .collect(toList()),
+                Space.EMPTY
+        );
+
+        // Replace the statements with the block
+        List<Statement> newStatements = new ArrayList<>();
+        for (int i = 0; i < startIdx; i++) {
+            newStatements.add(statements.get(i));
+        }
+        newStatements.add(wrapperBlock);
+        for (int i = endIdx; i < statements.size(); i++) {
+            newStatements.add(statements.get(i));
+        }
+
+        methodBody = methodBody.withStatements(newStatements);
+    }
+
     private boolean isFullVerifications() {
         return this.blockType == FullVerifications;
     }
@@ -152,6 +225,11 @@ private boolean isVerificationsInOrder() {
         return this.blockType == VerificationsInOrder;
     }
 
+    private static boolean isSetupStatement(Statement statement) {
+        // Variable declarations are setup statements
+        return statement instanceof J.VariableDeclarations;
+    }
+
     private void rewriteMethodInvocation(List<Statement> statementsToRewrite) {
         final MockInvocationResults mockInvocationResults = buildMockInvocationResults(statementsToRewrite);
         if (mockInvocationResults == null) {
@@ -169,6 +247,8 @@ private void rewriteMethodInvocation(List<Statement> statementsToRewrite) {
 
         if (!hasResults && !hasTimes && (this.blockType == JMockitBlockType.Expectations || this.blockType.isVerifications())) {
             rewriteVerify(invocation, null, "");
+            // Output setup statements that follow this mock invocation
+            outputSetupStatements(mockInvocationResults.getSetupStatements());
             return;
         }
         if (mockInvocationResults.getTimes() != null) {
@@ -185,6 +265,15 @@ private void rewriteMethodInvocation(List<Statement> statementsToRewrite) {
         if (mockInvocationResults.getMaxTimes() != null) {
             rewriteVerify(invocation, mockInvocationResults.getMaxTimes(), "atMost");
         }
+
+        // Output setup statements that follow this mock invocation
+        outputSetupStatements(mockInvocationResults.getSetupStatements());
+    }
+
+    private void outputSetupStatements(List<Statement> setupStatements) {
+        for (Statement setupStatement : setupStatements) {
+            insertStatementAtCurrentPosition(setupStatement);
+        }
     }
 
     private void removeBlock() {
@@ -196,6 +285,20 @@ private void removeBlock() {
         setNextStatementCoordinates(0);
     }
 
+    private void insertStatementAtCurrentPosition(Statement statement) {
+        List<Statement> statements = new ArrayList<>(methodBody.getStatements());
+        int insertIdx = bodyStatementIndex + numStatementsAdded;
+        if (insertIdx < 0) {
+            insertIdx = 0;
+        }
+        if (insertIdx > statements.size()) {
+            insertIdx = statements.size();
+        }
+        statements.add(insertIdx, statement);
+        methodBody = methodBody.withStatements(statements);
+        setNextStatementCoordinates(++numStatementsAdded);
+    }
+
     private void rewriteResult(J.MethodInvocation invocation, List<Expression> results, boolean hasTimes) {
         boolean lenient = this.blockType == NonStrictExpectations && !hasTimes;
         String template = getWhenTemplate(results, lenient);
@@ -457,6 +560,15 @@ private static void appendToTemplate(StringBuilder templateBuilder, boolean buil
                 }
                 continue;
             }
+            // Skip setup statements (variable declarations) - they're tracked separately
+            if (isSetupStatement(expectationStatement)) {
+                resultWrapper.addSetupStatement(expectationStatement);
+                continue;
+            }
+            if (!(expectationStatement instanceof J.Assignment)) {
+                // unhandled statement type, skip it
+                continue;
+            }
             J.Assignment assignment = (J.Assignment) expectationStatement;
             String variableName = getVariableNameFromAssignment(assignment);
             if (variableName == null) {
@@ -525,6 +637,8 @@ private static void appendToTemplate(StringBuilder templateBuilder, boolean buil
     private static class MockInvocationResults {
         @Setter(AccessLevel.NONE)
         private final List<Expression> results = new ArrayList<>();
+        @Setter(AccessLevel.NONE)
+        private final List<Statement> setupStatements = new ArrayList<>();
         private @Nullable Expression times;
         private @Nullable Expression minTimes;
         private @Nullable Expression maxTimes;
@@ -533,6 +647,10 @@ private void addResult(Expression result) {
             results.add(result);
         }
 
+        private void addSetupStatement(Statement statement) {
+            setupStatements.add(statement);
+        }
+
         private boolean hasAnyTimes() {
             return times != null || minTimes != null || maxTimes != null;
         }

src/main/java/org/openrewrite/java/testing/jmockit/SetupStatementsRewriter.java

@@ -87,19 +87,20 @@ J.Block rewriteMethodBody() {
             Set<String> methodBodyVariableNames = getVariableNames(methodBody.getStatements());
             boolean hasConflict = setupVariableNames.stream().anyMatch(methodBodyVariableNames::contains);
 
+            if (hasConflict) {
+                // When there's a conflict, don't extract setup statements.
+                // Leave them in the expectations block so JMockitBlockRewriter can handle them
+                // and wrap everything in a block to avoid variable name conflicts.
+                continue;
+            }
+
             // move setup statements before the expectations block
             JavaCoordinates coordinates = nc.getCoordinates().before();
             if (!setupStatements.isEmpty()) {
-                if (hasConflict) {
-                    // wrap in a block to avoid variable name conflicts
-                    J.Block setupBlock = expectationsBlock.withStatements(setupStatements);
-                    rewriteBodyStatement(setupBlock, coordinates);
-                } else {
-                    // move statements individually
-                    for (Statement setupStatement : setupStatements) {
-                        rewriteBodyStatement(setupStatement, coordinates);
-                        coordinates = setupStatement.getCoordinates().after();
-                    }
+                // move statements individually
+                for (Statement setupStatement : setupStatements) {
+                    rewriteBodyStatement(setupStatement, coordinates);
+                    coordinates = setupStatement.getCoordinates().after();
                 }
             }
 

src/test/java/org/openrewrite/java/testing/jmockit/JMockitExpectationsToMockitoTest.java

@@ -18,6 +18,7 @@
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
@@ -1762,8 +1763,8 @@ class MyTest {
                   void test() {
                       {
                           String s = "setup";
+                          when(myObject.toString()).thenReturn("foo");
                       }
-                      when(myObject.toString()).thenReturn("foo");
                       String s = "test";
                   }
               }
@@ -1820,4 +1821,103 @@ interface MyInterface {
           )
         );
     }
+
+    @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/844")
+    @Test
+    void setupStatementVariableUsedInResultAndTestCode() {
+        // Regression test for issue #844 - variables declared in Expectations block should be accessible
+        // to when() calls when there's a variable name conflict with test code
+        //language=java
+        rewriteRun(
+          java(
+            """
+              class Config {
+                  private String value;
+                  private String name;
+
+                  public String getValue() {
+                      return value;
+                  }
+
+                  public void setValue(String value) {
+                      this.value = value;
+                  }
+
+                  public String getName() {
+                      return name;
+                  }
+
+                  public void setName(String name) {
+                      this.name = name;
+                  }
+              }
+              """
+          ),
+          java(
+            """
+              import mockit.Expectations;
+              import mockit.Mocked;
+              import mockit.integration.junit5.JMockitExtension;
+              import org.junit.jupiter.api.extension.ExtendWith;
+
+              import static org.junit.jupiter.api.Assertions.assertEquals;
+
+              @ExtendWith(JMockitExtension.class)
+              public class SameNameTest {
+                  @Mocked
+                  Config config;
+
+                  void testFoo() {
+                      new Expectations() {{
+                          String res = "mockedValue";
+                          config.getValue();
+                          result = res;
+
+                          String name = "mockedName";
+                          config.getName();
+                          result = name;
+                      }};
+
+                      String res = config.getValue();
+                      assertEquals("mockedValue", res);
+
+                      String name = config.getName();
+                      assertEquals("mockedName", name);
+                  }
+              }
+              """,
+            """
+              import org.junit.jupiter.api.extension.ExtendWith;
+              import org.mockito.Mock;
+              import org.mockito.junit.jupiter.MockitoExtension;
+
+              import static org.junit.jupiter.api.Assertions.assertEquals;
+              import static org.mockito.Mockito.when;
+
+              @ExtendWith(MockitoExtension.class)
+              public class SameNameTest {
+                  @Mock
+                  Config config;
+
+                  void testFoo() {
+                      {
+                          String res = "mockedValue";
+                          when(config.getValue()).thenReturn(res);
+
+                          String name = "mockedName";
+
+                          when(config.getName()).thenReturn(name);
+                      }
+
+                      String res = config.getValue();
+                      assertEquals("mockedValue", res);
+
+                      String name = config.getName();
+                      assertEquals("mockedName", name);
+                  }
+              }
+              """
+          )
+        );
+    }
 }