Skip to content

Commit 9093b7c

Browse files
committed
Do not use diamond operator for ambiguous constructors
Fixes #603
1 parent 2127a4e commit 9093b7c

File tree

2 files changed

+99
-3
lines changed

2 files changed

+99
-3
lines changed

src/main/java/org/openrewrite/staticanalysis/UseDiamondOperator.java

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
8181
varDecls.getTypeExpression() instanceof J.ParameterizedType) {
8282
varDecls = varDecls.withVariables(ListUtils.map(varDecls.getVariables(), nv -> {
8383
if (nv.getInitializer() instanceof J.NewClass) {
84-
nv = nv.withInitializer(maybeRemoveParams(parameterizedTypes((J.ParameterizedType) varDeclsTypeExpression), (J.NewClass) nv.getInitializer()));
84+
J.NewClass nc = (J.NewClass) nv.getInitializer();
85+
if (!hasAmbiguousConstructors(nc)) {
86+
nv = nv.withInitializer(maybeRemoveParams(parameterizedTypes((J.ParameterizedType) varDeclsTypeExpression), nc));
87+
}
8588
}
8689
return nv;
8790
}));
@@ -95,7 +98,7 @@ public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ct
9598
if (asgn.getAssignment() instanceof J.NewClass) {
9699
JavaType.Parameterized assignmentType = TypeUtils.asParameterized(asgn.getType());
97100
J.NewClass nc = (J.NewClass) asgn.getAssignment();
98-
if (assignmentType != null && nc.getClazz() instanceof J.ParameterizedType) {
101+
if (assignmentType != null && nc.getClazz() instanceof J.ParameterizedType && !hasAmbiguousConstructors(nc)) {
99102
asgn = asgn.withAssignment(maybeRemoveParams(assignmentType.getTypeParameters(), nc));
100103
}
101104
}
@@ -144,7 +147,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
144147
J.NewClass nc = (J.NewClass) arg;
145148
if ((java9 || nc.getBody() == null) && !methodType.getParameterTypes().isEmpty()) {
146149
JavaType.Parameterized paramType = TypeUtils.asParameterized(getMethodParamType(methodType, i));
147-
if (paramType != null && nc.getClazz() instanceof J.ParameterizedType) {
150+
if (paramType != null && nc.getClazz() instanceof J.ParameterizedType && !hasAmbiguousConstructors(nc)) {
148151
return maybeRemoveParams(paramType.getTypeParameters(), nc);
149152
}
150153
}
@@ -188,6 +191,10 @@ public J.Return visitReturn(J.Return _return, ExecutionContext ctx) {
188191
J.Return return_ = super.visitReturn(_return, ctx);
189192
J.NewClass nc = return_.getExpression() instanceof J.NewClass ? (J.NewClass) return_.getExpression() : null;
190193
if (nc != null && (java9 || nc.getBody() == null) && nc.getClazz() instanceof J.ParameterizedType) {
194+
// Check if there could be constructor ambiguity
195+
if (hasAmbiguousConstructors(nc)) {
196+
return return_;
197+
}
191198
J parentBlock = getCursor().dropParentUntil(v -> v instanceof J.MethodDeclaration || v instanceof J.Lambda).getValue();
192199
if (parentBlock instanceof J.MethodDeclaration) {
193200
J.MethodDeclaration md = (J.MethodDeclaration) parentBlock;
@@ -259,5 +266,47 @@ private boolean isAParameter() {
259266
p instanceof J.Block ||
260267
p == Cursor.ROOT_VALUE).getValue() instanceof J.MethodInvocation;
261268
}
269+
270+
private boolean hasAmbiguousConstructors(J.NewClass newClass) {
271+
// If there are no arguments or no type info, no ambiguity
272+
if (newClass.getArguments().isEmpty() || newClass.getConstructorType() == null) {
273+
return false;
274+
}
275+
276+
JavaType.FullyQualified type = TypeUtils.asFullyQualified(newClass.getType());
277+
if (type == null) {
278+
return false;
279+
}
280+
281+
// Check if any argument contains a lambda or method reference
282+
boolean hasLambdaOrMethodRef = false;
283+
for (Expression arg : newClass.getArguments()) {
284+
if (arg instanceof J.Lambda || arg instanceof J.MemberReference) {
285+
hasLambdaOrMethodRef = true;
286+
break;
287+
}
288+
}
289+
290+
// If no lambdas/method references, no ambiguity
291+
if (!hasLambdaOrMethodRef) {
292+
return false;
293+
}
294+
295+
// Check if there are multiple constructors with the same number of parameters
296+
int argCount = newClass.getArguments().size();
297+
int constructorsWithSameArgCount = 0;
298+
299+
for (JavaType.Method method : type.getMethods()) {
300+
if (method.isConstructor() && method.getParameterTypes().size() == argCount) {
301+
constructorsWithSameArgCount++;
302+
if (constructorsWithSameArgCount > 1) {
303+
// Multiple constructors with same arg count + lambda/method ref = potential ambiguity
304+
return true;
305+
}
306+
}
307+
}
308+
309+
return false;
310+
}
262311
}
263312
}

src/test/java/org/openrewrite/staticanalysis/UseDiamondOperatorTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,4 +542,51 @@ private void test(Object t) {
542542
)
543543
);
544544
}
545+
546+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/603")
547+
@Test
548+
void doNotRemoveTypeParameterWhenItCausesAmbiguity() {
549+
rewriteRun(
550+
//language=java
551+
java(
552+
"""
553+
import java.util.function.Supplier;
554+
555+
class Test {
556+
static class Ambiguous<T> {
557+
Ambiguous(Supplier<? extends T> supplier, Runnable runnable) {}
558+
Ambiguous(T object, Runnable runnable) {}
559+
}
560+
561+
<T extends AutoCloseable> Ambiguous<T> m(T t) {
562+
return new Ambiguous<T>(() -> { return t; }, () -> { System.out.println("A"); });
563+
}
564+
}
565+
"""
566+
)
567+
);
568+
}
569+
570+
@Test
571+
void doNotRemoveTypeParameterWhenItCausesAmbiguityInVariableDeclaration() {
572+
rewriteRun(
573+
//language=java
574+
java(
575+
"""
576+
import java.util.function.Supplier;
577+
578+
class Test {
579+
static class Ambiguous<T> {
580+
Ambiguous(Supplier<T> supplier, String s) {}
581+
Ambiguous(T object, String s) {}
582+
}
583+
584+
void test() {
585+
Ambiguous<String> amb = new Ambiguous<String>(() -> "test", "arg");
586+
}
587+
}
588+
"""
589+
)
590+
);
591+
}
545592
}

0 commit comments

Comments
 (0)