Collections.singleton and Collections.singletonList allow null, Set.of and List.of not

[JosRoseboom]

Using openrewrite 6.1.8 using gradle:

rewrite {
    activeRecipe(
            "org.openrewrite.java.migrate.UpgradeToJava17"
    )
}

dependencies {
    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.4"))
    rewrite("org.openrewrite.recipe:rewrite-migrate-java")
}

This recipe replaces Collections.singleton with Set.of and Collections.singletonList by List.of . However, the former allows null while the latter does not. So these 2 lines:

var mySet = Collections.singleton(myVarThatIsNull); // results in a Set: [null]
var myList = Collections.singletonList(myVarThatIsNull); // results in a List: [null]

are replaced by the recipe by:

var mySet = Set.of(myVarThatIsNull); // results in a NPE thrown
var myList = List.of(myVarThatIsNull); // results in a NPE thrown

which creates a potential NPE that would not happened in the original code.

[timtebeek]

Hmm; that's odd; we had a similar case come up in review recently, where we specifically recommended to introduce data flow analysis before applying the change there; sounds like this recipe needs a similar change. Would you be open to making that change with help?

[timtebeek]

In the short term it's probably best to disable this recipe in any collections of recipes; then people can still decide to run it explicitly but know they might need to review for nullability.

[knutwannheden]

Currently this recipe is part of org.openrewrite.java.migrate.util.JavaUtilAPIs which in turn is part of org.openrewrite.java.migrate.Java8toJava11. Since all of the recipes in org.openrewrite.java.migrate.util.JavaUtilAPIs have this flaw, we should probably remove org.openrewrite.java.migrate.util.JavaUtilAPIs entirely from org.openrewrite.java.migrate.Java8toJava11.