Fix possible out of bounds exception in switch pattern hint

- index can be less than 0 in some situations - added test and code simplification suggestions from Jan Lahoda Co-authored-by: Jan Lahoda <jan.lahoda@oracle.com>
apache · Nov 19, 2024 · 0fa77ff · 0fa77ff
1 parent 9b572bd
commit 0fa77ff
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 12 deletions.
diff --git a/.../java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java b/.../java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java
@@ -28,6 +28,7 @@
 import com.sun.source.tree.SwitchTree;
 import com.sun.source.tree.ThrowTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import java.util.ArrayList;
@@ -272,18 +273,21 @@ public static ErrorDescription switchPatternMatchToSwitchNull(HintContext ctx) {
         }
         Tree expression = ((ParenthesizedTree) switchTree.getExpression()).getExpression();
         Tree parent = (Tree) ctx.getPath().getParentPath().getLeaf();
-        int indexOf;
-        if (parent instanceof BlockTree) {
-            indexOf = ((BlockTree) parent).getStatements().indexOf(switchTree) - 1;
+        int indexOfSwitch;
+        if (parent.getKind() == Kind.BLOCK) {
+            indexOfSwitch = ((BlockTree) parent).getStatements().indexOf(switchTree);
+            if (indexOfSwitch < 1) {
+                return null;
+            }
         } else {
             return null;
         }
-        Tree ifTree = ((BlockTree) parent).getStatements().get(indexOf);
-        if ((!(ifTree instanceof IfTree) || !MatcherUtilities.matches(ctx, new TreePath(ctx.getPath(), ((IfTree) ifTree).getCondition()), "($expr0 == null)", true))
+        Tree ifTree = ((BlockTree) parent).getStatements().get(indexOfSwitch - 1);
+        if (ifTree.getKind() != Kind.IF || !MatcherUtilities.matches(ctx, new TreePath(ctx.getPath(), ((IfTree) ifTree).getCondition()), "($expr0 == null)", true)
                 || !(ctx.getVariables().get("$expr0").getLeaf().toString().equals(expression.toString()))) {
             return null;
         }
-        Fix fix = new FixSwitchPatternMatchToSwitchNull(ctx.getInfo(), ctx.getPath().getParentPath(), indexOf).toEditorFix();
+        Fix fix = new FixSwitchPatternMatchToSwitchNull(ctx.getInfo(), ctx.getPath().getParentPath(), indexOfSwitch).toEditorFix();
         return ErrorDescriptionFactory.forTree(ctx, ifTree, Bundle.ERR_ConvertToSwitchPatternInstanceOf(), fix);
 
     }
@@ -300,11 +304,11 @@ public static boolean isPatternMatch(Tree node) {
 
     private static final class FixSwitchPatternMatchToSwitchNull extends JavaFix {
 
-        private final int indexOf;
+        private final int indexOfSwitch;
 
-        public FixSwitchPatternMatchToSwitchNull(CompilationInfo info, TreePath path, int indexOf) {
+        public FixSwitchPatternMatchToSwitchNull(CompilationInfo info, TreePath path, int indexOfSwitch) {
             super(info, path);
-            this.indexOf = indexOf;
+            this.indexOfSwitch = indexOfSwitch;
         }
 
         @Override
@@ -318,17 +322,17 @@ protected void performRewrite(TransformationContext ctx) throws Exception {
             TreePath main = ctx.getPath();
             TreeMaker make = wc.getTreeMaker();
             List<Tree> caseNullLabel = new LinkedList<>();
-            SwitchTree switchTree = (SwitchTree) ((BlockTree) main.getLeaf()).getStatements().get(indexOf + 1);
+            SwitchTree switchTree = (SwitchTree) ((BlockTree) main.getLeaf()).getStatements().get(indexOfSwitch);
 
-            Tree ifTree = ((BlockTree) main.getLeaf()).getStatements().get(indexOf);
+            Tree ifTree = ((BlockTree) main.getLeaf()).getStatements().get(indexOfSwitch - 1);
             StatementTree thenStatement = ((IfTree) ifTree).getThenStatement();
             caseNullLabel.add(wc.getTreeMaker().Identifier("null"));
             BlockTree blockTree = (BlockTree)thenStatement;
             Tree statementTree = blockTree.getStatements().size() == 1 && isValidCaseTree(blockTree.getStatements().get(0))? blockTree.getStatements().get(0) : blockTree;
             CaseTree caseMultipleSwitchPatterns = wc.getTreeMaker().CasePatterns(caseNullLabel, statementTree);
             SwitchTree insertSwitchCase = make.insertSwitchCase(switchTree, 0, caseMultipleSwitchPatterns);
             wc.rewrite(switchTree, insertSwitchCase);
-            BlockTree removeBlockStatement = make.removeBlockStatement((BlockTree) main.getLeaf(), indexOf);
+            BlockTree removeBlockStatement = make.removeBlockStatement((BlockTree) main.getLeaf(), indexOfSwitch - 1);
             wc.rewrite(main.getLeaf(), removeBlockStatement);
         }
     }

diff --git a/...st/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java b/...st/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java
@@ -209,6 +209,24 @@ public void testSimpleSwitchWithNull() throws Exception {
                         + "}\n");
     }
 
+    @Test
+    public void testSwitchWithNullExceptionSwitchIsFirst() throws Exception {
+        HintTest.create()
+                .input("""
+                       package test;
+                       public class Test {
+                           private static void test(Object o) {
+                               switch (o) {
+                                   case String s -> {}
+                               }
+                           }
+                       }
+                       """, false)
+                .sourceLevel("21")
+                .run(ConvertToSwitchPatternInstanceOf.class)
+                .assertWarnings();
+    }
+
     @Test
     public void testSimpleSwitchWithNullNoHint() throws Exception {
         HintTest.create()