From 02d9d0c2b2690d83e7114a2cb46de619103ad59a Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran <131454720+srikanth-sankaran@users.noreply.github.com> Date: Fri, 1 Nov 2024 06:09:21 +0530 Subject: [PATCH] Alternate fixes for https://bugs.eclipse.org/bugs/show_bug.cgi?id=576861 (#3208) * Also back out its follow on bug fix https://github.com/eclipse-jdt/eclipse.jdt.core/issues/520 as it loses relevance --- .../jdt/internal/compiler/ast/Expression.java | 4 + .../compiler/ast/LambdaExpression.java | 1 + .../compiler/ast/SwitchExpression.java | 47 ++++--- .../internal/compiler/ast/YieldStatement.java | 4 +- .../compiler/lookup/PolyTypeBinding.java | 4 +- .../SwitchExpressionsYieldTest.java | 131 ++++++++++++++++-- 6 files changed, 156 insertions(+), 35 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java index facd8568eff..0d6bd13b404 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java @@ -1100,6 +1100,10 @@ public StringBuilder print(int indent, StringBuilder output) { public abstract StringBuilder printExpression(int indent, StringBuilder output); +public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) { + return printExpression(tab, output); +} + @Override public StringBuilder printStatement(int indent, StringBuilder output) { return print(indent, output).append(";"); //$NON-NLS-1$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/LambdaExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/LambdaExpression.java index c0b9f2d2f97..8eadf846dad 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/LambdaExpression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/LambdaExpression.java @@ -734,6 +734,7 @@ public StringBuilder printExpression(int tab, StringBuilder output) { return printExpression(tab, output, false); } + @Override public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) { int parenthesesCount = (this.bits & ASTNode.ParenthesizedMASK) >> ASTNode.ParenthesizedSHIFT; String suffix = ""; //$NON-NLS-1$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java index 0d89b5d91a8..8f9c7d59c61 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java @@ -31,7 +31,15 @@ import org.eclipse.jdt.internal.compiler.flow.FlowInfo; import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; import org.eclipse.jdt.internal.compiler.impl.Constant; -import org.eclipse.jdt.internal.compiler.lookup.*; +import org.eclipse.jdt.internal.compiler.lookup.BlockScope; +import org.eclipse.jdt.internal.compiler.lookup.FieldBinding; +import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding; +import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment; +import org.eclipse.jdt.internal.compiler.lookup.MethodBinding; +import org.eclipse.jdt.internal.compiler.lookup.PolyTypeBinding; +import org.eclipse.jdt.internal.compiler.lookup.Scope; +import org.eclipse.jdt.internal.compiler.lookup.TypeBinding; +import org.eclipse.jdt.internal.compiler.lookup.TypeIds; public class SwitchExpression extends SwitchStatement implements IPolyExpression { @@ -40,8 +48,6 @@ public class SwitchExpression extends SwitchStatement implements IPolyExpression private boolean isPolyExpression = false; private TypeBinding[] originalValueResultExpressionTypes; private TypeBinding[] finalValueResultExpressionTypes; - /* package */ Map originalTypeMap = new HashMap<>(); - private int nullStatus = FlowInfo.UNKNOWN; public List resultExpressions = new ArrayList<>(0); @@ -330,9 +336,6 @@ public TypeBinding resolveType(BlockScope upperScope) { if (this.constant != Constant.NotAConstant) { this.constant = Constant.NotAConstant; - if (this.originalTypeMap == null) - this.originalTypeMap = new HashMap<>(); - super.resolve(upperScope); resultExpressionsCount = this.resultExpressions.size(); @@ -341,31 +344,26 @@ public TypeBinding resolveType(BlockScope upperScope) { return this.resolvedType = null; } - if (this.originalValueResultExpressionTypes == null) { - this.originalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount]; - this.finalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount]; - for (int i = 0; i < resultExpressionsCount; ++i) { - this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] = - this.resultExpressions.get(i).resolvedType; - } + this.originalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount]; + this.finalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount]; + for (int i = 0; i < resultExpressionsCount; ++i) { + this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] = this.resultExpressions.get(i).resolvedType; } + if (isPolyExpression()) { //The type of a poly switch expression is the same as its target type. if (this.expectedType == null || !this.expectedType.isProperType(true)) { return new PolyTypeBinding(this); } return this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null; } - } else { - // re-resolving of poly expression: + } else { // re-resolving of poly expression: resultExpressionsCount = this.resultExpressions.size(); if (resultExpressionsCount == 0) return this.resolvedType = null; // error flagging would have been done during the earlier phase. boolean yieldErrors = false; for (int i = 0; i < resultExpressionsCount; i++) { Expression resultExpr = this.resultExpressions.get(i); - TypeBinding origType = this.originalTypeMap.get(resultExpr); - // NB: if origType == null we assume that initial resolving failed hard, rendering re-resolving impossible - if (origType != null && origType.kind() == Binding.POLY_TYPE) { + if (resultExpr.isPolyExpression()) { this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] = resultExpr.resolveTypeExpecting(upperScope, this.expectedType); } @@ -374,7 +372,7 @@ public TypeBinding resolveType(BlockScope upperScope) { } if (yieldErrors) return this.resolvedType = null; - this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null; + return this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null; } boolean uniformYield = true; @@ -641,6 +639,17 @@ public boolean sIsMoreSpecific(TypeBinding s, TypeBinding t, Scope skope) { } return true; } + + @Override + public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) { + if (!makeShort) { + return super.printExpression(tab, output); + } else { + printIndent(tab, output).append("switch ("); //$NON-NLS-1$ + return this.expression.printExpression(0, output).append(") { ... }"); //$NON-NLS-1$ + } + } + @Override public TypeBinding expectedType() { return this.expectedType; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java index 0172477734c..4f95951282c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java @@ -222,9 +222,7 @@ public void resolve(BlockScope scope) { } else if (this.switchExpression == null) { scope.problemReporter().yieldOutsideSwitchExpression(this); } - TypeBinding type = this.expression.resolveType(scope); - if (this.switchExpression != null && type != null) - this.switchExpression.originalTypeMap.put(this.expression, type); + this.expression.resolveType(scope); } @Override diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/PolyTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/PolyTypeBinding.java index 13d4382d16a..72b08396309 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/PolyTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/PolyTypeBinding.java @@ -17,7 +17,6 @@ package org.eclipse.jdt.internal.compiler.lookup; import org.eclipse.jdt.internal.compiler.ast.Expression; -import org.eclipse.jdt.internal.compiler.ast.LambdaExpression; public class PolyTypeBinding extends TypeBinding { @@ -75,8 +74,7 @@ public char[] readableName() { @Override public char[] shortReadableName() { - return this.expression instanceof LambdaExpression ? - ((LambdaExpression) this.expression).printExpression(0, new StringBuilder(), true).toString().toCharArray() : readableName(); + return this.expression.printExpression(0, new StringBuilder(), true).toString().toCharArray(); } @Override diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java index a2f376fcb39..e9c9e934613 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java @@ -6021,7 +6021,7 @@ public void testBug566125_06() { "1. ERROR in X.java (at line 7)\n" + " boolean b = foo( switch(i+1) {\n" + " ^^^\n" + - "The method foo(short) in the type X is not applicable for the arguments (double)\n" + + "The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" + "----------\n" + "2. ERROR in X.java (at line 9)\n" + " default -> Double.valueOf(2.0d);\n" + @@ -6057,7 +6057,7 @@ public void testBug566125_07() { "1. ERROR in X.java (at line 7)\n" + " boolean b = foo( switch(i+1) {\n" + " ^^^\n" + - "The method foo(short) in the type X is not applicable for the arguments (double)\n" + + "The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" + "----------\n" + "2. ERROR in X.java (at line 9)\n" + " default -> 2.0d;\n" + @@ -6094,7 +6094,7 @@ public void testBug566125_08() { "1. ERROR in X.java (at line 7)\n" + " boolean b = foo( switch(i+1) {\n" + " ^^^\n" + - "The method foo(short) in the type X is not applicable for the arguments (double)\n" + + "The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" + "----------\n" + "2. ERROR in X.java (at line 9)\n" + " default : yield 2.0d;\n" + @@ -6341,12 +6341,7 @@ public void testGH520() throws Exception { "1. ERROR in X.java (at line 3)\n" + " foo(switch (i) {\n" + " ^^^\n" + - "The method foo(T) in the type X is not applicable for the arguments (switch (i) {\n" + - "case 0 ->\n" + - " m.call();\n" + - "default ->\n" + - " null;\n" + - "})\n" + + "The method foo(T) in the type X is not applicable for the arguments (switch (i) { ... })\n" + "----------\n" + "2. ERROR in X.java (at line 4)\n" + " case 0 -> m.call();\n" + @@ -8087,4 +8082,120 @@ public static void main(String[] args) { "----------\n"); } -} + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3205 + // [Switch Expressions] ECJ accepts ambiguous method invocation involving switch expressions with poly type result expression + public void testIssue3205() { + this.runConformTest( + new String[] { + "X.java", + """ + interface I { + void foo(); + } + + interface J { + void foo(); + } + + public class X implements I, J { + + public void foo() { + } + + static void doit() {} + + public static void foo(Object o, J j) { + System.out.println("Object"); + } + + public static void foo(String s, I i) { + System.out.println(s); + } + + public static void main(String[] args) { + + // Compiles with both ECJ and javac + foo("OK", () -> {}); + + // Compiles with both ECJ and javac + foo("OK", args == null ? () -> {} : () -> {}); + + // Compiles with both ECJ and javac + foo("OK", X::doit); + + // Rejected by both ECJ and javac. + //foo("OK", new X()); + + // Rejected by javac, accepted by ECJ. + foo("OK", switch (0) { + case 0 -> () -> {}; + default -> () -> {}; + }); + } + } + """ + }, + "OK\nOK\nOK\nOK"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3205 + // [Switch Expressions] ECJ accepts ambiguous method invocation involving switch expressions with poly type result expression + public void testIssue3205_2() { + this.runNegativeTest( + new String[] { + "X.java", + """ + interface I { + void foo(); + } + + interface J { + void foo(); + } + + public class X implements I, J { + + public void foo() { + } + + static void doit() {} + + public static void foo(Object o, J j) { + System.out.println("Object"); + } + + public static void foo(String s, I i) { + System.out.println(s); + } + + public static void main(String[] args) { + + // Compiles with both ECJ and javac + foo("OK", () -> {}); + + // Compiles with both ECJ and javac + foo("OK", args == null ? () -> {} : () -> {}); + + // Compiles with both ECJ and javac + foo("OK", X::doit); + + // Rejected by both ECJ and javac. + foo("OK", new X()); + + // Rejected by javac, accepted by ECJ. + foo("OK", switch (0) { + case 0 -> () -> {}; + default -> () -> {}; + }); + } + } + """ + }, + "----------\n" + + "1. ERROR in X.java (at line 36)\n" + + " foo(\"OK\", new X());\n" + + " ^^^\n" + + "The method foo(Object, J) is ambiguous for the type X\n" + + "----------\n"); + } +} \ No newline at end of file