Skip to content

Commit a9b4ef9

Browse files
Preserve nullable values in UseMapOf prose maps (#1204)
1 parent 91443e8 commit a9b4ef9

2 files changed

Lines changed: 143 additions & 3 deletions

File tree

‎src/main/java/org/openrewrite/java/migrate/util/UseMapOf.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.openrewrite.java.search.UsesMethod;
3030
import org.openrewrite.java.tree.Expression;
3131
import org.openrewrite.java.tree.J;
32+
import org.openrewrite.java.tree.JavaType;
3233
import org.openrewrite.java.tree.Statement;
3334
import org.openrewrite.java.tree.TypeUtils;
3435

@@ -78,7 +79,9 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
7879
new StringJoiner(", ", "Map.ofEntries(", ")") :
7980
new StringJoiner(", ", "Map.of(", ")");
8081
for (J.MethodInvocation put : puts) {
81-
args.addAll(put.getArguments());
82+
for (Expression arg : put.getArguments()) {
83+
args.add(arg.unwrap());
84+
}
8285
if (useEntries) {
8386
inner.add("Map.entry(#{any()}, #{any()})");
8487
} else {
@@ -317,7 +320,7 @@ private String matchingTargetName(J.VariableDeclarations decl) {
317320
/**
318321
* If {@code stmt} is {@code targetName.put(k, v)} matching {@link #MAP_PUT},
319322
* returns [key, value] as a list; otherwise {@code null}. Returns {@code null}
320-
* if either argument is the {@code null} literal, since {@code Map.of(..)} and
323+
* unless both arguments are proven non-null, since {@code Map.of(..)} and
321324
* {@code Map.entry(..)} reject nulls.
322325
*/
323326
private List<Expression> matchPutCallOn(Statement stmt, String targetName) {
@@ -338,13 +341,35 @@ private List<Expression> matchPutCallOn(Statement stmt, String targetName) {
338341
return null;
339342
}
340343
for (Expression arg : mi.getArguments()) {
341-
if (J.Literal.isLiteralValue( arg, null )) {
344+
if (!isProvablyNonNull(arg)) {
342345
return null;
343346
}
344347
}
345348
return mi.getArguments();
346349
}
347350

351+
private boolean isProvablyNonNull(Expression expression) {
352+
expression = expression.unwrap();
353+
if (expression instanceof J.Literal) {
354+
return !J.Literal.isLiteralValue(expression, null);
355+
}
356+
if (expression instanceof J.NewClass ||
357+
expression instanceof J.NewArray ||
358+
expression instanceof J.Lambda ||
359+
expression instanceof J.MemberReference) {
360+
return true;
361+
}
362+
JavaType type = expression.getType();
363+
return type == JavaType.Primitive.Boolean ||
364+
type == JavaType.Primitive.Byte ||
365+
type == JavaType.Primitive.Char ||
366+
type == JavaType.Primitive.Double ||
367+
type == JavaType.Primitive.Float ||
368+
type == JavaType.Primitive.Int ||
369+
type == JavaType.Primitive.Long ||
370+
type == JavaType.Primitive.Short;
371+
}
372+
348373
private boolean expressionReferences(Expression expr, String name) {
349374
return new JavaIsoVisitor<AtomicBoolean>() {
350375
@Override

‎src/test/java/org/openrewrite/java/migrate/util/UseMapOfTest.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,4 +574,119 @@ void m() {
574574
)
575575
);
576576
}
577+
578+
@Test
579+
void proseMapsWithProvablyNonNullValuesAreCollapsed() {
580+
rewriteRun(
581+
//language=java
582+
java(
583+
"""
584+
import java.util.HashMap;
585+
import java.util.Map;
586+
587+
class PrimitiveValues {
588+
Map<String, Integer> values(int first, int second) {
589+
Map<String, Integer> values = new HashMap<>();
590+
values.put("first", first);
591+
values.put("second", second);
592+
return values;
593+
}
594+
}
595+
""",
596+
"""
597+
import java.util.HashMap;
598+
import java.util.Map;
599+
600+
class PrimitiveValues {
601+
Map<String, Integer> values(int first, int second) {
602+
Map<String, Integer> values = new HashMap<>(Map.of(
603+
"first", first,
604+
"second", second));
605+
return values;
606+
}
607+
}
608+
"""
609+
),
610+
//language=java
611+
java(
612+
"""
613+
import java.util.HashMap;
614+
import java.util.Map;
615+
616+
class ParenthesizedValues {
617+
Map<String, Integer> values(int first, int second) {
618+
Map<String, Integer> values = new HashMap<>();
619+
values.put(("first"), first);
620+
values.put(("second"), second);
621+
return values;
622+
}
623+
}
624+
""",
625+
"""
626+
import java.util.HashMap;
627+
import java.util.Map;
628+
629+
class ParenthesizedValues {
630+
Map<String, Integer> values(int first, int second) {
631+
Map<String, Integer> values = new HashMap<>(Map.of(
632+
"first", first,
633+
"second", second));
634+
return values;
635+
}
636+
}
637+
"""
638+
),
639+
//language=java
640+
java(
641+
"""
642+
import java.util.HashMap;
643+
import java.util.Map;
644+
645+
class NewClassValues {
646+
Map<String, Object> values() {
647+
Map<String, Object> values = new HashMap<>();
648+
values.put("first", new Object());
649+
values.put("second", new Object());
650+
return values;
651+
}
652+
}
653+
""",
654+
"""
655+
import java.util.HashMap;
656+
import java.util.Map;
657+
658+
class NewClassValues {
659+
Map<String, Object> values() {
660+
Map<String, Object> values = new HashMap<>(Map.of(
661+
"first", new Object(),
662+
"second", new Object()));
663+
return values;
664+
}
665+
}
666+
"""
667+
)
668+
);
669+
}
670+
671+
@Test
672+
void proseMapWithUnknownNullableValueIsLeftAlone() {
673+
rewriteRun(
674+
//language=java
675+
java(
676+
"""
677+
import java.util.HashMap;
678+
import java.util.Map;
679+
680+
class Test {
681+
Map<String, Object> prompt(Object card, Object attempt) {
682+
Map<String, Object> values = new HashMap<>();
683+
values.put("card", card);
684+
values.put("attempt", attempt);
685+
return values;
686+
}
687+
}
688+
"""
689+
)
690+
);
691+
}
577692
}

0 commit comments

Comments
 (0)