lahodaj commented on code in PR #8591:
URL: https://github.com/apache/netbeans/pull/8591#discussion_r2200798350


##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RenameRecordTest.java:
##########
@@ -382,6 +382,63 @@ private NewName test() {
                                     """));
 
     }
+

Review Comment:
   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
   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());
   ```



##########
java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java:
##########
@@ -2131,7 +2131,11 @@ protected int diffCase(JCCase oldT, JCCase newT, int[] 
bounds) {
         do { } while (tokenSequence.moveNext() && JavaTokenId.COLON != 
tokenSequence.token().id() && JavaTokenId.ARROW != tokenSequence.token().id());
         boolean reindentStatements = false;
         if (Objects.equals(oldT.getCaseKind(), newT.getCaseKind())) {
+            localPointer= tokenSequence.offset();

Review Comment:
   I am sorry, but the change should (I think) rather be:
   ```diff
   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) {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to