Skip to content

Commit

Permalink
Chained AssertJ assertions should be simplified to the corresponding …
Browse files Browse the repository at this point in the history
…dedicated assertion (#556)

* Chained AssertJ assertions should be simplified to the corresponding dedicated assertion

* Adopt Traits.methodAccess and match overrides

* Apply formatter to SonarDedicatedAssertions

* Fix imports

* Group IsEqualTo* Logic into reusable ShortenChainedAssertJAssertion

* Apply formatter

* Rename to have differences stand out more

* Clear out empty lines

* Cover one additional case

* Rename class to reflect tested recipe

* Fix expectations around Path

* Enable one more case after updating expected outcome

* Update documented options for clarity

* Move test to illustrate reliance on recipe order

* Remove unnecessary classpath entry

---------

Co-authored-by: Tim te Beek <[email protected]>
  • Loading branch information
velo and timtebeek authored Aug 11, 2024
1 parent 37e9667 commit 8a037e8
Show file tree
Hide file tree
Showing 12 changed files with 749 additions and 655 deletions.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright 2023 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.java.testing.assertj;

import lombok.AllArgsConstructor;
import lombok.NoArgsConstructor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Option;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.TypeUtils;

@AllArgsConstructor
@NoArgsConstructor
public class SimplifyAssertJAssertion extends Recipe {

@Option(displayName = "AssertJ assertion",
description = "The assertion method that should be replaced.",
example = "hasSize",
required = false)
@Nullable
String assertToReplace;

@Option(displayName = "Assertion argument literal",
description = "The literal argument passed into the assertion to replace; use \"null\" for `null`.",
example = "0")
String literalArgument;

@Option(displayName = "Dedicated assertion",
description = "The zero argument assertion to adopt instead.",
example = "isEmpty")
String dedicatedAssertion;

@Option(displayName = "Required type",
description = "The type of the actual assertion argument.",
example = "java.lang.String")
String requiredType;

@Override
public String getDisplayName() {
return "Simplify AssertJ assertions with literal arguments";
}

@Override
public String getDescription() {
return "Simplify AssertJ assertions by replacing them with more expressiove dedicated assertions.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new ShorthenChainedAssertJAssertionsVisitor();
}

private class ShorthenChainedAssertJAssertionsVisitor extends JavaIsoVisitor<ExecutionContext> {
private final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.assertj.core.api.Assertions assertThat(..)");
private final MethodMatcher ASSERT_TO_REPLACE = new MethodMatcher("org.assertj.core.api.* " + assertToReplace + "(..)");

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) {
J.MethodInvocation mi = super.visitMethodInvocation(methodInvocation, ctx);

// Match the end of the chain first, then the select to avoid matching the wrong method chain
if (!ASSERT_TO_REPLACE.matches(mi) || !ASSERT_THAT_MATCHER.matches(mi.getSelect())) {
return mi;
}

// Compare argument with passed in literal
if (!(mi.getArguments().get(0) instanceof J.Literal) ||
!literalArgument.equals(((J.Literal) mi.getArguments().get(0)).getValueSource())) { // Implies "null" is `null`
return mi;
}

// Check argument type of assertThat
if (!TypeUtils.isAssignableTo(requiredType, ((J.MethodInvocation) mi.getSelect()).getArguments().get(0).getType())) {
return mi;
}

// Assume zero argument replacement method
return JavaTemplate.builder(dedicatedAssertion + "()")
.contextSensitive()
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
.build()
.apply(getCursor(), mi.getCoordinates().replaceMethod());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,29 @@
@AllArgsConstructor
@NoArgsConstructor
public class SimplifyChainedAssertJAssertion extends Recipe {
@Option(displayName = "AssertJ Assertion",
@Option(displayName = "AssertJ chained assertion",
description = "The chained AssertJ assertion to move to dedicated assertion.",
example = "equals",
required = false)
@Nullable
String chainedAssertion;

@Option(displayName = "AssertJ Assertion",
@Option(displayName = "AssertJ replaced assertion",
description = "The AssertJ assert that should be replaced.",
example = "isTrue",
required = false)
@Nullable
String assertToReplace;

@Option(displayName = "AssertJ Assertion",
@Option(displayName = "AssertJ replacement assertion",
description = "The AssertJ method to migrate to.",
example = "isEqualTo",
required = false)
@Nullable
String dedicatedAssertion;

@Option(displayName = "Required Type",
description = "Specifies the type the recipe should run on.",
@Option(displayName = "Required type",
description = "The type of the actual assertion argument.",
example = "java.lang.String",
required = false)
@Nullable
Expand Down Expand Up @@ -120,11 +120,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat
List<Expression> arguments = new ArrayList<>();
arguments.add(actual);

// Special case for more expressive assertions: assertThat(x.size()).isEqualTo(0) -> isEmpty()
if ("size".equals(chainedAssertion) && "isEqualTo".equals(assertToReplace) && hasZeroArgument(mi)) {
return applyTemplate("assertThat(#{any()}).isEmpty()", arguments, mi, ctx);
}

String template = getStringTemplateAndAppendArguments(assertThatArg, mi, arguments);
return applyTemplate(String.format(template, dedicatedAssertion), arguments, mi, ctx);
}
Expand All @@ -136,38 +131,38 @@ private J.MethodInvocation applyTemplate(String formattedTemplate, List<Expressi
.build()
.apply(getCursor(), mi.getCoordinates().replace(), arguments.toArray());
}
}

private String getStringTemplateAndAppendArguments(J.MethodInvocation assertThatArg, J.MethodInvocation methodToReplace, List<Expression> arguments) {
Expression assertThatArgument = assertThatArg.getArguments().get(0);
Expression methodToReplaceArgument = methodToReplace.getArguments().get(0);
boolean assertThatArgumentIsEmpty = assertThatArgument instanceof J.Empty;
boolean methodToReplaceArgumentIsEmpty = methodToReplaceArgument instanceof J.Empty;
private String getStringTemplateAndAppendArguments(J.MethodInvocation assertThatArg, J.MethodInvocation methodToReplace, List<Expression> arguments) {
Expression assertThatArgument = assertThatArg.getArguments().get(0);
Expression methodToReplaceArgument = methodToReplace.getArguments().get(0);
boolean assertThatArgumentIsEmpty = assertThatArgument instanceof J.Empty;
boolean methodToReplaceArgumentIsEmpty = methodToReplaceArgument instanceof J.Empty;

// If both arguments are empty, then the select is already added to the arguments list, and we use a minimal template
if (assertThatArgumentIsEmpty && methodToReplaceArgumentIsEmpty) {
return "assertThat(#{any()}).%s()";
}

// If both arguments are not empty, then we add both to the arguments to the arguments list, and return a template with two arguments
if (!assertThatArgumentIsEmpty && !methodToReplaceArgumentIsEmpty) {
// This should only happen for map assertions using a key and value
arguments.add(assertThatArgument);
arguments.add(methodToReplaceArgument);
return "assertThat(#{any()}).%s(#{any()}, #{any()})";
}
// If both arguments are empty, then the select is already added to the arguments list, and we use a minimal template
if (assertThatArgumentIsEmpty && methodToReplaceArgumentIsEmpty) {
return "assertThat(#{any()}).%s()";
}

// If either argument is empty, we choose which one to add to the arguments list, and optionally extract the select
arguments.add(extractEitherArgument(assertThatArgumentIsEmpty, assertThatArgument, methodToReplaceArgument));
// If both arguments are not empty, then we add both to the arguments to the arguments list, and return a template with two arguments
if (!assertThatArgumentIsEmpty && !methodToReplaceArgumentIsEmpty) {
// This should only happen for map assertions using a key and value
arguments.add(assertThatArgument);
arguments.add(methodToReplaceArgument);
return "assertThat(#{any()}).%s(#{any()}, #{any()})";
}

// Special case for Path.of() assertions
if ("java.nio.file.Path".equals(requiredType) && dedicatedAssertion.contains("Raw")
&& TypeUtils.isAssignableTo("java.lang.String", assertThatArgument.getType())) {
return "assertThat(#{any()}).%s(Path.of(#{any()}))";
}
// If either argument is empty, we choose which one to add to the arguments list, and optionally extract the select
arguments.add(extractEitherArgument(assertThatArgumentIsEmpty, assertThatArgument, methodToReplaceArgument));

return "assertThat(#{any()}).%s(#{any()})";
// Special case for Path.of() assertions
if ("java.nio.file.Path".equals(requiredType) && dedicatedAssertion.contains("Raw")
&& TypeUtils.isAssignableTo("java.lang.String", assertThatArgument.getType())) {
maybeAddImport("java.nio.file.Path");
return "assertThat(#{any()}).%s(Path.of(#{any()}))";
}

return "assertThat(#{any()}).%s(#{any()})";
}
}

private static Expression extractEitherArgument(boolean assertThatArgumentIsEmpty, Expression assertThatArgument, Expression methodToReplaceArgument) {
Expand All @@ -183,13 +178,4 @@ private static Expression extractEitherArgument(boolean assertThatArgumentIsEmpt
}
return assertThatArgument;
}

private boolean hasZeroArgument(J.MethodInvocation method) {
List<Expression> arguments = method.getArguments();
if (arguments.size() == 1 && arguments.get(0) instanceof J.Literal) {
J.Literal literalArg = (J.Literal) arguments.get(0);
return literalArg.getValue() != null && literalArg.getValue().equals(0);
}
return false;
}
}
Loading

0 comments on commit 8a037e8

Please sign in to comment.