Various JMockit fixes (#824)

* Fix mockStatic error in JMockitToMockito (#776)

- Use getFullyQualifiedName() instead of getClassName() for nullable() matchers to avoid ambiguity with same-named classes
- Fix syntax error in getCallRealMethod() by properly handling semicolon
- Add ShortenFullyQualifiedTypeReferences to shorten FQN types where safe
- Add test case to verify thenCallRealMethod() generates correct syntax

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Fix variable name conflicts in JMockitToMockito (#777)

When setup statements are moved out of Expectations blocks, variable
declarations in the setup statements could conflict with variable
declarations in the method body, causing compilation errors.

This fix detects such conflicts and wraps the setup statements in a
block to maintain proper scoping when conflicts are detected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Fix IndexOutOfBoundsException in RemoveTimesZeroAndOne with custom classes

Fixes #748

When converting JMockit Verifications with `times = 1` to Mockito for custom classes,
the RemoveTimesZeroAndOne recipe was failing with IndexOutOfBoundsException because
getParameterTypes() returned an empty list when type information was incomplete.

Changes:
- Check if parameter types list is empty before calling subList()
- If empty, only remove the times() argument without modifying method type
- Add test case with custom class to verify the fix
- Disable method invocation type validation in the test as type info may be incomplete

* Refactor to use direct AST manipulation instead of JavaTemplate

Previously, `removeBlock()` and `rewriteBodyStatement()` were using
JavaTemplate inappropriately - using empty strings or `#{any()}` just
to manipulate AST structure. This was inefficient and error-prone.

Changes:
- Refactored `removeBlock()` to directly manipulate the statements list
- Refactored `rewriteBodyStatement()` to use direct AST manipulation
  with proper tree node comparison using `.isScope()`
- Fixed issue #687: Added check for empty Expectations blocks to
  prevent IndexOutOfBoundsException
- Added regression test `emptyExpectationsBlockDoesNotCrash()`

* Add contextSensitive() to JavaTemplate in JMockitBlockRewriter

Using `.contextSensitive()` allows the JavaTemplate to use type
information from the surrounding code context, which can improve type
attribution in generated code. While this doesn't fully solve issue #748's
type attribution problem with custom classes (which still requires
disabling type validation for method invocations), it's a best practice
that may help in other scenarios.

All 91 tests pass with 100% success.

* Fix type attribution in verify() calls by using AST manipulation

Issue #748: verify() calls with custom classes were generating
"MethodInvocation type is missing or malformed" errors.

Root cause: JavaTemplate can only resolve types on the classpath,
not types defined in the source code being transformed.

Solution: Template verify() with placeholder types (String, int),
then replace placeholders with actual AST nodes from the original code:
1. Template: verify("", times(1)).toString()
2. Replace "" with actual mock
3. Replace 1 with actual times value
4. Replace toString() with actual method invocation

This preserves all type information from the original invocation.

Also fixed: Filter out times(1) since it's the default.

Fixes #748

All 91 JMockit tests pass with full type validation enabled.

* Polish

* Polish

* Polish

---------

Co-authored-by: Claude <[email protected]>

Author: knutwannheden

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

@@ -21,7 +21,6 @@
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.Cursor;
 import org.openrewrite.ExecutionContext;
-import org.openrewrite.java.JavaParser;
 import org.openrewrite.java.JavaTemplate;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.tree.*;
@@ -174,7 +173,12 @@ private void rewriteMethodInvocation(List<Statement> statementsToRewrite) {
             return;
         }
         if (mockInvocationResults.getTimes() != null) {
-            rewriteVerify(invocation, mockInvocationResults.getTimes(), "times");
+            // Don't add times(1) since that's the default
+            if (!J.Literal.isLiteralValue(mockInvocationResults.getTimes(), 1)) {
+                rewriteVerify(invocation, mockInvocationResults.getTimes(), "times");
+            } else {
+                rewriteVerify(invocation, null, "");
+            }
         }
         if (mockInvocationResults.getMinTimes() != null) {
             rewriteVerify(invocation, mockInvocationResults.getMinTimes(), "atLeast");
@@ -185,10 +189,11 @@ private void rewriteMethodInvocation(List<Statement> statementsToRewrite) {
     }
 
     private void removeBlock() {
-        methodBody = JavaTemplate.builder("")
-                .javaParser(JavaParser.fromJavaVersion())
-                .build()
-                .apply(new Cursor(visitor.getCursor(), methodBody), nextStatementCoordinates);
+        List<Statement> statements = new ArrayList<>(methodBody.getStatements());
+        if (bodyStatementIndex >= 0 && bodyStatementIndex < statements.size()) {
+            statements.remove(bodyStatementIndex);
+            methodBody = methodBody.withStatements(statements);
+        }
         setNextStatementCoordinates(0);
     }
 
@@ -224,32 +229,108 @@ private void rewriteVerify(J.MethodInvocation invocation, @Nullable Expression t
             return;
         }
 
-        List<Object> templateParams = new ArrayList<>();
-        templateParams.add(invocation.getSelect());
-        if (times != null) {
-            templateParams.add(times);
+        // Build a template for verify() using placeholder types to avoid classpath dependencies
+        // We'll template something like: verify("", times(1)).toString();
+        // Then replace "" with the mock, 1 with the actual times, and toString() with the actual method invocation
+        StringBuilder templateBuilder = new StringBuilder();
+
+        if (isVerificationsInOrder()) {
+            templateBuilder.append("inOrder");
+            if (this.verificationsInOrderIdx > 0) {
+                templateBuilder.append(this.verificationsInOrderIdx);
+            }
+            templateBuilder.append(".");
         }
-        templateParams.add(invocation.getName().getSimpleName());
-        String verifyTemplate = getVerifyTemplate(invocation.getArguments(), verificationMode, templateParams);
+
+        templateBuilder.append("verify(\"\"");
+        if (!verificationMode.isEmpty()) {
+            templateBuilder.append(", ").append(verificationMode).append("(1)");
+        }
+        templateBuilder.append(").toString();");
+
         JavaCoordinates verifyCoordinates;
         if (this.blockType.isVerifications()) {
-            // for Verifications, replace the Verifications block
             verifyCoordinates = nextStatementCoordinates;
         } else {
-            // for Expectations put verify at the end of the method
             verifyCoordinates = methodBody.getCoordinates().lastStatement();
         }
-        rewriteTemplate(verifyTemplate, templateParams, verifyCoordinates);
-        if (this.rewriteFailed) {
+
+        // Apply template to add the verify wrapper
+        int numStatementsBefore = methodBody.getStatements().size();
+        methodBody = JavaTemplate.builder(templateBuilder.toString())
+                .javaParser(getJavaParser(ctx))
+                .staticImports(MOCKITO_ALL_IMPORT)
+                .imports(IN_ORDER_IMPORT_FQN)
+                .build()
+                .apply(new Cursor(visitor.getCursor(), methodBody), verifyCoordinates);
+
+        if (methodBody.getStatements().size() <= numStatementsBefore) {
+            this.rewriteFailed = true;
+            return;
+        }
+
+        // Find the newly added statement
+        int newStatementIdx = this.blockType.isVerifications() ?
+            bodyStatementIndex + numStatementsAdded :
+            methodBody.getStatements().size() - 1;
+
+        Statement newStatement = methodBody.getStatements().get(newStatementIdx);
+        if (!(newStatement instanceof J.MethodInvocation)) {
+            this.rewriteFailed = true;
+            return;
+        }
+
+        // newStatement is something like: verify("").toString() or verify("", times(1)).toString()
+        // We need to replace "" with the actual mock, 1 with the actual times, and toString() with the actual method invocation
+        J.MethodInvocation toStringCall = (J.MethodInvocation) newStatement;
+
+        // Get the verify() call
+        Expression selectExpr = toStringCall.getSelect();
+        if (!(selectExpr instanceof J.MethodInvocation)) {
+            this.rewriteFailed = true;
+            return;
+        }
+
+        J.MethodInvocation verifyCall = (J.MethodInvocation) selectExpr;
+
+        // Replace the placeholders in verify()'s arguments
+        // First argument is always the mock ("")
+        // Second argument (if present) is times(1)
+        List<Expression> verifyArgs = new ArrayList<>(verifyCall.getArguments());
+        if (verifyArgs.isEmpty()) {
+            this.rewriteFailed = true;
             return;
         }
 
+        // Replace the mock placeholder
+        verifyArgs.set(0, invocation.getSelect());
+
+        // If there's a second argument, it's the times() call - replace its argument
+        if (verifyArgs.size() > 1 && verifyArgs.get(1) instanceof J.MethodInvocation) {
+            J.MethodInvocation timesCall = (J.MethodInvocation) verifyArgs.get(1);
+            if (times != null) {
+                // Replace the placeholder argument with the actual times value
+                List<Expression> timesArgs = new ArrayList<>();
+                timesArgs.add(times);
+                timesCall = timesCall.withArguments(timesArgs);
+                verifyArgs.set(1, timesCall);
+            }
+        }
+
+        verifyCall = verifyCall.withArguments(verifyArgs);
+
+        // Replace toString() with the actual method invocation
+        J.MethodInvocation wrappedInvocation = invocation.withSelect(verifyCall);
+
+        // Update the statement in the method body
+        List<Statement> statements = new ArrayList<>(methodBody.getStatements());
+        statements.set(newStatementIdx, wrappedInvocation);
+        methodBody = methodBody.withStatements(statements);
+
         if (this.blockType.isVerifications()) {
-            setNextStatementCoordinates(++numStatementsAdded); // for Expectations, verify statements added to end of method
+            setNextStatementCoordinates(++numStatementsAdded);
         }
 
-        // do this last making sure rewrite worked and specify onlyifReferenced=false because framework cannot find the
-        // static reference to verify when another static mockit reference is added
         visitor.maybeAddImport(MOCKITO_IMPORT_FQN_PREFX, "verify", false);
         if (!verificationMode.isEmpty()) {
             visitor.maybeAddImport(MOCKITO_IMPORT_FQN_PREFX, verificationMode);
@@ -487,9 +568,9 @@ private String getVerifyTemplate(List<Expression> arguments, String
     private static class MockInvocationResults {
         @Setter(AccessLevel.NONE)
         private final List<Expression> results = new ArrayList<>();
-        private Expression times;
-        private Expression minTimes;
-        private Expression maxTimes;
+        private @Nullable Expression times;
+        private @Nullable Expression minTimes;
+        private @Nullable Expression maxTimes;
 
         private void addResult(Expression result) {
             results.add(result);

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

@@ -93,7 +93,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
                     bodyStatementIndex++;
                 }
             }
-            return md.withBody(methodBody);
+            return maybeAutoFormat(methodDeclaration, md.withBody(methodBody), ctx);
         }
     }
 }

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

@@ -18,6 +18,7 @@
 import org.openrewrite.*;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.JavaTemplate;
+import org.openrewrite.java.ShortenFullyQualifiedTypeReferences;
 import org.openrewrite.java.search.UsesType;
 import org.openrewrite.java.tree.*;
 import org.openrewrite.marker.SearchResult;
@@ -157,9 +158,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
             }
             if (isTearDownMethod(md)) {
                 for (J.Identifier id : tearDownMocks.values()) {
-                    String type = TypeUtils.asFullyQualified(id.getFieldType().getType()).getFullyQualifiedName();
-                    md = JavaTemplate.builder("#{any(" + type + ")}.closeOnDemand();")
-                            .contextSensitive()
+                    md = JavaTemplate.builder("#{any()}.closeOnDemand();")
                             .javaParser(getJavaParser(ctx))
                             .imports(MOCKITO_STATIC_IMPORT, MOCKITO_CONSTRUCTION_IMPORT)
                             .staticImports(MOCKITO_ALL_IMPORT)
@@ -307,6 +306,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
             maybeRemoveImport(JMOCKIT_MOCKUP_IMPORT);
 
             doAfterVisit(new LambdaBlockToExpression().getVisitor());
+            doAfterVisit(ShortenFullyQualifiedTypeReferences.modifyOnly(md));
             return maybeAutoFormat(methodDecl, md, ctx);
         }
 
@@ -335,7 +335,7 @@ private String getMatcher(JavaType s) {
                 String elem = TypeUtils.asArray(s).getElemType().toString();
                 return "nullable(" + elem + "[].class)";
             }
-            return "nullable(" + TypeUtils.asFullyQualified(s).getClassName() + ".class)";
+            return "nullable(" + TypeUtils.asFullyQualified(s).getFullyQualifiedName() + ".class)";
         }
 
         private String getAnswerBody(J.MethodDeclaration md) {
@@ -435,7 +435,6 @@ private String getMockStaticMethods(JavaType.Class clazz, String className, Map<
                     .forEach(m -> tpl.append("mockStatic").append(className).append(".when(() -> ")
                             .append(className).append(".").append(m.getName())
                             .append(getCallRealMethod(m))
-                            .append(");")
                     );
 
             return tpl.toString();

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

@@ -15,10 +15,7 @@
  */
 package org.openrewrite.java.testing.jmockit;
 
-import org.openrewrite.Cursor;
 import org.openrewrite.ExecutionContext;
-import org.openrewrite.java.JavaParser;
-import org.openrewrite.java.JavaTemplate;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.tree.*;
 
@@ -55,6 +52,12 @@ J.Block rewriteMethodBody() {
             }
 
             assert nc.getBody() != null;
+
+            // Check if the body has any statements - if empty, skip processing
+            if (nc.getBody().getStatements().isEmpty()) {
+                continue;
+            }
+
             J.Block expectationsBlock = (J.Block) nc.getBody().getStatements().get(0);
 
             // Account for Expectations which may contain multiple blocks
@@ -66,18 +69,37 @@ J.Block rewriteMethodBody() {
                 statementList.add(expectationsBlock);
             }
 
-            // statement needs to be moved directly before expectations class instantiation
-            JavaCoordinates coordinates = nc.getCoordinates().before();
+            // collect setup statements
+            List<Statement> setupStatements = new ArrayList<>();
             List<Statement> newExpectationsBlockStatements = new ArrayList<>();
             for (Statement st : statementList) {
                 for (Statement expectationStatement : ((J.Block) st).getStatements()) {
                     if (!isSetupStatement(expectationStatement, spies)) {
                         newExpectationsBlockStatements.add(expectationStatement);
                         continue;
                     }
-                    rewriteBodyStatement(expectationStatement, coordinates);
-                    // subsequent setup statements are moved in order
-                    coordinates = expectationStatement.getCoordinates().after();
+                    setupStatements.add(expectationStatement);
+                }
+            }
+
+            // check if setup statement variable names conflict with method body variable names
+            Set<String> setupVariableNames = getVariableNames(setupStatements);
+            Set<String> methodBodyVariableNames = getVariableNames(methodBody.getStatements());
+            boolean hasConflict = setupVariableNames.stream().anyMatch(methodBodyVariableNames::contains);
+
+            // 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();
+                    }
                 }
             }
 
@@ -91,14 +113,31 @@ J.Block rewriteMethodBody() {
     }
 
     private void rewriteBodyStatement(Statement statement, JavaCoordinates coordinates) {
-        methodBody = JavaTemplate.builder("#{any()}")
-                .javaParser(JavaParser.fromJavaVersion())
-                .build()
-                .apply(
-                        new Cursor(visitor.getCursor(), methodBody),
-                        coordinates,
-                        statement
-                );
+        List<Statement> statements = new ArrayList<>(methodBody.getStatements());
+
+        if (coordinates.getMode() == JavaCoordinates.Mode.REPLACEMENT) {
+            // Replace the statement at the specified position
+            for (int i = 0; i < statements.size(); i++) {
+                if (statements.get(i).isScope(coordinates.getTree())) {
+                    statements.set(i, statement);
+                    break;
+                }
+            }
+        } else {
+            // Find the reference statement and insert before/after it
+            for (int i = 0; i < statements.size(); i++) {
+                if (statements.get(i).isScope(coordinates.getTree())) {
+                    if (coordinates.getMode() == JavaCoordinates.Mode.BEFORE) {
+                        statements.add(i, statement);
+                    } else { // AFTER
+                        statements.add(i + 1, statement);
+                    }
+                    break;
+                }
+            }
+        }
+
+        methodBody = methodBody.withStatements(statements);
     }
 
     private boolean isSetupStatement(Statement expectationStatement, Set<String> spies) {
@@ -163,4 +202,17 @@ private static boolean isNotMockIdentifier(J.Identifier identifier, Set<String>
         }
         return true;
     }
+
+    private static Set<String> getVariableNames(List<Statement> statements) {
+        Set<String> variableNames = new HashSet<>();
+        for (Statement statement : statements) {
+            if (statement instanceof J.VariableDeclarations) {
+                J.VariableDeclarations varDecls = (J.VariableDeclarations) statement;
+                for (J.VariableDeclarations.NamedVariable namedVar : varDecls.getVariables()) {
+                    variableNames.add(namedVar.getSimpleName());
+                }
+            }
+        }
+        return variableNames;
+    }
 }

src/main/java/org/openrewrite/java/testing/mockito/RemoveTimesZeroAndOne.java

@@ -66,13 +66,20 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
                             J.MethodInvocation times = (J.MethodInvocation) mi.getArguments().get(1);
                             if (timesMatcher.matches(times) && J.Literal.isLiteralValue(times.getArguments().get(0), 1)) {
                                 maybeRemoveImport("org.mockito.Mockito.times");
-                                JavaType.Method methodType = mi.getMethodType()
-                                        .withParameterNames(mi.getMethodType().getParameterNames().subList(0, 1))
-                                        .withParameterTypes(mi.getMethodType().getParameterTypes().subList(0, 1));
-                                return mi
-                                        .withArguments(mi.getArguments().subList(0, 1))
-                                        .withMethodType(methodType)
-                                        .withName(mi.getName().withType(methodType));
+                                // Only update method type if parameter types are available
+                                JavaType.Method methodType = mi.getMethodType();
+                                if (!methodType.getParameterTypes().isEmpty()) {
+                                    methodType = methodType
+                                            .withParameterNames(methodType.getParameterNames().subList(0, 1))
+                                            .withParameterTypes(methodType.getParameterTypes().subList(0, 1));
+                                    return mi
+                                            .withArguments(mi.getArguments().subList(0, 1))
+                                            .withMethodType(methodType)
+                                            .withName(mi.getName().withType(methodType));
+                                }
+                                // If parameter types are not available, just remove the second argument
+                                // and let the type attribution be redone later
+                                return mi.withArguments(mi.getArguments().subList(0, 1));
                             }
                         }
                         return mi;