From 75199a9f965976bf6bd86994d567e3ae6b2ded93 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Sun, 2 Feb 2025 14:27:47 +0100 Subject: [PATCH] Organize Members hint: don't rearrange record components if the formatter is configured to sort members alphabetically, the hint should not rearrange record components. --- .../modules/java/hints/OrganizeMembers.java | 65 ++++----- .../java/hints/OrganizeMembersTest.java | 133 ++++++++++++++++++ .../api/java/source/TreeUtilities.java | 9 +- 3 files changed, 171 insertions(+), 36 deletions(-) create mode 100644 java/java.hints/test/unit/src/org/netbeans/modules/java/hints/OrganizeMembersTest.java 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 index 26639d0d9f4c..5fb493a34d93 100644 --- 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.emptyList()); - List members = new ArrayList<>(clazz.getMembers().size()); - Map memberMap = new HashMap<>(clazz.getMembers().size()); + List members = new ArrayList<>(); + Map memberMap = new HashMap<>(); - List enumValues = new ArrayList<>(); + List 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 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 new file mode 100644 index 000000000000..5a836a9a61d9 --- /dev/null +++ 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 index d8c6a4ea0db3..1cfa90ee7182 100644 --- 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, Tree.getKind() == Kind.ANNOTATION_TYPE should be used instead. */