Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Organize Members hint: don't rearrange record components #8220

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+35 −35
Draft

Organize Members hint: don't rearrange record components #8220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 27 additions & 34 deletions java/java.hints/src/org/netbeans/modules/java/hints/OrganizeMembers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -123,48 +123,41 @@ private static void doOrganizeMembers(WorkingCopy copy, TreePath path) {
clazz = gu.importComments(clazz, copy.getCompilationUnit());
TreeMaker maker = copy.getTreeMaker();
ClassTree nue = maker.Class(clazz.getModifiers(), clazz.getSimpleName(), clazz.getTypeParameters(), clazz.getExtendsClause(), clazz.getImplementsClause(), 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))) {
if (copy.getTreeUtilities().isSynthetic(new TreePath(path, tree))
&& !(tree.getKind() == Kind.VARIABLE && copy.getTreeUtilities().isRecordComponent((VariableTree)tree))) {
Comment on lines -131 to +132
Copy link
Member Author

@mbien mbien Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please note that the !isRecordComponent() check is currently redundant since isSynthetic() does currently not return true for synthetic record members, representing the record components.

inserting

            if (path.getLeaf().getKind() == Kind.VARIABLE &&
                path.getParentPath() != null &&
                path.getParentPath().getLeaf().getKind() == Kind.RECORD) {
                Set<Modifier> mods = ((VariableTree) path.getLeaf()).getModifiers().getFlags();
                if (!mods.contains(Modifier.STATIC)) {
                    return true; // all non static record fields are synthetic
                }
            }

to

netbeans/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java

Line 212 in e38f618

}

would update it but it could break other things

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);
Expand All @@ -181,8 +174,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;
Expand Down Expand Up @@ -226,7 +219,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 {
Expand Down
9 changes: 8 additions & 1 deletion java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Loading