Code review driven iterative clean up of SwitchExpressions support (#…

…3186)

* Get rid of `SwitchExpression.resolveAll` and the kludge introduced by https://bugs.eclipse.org/bugs/show_bug.cgi?id=542560
* Withdraw the error `A switch expression should have a non-empty switch block` as it is subsumed by `A switch expression should have at least one result expression`
* Delete obsolete diagnostic property strings from messages.properties leaving corresponding IProblem constants deprecated
* Remove dead code in the Scanner
* Revert earlier ill thought out renaming of `YieldStatement.switchExpression` that actually _reduces_ readability
* Remove misplaced attempt to diagnose about expression in place of statement from `SwitchStatement.analyseCode` - this is already diagnosed in `SwitchExpression.resolveType()`
* SwitchExpression.java: Eliminate unnecessary null checks; Opt for clearer variable names; Withdraw AST traversal method that is identical to super implementation.

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java

@@ -2317,7 +2317,9 @@ public interface IProblem {
 	/* Java14 errors - begin */
 	/** @since 3.21  */
 	int SwitchExpressionsYieldIncompatibleResultExpressionTypes = TypeRelated + 1700;
-	/** @since 3.21  */
+	/** @since 3.21
+	 * @deprecated no longer issued - will be removed
+	 */
 	int SwitchExpressionsYieldEmptySwitchBlock = Syntax + 1701;
 	/** @since 3.21  */
 	int SwitchExpressionsYieldNoResultExpression = Internal + 1702;

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java

@@ -25,7 +25,6 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.eclipse.jdt.core.compiler.CharOperation;
-import org.eclipse.jdt.internal.compiler.ASTVisitor;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
 import org.eclipse.jdt.internal.compiler.flow.FlowContext;
@@ -46,7 +45,6 @@ public class SwitchExpression extends SwitchStatement implements IPolyExpression
 
 	private int nullStatus = FlowInfo.UNKNOWN;
 	public List<Expression> resultExpressions = new ArrayList<>(0);
-	public boolean resolveAll;
 	/* package */ List<Integer> resultExpressionNullStatus;
 	public boolean jvmStackVolatile = false;
 	private static Map<TypeBinding, TypeBinding[]> type_map;
@@ -376,20 +374,13 @@ public TypeBinding resolveType(BlockScope upperScope) {
 
 				if (this.originalTypeMap == null)
 					this.originalTypeMap = new HashMap<>();
-				super.resolve(upperScope);
 
-				if (this.statements == null || this.statements.length == 0) {
-					//	Report Error JLS 13 15.28.1  The switch block must not be empty.
-					upperScope.problemReporter().switchExpressionEmptySwitchBlock(this);
-					return null;
-				}
+				super.resolve(upperScope);
 
-				resultExpressionsCount = this.resultExpressions != null ? this.resultExpressions.size() : 0;
+				resultExpressionsCount = this.resultExpressions.size();
 				if (resultExpressionsCount == 0) {
-					//  Report Error JLS 13 15.28.1
-					// It is a compile-time error if a switch expression has no result expressions.
 					upperScope.problemReporter().switchExpressionNoResultExpressions(this);
-					return null;
+					return this.resolvedType = null;
 				}
 
 				if (this.originalValueResultExpressionTypes == null) {
@@ -409,9 +400,10 @@ public TypeBinding resolveType(BlockScope upperScope) {
 				// fall through
 			} else {
 				// re-resolving of poly expression:
-				resultExpressionsCount = this.resultExpressions != null ? this.resultExpressions.size() : 0;
+				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);
@@ -420,33 +412,27 @@ public TypeBinding resolveType(BlockScope upperScope) {
 						this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] =
 							resultExpr.resolveTypeExpecting(upperScope, this.expectedType);
 					}
-					// This is a kludge and only way completion can tell this node to resolve all
-					// resultExpressions. Ideal solution is to remove all other expressions except
-					// the one that contain the completion node.
-					if (this.resolveAll) continue;
 					if (resultExpr.resolvedType == null || !resultExpr.resolvedType.isValidBinding())
-						return this.resolvedType = null;
+						yieldErrors = true;
 				}
+				if (yieldErrors)
+					return this.resolvedType = null;
 				this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null;
 				// fall through
 			}
 
-			if (resultExpressionsCount == 1)
-				return this.resolvedType = this.originalValueResultExpressionTypes[0];
-
-			boolean typeUniformAcrossAllArms = true;
+			boolean uniformYield = true;
 			TypeBinding tmp = this.originalValueResultExpressionTypes[0];
-			for (int i = 1, l = this.originalValueResultExpressionTypes.length; i < l; ++i) {
+			for (int i = 1; i < resultExpressionsCount; ++i) {
 				TypeBinding originalType = this.originalValueResultExpressionTypes[i];
 				if (originalType != null && TypeBinding.notEquals(tmp, originalType)) {
-					typeUniformAcrossAllArms = false;
+					uniformYield = false;
 					break;
 				}
 			}
 			// If the result expressions all have the same type (which may be the null type),
 			// then that is the type of the switch expression.
-			if (typeUniformAcrossAllArms) {
-				tmp = this.originalValueResultExpressionTypes[0];
+			if (uniformYield) {
 				for (int i = 1; i < resultExpressionsCount; ++i) {
 					if (this.originalValueResultExpressionTypes[i] != null)
 						tmp = NullAnnotationMatching.moreDangerousType(tmp, this.originalValueResultExpressionTypes[i]);
@@ -506,11 +492,7 @@ public TypeBinding resolveType(BlockScope upperScope) {
 				 *  Otherwise, if any result expression is of type long, then other result expressions that are not of
 				 *  type long are widened to long.
 				 */
-				TypeBinding[] dfl = new TypeBinding[]{// do not change the order JLS 13 5.6
-						TypeBinding.DOUBLE,
-						TypeBinding.FLOAT,
-						TypeBinding.LONG};
-				for (TypeBinding binding : dfl) {
+				for (TypeBinding binding : new TypeBinding[] { TypeBinding.DOUBLE, TypeBinding.FLOAT, TypeBinding.LONG }) { // order important per JLS
 					if (typeSet.contains(binding)) {
 						resultNumeric = binding;
 						break;
@@ -695,19 +677,4 @@ public boolean sIsMoreSpecific(TypeBinding s, TypeBinding t, Scope skope) {
 	public TypeBinding expectedType() {
 		return this.expectedType;
 	}
-	@Override
-	public void traverse(
-			ASTVisitor visitor,
-			BlockScope blockScope) {
-
-		if (visitor.visit(this, blockScope)) {
-			this.expression.traverse(visitor, blockScope);
-			if (this.statements != null) {
-				int statementsLength = this.statements.length;
-				for (int i = 0; i < statementsLength; i++)
-					this.statements[i].traverse(visitor, this.scope);
-			}
-		}
-		visitor.endVisit(this, blockScope);
-	}
 }

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java

@@ -478,19 +478,6 @@ else if ((statement.bits & ASTNode.DocumentedFallthrough) == 0) { // the case is
 						complaintLevel = initialComplaintLevel; // reset complaint
 						fallThroughState = this.containsPatterns ? FALLTHROUGH : CASE;
 					} else {
-						if (!isTrulyExpression() &&
-							compilerOptions.complianceLevel >= ClassFileConstants.JDK14 &&
-							statement instanceof YieldStatement &&
-							((YieldStatement) statement).isImplicit) {
-							YieldStatement y = (YieldStatement) statement;
-							Expression e = ((YieldStatement) statement).expression;
-							/* JLS 13 14.11.2
-									Switch labeled rules in switch statements differ from those in switch expressions (15.28).
-									In switch statements they must be switch labeled statement expressions, ... */
-							if (!y.expression.statementExpression()) {
-								this.scope.problemReporter().invalidExpressionAsStatement(e);
-							}
-						}
 						fallThroughState = getFallThroughState(statement, currentScope); // reset below if needed
 					}
 					if ((complaintLevel = statement.complainIfUnreachable(caseInits, this.scope, complaintLevel, true)) < Statement.COMPLAINED_UNREACHABLE) {

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java

@@ -31,7 +31,7 @@
 public class YieldStatement extends BranchStatement {
 
 	public Expression expression;
-	public SwitchExpression swich; // innermost enclosing switch **expression**
+	public SwitchExpression switchExpression;
 
 	public boolean isImplicit;
 	static final char[] SECRET_YIELD_RESULT_VALUE_NAME = " secretYieldValue".toCharArray(); //$NON-NLS-1$
@@ -105,11 +105,11 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 
 @Override
 protected void setSubroutineSwitchExpression(SubRoutineStatement sub) {
-	sub.setSwitchExpression(this.swich);
+	sub.setSwitchExpression(this.switchExpression);
 }
 
 protected void addSecretYieldResultValue(BlockScope scope) {
-	SwitchExpression se = this.swich;
+	SwitchExpression se = this.switchExpression;
 	if (se == null || !se.jvmStackVolatile)
 		return;
 	LocalVariableBinding local = new LocalVariableBinding(
@@ -138,7 +138,7 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
 		return;
 	}
 	boolean generateExpressionResultCodeExpanded = false;
-	if (this.swich != null && this.swich.jvmStackVolatile && this.swich.resolvedType != null ) {
+	if (this.switchExpression != null && this.switchExpression.jvmStackVolatile && this.switchExpression.resolvedType != null ) {
 		generateExpressionResultCodeExpanded = true;
 		addSecretYieldResultValue(currentScope);
 		assert this.secretYieldResultValue != null;
@@ -152,12 +152,12 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
 		Assignment assignment = new Assignment(lhs, this.expression, 0);
 		assignment.generateCode(currentScope, codeStream);
 	} else {
-		this.expression.generateCode(currentScope, codeStream, this.swich != null);
+		this.expression.generateCode(currentScope, codeStream, this.switchExpression != null);
 		if (this.expression.resolvedType == TypeBinding.NULL) {
-			if (!this.swich.resolvedType.isBaseType()) {
+			if (!this.switchExpression.resolvedType.isBaseType()) {
 				// no opcode called for to align the types, but we need to adjust the notion of type of TOS.
 				codeStream.operandStack.pop(TypeBinding.NULL);
-				codeStream.operandStack.push(this.swich.resolvedType);
+				codeStream.operandStack.push(this.switchExpression.resolvedType);
 			}
 		}
 	}
@@ -187,7 +187,7 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
 		}
 	}
 	if (generateExpressionResultCodeExpanded) {
-		this.swich.refillOperandStack(codeStream);
+		this.switchExpression.refillOperandStack(codeStream);
 		codeStream.load(this.secretYieldResultValue);
 		codeStream.removeVariable(this.secretYieldResultValue);
 	}
@@ -203,19 +203,19 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
 @Override
 public void resolve(BlockScope scope) {
 
-	if (this.swich == null) {
-		this.swich = scope.enclosingSwitchExpression();
-		if (this.swich != null) {
-			this.swich.resultExpressions.add(this.expression);
-			if (this.swich.expressionContext == ASSIGNMENT_CONTEXT || this.swich.expressionContext == INVOCATION_CONTEXT) { // poly switch expression
-				this.expression.setExpressionContext(this.swich.expressionContext); // result expressions feature in same context ...
-				this.expression.setExpectedType(this.swich.expectedType);           // ... with the same target type
+	if (this.switchExpression == null) {
+		this.switchExpression = scope.enclosingSwitchExpression();
+		if (this.switchExpression != null) {
+			this.switchExpression.resultExpressions.add(this.expression);
+			if (this.switchExpression.expressionContext == ASSIGNMENT_CONTEXT || this.switchExpression.expressionContext == INVOCATION_CONTEXT) { // poly switch expression
+				this.expression.setExpressionContext(this.switchExpression.expressionContext); // result expressions feature in same context ...
+				this.expression.setExpectedType(this.switchExpression.expectedType);           // ... with the same target type
 			}
 		}
 	}
 
-	if (this.swich != null || this.isImplicit) {
-		if (this.swich == null && this.isImplicit && !this.expression.statementExpression()) {
+	if (this.switchExpression != null || this.isImplicit) {
+		if (this.switchExpression == null && this.isImplicit && !this.expression.statementExpression()) {
 			if (scope.compilerOptions().sourceLevel >= ClassFileConstants.JDK14) {
 				/* JLS 13 14.11.2
 				Switch labeled rules in switch statements differ from those in switch expressions (15.28).
@@ -230,8 +230,8 @@ Switch labeled rules in switch statements differ from those in switch expression
 		}
 	}
 	TypeBinding type = this.expression.resolveType(scope);
-	if (this.swich != null && type != null)
-		this.swich.originalTypeMap.put(this.expression, type);
+	if (this.switchExpression != null && type != null)
+		this.switchExpression.originalTypeMap.put(this.expression, type);
 }
 
 @Override

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Scanner.java

@@ -5615,9 +5615,6 @@ int disambiguatedRestrictedIdentifierWhen(int restrictedIdentifierToken) {
 					TokenNameIdentifier : TokenNameRestrictedIdentifierWhen;
 }
 int disambiguatedRestrictedIdentifierYield(int restrictedIdentifierToken) {
-	// and here's the kludge
-	if (restrictedIdentifierToken != TokenNameRestrictedIdentifierYield)
-		return restrictedIdentifierToken;
 	if (this.sourceLevel < ClassFileConstants.JDK14)
 		return TokenNameIdentifier;
 

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java

@@ -11680,14 +11680,6 @@ public void switchExpressionIncompatibleResultExpressions(SwitchExpression expre
 		expression.sourceStart,
 		expression.sourceEnd);
 }
-public void switchExpressionEmptySwitchBlock(SwitchExpression expression) {
-	this.handle(
-		IProblem.SwitchExpressionsYieldEmptySwitchBlock,
-		NoArgument,
-		NoArgument,
-		expression.sourceStart,
-		expression.sourceEnd);
-}
 public void switchExpressionNoResultExpressions(SwitchExpression expression) {
 	this.handle(
 		IProblem.SwitchExpressionsYieldNoResultExpression,

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties

@@ -1014,23 +1014,23 @@
 1513 = 'var' cannot be used with type arguments
 
 # Switch-Expressions Java 12 Preview
-1600 = Incompatible switch results expressions {0}
-1601 = A switch expression should have a non-empty switch block
-1602 = A switch expression should have at least one result expression
-1603 = A switch labeled block in a switch expression should not complete normally
-1604 = The last statement of a switch block in a switch expression should not complete normally
-1605 = Trailing switch labels are not allowed in a switch expression.
-1606 = Mixing of different kinds of case statements '->' and  ':' is not allowed within a switch
-1607 = A switch expression should have a default case
-1608 = Switch expressions are allowed only at source level 12 or above
-1609 = Switch Case Labels with '->' are allowed only at source level 12 or above
-1610 = Break of a switch expression should have a value
-1611 = A Switch expression should cover all possible values
-1612 = 'continue' or 'return' cannot be the last statement in a Switch expression case body
+###[obsolete] 1600 = Incompatible switch results expressions {0}
+###[obsolete] 1601 = A switch expression should have a non-empty switch block
+###[obsolete] 1602 = A switch expression should have at least one result expression
+###[obsolete] 1603 = A switch labeled block in a switch expression should not complete normally
+###[obsolete] 1604 = The last statement of a switch block in a switch expression should not complete normally
+###[obsolete] 1605 = Trailing switch labels are not allowed in a switch expression.
+###[obsolete] 1606 = Mixing of different kinds of case statements '->' and  ':' is not allowed within a switch
+###[obsolete] 1607 = A switch expression should have a default case
+###[obsolete] 1608 = Switch expressions are allowed only at source level 12 or above
+###[obsolete] 1609 = Switch Case Labels with '->' are allowed only at source level 12 or above
+###[obsolete] 1610 = Break of a switch expression should have a value
+###[obsolete] 1611 = A Switch expression should cover all possible values
+###[obsolete] 1612 = 'continue' or 'return' cannot be the last statement in a Switch expression case body
 
 # Switch-Expressions Java 14
 1700 = Incompatible switch results expressions {0}
-1701 = A switch expression should have a non-empty switch block
+###[obsolete] 1701 = A switch expression should have a non-empty switch block
 1702 = A switch expression should have at least one result expression
 1703 = A switch labeled block in a switch expression should not complete normally
 1704 = The last statement of a switch block in a switch expression should not complete normally

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java

@@ -168,7 +168,7 @@ public void testBug544073_003() {
 			"	int tw = switch (i) {\n" +
 			"		};\n" +
 			"	         ^^^^^^^^^^^^^^^^\n" +
-			"A switch expression should have a non-empty switch block\n" +
+			"A switch expression should have at least one result expression\n" +
 			"----------\n" +
 			"2. ERROR in X.java (at line 6)\n" +
 			"	int tw = switch (i) {\n" +

org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java

@@ -2696,14 +2696,6 @@ protected void consumeCompilationUnit() {
 	super.consumeCompilationUnit();
 }
 @Override
-protected void consumeSwitchExpression() {
-	super.consumeSwitchExpression();
-	if (this.assistNode != null) {
-		SwitchExpression expr = (SwitchExpression) this.expressionStack[this.expressionPtr];
-		expr.resolveAll = true;
-	}
-}
-@Override
 protected void consumeConditionalExpression(int op) {
 	popElement(K_CONDITIONAL_OPERATOR);
 	super.consumeConditionalExpression(op);