Organize Members hint: don't rearrange record components

mbien · mbien · commit bb696b9049f0 · 2025-06-23T19:00:18.000+02:00
if the formatter is configured to sort members alphabetically,
the hint should not rearrange record components.
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/OrganizeMembers.java b/java/java.hints/src/org/netbeans/modules/java/hints/OrganizeMembers.java
@@ -45,7 +45,7 @@
 import org.netbeans.api.java.source.*;
 import org.netbeans.api.java.source.JavaSource.Phase;
 import org.netbeans.api.java.source.ModificationResult.Difference;
-import org.netbeans.api.progress.ProgressUtils;
+import org.netbeans.api.progress.BaseProgressUtils;
 import org.netbeans.editor.BaseAction;
 import org.netbeans.editor.BaseDocument;
 import org.netbeans.editor.GuardedDocument;
@@ -73,7 +73,7 @@
 @Hint(displayName = "#DN_org.netbeans.modules.java.hints.OrganizeMembers", description = "#DESC_org.netbeans.modules.java.hints.OrganizeMembers", category = "class_structure", enabled = false)
 public class OrganizeMembers {
 
-    @TriggerTreeKind(Kind.CLASS)
+    @TriggerTreeKind({Kind.CLASS, Kind.RECORD, Kind.ENUM})
     public static ErrorDescription checkMembers(final HintContext context) {
         for (Diagnostic<?> d : context.getInfo().getDiagnostics()) {
             if (Hacks.isSyntaxError(d)) {
@@ -122,49 +122,44 @@ private static void doOrganizeMembers(WorkingCopy copy, TreePath path) {
         ClassTree clazz = (ClassTree) path.getLeaf();
         clazz = gu.importComments(clazz, copy.getCompilationUnit());
         TreeMaker maker = copy.getTreeMaker();
+
         ClassTree nue = maker.Class(clazz.getModifiers(), clazz.getSimpleName(), clazz.getTypeParameters(), clazz.getExtendsClause(), clazz.getImplementsClause(), clazz.getPermitsClause(), Collections.<Tree>emptyList());
-        List<Tree> members = new ArrayList<>(clazz.getMembers().size());
-        Map<Tree, Tree> memberMap = new HashMap<>(clazz.getMembers().size());
+        List<Tree> members = new ArrayList<>();
+        Map<Tree, Tree> memberMap = new HashMap<>();
         
-        List<Tree> enumValues = new ArrayList<>();
+        List<Tree> fixedMembers = new ArrayList<>();
         for (Tree tree : clazz.getMembers()) {
-            if (copy.getTreeUtilities().isSynthetic(new TreePath(path, tree))) {
+            // isSynthetic does not consider record components to be synthetic
+            if (copy.getTreeUtilities().isSynthetic(new TreePath(path, tree))
+                    && !(tree.getKind() == Kind.VARIABLE && copy.getTreeUtilities().isRecordComponent((VariableTree)tree))) {
                 continue;
             }
-            Tree member;
-            switch (tree.getKind()) {
-                case CLASS:
-                case INTERFACE:
-                case ENUM:
-                case ANNOTATION_TYPE:
-                    member = maker.setLabel(tree, ((ClassTree)tree).getSimpleName());
-                    break;
-                case VARIABLE:
-                    member = maker.setLabel(tree, ((VariableTree)tree).getName());
-                    if (copy.getTreeUtilities().isEnumConstant((VariableTree)tree)) {
-                        enumValues.add(member);
+            Tree member = switch (tree.getKind()) {
+                case CLASS, INTERFACE, ENUM, RECORD, ANNOTATION_TYPE -> 
+                    maker.setLabel(tree, ((ClassTree)tree).getSimpleName());
+                case VARIABLE -> {
+                    VariableTree vt = (VariableTree)tree;
+                    Tree mem = maker.setLabel(tree, vt.getName());
+                    if (copy.getTreeUtilities().isEnumConstant(vt) || copy.getTreeUtilities().isRecordComponent(vt)) {
+                        fixedMembers.add(mem);
                     }
-                    break;
-                case METHOD:
-                    member = maker.setLabel(tree, ((MethodTree)tree).getName());
-                    break;
-                case BLOCK:
-                    member = maker.asReplacementOf(maker.Block(((BlockTree)tree).getStatements(), ((BlockTree)tree).isStatic()), tree, true);
-                    break;
-                default:
-                    member = tree;    
-            }
+                    yield mem;
+                }
+                case METHOD -> maker.setLabel(tree, ((MethodTree)tree).getName());
+                case BLOCK -> maker.asReplacementOf(maker.Block(((BlockTree)tree).getStatements(), ((BlockTree)tree).isStatic()), tree, true);
+                default -> tree;    
+            };
             members.add(member);
             memberMap.put(member, tree);
         }
         // fool the generator utilities with cloned members, so it does not take positions into account
-        if (enumValues.isEmpty()) {
+        if (fixedMembers.isEmpty()) {
             nue = GeneratorUtilities.get(copy).insertClassMembers(nue, members);
         } else {
-            members.removeAll(enumValues);
+            members.removeAll(fixedMembers);
             int max = nue.getMembers().size();
-            // insert the enum values in the original order
-            for (Tree t : enumValues) {
+            // insert the enum values or record components in the original order
+            for (Tree t : fixedMembers) {
                 nue = maker.insertClassMember(nue, max++, t);
             }
             nue = GeneratorUtilities.get(copy).insertClassMembers(nue, members);
@@ -181,8 +176,8 @@ private static void doOrganizeMembers(WorkingCopy copy, TreePath path) {
     }
     
     private static boolean checkGuarded(Document doc, List<? extends Difference> diffs) {
-        if (doc instanceof GuardedDocument) {
-            MarkBlockChain chain = ((GuardedDocument) doc).getGuardedBlockChain();
+        if (doc instanceof GuardedDocument guardedDocument) {
+            MarkBlockChain chain = guardedDocument.getGuardedBlockChain();
             for (Difference diff : diffs) {
                 if ((chain.compareBlock(diff.getStartPosition().getOffset(), diff.getEndPosition().getOffset()) & MarkBlock.OVERLAP) != 0) {
                     return true;
@@ -226,7 +221,7 @@ public void actionPerformed(final ActionEvent evt, final JTextComponent componen
             final Source source = Source.create(doc);
             if (source != null) {
                 final AtomicBoolean cancel = new AtomicBoolean();
-                ProgressUtils.runOffEventDispatchThread(new Runnable() {
+                BaseProgressUtils.runOffEventDispatchThread(new Runnable() {
                     @Override
                     public void run() {
                         try {
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/OrganizeMembersTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/OrganizeMembersTest.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.hints;
+
+import java.util.prefs.Preferences;
+import org.netbeans.api.editor.mimelookup.MimeLookup;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.modules.editor.java.JavaKit;
+import org.netbeans.modules.java.hints.test.api.HintTest;
+import org.netbeans.modules.java.ui.FmtOptions;
+import org.openide.util.NbBundle;
+
+/**
+ * @author mbien
+ */
+public class OrganizeMembersTest extends NbTestCase {
+    
+    public OrganizeMembersTest(String name) {
+        super(name);
+    }
+    
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        HintTest.create(); // initializes lookup (stolen from bugs.TinyTest)
+        MimeLookup.getLookup(JavaKit.JAVA_MIME_TYPE)
+                  .lookup(Preferences.class)
+                  .putBoolean(FmtOptions.sortMembersInGroups, true);
+    }
+
+    @Override
+    protected boolean runInEQ() {
+        // without it, hint markers are sometimes not found
+        return true;
+    }
+    
+    public void testSimpleMemberSort() throws Exception {
+        HintTest.create()
+            .sourceLevel(11)
+            .input(
+                """
+                package test;
+                public class Test {
+                    public static int b;
+                    public static int a;
+                }
+                """
+            )
+            .run(OrganizeMembers.class)
+            .findWarning("2:4-2:24:verifier:" + NbBundle.getMessage(OrganizeMembers.class, "MSG_OragnizeMembers"))
+            .applyFix()
+            .assertOutput(
+                """
+                package test;
+                public class Test {
+                    public static int a;
+                    public static int b;
+                }
+                """
+            );
+    }
+    
+    public void testRecordMemberSort() throws Exception {
+        HintTest.create()
+            .sourceLevel(17)
+            .input(
+                """
+                package test;
+                public record Test(int d, int c) {
+                    public static int b;
+                    public static int a;
+                }
+                """
+            )
+            .run(OrganizeMembers.class)
+            .findWarning("2:4-2:24:verifier:" + NbBundle.getMessage(OrganizeMembers.class, "MSG_OragnizeMembers"))
+            .applyFix()
+            .assertOutput(
+                """
+                package test;
+                public record Test(int d, int c) {
+                    public static int a;
+                    public static int b;
+                }
+                """
+            );
+    }
+    
+    public void testEnumMemberSort() throws Exception {
+        HintTest.create()
+            .sourceLevel(11)
+            .input(
+                """
+                package test;
+                public enum Test {
+                    B, A;
+                    public static int b;
+                    public static int a;
+                }
+                """
+            )
+            .run(OrganizeMembers.class)
+            .findWarning("3:4-3:24:verifier:" + NbBundle.getMessage(OrganizeMembers.class, "MSG_OragnizeMembers"))
+            .applyFix()
+            .assertOutput(
+                """
+                package test;
+                public enum Test {
+                    B, A;
+                    public static int a;
+                    public static int b;
+                }
+                """
+            );
+    }
+
+}
diff --git a/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java b/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java
@@ -151,7 +151,14 @@ public boolean isEnum(ClassTree tree) {
     public boolean isEnumConstant(VariableTree tree) {
         return (((JCTree.JCModifiers) tree.getModifiers()).flags & Flags.ENUM) != 0;
     }
-    
+
+    /**
+     * Checks whether given variable tree represents a record component.
+     */
+    public boolean isRecordComponent(VariableTree tree) {
+        return (((JCTree.JCModifiers) tree.getModifiers()).flags & Flags.RECORD) != 0;
+    }
+
     /**Checks whether the given tree represents an annotation.
      * @deprecated since 0.67, <code>Tree.getKind() == Kind.ANNOTATION_TYPE</code> should be used instead.
      */
