openrewrite / rewrite-static-analysis

OpenRewrite recipes for identifying and fixing static analysis issues.
Apache License 2.0
28 stars 43 forks source link

`Use lambdas where possible` leads to ambiguity in WireMock `Xml.java` #10

Closed moderne-meeseeks[bot] closed 9 months ago

moderne-meeseeks[bot] commented 1 year ago

Problem

Ambiguity as there's two methods that match the use of the lambda here:

public static <T> T doPrivileged(PrivilegedAction<T> action);
public static <T> T doPrivileged(PrivilegedExceptionAction<T> action)

Expected behavior

Not converted to lambda, as there's ambiguity,

Example diff

From: src/main/java/com/github/tomakehurst/wiremock/common/xml/Xml.java

import java.io.StringReader;
import java.io.StringWriter;
import java.security.AccessController;
-import java.security.PrivilegedAction;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;

private static String setProperty(final String name, final String value) {
return AccessController.doPrivileged(
-        new PrivilegedAction<String>() {
-          @Override
-          public String run() {
-            return System.setProperty(name, value);
-          }
-        });
+        () -> System.setProperty(name, value));
}

public static String prettyPrint(String xml) {

Recipes in example diff:

References:


This issue originally created by @timtebeek on: https://github.com/moderneinc/support-public/issues/35

moderne-meeseeks[bot] commented 1 year ago

This comment originially written by @ on:

knutwannheden commented 1 year ago

This is surprisingly tricky to solve and some of the internals make it even harder than it should be.

jbessels commented 1 year ago

I'm having a similar problem. When solving this problem I'm hoping it can be solved in a generic way and that in some cases the ambiguity can be solved without having to resort to the kill solution of 'don't convert to Lamba'. We are using the Wicket framework and there are lots of places where 6-7 lines can be changed into a single Lambda which is superb. Most of the times this works but sometimes it doesn't work. It always goes wrong with CssClassNameAppender. I'm really hoping that there is a way to solve this ambiguity by looking at the context or something like that. If needed I can create a separate ticket for it.
image

jbessels commented 7 months ago

As mentioned earlier this recipe caused problems for my IDB repo as well. Had to exclude files for it. Re tested all excluded files. The new version of the recipe now handles it correctly. As in no compiling errors any longer.