Refactoring class/record duplicates switch guard clause in case statements

[homberghp]

Closes #8495.

Added test, to show the problem is real.

Changed org.netbeans.modules.java.source.save.CasualDiff.java

netbeans/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java

Lines 2133 to 2136 in 37f95bd

	if (Objects.equals(oldT.getCaseKind(), newT.getCaseKind())) {
	tokenSequence.moveNext();
	copyTo(localPointer, localPointer = tokenSequence.offset());
	} else {

Line 2135 copies too much because localPointer is not properly advanced.

Solution: move localPointer to behind token that precedes the COLON od ARROW token.
Then copy, preserving the space that (optionally) precedes the COLON or ARROW from the updated localPointer
to the offset of COLON or ARROW.

---

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

• are all your own work, and you have the right to contribute them.
• are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

1. Was this PR correctly labeled, did the right tests run? When did they run?
2. Is this PR squashed?
3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

[homberghp]

Cleanup as necessary to prepare the PR for review.

[lahodaj]

Overall looks like a good direction. Please see inline comments.

[lahodaj]

I am sorry, but the change should (I think) rather be:

diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java b/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
index 59e9a01810..c8c454e899 100644
--- a/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
+++ b/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
@@ -2122,7 +2122,7 @@ public class CasualDiff {
         if (oldT.guard != null && newT.guard != null) {
             int[] guardBounds = getBounds(oldT.guard);
             copyTo(localPointer, guardBounds[0]);
-            diffTree(oldT.guard, newT.guard, guardBounds);
+            localPointer = diffTree(oldT.guard, newT.guard, guardBounds);
         } else if (oldT.guard != null && newT.guard == null) {
             //TODO:
         } else if (oldT.guard == null && newT.guard != null) {

[homberghp]

done

[lahodaj]

This is not a bad test, but for codegen (like CasualDiff) changes, it is better to also have tests under java.source.base. E.g.:

diff --git a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/SwitchTest.java b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/SwitchTest.java
index 252714d433..6688b0ddaa 100644
--- a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/SwitchTest.java
+++ b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/SwitchTest.java
@@ -613,6 +613,56 @@ public class SwitchTest extends GeneratorTestBase {
         assertEquals(golden, res);
     }
     
+    public void testGuardPreserved() throws Exception {
+        testFile = new File(getWorkDir(), "Test.java");
+        String test =
+                """
+                public class Test {
+                    void m(Object o) {
+                        switch (o) {
+                            case String str when str.isEmpty() -> System.err.println();
+                            default -> {}
+                        }
+                    }
+                }
+                """;
+        String golden =
+                """
+                public class Test {
+                    void m(Object o) {
+                        switch (o) {
+                            case String str when str.isEmpty() -> {
+                            }
+                            default -> {}
+                        }
+                    }
+                }
+                """;
+        TestUtilities.copyStringToFile(testFile, test.replace("|", ""));
+        JavaSource src = getJavaSource(testFile);
+        Task<WorkingCopy> task = new Task<WorkingCopy>() {
+            public void run(final WorkingCopy copy) throws IOException {
+                if (copy.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0) {
+                    return;
+                }
+                final TreeMaker make = copy.getTreeMaker();
+                new ErrorAwareTreePathScanner<Void, Void>() {
+                    @Override public Void visitCase(CaseTree node, Void p) {
+                        if (!node.getLabels().isEmpty()) {
+                            copy.rewrite(getCurrentPath().getLeaf(),
+                                         make.CasePatterns(node.getLabels(), node.getGuard(), make.Block(List.of(), false)));
+                        }
+                        return super.visitCase(node, p);
+                    }
+                }.scan(copy.getCompilationUnit(), null);
+            }
+        };
+        src.runModificationTask(task).commit();
+        String res = TestUtilities.copyFileToString(testFile);
+        //System.err.println(res);
+        assertEquals(golden, res);
+    }
+
     private boolean enhancedSwitchAvailable() {
         try {
             Class.forName("com.sun.source.tree.CaseTree$CaseKind", false, JCTree.class.getClassLoader());

[homberghp]

did not know that.
Done. Waiting for tests (local run) to pass. BTW, some fail with an error on JDK 21. Running tests with 17.
Passed.