* Alternate approach for checking effective finality of locals referred

from guard expression * Fixes #2077
eclipse-jdt · Mar 5, 2024 · fd09117 · fd09117
1 parent 6c652c6
commit fd09117
Show file tree

Hide file tree

Showing 4 changed files with 136 additions and 24 deletions.
diff --git a/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/GuardedPattern.java b/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/GuardedPattern.java
@@ -19,7 +19,6 @@
 import org.eclipse.jdt.internal.compiler.flow.FlowContext;
 import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
 import org.eclipse.jdt.internal.compiler.impl.Constant;
-import org.eclipse.jdt.internal.compiler.lookup.Binding;
 import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
 import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
 import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
@@ -103,8 +102,12 @@ public TypeBinding resolveType(BlockScope scope) {
 		if (this.resolvedType != null || this.primaryPattern == null)
 			return this.resolvedType;
 		this.resolvedType = this.primaryPattern.resolveType(scope);
-
-		this.condition.resolveTypeExpectingWithBindings(this.primaryPattern.bindingsWhenTrue(), scope, TypeBinding.BOOLEAN);
+		try {
+			scope.resolvingGuardExpression = true; // as guards cannot nest in the same scope, no save & restore called for
+			this.condition.resolveTypeExpectingWithBindings(this.primaryPattern.bindingsWhenTrue(), scope, TypeBinding.BOOLEAN);
+		} finally {
+			scope.resolvingGuardExpression = false;
+		}
 		Constant cst = this.condition.optimizedBooleanConstant();
 		if (cst.typeID() == TypeIds.T_boolean && cst.booleanValue() == false) {
 			scope.problemReporter().falseLiteralInGuard(this.condition);
@@ -113,27 +116,6 @@ public TypeBinding resolveType(BlockScope scope) {
 		if (!isEffectivelyUnguarded())
 			this.primaryPattern.setIsEffectivelyGuarded();
 
-		this.condition.traverse(new ASTVisitor() {
-			@Override
-			public boolean visit(
-					SingleNameReference ref,
-					BlockScope skope) {
-				LocalVariableBinding local = ref.localVariableBinding();
-				if (local != null) {
-					ref.bits |= ASTNode.IsUsedInPatternGuard;
-				}
-				return false;
-			}
-			@Override
-			public boolean visit(
-					QualifiedNameReference ref,
-					BlockScope skope) {
-				if ((ref.bits & ASTNode.RestrictiveFlagMASK) == Binding.LOCAL) {
-					ref.bits |= ASTNode.IsUsedInPatternGuard;
-				}
-				return false;
-			}
-		}, scope);
 		return this.resolvedType = this.primaryPattern.resolvedType;
 	}
 

diff --git a/...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java b/...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java
@@ -70,6 +70,7 @@ public class BlockScope extends Scope {
 	// annotation support
 	public boolean insideTypeAnnotation = false;
 	public Statement blockStatement;
+	public boolean resolvingGuardExpression = false;
 
     private boolean reparentLocals = false;
 
@@ -1431,6 +1432,12 @@ public void reportClashingDeclarations(LocalVariableBinding [] left, LocalVariab
 		}
 	}
 }
+
+@Override
+public boolean resolvingGuardExpression() {
+	return this.resolvingGuardExpression;
+}
+
 public void include(LocalVariableBinding[] bindings) {
 	// `this` is assumed to be populated with bindings.
 	if (bindings != null) {

diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java
@@ -119,6 +119,7 @@ public char[] readableName() {
 	public final CompilationUnitScope compilationUnitScope;
 	private Map<String, Supplier<ReferenceBinding>> commonTypeBindings = null;
 
+
 	private static class NullDefaultRange {
 		final int start, end;
 		int value;
@@ -2035,6 +2036,10 @@ public LocalVariableBinding findVariable(char[] variable, InvocationSite invocat
 		return null;
 	}
 
+	public boolean resolvingGuardExpression() {
+		return false;
+	}
+
 	/* API
 	 *
 	 *	Answer the binding that corresponds to the argument name.
@@ -2075,6 +2080,7 @@ public Binding getBinding(char[] name, int mask, InvocationSite invocationSite,
 				int depth = 0;
 				int foundDepth = 0;
 				boolean shouldTrackOuterLocals = false;
+				boolean resolvingGuardExpression = false;
 				ReferenceBinding foundActualReceiverType = null;
 				done : while (true) { // done when a COMPILATION_UNIT_SCOPE is found
 					switch (scope.kind) {
@@ -2086,6 +2092,8 @@ public Binding getBinding(char[] name, int mask, InvocationSite invocationSite,
 
 							//$FALL-THROUGH$ could duplicate the code below to save a cast - questionable optimization
 						case BLOCK_SCOPE :
+							if (!resolvingGuardExpression)
+								resolvingGuardExpression = scope.resolvingGuardExpression();
 							LocalVariableBinding variableBinding = scope.findVariable(name, invocationSite);
 							// looks in this scope only
 							if (variableBinding != null) {
@@ -2106,6 +2114,8 @@ public Binding getBinding(char[] name, int mask, InvocationSite invocationSite,
 										variableDeclaration.bits |= ASTNode.ShadowsOuterLocal;
 									}
 								}
+								if (resolvingGuardExpression && invocationSite instanceof NameReference nameReference)
+									nameReference.bits |= ASTNode.IsUsedInPatternGuard;
 								return variableBinding;
 							}
 							break;

diff --git a/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java b/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java
@@ -7707,4 +7707,117 @@ public static void main(String argv[]) {
 			},
 		 "133");
 	}
+	// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2077
+	// [Patterns] Incorrect complaint about non-final variable reference from guard expression
+	public void testIssue2077() throws Exception {
+		runConformTest(
+			new String[] {
+				"X.java",
+				"""
+				public class X {
+				    public static void main(String [] args) {
+				        new X().foo("Hello ", 1);
+				    }
+				    public void foo(Object obj, int x) {
+				        int y = 10;
+				        y = 20;
+				        y = 30;
+				        switch (obj) {
+				         case String s when switch (x) {
+				                    case 1 -> { int y1 = 10; y1 = 30; yield y1!=20; }
+				                    default -> { yield false; }
+				                }
+				                 -> {
+				                    System.out.println(s + "OK");
+				                    if (y == 0)
+				                    	System.out.println(s + "OK");
+				                 }
+
+				         default -> {}
+				        }
+				    }
+				}
+				"""
+			},
+		 "Hello OK");
+	}
+	// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2077
+	// [Patterns] Incorrect complaint about non-final variable reference from guard expression
+	public void testIssue2077_2() throws Exception {
+		runNegativeTest(
+			new String[] {
+				"X.java",
+				"""
+				public class X {
+				    public static void main(String [] args) {
+				        new X().foo("Hello ", 1);
+				    }
+				    public void foo(Object obj, int x) {
+				        int y = 10;
+				        y = 20;
+				        y = 30;
+				        switch (obj) {
+				         case String s when switch (x) {
+				                    case 1 -> { int y1 = 10; y1 = 30; yield 30 != y1; }
+				                    default -> { yield false; }
+				                } && y != 0
+				                 -> {
+				                    System.out.println(s + "OK");
+				                    if (y == 0)
+				                    	System.out.println(s + "OK");
+				                 }
+
+				         default -> {}
+				        }
+				    }
+				}
+				"""
+			},
+			"----------\n" +
+			"1. ERROR in X.java (at line 13)\r\n" +
+			"	} && y != 0\r\n" +
+			"	     ^\n" +
+			"Local variable y referenced from a guard must be final or effectively final\n" +
+			"----------\n");
+	}
+	// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2077
+	// [Patterns] Incorrect complaint about non-final variable reference from guard expression
+	public void testIssue2077_3() throws Exception {
+		runNegativeTest(
+			new String[] {
+				"X.java",
+				"""
+				public class X {
+				    public static void main(String [] args) {
+				        new X().foo("Hello ", 1);
+				    }
+				    public void foo(Object obj, int x) {
+				        int y = 10;
+				        y = 20;
+				        y = 30;
+				        switch (obj) {
+				         case String s when switch (x) {
+				                    case 1 -> { int y1 = 10; y1 = 30; yield y != y1; }
+				                    default -> { yield false; }
+				                } && y != 0
+				                 -> {
+				                    System.out.println(s + "OK");
+				                    if (y == 0)
+				                    	System.out.println(s + "OK");
+				                 }
+
+				         default -> {}
+				        }
+				    }
+				}
+				"""
+			},
+			"----------\n" +
+			"1. ERROR in X.java (at line 11)\r\n" +
+			"	case 1 -> { int y1 = 10; y1 = 30; yield y != y1; }\r\n" +
+			"	                                        ^\n" +
+			"Local variable y referenced from a guard must be final or effectively final\n" +
+			"----------\n");
+		    // We throw AbortMethod after first error, so second error doesn't surface
+	}
 }