diff --git a/src/main/java/org/openrewrite/java/testing/mockito/MockUtilsToStatic.java b/src/main/java/org/openrewrite/java/testing/mockito/MockUtilsToStatic.java index c09dad648..91f14720b 100644 --- a/src/main/java/org/openrewrite/java/testing/mockito/MockUtilsToStatic.java +++ b/src/main/java/org/openrewrite/java/testing/mockito/MockUtilsToStatic.java @@ -16,13 +16,23 @@ package org.openrewrite.java.testing.mockito; import lombok.Getter; +import lombok.RequiredArgsConstructor; +import org.jspecify.annotations.Nullable; import org.openrewrite.*; +import org.openrewrite.internal.ListUtils; import org.openrewrite.java.ChangeMethodTargetToStatic; -import org.openrewrite.java.DeleteStatement; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JRightPadded; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Space; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; /** * In Mockito 1 you use a code snippet like: @@ -50,8 +60,11 @@ public TreeVisitor getVisitor() { } public static class MockUtilsToStaticVisitor extends JavaVisitor { - private static final MethodMatcher METHOD_MATCHER = new MethodMatcher("org.mockito.internal.util.MockUtil ()"); - private final ChangeMethodTargetToStatic changeMethodTargetToStatic = new ChangeMethodTargetToStatic("org.mockito.internal.util.MockUtil *(..)", "org.mockito.internal.util.MockUtil", null, null, false); + private static final String MOCK_UTIL = "org.mockito.internal.util.MockUtil"; + private static final String MOCK_UTIL_METHODS = MOCK_UTIL + " *(..)"; + private static final MethodMatcher METHOD_MATCHER = new MethodMatcher(MOCK_UTIL + " ()"); + private static final MethodMatcher MIGRATED_METHOD_MATCHER = new MethodMatcher(MOCK_UTIL_METHODS); + private final ChangeMethodTargetToStatic changeMethodTargetToStatic = new ChangeMethodTargetToStatic(MOCK_UTIL_METHODS, MOCK_UTIL, null, null, false); @Override public J visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext ctx) { @@ -60,21 +73,160 @@ public J visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContex } @Override - public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { - if (METHOD_MATCHER.matches(newClass)) { - // Check to see if the new MockUtil() is being assigned to a variable or field, like - // MockUtil util = new MockUtil(); - // If it is, then we'll get rid of it - - Cursor parent = getCursor().dropParentUntil(J.class::isInstance); - if (parent.getValue() instanceof J.VariableDeclarations.NamedVariable) { - Object namedVar = parent.dropParentUntil(J.class::isInstance).getValue(); - if (namedVar instanceof J.VariableDeclarations) { - doAfterVisit(new DeleteStatement<>((J.VariableDeclarations) namedVar)); - } + public J visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + J.VariableDeclarations vd = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx); + // Uses of a visible field from another source file are not analysed + J.CompilationUnit scope = getCursor().firstEnclosing(J.CompilationUnit.class); + if (scope == null) { + return vd; + } + + List> original = vd.getPadding().getVariables(); + List> variables = + ListUtils.map(original, v -> isObsoleteMockUtilInstance(v.getElement(), scope) ? null : v); + if (variables.size() == original.size()) { + return vd; + } + if (variables.isEmpty()) { + if (getCursor().getParentTreeCursor().getValue() instanceof J.Block) { + maybeRemoveImport(MOCK_UTIL); + //noinspection DataFlowIssue + return null; + } + return vd; + } + if (original.get(0) != variables.get(0)) { + // The next declarator now carries the separation from the type expression + variables = ListUtils.mapFirst(variables, v -> v.getElement().getPrefix().getComments().isEmpty() ? + v.withElement(v.getElement().withPrefix(Space.SINGLE_SPACE)) : v); + } + JRightPadded last = original.get(original.size() - 1); + if (variables.get(variables.size() - 1) != last) { + // The previous declarator now carries the separation from the semicolon + variables = ListUtils.mapLast(variables, v -> v.withAfter(last.getAfter())); + } + return vd.getPadding().withVariables(variables); + } + + private static boolean isObsoleteMockUtilInstance(J.VariableDeclarations.NamedVariable variable, J.CompilationUnit scope) { + if (!(variable.getInitializer() instanceof J.NewClass) || !METHOD_MATCHER.matches((J.NewClass) variable.getInitializer())) { + return false; + } + JavaType.Variable variableType = variable.getVariableType(); + // Without symbol attribution the uses cannot be proven obsolete + return variableType != null && + !new FindUnmigratedUses(variableType, variable.getSimpleName()).reduce(scope, new AtomicBoolean()).get(); + } + + @RequiredArgsConstructor + private static class FindUnmigratedUses extends JavaIsoVisitor { + private final JavaType.Variable variableType; + private final String name; + + @Override + public @Nullable J visit(@Nullable Tree tree, AtomicBoolean found) { + return found.get() ? (J) tree : super.visit(tree, found); + } + + @Override + public J.Package visitPackage(J.Package pkg, AtomicBoolean found) { + // Package and import name segments are not uses of the variable + return pkg; + } + + @Override + public J.Import visitImport(J.Import anImport, AtomicBoolean found) { + return anImport; + } + + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean found) { + if (!name.equals(identifier.getSimpleName()) || + identifier.getFieldType() != null && !variableType.equals(identifier.getFieldType())) { + return identifier; + } + Cursor parent = getCursor().getParentTreeCursor(); + if (!isNeverVariableReference(identifier, parent) && !isMigratedUse(identifier, parent)) { + found.set(true); + } + return identifier; + } + + /** + * @return whether the identifier declares a variable or names something in another namespace, so that + * it can never reference the variable under analysis. + */ + private static boolean isNeverVariableReference(J.Identifier identifier, Cursor parentCursor) { + if (identifier.getFieldType() == null && + (identifier.getType() instanceof JavaType.Class || identifier.getType() instanceof JavaType.GenericTypeVariable)) { + return true; + } + Object parent = parentCursor.getValue(); + if (parent instanceof J.VariableDeclarations.NamedVariable) { + return ((J.VariableDeclarations.NamedVariable) parent).getName() == identifier; + } + if (parent instanceof J.MethodInvocation) { + return ((J.MethodInvocation) parent).getName() == identifier; + } + if (parent instanceof J.MethodDeclaration) { + return ((J.MethodDeclaration) parent).getName() == identifier; + } + if (parent instanceof J.ClassDeclaration) { + return ((J.ClassDeclaration) parent).getName() == identifier; + } + if (parent instanceof J.MemberReference) { + return ((J.MemberReference) parent).getReference() == identifier; + } + if (parent instanceof J.Label) { + return ((J.Label) parent).getLabel() == identifier; + } + if (parent instanceof J.Break) { + return ((J.Break) parent).getLabel() == identifier; + } + if (parent instanceof J.Continue) { + return ((J.Continue) parent).getLabel() == identifier; + } + if (parent instanceof J.EnumValue) { + return ((J.EnumValue) parent).getName() == identifier; + } + if (parent instanceof J.TypeParameter) { + return ((J.TypeParameter) parent).getName() == identifier; + } + if (parent instanceof J.Assignment) { + // The left side of an annotation element assignment names the element, not a variable + return ((J.Assignment) parent).getVariable() == identifier && + parentCursor.getParentTreeCursor().getValue() instanceof J.Annotation; + } + if (parent instanceof J.FieldAccess) { + // Package and type segments carry no field type, unlike an attributed variable + return identifier.getFieldType() == null; + } + return false; + } + + /** + * @return whether the identifier is the receiver of a call {@link ChangeMethodTargetToStatic} makes + * static, bare or through a field access, in which case the rewrite drops it and the instance with it. + */ + private static boolean isMigratedUse(J.Identifier identifier, Cursor parent) { + Object parentValue = parent.getValue(); + if (parentValue instanceof J.FieldAccess && ((J.FieldAccess) parentValue).getName() == identifier) { + return isMigratedReceiver((J.FieldAccess) parentValue, parent.getParentTreeCursor().getValue()); + } + return isMigratedReceiver(identifier, parentValue); + } + + private static boolean isMigratedReceiver(Expression receiver, Object parent) { + if (parent instanceof J.MethodInvocation) { + J.MethodInvocation method = (J.MethodInvocation) parent; + return method.getSelect() == receiver && MIGRATED_METHOD_MATCHER.matches(method); + } + if (parent instanceof J.MemberReference) { + J.MemberReference reference = (J.MemberReference) parent; + return reference.getContaining() == receiver && MIGRATED_METHOD_MATCHER.matches(reference); } + return false; } - return super.visitNewClass(newClass, ctx); } } } diff --git a/src/test/java/org/openrewrite/java/testing/mockito/MockUtilsToStaticTest.java b/src/test/java/org/openrewrite/java/testing/mockito/MockUtilsToStaticTest.java index dbe746cc3..47bf7ee7f 100644 --- a/src/test/java/org/openrewrite/java/testing/mockito/MockUtilsToStaticTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/MockUtilsToStaticTest.java @@ -16,6 +16,8 @@ package org.openrewrite.java.testing.mockito; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.java.JavaParser; @@ -129,4 +131,666 @@ public void isMockExample() { ) ); } + + @Test + void retainLocalVariableUsedAsMethodArgument() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + observe(util); + return util.isMock(value); + } + + void observe(Object value) { + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + observe(util); + return MockUtil.isMock(value); + } + + void observe(Object value) { + } + } + """ + ) + ); + } + + @Test + void retainFieldUsedAsReturnValue() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private MockUtil util = new MockUtil(); + + MockUtil expose() { + return util; + } + + boolean test(Object value) { + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private MockUtil util = new MockUtil(); + + MockUtil expose() { + return util; + } + + boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void retainStaticFieldUsedInComparison() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private static MockUtil util = new MockUtil(); + + static boolean isShared(MockUtil other) { + return other == util; + } + + static boolean test(Object value) { + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private static MockUtil util = new MockUtil(); + + static boolean isShared(MockUtil other) { + return other == util; + } + + static boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void retainVariableUsedToInitializeAnAlias() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + MockUtil alias = util; + return alias.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + MockUtil alias = util; + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @ParameterizedTest + @ValueSource(strings = { + "MockUtil util = new MockUtil(), observed = createObserved();", + "MockUtil observed = createObserved(), util = new MockUtil();", + """ + MockUtil util = new MockUtil(), + observed = createObserved();""", + "MockUtil observed = createObserved() , util = new MockUtil();", + "MockUtil util = new MockUtil(),observed = createObserved();" + }) + void removeOnlyTheMigratedDeclarator(String declaration) { + //language=java + var source = """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + %s + observe(observed); + return %s; + } + + MockUtil createObserved() { + return new MockUtil(); + } + + void observe(Object value) { + } + } + """; + rewriteRun( + java( + source.formatted(declaration, "util.isMock(value)"), + source.formatted("MockUtil observed = createObserved();", "MockUtil.isMock(value)") + ) + ); + } + + @Test + void removeFieldWhoseUsesAreMigratedThroughThisReceiver() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private MockUtil util = new MockUtil(); + private MockUtil kept = new MockUtil(); + + MockUtil expose() { + return kept; + } + + boolean test(Object value) { + return this.util.isMock(value); + } + + boolean keptTest(Object value) { + return kept.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + private MockUtil kept = new MockUtil(); + + MockUtil expose() { + return kept; + } + + boolean test(Object value) { + return MockUtil.isMock(value); + } + + boolean keptTest(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void removeVariableWhoseNameCollidesWithMethodDeclarationTypeDeclarationAndLabelNames() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class util { + } + + boolean test(Object value) { + MockUtil util = new MockUtil(); + MockUtil kept = new MockUtil(); + observe(kept); + boolean result = util.isMock(value) && kept.isMock(value); + util(); + util: + while (result) { + break util; + } + return result; + } + + void util() { + } + + void observe(Object value) { + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class util { + } + + boolean test(Object value) { + MockUtil kept = new MockUtil(); + observe(kept); + boolean result = MockUtil.isMock(value) && MockUtil.isMock(value); + util(); + util: + while (result) { + break util; + } + return result; + } + + void util() { + } + + void observe(Object value) { + } + } + """ + ) + ); + } + + @Test + void removeVariableWhoseUseIsAMigratedMethodReference() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + import java.util.function.Predicate; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + MockUtil kept = new MockUtil(); + observe(kept); + Predicate isMock = util::isMock; + return isMock.test(value) && kept.isMock(value); + } + + void observe(Object value) { + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + import java.util.function.Predicate; + + class Test { + boolean test(Object value) { + MockUtil kept = new MockUtil(); + observe(kept); + Predicate isMock = MockUtil::isMock; + return isMock.test(value) && MockUtil.isMock(value); + } + + void observe(Object value) { + } + } + """ + ) + ); + } + + @Test + void removeFieldWhoseUseIsAMigratedMethodReferenceThroughThisReceiver() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + import java.util.function.Predicate; + + class Test { + private MockUtil util = new MockUtil(); + private MockUtil kept = new MockUtil(); + + MockUtil expose() { + return kept; + } + + Predicate checker() { + return this.util::isMock; + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + import java.util.function.Predicate; + + class Test { + private MockUtil kept = new MockUtil(); + + MockUtil expose() { + return kept; + } + + Predicate checker() { + return MockUtil::isMock; + } + } + """ + ) + ); + } + + @Test + void removeDeclarationDespiteSameNamedPackageSegment() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + MockUtil util = new MockUtil(); + java.util.Date date = new java.util.Date(); + return util.isMock(value) && date.getTime() >= 0L; + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + java.util.Date date = new java.util.Date(); + return MockUtil.isMock(value) && date.getTime() >= 0L; + } + } + """ + ) + ); + } + + @Test + void removeFullyQualifiedDeclarationDespiteSameNamedPackageSegment() { + //language=java + rewriteRun( + java( + """ + class Test { + boolean test(Object value) { + org.mockito.internal.util.MockUtil util = new org.mockito.internal.util.MockUtil(); + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void removeDeclarationDespiteSameNamedEnumConstant() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + enum Kind { util, other } + + boolean test(Object value) { + MockUtil util = new MockUtil(); + return util.isMock(value) && Kind.util != Kind.other; + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + enum Kind { util, other } + + boolean test(Object value) { + return MockUtil.isMock(value) && Kind.util != Kind.other; + } + } + """ + ) + ); + } + + @Test + void removeDeclarationDespiteSameNamedAnnotationAttribute() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + @interface Marker { + String util(); + } + + @Marker(util = "x") + boolean test(Object value) { + MockUtil util = new MockUtil(); + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + @interface Marker { + String util(); + } + + @Marker(util = "x") + boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void removeDeclarationDespiteSameNamedNestedTypeUsedAsStaticReceiver() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class util { + static boolean flag() { + return true; + } + } + + boolean other() { + return util.flag(); + } + + boolean test(Object value) { + MockUtil util = new MockUtil(); + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class util { + static boolean flag() { + return true; + } + } + + boolean other() { + return util.flag(); + } + + boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void removeDeclarationDespiteSameNamedTypeParameter() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + import java.util.List; + + class Test { + util pick(List items) { + return items.get(0); + } + + boolean test(Object value) { + MockUtil util = new MockUtil(); + return util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + import java.util.List; + + class Test { + util pick(List items) { + return items.get(0); + } + + boolean test(Object value) { + return MockUtil.isMock(value); + } + } + """ + ) + ); + } + + @Test + void removeImportWithLastDeclaration() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + void test() { + MockUtil util = new MockUtil(); + } + } + """, + """ + class Test { + void test() { + } + } + """ + ) + ); + } + + @Test + void doNotRemoveDeclarationsOfSimilarlyNamedTypes() { + //language=java + rewriteRun( + java( + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class FakeUtil { + boolean isMock(Object value) { + return true; + } + } + + boolean test(Object value) { + FakeUtil fake = new FakeUtil(); + MockUtil util = new MockUtil(); + return fake.isMock(value) && util.isMock(value); + } + } + """, + """ + import org.mockito.internal.util.MockUtil; + + class Test { + static class FakeUtil { + boolean isMock(Object value) { + return true; + } + } + + boolean test(Object value) { + FakeUtil fake = new FakeUtil(); + return fake.isMock(value) && MockUtil.isMock(value); + } + } + """ + ) + ); + } }