Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Failures in UseVar Recipe discovered by moderne.io #285

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Fix Failures in UseVar Recipe discovered by moderne.io #285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
023a0f4
add reproducing tests for moderne.io failures
MBoegers Sep 6, 2023
bffa5f4
Revert ternary simplifications
timtebeek Sep 14, 2023
36546c2
Merge branch 'main' into bugfixing_moderneio_findings
timtebeek Sep 14, 2023
2bb570b
Minimize test before/after differences
timtebeek Sep 14, 2023
8343c0a
Add `.contextSensitive()` as suggested
timtebeek Sep 14, 2023
31a466f
Merge branch 'openrewrite:main' into bugfixing_moderneio_findings
MBoegers Oct 4, 2023
4fad404
add test to assure that streams are handled correctly
MBoegers Oct 5, 2023
e1db7a8
add test to show streams are handled somehow correct
MBoegers Oct 5, 2023
122c1cd
Merge branch 'main' into bugfixing_moderneio_findings
timtebeek Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForGenericMethodInvocations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
*/
package org.openrewrite.java.migrate.lang.var;

import java.util.ArrayList;
import java.util.List;

import org.openrewrite.*;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
Expand All @@ -26,6 +23,9 @@
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import java.util.ArrayList;
import java.util.List;

import static java.util.Collections.emptyList;

public class UseVarForGenericMethodInvocations extends Recipe {
Expand All @@ -51,6 +51,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {

static final class UseVarForGenericsVisitor extends JavaIsoVisitor<ExecutionContext> {
private final JavaTemplate template = JavaTemplate.builder("var #{} = #{any()}")
.contextSensitive()
.javaParser(JavaParser.fromJavaVersion()).build();

@Override
Expand All @@ -77,9 +78,29 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations v
if (hasNoTypeParams && argumentsEmpty) return vd;

// mark imports for removal if unused
if (vd.getType() instanceof JavaType.FullyQualified) maybeRemoveImport((JavaType.FullyQualified) vd.getType());
JavaType typeRemoved = vd.getType();
if (typeRemoved instanceof JavaType.FullyQualified) {
if (typeRemoved instanceof JavaType.Parameterized) { // parameterized have to be decomposed
typeRemoved = ((JavaType.Parameterized) typeRemoved).getType();
}
maybeRemoveImport((JavaType.FullyQualified) typeRemoved);
}

return transformToVar(vd, new ArrayList<>(), new ArrayList<>());

//determine types
List<JavaType> leftTypes = new ArrayList<>();
TypeTree leftTypeExpression = vd.getTypeExpression();
if (leftTypeExpression != null) {
leftTypes.add(leftTypeExpression.getType());
}

List<JavaType> rightTypes = new ArrayList<>();
JavaType initializerType = initializer.getType();
if (initializerType != null) {
rightTypes.add(initializerType);
}

return transformToVar(vd, leftTypes, rightTypes);
}

private static boolean allArgumentsEmpty(J.MethodInvocation invocation) {
Expand All @@ -100,10 +121,13 @@ private J.VariableDeclarations transformToVar(J.VariableDeclarations vd, List<Ja
// we need to switch type infos from left to right here
List<Expression> typeArgument = new ArrayList<>();
for (JavaType t : leftTypes) {
typeArgument.add(new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), ((JavaType.Class) t).getClassName(), t, null));
typeArgument.add(new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), ((JavaType.FullyQualified) t).getClassName(), t, null));
}

if (initializer instanceof J.NewClass) { // for constructor invocations we need to handle generics
J.ParameterizedType typedInitializerClazz = ((J.ParameterizedType) ((J.NewClass) initializer).getClazz()).withTypeParameters(typeArgument);
initializer = ((J.NewClass) initializer).withClazz(typedInitializerClazz);
}
J.ParameterizedType typedInitializerClazz = ((J.ParameterizedType) ((J.NewClass) initializer).getClazz()).withTypeParameters(typeArgument);
initializer = ((J.NewClass) initializer).withClazz(typedInitializerClazz);
}

J.VariableDeclarations result = template.<J.VariableDeclarations>apply(getCursor(), vd.getCoordinates().replace(), simpleName, initializer)
Expand Down
249 changes: 238 additions & 11 deletions src/test/java/org/openrewrite/java/migrate/lang/UseVarKeywordTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
*/
package org.openrewrite.java.migrate.lang;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.java.Assertions.version;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
Expand All @@ -26,6 +23,9 @@
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.java.Assertions.version;

class UseVarKeywordTest implements RewriteTest {

@Override
Expand All @@ -37,6 +37,233 @@ public void defaults(RecipeSpec spec) {
.activateRecipes("org.openrewrite.java.migrate.lang.UseVar"));
}

@Nested
class BugFixing {

@Test
void anonymousClass() {
// spring-projects/spring-data-commons @ main: src/test/java/org/springframework/data/domain/ManagedTypesUnitTests.java
// solving: Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template:
//var typesSupplier = __P__.<org.springframework.data.domain.ManagedTypesUnitTests.1>/*__p1__*/p()
//language=java
rewriteRun(
version(
java("""
package com.example.app;

import java.util.Collections;
import java.util.function.Supplier;

class ManagedTypesUnitTests {
void supplierBasedManagedTypesAreEvaluatedLazily() {
Supplier<Iterable<Class<?>>> typesSupplier = spy(new Supplier<Iterable<Class<?>>>() {
@Override
public Iterable<Class<?>> get() {
return Collections.singleton(Object.class);
}
});
}

// mock for mockito method
private Supplier<Iterable<Class<?>>> spy(Supplier<Iterable<Class<?>>> supplier) {
return null;
}
}
""", """
package com.example.app;

import java.util.Collections;
import java.util.function.Supplier;

class ManagedTypesUnitTests {
void supplierBasedManagedTypesAreEvaluatedLazily() {
var typesSupplier = spy(new Supplier<Iterable<Class<?>>>() {
@Override
public Iterable<Class<?>> get() {
return Collections.singleton(Object.class);
}
});
}

// mock for mockito method
private Supplier<Iterable<Class<?>>> spy(Supplier<Iterable<Class<?>>> supplier) {
return null;
}
}
"""),
10
)
);
}

@Test
void multiGenerics() {
// spring-cloud/spring-cloud-contract @ main: spring-cloud-contract-verifier/src/test/resources/contractsToCompile/contract_multipart.java
// solving java.lang.IllegalArgumentException: Unable to parse expression from JavaType Unknown
//language=java
rewriteRun(
version(
java("""
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

class contract_multipart implements Supplier<Collection<Contract>> { \s
private static Map<String, DslProperty> namedProps(HttpSender.Request r) {
Map<String, DslProperty> map = new HashMap<>();
return map;
}

@Override
public Collection<Contract> get() { return null; }
}
// replacements
class Contract{}
class DslProperty{}
class HttpSender {
static class Request {}
}
""", """
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

class contract_multipart implements Supplier<Collection<Contract>> { \s
private static Map<String, DslProperty> namedProps(HttpSender.Request r) {
var map = new HashMap<String, DslProperty>();
return map;
}

@Override
public Collection<Contract> get() { return null; }
}
// replacements
class Contract{}
class DslProperty{}
class HttpSender {
static class Request {}
}
"""),
10
)
);
}

@Test
void duplicateTemplate() {
// spring-projects/spring-hateoas @ main src/test/java/org/springframework/hateoas/mediatype/html/HtmlInputTypeUnitTests.java
// solving Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template:
//var numbers = __P__.<java.util.stream.Stream<org.springframework.hateoas.mediatype.html.HtmlInputTypeUnitTests..>>/*__p1__*/p()
//language=java
rewriteRun(
version(
java("""
import java.math.BigDecimal;
import java.util.Arrays;
import java.util.Collection;
import java.util.stream.Stream;

class HtmlInputTypeUnitTests {
Stream<DynamicTest> derivesInputTypesFromType() {
Stream<$> numbers = HtmlInputType.NUMERIC_TYPES.stream() //
.map(it -> $.of(it, HtmlInputType.NUMBER));
return null;
}

static class HtmlInputType {
static final Collection<Class<?>> NUMERIC_TYPES = Arrays.asList(int.class, long.class, float.class,
double.class, short.class, Integer.class, Long.class, Float.class, Double.class, Short.class, BigDecimal.class);

public static final HtmlInputType NUMBER = new HtmlInputType();

public static HtmlInputType from(Class<?> type) { return null; }
}

static class $ {

Class<?> type;
HtmlInputType expected;

static $ of(Class<?> it, HtmlInputType number){ return null; }\s

public void verify() {
assertThat(HtmlInputType.from(type)).isEqualTo(expected);
}

@Override
public String toString() {
return String.format("Derives %s from %s.", expected, type);
}
//mocking
private <SELF extends AbstractBigDecimalAssert<SELF>> AbstractBigDecimalAssert assertThat(HtmlInputType from) {
return null;
}
}
}
// replacement
class DynamicTest {}
class AbstractBigDecimalAssert<T> {
public void isEqualTo(Object expected) {}
}
""", """
import java.math.BigDecimal;
import java.util.Arrays;
import java.util.Collection;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;

class HtmlInputTypeUnitTests {
Stream<DynamicTest> derivesInputTypesFromType() {
var numbers = HtmlInputType.NUMERIC_TYPES.stream() //
.map(it -> $.of(it, HtmlInputType.NUMBER));
return null;
}

static class HtmlInputType {
static final Collection<Class<?>> NUMERIC_TYPES = Arrays.asList(int.class, long.class, float.class,
double.class, short.class, Integer.class, Long.class, Float.class, Double.class, Short.class, BigDecimal.class);

public static final HtmlInputType NUMBER = new HtmlInputType();

public static HtmlInputType from(Class<?> type) { return null; }
}

static class $ {

Class<?> type;
HtmlInputType expected;

static $ of(Class<?> it, HtmlInputType number){ return null; }\s

public void verify() {
assertThat(HtmlInputType.from(type)).isEqualTo(expected);
}

@Override
public String toString() {
return String.format("Derives %s from %s.", expected, type);
}
//mocking
private <SELF extends AbstractBigDecimalAssert<SELF>> AbstractBigDecimalAssert assertThat(HtmlInputType from) {
return null;
}
}
}
// replacement
class DynamicTest {}
class AbstractBigDecimalAssert<T> {
public void isEqualTo(Object expected) {}
}
"""),
10
)
);
}
}

@Nested
class GeneralNotApplicable {

Expand Down Expand Up @@ -142,14 +369,14 @@ void withTernary() {
rewriteRun(
version(
java("""
package com.example.app;
class A {
void m() {
String o = true ? "isTrue" : "Test";
}
}
"""),
package com.example.app;

class A {
void m() {
String o = true ? "isTrue" : "Test";
}
}
"""),
10
)
);
Expand Down
Loading
Loading