From f4ca98215b0ad660292e17ffb5af3e05cb89f8cd Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Tue, 26 Nov 2024 16:54:35 +0100 Subject: [PATCH] Replace `UnaryOperator` with `Function` in `ListUtils` methods (#4720) * 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 --- .../org/openrewrite/internal/ListUtils.java | 27 ++++++++++--------- .../java/org/openrewrite/hcl/HclTemplate.java | 7 ++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java b/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java index bd58337f85b..1ee4f4ed59f 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/ListUtils.java @@ -15,6 +15,7 @@ */ package org.openrewrite.internal; +import org.jetbrains.annotations.Contract; import org.jspecify.annotations.Nullable; import java.util.ArrayList; @@ -22,7 +23,6 @@ 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; @@ -117,7 +117,8 @@ public static List insert(@Nullable List ls, @Nullable T t, int index) * @param The type of elements in the list. * @return A new list with the modified last element, or the original list if unchanged. */ - public static List mapLast(@Nullable List ls, UnaryOperator<@Nullable T> mapLast) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List mapLast(@Nullable List ls, Function mapLast) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions return ls; @@ -145,7 +146,8 @@ public static List mapLast(@Nullable List ls, UnaryOperator<@Nullable * @param The type of elements in the list. * @return A new list with the modified first element, or the original list if unchanged. */ - public static List mapFirst(@Nullable List ls, UnaryOperator<@Nullable T> mapFirst) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List mapFirst(@Nullable List ls, Function mapFirst) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions return ls; @@ -173,7 +175,8 @@ public static List mapFirst(@Nullable List ls, UnaryOperator<@Nullable * @param The type of elements in the list. * @return A new list with modified elements, or the original list if unchanged. */ - public static List map(@Nullable List ls, BiFunction map) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List map(@Nullable List ls, BiFunction map) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions return ls; @@ -198,7 +201,6 @@ public static List map(@Nullable List ls, BiFunction List map(@Nullable List ls, BiFunction List map(@Nullable List ls, UnaryOperator<@Nullable T> map) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List map(@Nullable List ls, Function map) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions return ls; @@ -236,7 +239,6 @@ public static List map(@Nullable List ls, UnaryOperator<@Nullable T> m while (newLs.remove(null)) ; } - //noinspection NullableProblems return newLs; } @@ -250,7 +252,8 @@ public static List map(@Nullable List ls, UnaryOperator<@Nullable T> m * @param The type of elements in the list. * @return A new list with expanded or modified elements, or the original list if unchanged. */ - public static List flatMap(@Nullable List ls, BiFunction flatMap) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List flatMap(@Nullable List ls, BiFunction flatMap) { if (ls == null || ls.isEmpty()) { //noinspection ConstantConditions return ls; @@ -310,7 +313,6 @@ public static List flatMap(@Nullable List ls, BiFunction List flatMap(@Nullable List ls, BiFunction The type of elements in the list. * @return A new list with expanded or modified elements, or the original list if unchanged. */ - public static List flatMap(@Nullable List ls, Function flatMap) { + @Contract("null, _ -> null; !null, _ -> !null") + public static @Nullable List flatMap(@Nullable List ls, Function flatMap) { return flatMap(ls, (i, t) -> flatMap.apply(t)); } @@ -355,6 +358,7 @@ public static List concat(@Nullable List 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 @Nullable List concat(@Nullable T t, @Nullable List ls) { if (t == null && ls == null) { //noinspection ConstantConditions @@ -378,12 +382,11 @@ public static List concat(@Nullable List ls, @Nullable T t) { * @param 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 @Nullable List concatAll(@Nullable List ls, @Nullable List 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 diff --git a/rewrite-hcl/src/main/java/org/openrewrite/hcl/HclTemplate.java b/rewrite-hcl/src/main/java/org/openrewrite/hcl/HclTemplate.java index bf042fcc305..5fb3297f4c3 100644 --- a/rewrite-hcl/src/main/java/org/openrewrite/hcl/HclTemplate.java +++ b/rewrite-hcl/src/main/java/org/openrewrite/hcl/HclTemplate.java @@ -65,7 +65,7 @@ public H apply(Cursor scope, HclCoordinates coordinates, Object. Space.Location loc = coordinates.getSpaceLocation(); //noinspection unchecked - H h = (H) new HclVisitor() { + return (H) new HclVisitor() { @Override public Hcl visitConfigFile(Hcl.ConfigFile configFile, Integer p) { Hcl.ConfigFile c = (Hcl.ConfigFile) super.visitConfigFile(configFile, p); @@ -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) {