[Enhancement] Add warnings for unused patterns

Fixes eclipse-jdt#3051
srikanth-sankaran · Oct 6, 2024 · e303bef · e303bef
1 parent 9b4746e
commit e303bef
Show file tree

Hide file tree

Showing 3 changed files with 156 additions and 8 deletions.
diff --git a/...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java b/...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java
@@ -74,11 +74,16 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
  int nullPatternCount = 0;
  for (int i = 0, length = this.constantExpressions.length; i < length; i++) {
  Expression e = this.constantExpressions[i];
- for (LocalVariableBinding local : e.bindingsWhenTrue()) {
- local.useFlag = LocalVariableBinding.USED; // these are structurally required even if not touched
+ CompilerOptions compilerOptions = currentScope.compilerOptions();
+ long sourceLevel = compilerOptions.sourceLevel;
+ boolean enablePreviewFeatures = compilerOptions.enablePreviewFeatures;
+ if (!JavaFeature.UNNAMMED_PATTERNS_AND_VARS.isSupported(sourceLevel, enablePreviewFeatures)) {
+ for (LocalVariableBinding local : e.bindingsWhenTrue()) {
+ local.useFlag = LocalVariableBinding.USED; // these are structurally required even if not touched
+ }
  }
  nullPatternCount += e instanceof NullLiteral ? 1 : 0;
- if (i > 0 && (e instanceof Pattern) && !JavaFeature.UNNAMMED_PATTERNS_AND_VARS.isSupported(currentScope.compilerOptions().sourceLevel, currentScope.compilerOptions().enablePreviewFeatures)) {
+ if (i > 0 && (e instanceof Pattern) && !JavaFeature.UNNAMMED_PATTERNS_AND_VARS.isSupported(sourceLevel, enablePreviewFeatures)) {
  if (!(i == nullPatternCount && e instanceof TypePattern))
  currentScope.problemReporter().IllegalFallThroughToPattern(e);
  }

diff --git a/...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypePattern.java b/...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypePattern.java
@@ -18,6 +18,7 @@
 import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
 import org.eclipse.jdt.internal.compiler.flow.FlowContext;
 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.impl.JavaFeature;
 import org.eclipse.jdt.internal.compiler.lookup.*;
@@ -233,8 +234,11 @@ public TypeBinding resolveType(BlockScope scope) {
  this.local.resolve(scope, true);
  if (this.local.binding != null) {
  this.local.binding.modifiers |= ExtraCompilerModifiers.AccOutOfFlowScope; // start out this way, will be BlockScope.include'd when definitely assigned
- if (enclosingPattern != null)
- this.local.binding.useFlag = LocalVariableBinding.USED; // syntactically required even if untouched
+ CompilerOptions compilerOptions = scope.compilerOptions();
+ if (!JavaFeature.UNNAMMED_PATTERNS_AND_VARS.isSupported(compilerOptions.sourceLevel, compilerOptions.enablePreviewFeatures)) {
+ if (enclosingPattern != null)
+ this.local.binding.useFlag = LocalVariableBinding.USED; // syntactically required even if untouched
+ }
  if (this.local.type != null)
  this.resolvedType = this.local.binding.type;
  }

diff --git a/....compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java b/....compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java
@@ -3334,9 +3334,9 @@ public void testGH567() {
  " record Point (int x, int y) {}\n" +
  " void foo(Object o) {\n" +
  " if (o instanceof String s) { int x; }\n" +
- " if (o instanceof Point (int xVal, int yVal)) {}\n" + // Should not report as unused locals - structurally required
+ " if (o instanceof Point (int xVal, int yVal)) {}\n" + // Should not report as unused locals in 21 - structurally required
  " switch (o) {\n" +
- " case String c : \n" + // Should not report as unused local - structurally required
+ " case String c : \n" + // Should not report as unused local in 21 - structurally required
  " break;\n" +
  " default :\n" +
  " break;\n" +
@@ -3345,6 +3345,8 @@ public void testGH567() {
  " }\n" +
  "}"
  },
+ this.complianceLevel == ClassFileConstants.JDK21 ?
+
  "----------\n"
  + "1. WARNING in X.java (at line 4)\n"
  + " if (o instanceof String s) { int x; }\n"
@@ -3355,7 +3357,144 @@ public void testGH567() {
  + " if (o instanceof String s) { int x; }\n"
  + " ^\n"
  + "The value of the local variable x is not used\n"
- + "----------\n",
+ + "----------\n" :
+ "----------\n" +
+ "1. WARNING in X.java (at line 4)\n" +
+ " if (o instanceof String s) { int x; }\n" +
+ " ^\n" +
+ "The value of the local variable s is not used\n" +
+ "----------\n" +
+ "2. WARNING in X.java (at line 4)\n" +
+ " if (o instanceof String s) { int x; }\n" +
+ " ^\n" +
+ "The value of the local variable x is not used\n" +
+ "----------\n" +
+ "3. WARNING in X.java (at line 5)\n" +
+ " if (o instanceof Point (int xVal, int yVal)) {}\n" +
+ " ^^^^\n" +
+ "The value of the local variable xVal is not used\n" +
+ "----------\n" +
+ "4. WARNING in X.java (at line 5)\n" +
+ " if (o instanceof Point (int xVal, int yVal)) {}\n" +
+ " ^^^^\n" +
+ "The value of the local variable yVal is not used\n" +
+ "----------\n" +
+ "5. WARNING in X.java (at line 7)\n" +
+ " case String c : \n" +
+ " ^\n" +
+ "The value of the local variable c is not used\n" +
+ "----------\n",
+ null/*classLibraries*/,
+ true/*shouldFlushOutputDirectory*/,
+ customOptions);
+}
+// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3051
+// [Enhancement] Add warnings for unused patterns
+public void testIssue3051() {
+ if (this.complianceLevel < ClassFileConstants.JDK22)
+ return;
+ Map customOptions = getCompilerOptions();
+ customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
+ this.runNegativeTest(
+ new String[] {
+ "Unused.java",
+ """
+ public class Unused {
+
+ public static void main(String[] args) {
+ record R(int i, long l) {}
+ Object o = null;
+ if (o instanceof String s) {
+
+ }
+ R r = new R(1, 1);
+ switch (r) {
+ case R(_, long lvar) -> {}
+ case R scpatvar -> {}
+ }
+ }
+ }
+ """
+ },
+ "----------\n" +
+ "1. WARNING in Unused.java (at line 6)\n" +
+ " if (o instanceof String s) {\n" +
+ " ^\n" +
+ "The value of the local variable s is not used\n" +
+ "----------\n" +
+ "2. WARNING in Unused.java (at line 11)\n" +
+ " case R(_, long lvar) -> {}\n" +
+ " ^^^^\n" +
+ "The value of the local variable lvar is not used\n" +
+ "----------\n" +
+ "3. WARNING in Unused.java (at line 12)\n" +
+ " case R scpatvar -> {}\n" +
+ " ^^^^^^^^\n" +
+ "The value of the local variable scpatvar is not used\n" +
+ "----------\n",
+ null/*classLibraries*/,
+ true/*shouldFlushOutputDirectory*/,
+ customOptions);
+}
+// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3051
+// [Enhancement] Add warnings for unused patterns
+public void testIssue3051_2() {
+ if (this.complianceLevel < ClassFileConstants.JDK21)
+ return;
+ Map customOptions = getCompilerOptions();
+ customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
+ this.runNegativeTest(
+ new String[] {
+ "Unused.java",
+ """
+ public class Unused {
+
+ public static void main(String[] args) {
+ record R(int i, long l, float f) {}
+ Object o = null;
+ if (o instanceof String s) {
+
+ }
+ R r = new R(1, 1, 1.0f);
+ switch (r) {
+ case R(int ivar, long lvar, float fvar) -> {
+ System.out.println(ivar++);
+ lvar++;
+ }
+ case R scpatvar -> {}
+ }
+ }
+ }
+ """
+ },
+ this.complianceLevel == ClassFileConstants.JDK21 ?
+ "----------\n" +
+ "1. WARNING in Unused.java (at line 6)\n" +
+ " if (o instanceof String s) {\n" +
+ " ^\n" +
+ "The value of the local variable s is not used\n" +
+ "----------\n" :
+ "----------\n" +
+ "1. WARNING in Unused.java (at line 6)\n" +
+ " if (o instanceof String s) {\n" +
+ " ^\n" +
+ "The value of the local variable s is not used\n" +
+ "----------\n" +
+ "2. WARNING in Unused.java (at line 11)\n" +
+ " case R(int ivar, long lvar, float fvar) -> {\n" +
+ " ^^^^\n" +
+ "The value of the local variable lvar is not used\n" +
+ "----------\n" +
+ "3. WARNING in Unused.java (at line 11)\n" +
+ " case R(int ivar, long lvar, float fvar) -> {\n" +
+ " ^^^^\n" +
+ "The value of the local variable fvar is not used\n" +
+ "----------\n" +
+ "4. WARNING in Unused.java (at line 15)\n" +
+ " case R scpatvar -> {}\n" +
+ " ^^^^^^^^\n" +
+ "The value of the local variable scpatvar is not used\n" +
+ "----------\n",
  null/*classLibraries*/,
  true/*shouldFlushOutputDirectory*/,
  customOptions);