Skip to content

Commit

Permalink
Adopt SequencedCollection First and Last methods (#339)
Browse files Browse the repository at this point in the history
* Adopt SequencedCollection First and Last methods

* Add license header

* First passing tests when running on Java 21

* Add negative tests and simplify condition

* Add IteratorNext recipe that only transforms sequenced collections

* Limit recipes to running on Java 21

* Do not use `addLast` as the index is off by one

* Use correct method type on getFirst

* Use correct return type and declaring type

* Drop JavaTemplate as template was context sensitive

* Drop afterTypeValidationOptions by adding rewrite-java-21

* More descriptive test names

* Document improbable comment loss
  • Loading branch information
timtebeek authored Nov 6, 2023
1 parent 9535f5b commit ff1da3f
Show file tree
Hide file tree
Showing 7 changed files with 650 additions and 41 deletions.
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ dependencies {
runtimeOnly("org.openrewrite:rewrite-java-8")
runtimeOnly("org.openrewrite:rewrite-java-11")
runtimeOnly("org.openrewrite:rewrite-java-17")
runtimeOnly("org.openrewrite:rewrite-java-21")

testImplementation("org.junit.jupiter:junit-jupiter-api:latest.release")
testImplementation("org.junit.jupiter:junit-jupiter-params:latest.release")
Expand Down
74 changes: 74 additions & 0 deletions src/main/java/org/openrewrite/java/migrate/util/IteratorNext.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* 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.migrate.util;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.UsesJavaVersion;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.TypeUtils;

public class IteratorNext extends Recipe {
private static final MethodMatcher ITERATOR_MATCHER = new MethodMatcher("java.util.Collection iterator()", true);
private static final MethodMatcher NEXT_MATCHER = new MethodMatcher("java.util.Iterator next()", true);

@Override
public String getDisplayName() {
return "Replace `iterator().next()` with `getFirst()`";
}

@Override
public String getDescription() {
return "Replace `SequencedCollection.iterator().next()` with `getFirst()`.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(
Preconditions.and(
new UsesJavaVersion<>(21),
Preconditions.and(
new UsesMethod<>(ITERATOR_MATCHER),
new UsesMethod<>(NEXT_MATCHER)
)
),
new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation nextInvocation = super.visitMethodInvocation(method, executionContext);
if (NEXT_MATCHER.matches(nextInvocation) && ITERATOR_MATCHER.matches(nextInvocation.getSelect())) {
J.MethodInvocation iteratorInvocation = (J.MethodInvocation) nextInvocation.getSelect();
if (TypeUtils.isAssignableTo("java.util.SequencedCollection", iteratorInvocation.getSelect().getType())) {
J.MethodInvocation getFirstInvocation = nextInvocation.withSelect(iteratorInvocation.getSelect())
.withName(nextInvocation.getName().withSimpleName("getFirst"))
.withMethodType(nextInvocation.getMethodType()
.withName("getFirst")
.withDeclaringType(iteratorInvocation.getMethodType().getDeclaringType()));
return getFirstInvocation;
}
}
return nextInvocation;
}
}
);
}
}
137 changes: 137 additions & 0 deletions src/main/java/org/openrewrite/java/migrate/util/ListFirstAndLast.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* 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.migrate.util;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.UsesJavaVersion;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.Space;

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

public class ListFirstAndLast extends Recipe {

private static final MethodMatcher ADD_MATCHER = new MethodMatcher("java.util.List add(int, ..)", true); // , * fails
private static final MethodMatcher GET_MATCHER = new MethodMatcher("java.util.List get(int)", true);
private static final MethodMatcher REMOVE_MATCHER = new MethodMatcher("java.util.List remove(int)", true);
private static final MethodMatcher SIZE_MATCHER = new MethodMatcher("java.util.List size()", true);

@Override
public String getDisplayName() {
return "Replace `List` `get`, `add`, and `remove` with `SequencedCollection` `*First` and `*Last` methods";
}

@Override
public String getDescription() {
return "Replace `list.get(0)` with `list.getFirst()`, `list.get(list.size() - 1)` with `list.getLast()`, and similar for `add(int, E)` and `remove(int)`.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(
Preconditions.and(
new UsesJavaVersion<>(21),
Preconditions.or(
new UsesMethod<>(ADD_MATCHER),
new UsesMethod<>(GET_MATCHER),
new UsesMethod<>(REMOVE_MATCHER)
)
),
new FirstLastVisitor());
}

private static class FirstLastVisitor extends JavaIsoVisitor<ExecutionContext> {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation mi = super.visitMethodInvocation(method, executionContext);
Expression select = mi.getSelect();
if (!(select instanceof J.Identifier)) {
return mi;
}
J.Identifier sequencedCollection = (J.Identifier) select;

final String operation;
if (ADD_MATCHER.matches(mi)) {
operation = "add";
} else if (GET_MATCHER.matches(mi)) {
operation = "get";
} else if (REMOVE_MATCHER.matches(mi)) {
operation = "remove";
} else {
return mi;
}

final String firstOrLast;
Expression expression = mi.getArguments().get(0);
if (J.Literal.isLiteralValue(expression, 0)) {
firstOrLast = "First";
} else if (!"add".equals(operation) && lastElementOfSequencedCollection(sequencedCollection, expression)) {
firstOrLast = "Last";
} else {
return mi;
}

List<Expression> arguments = new ArrayList<>();
final JavaType.Method newMethodType;
JavaType.Method originalMethodType = mi.getMethodType();
if ("add".equals(operation)) {
arguments.add(mi.getArguments().get(1).withPrefix(Space.EMPTY));
newMethodType = originalMethodType
.withName(operation + firstOrLast)
.withParameterNames(Collections.singletonList(originalMethodType.getParameterNames().get(1)))
.withParameterTypes(Collections.singletonList(originalMethodType.getParameterTypes().get(1)));
} else {
newMethodType = originalMethodType
.withName(operation + firstOrLast)
.withParameterNames(null)
.withParameterTypes(null);
}
return mi.withName(mi.getName().withSimpleName(operation + firstOrLast))
.withArguments(arguments)
.withMethodType(newMethodType);
}

/**
* @param sequencedCollection the identifier of the collection we're calling `get` on
* @param expression the expression we're passing to `get`
* @return true, if we're calling `sequencedCollection.size() - 1` in expression on the same collection
*/
private static boolean lastElementOfSequencedCollection(J.Identifier sequencedCollection, Expression expression) {
if (expression instanceof J.Binary) {
J.Binary binary = (J.Binary) expression;
if (binary.getOperator() == J.Binary.Type.Subtraction
&& J.Literal.isLiteralValue(binary.getRight(), 1)
&& SIZE_MATCHER.matches(binary.getLeft())) {
Expression sizeSelect = ((J.MethodInvocation) binary.getLeft()).getSelect();
if (sizeSelect instanceof J.Identifier) {
return sequencedCollection.getSimpleName().equals(((J.Identifier) sizeSelect).getSimpleName());
}
}
}
return false;
}
}
}
11 changes: 2 additions & 9 deletions src/main/resources/META-INF/rewrite/java-version-21.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ tags:
- java21
- collections
recipeList:
- org.openrewrite.java.migrate.util.FirstAndLast
- org.openrewrite.java.migrate.util.IteratorNext
# Iterator
- org.openrewrite.java.SimplifyMethodChain:
methodPatternChain: [ 'java.util.SequencedCollection iterator()', 'java.util.Iterator next()' ]
newMethodName: getFirst
- org.openrewrite.java.ChangeMethodName:
methodPattern: java.util.SortedSet first()
newMethodName: getFirst
Expand All @@ -68,11 +67,5 @@ recipeList:
methodPattern: java.util.NavigableSet descendingSet()
newMethodName: reversed
# TODO convert additional patterns to sequenced collections
# SequencedCollection.get(0) -> SequencedCollection.getFirst()
# SequencedCollection.get(SequencedCollection.size() - 1) -> SequencedCollection.getLast()
# list.add(0, E) -> SequencedCollection.addFirst(E)
# list.addLast(list.size(), E) -> SequencedCollection.addLast(E)
# list.remove(0) -> SequencedCollection.removeFirst()
# list.remove(list.size() - 1) -> SequencedCollection.removeLast()
# list.listIterator() -> ???
# list.listIterator(int) -> ???
144 changes: 144 additions & 0 deletions src/test/java/org/openrewrite/java/migrate/util/IteratorNextTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* 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.migrate.util;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledForJreRange;
import org.junit.jupiter.api.condition.JRE;
import org.openrewrite.Issue;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

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

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/243")
@EnabledForJreRange(min = JRE.JAVA_21)
class IteratorNextTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new IteratorNext())
.allSources(src -> src.markers(javaVersion(21)));
}

@Test
void listIteratorNextToGetFirst() {
rewriteRun(
//language=java
java(
"""
import java.util.*;
class Foo {
String bar(List<String> collection) {
return collection.iterator().next();
}
}
""",
"""
import java.util.*;
class Foo {
String bar(List<String> collection) {
return collection.getFirst();
}
}
"""
)
);
}

@Test
void nonSequencedCollectionUnchanged() {
rewriteRun(
//language=java
java(
"""
import java.util.*;
class Foo {
void bar(Collection<String> collection) {
String first = collection.iterator().next();
}
}
"""
)
);
}

@Test
void nextCommentRetained() {
rewriteRun(
//language=java
java(
"""
import java.util.*;
class Foo {
void bar(List<String> collection) {
String first = collection
.iterator()
// Next comment
.next();
}
}
""",
"""
import java.util.*;
class Foo {
void bar(List<String> collection) {
String first = collection
// Next comment
.getFirst();
}
}
"""
)
);
}

@Test
void iteratorCommentLost() {
rewriteRun(
//language=java
java(
"""
import java.util.*;
class Foo {
void bar(List<String> collection) {
String first = collection
// Iterator comment
.iterator()
.next();
}
}
""",
"""
import java.util.List;
class Foo {
void bar(List<String> collection) {
String first = collection
.getFirst();
}
}
"""
)
);
}
}
Loading

0 comments on commit ff1da3f

Please sign in to comment.