Skip to content

Commit

Permalink
Replace UnaryOperator with Function in ListUtils methods (#4720)
Browse files Browse the repository at this point in the history 
* Replace `UnaryOperator` with `Function` in `ListUtils` methods

The reason for this change is that the mapping functions should all expect a non-`null` input but are allowed to return `null` to signal that the corresponding result is to be removed from the list.

Additionally, the return type of the methods has been annotated with `@Nullable` where appropriate (typically when the input `List` is `null`). So that this doesn't result in new warnings getting reported, the methods have now additionally also been annotated with `@Contract`, using which we capture the details of when the method returns `null`. With powerful static analysis tools like IDEA, this should typically not result in any new unwanted warnings.

For example when a `with`-method is called by mapping the result of the corresponding `get`-method this is not an issue, as both the `get`-method return type and the `with`-method parameter are annotated as `@Nullable`.

* Review feedback
  • Loading branch information
@knutwannheden
knutwannheden authored Nov 26, 2024
1 parent 01a7224 commit f4ca982
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
27 changes: 15 additions & 12 deletions rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
*/
package org.openrewrite.internal;

import org.jetbrains.annotations.Contract;
import org.jspecify.annotations.Nullable;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.UnaryOperator;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -117,7 +117,8 @@ public static <T> List<T> insert(@Nullable List<T> ls, @Nullable T t, int index)
* @param <T> The type of elements in the list.
* @return A new list with the modified last element, or the original list if unchanged.
*/
public static <T> List<T> mapLast(@Nullable List<T> ls, UnaryOperator<@Nullable T> mapLast) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> mapLast(@Nullable List<T> ls, Function<T, @Nullable T> mapLast) {
if (ls == null || ls.isEmpty()) {
//noinspection ConstantConditions
return ls;
Expand Down Expand Up @@ -145,7 +146,8 @@ public static <T> List<T> mapLast(@Nullable List<T> ls, UnaryOperator<@Nullable
* @param <T> The type of elements in the list.
* @return A new list with the modified first element, or the original list if unchanged.
*/
public static <T> List<T> mapFirst(@Nullable List<T> ls, UnaryOperator<@Nullable T> mapFirst) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> mapFirst(@Nullable List<T> ls, Function<T, @Nullable T> mapFirst) {
if (ls == null || ls.isEmpty()) {
//noinspection ConstantConditions
return ls;
Expand Down Expand Up @@ -173,7 +175,8 @@ public static <T> List<T> mapFirst(@Nullable List<T> ls, UnaryOperator<@Nullable
* @param <T> The type of elements in the list.
* @return A new list with modified elements, or the original list if unchanged.
*/
public static <T> List<T> map(@Nullable List<T> ls, BiFunction<Integer, T, @Nullable T> map) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> map(@Nullable List<T> ls, BiFunction<Integer, T, @Nullable T> map) {
if (ls == null || ls.isEmpty()) {
//noinspection ConstantConditions
return ls;
Expand All @@ -198,7 +201,6 @@ public static <T> List<T> map(@Nullable List<T> ls, BiFunction<Integer, T, @Null
while (newLs.remove(null)) ;
}

//noinspection NullableProblems
return newLs;
}

Expand All @@ -211,7 +213,8 @@ public static <T> List<T> map(@Nullable List<T> ls, BiFunction<Integer, T, @Null
* @return A new list with modified elements, or the original list if unchanged.
*/
// inlined version of `map(List, BiFunction)` for memory efficiency (no overhead for lambda)
public static <T> List<T> map(@Nullable List<T> ls, UnaryOperator<@Nullable T> map) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> map(@Nullable List<T> ls, Function<T, @Nullable T> map) {
if (ls == null || ls.isEmpty()) {
//noinspection ConstantConditions
return ls;
Expand All @@ -236,7 +239,6 @@ public static <T> List<T> map(@Nullable List<T> ls, UnaryOperator<@Nullable T> m
while (newLs.remove(null)) ;
}

//noinspection NullableProblems
return newLs;
}

Expand All @@ -250,7 +252,8 @@ public static <T> List<T> map(@Nullable List<T> ls, UnaryOperator<@Nullable T> m
* @param <T> The type of elements in the list.
* @return A new list with expanded or modified elements, or the original list if unchanged.
*/
public static <T> List<T> flatMap(@Nullable List<T> ls, BiFunction<Integer, T, @Nullable Object> flatMap) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> flatMap(@Nullable List<T> ls, BiFunction<Integer, T, @Nullable Object> flatMap) {
if (ls == null || ls.isEmpty()) {
//noinspection ConstantConditions
return ls;
Expand Down Expand Up @@ -310,7 +313,6 @@ public static <T> List<T> flatMap(@Nullable List<T> ls, BiFunction<Integer, T, @
}
}

//noinspection NullableProblems
return newLs;
}

Expand All @@ -322,7 +324,8 @@ public static <T> List<T> flatMap(@Nullable List<T> ls, BiFunction<Integer, T, @
* @param <T> The type of elements in the list.
* @return A new list with expanded or modified elements, or the original list if unchanged.
*/
public static <T> List<T> flatMap(@Nullable List<T> ls, Function<T, Object> flatMap) {
@Contract("null, _ -> null; !null, _ -> !null")
public static <T> @Nullable List<T> flatMap(@Nullable List<T> ls, Function<T, Object> flatMap) {
return flatMap(ls, (i, t) -> flatMap.apply(t));
}

Expand Down Expand Up @@ -355,6 +358,7 @@ public static <T> List<T> concat(@Nullable List<T> ls, @Nullable T t) {
* @return A new list with the added element at the start, the original list if the element is null or a null
* object if both element and list are null.
*/
@Contract("null, null -> null; !null, _ -> !null; _, !null -> !null")
public static <T> @Nullable List<T> concat(@Nullable T t, @Nullable List<T> ls) {
if (t == null && ls == null) {
//noinspection ConstantConditions
Expand All @@ -378,12 +382,11 @@ public static <T> List<T> concat(@Nullable List<T> ls, @Nullable T t) {
* @param <T> The type of elements in the lists.
* @return A new list containing both lists, or one of the lists if the other is null.
*/
@Contract("null, null -> null; !null, _ -> !null; _, !null -> !null")
public static <T> @Nullable List<T> concatAll(@Nullable List<T> ls, @Nullable List<? extends T> t) {
if (ls == null && t == null) {
//noinspection ConstantConditions
return null;
} else if (t == null || t.isEmpty()) {
//noinspection ConstantConditions
return ls;
} else if (ls == null || ls.isEmpty()) {
//noinspection unchecked
Expand Down
7 changes: 2 additions & 5 deletions rewrite-hcl/src/main/java/org/openrewrite/hcl/HclTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public <H extends Hcl> H apply(Cursor scope, HclCoordinates coordinates, Object.
Space.Location loc = coordinates.getSpaceLocation();

//noinspection unchecked
H h = (H) new HclVisitor<Integer>() {
return (H) new HclVisitor<Integer>() {
@Override
public Hcl visitConfigFile(Hcl.ConfigFile configFile, Integer p) {
Hcl.ConfigFile c = (Hcl.ConfigFile) super.visitConfigFile(configFile, p);
Expand Down Expand Up @@ -166,10 +166,7 @@ public Hcl visitExpression(Expression expression, Integer p) {

return e;
}
}.visit(scope.getValue(), 0, scope.getParentOrThrow());

assert h != null;
return h;
}.visitNonNull(scope.getValue(), 0, scope.getParentOrThrow());
}

public static Builder builder(String code) {
Expand Down

0 comments on commit f4ca982

Please sign in to comment.