From 071fca5cda0a812f476f6b977d68eba86eb5dd7c Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Thu, 12 Nov 2026 16:26:54 +0000 Subject: [PATCH 1/3] Add TreeMaker.Record and record support to pretty printer and diff engine Assisted-by: qwen3.8-27b --- java/java.source.base/apichanges.xml | 12 + .../nbproject/project.properties | 2 +- .../netbeans/api/java/source/TreeMaker.java | 29 ++- .../java/source/builder/TreeFactory.java | 11 +- .../java/source/pretty/VeryPretty.java | 55 +++++ .../modules/java/source/save/CasualDiff.java | 64 +++++- .../source/gen/ClassToRecordDiffTest.java | 169 ++++++++++++++ .../source/gen/RecordToClassDiffTest.java | 208 ++++++++++++++++++ 8 files changed, 542 insertions(+), 8 deletions(-) create mode 100644 java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ClassToRecordDiffTest.java create mode 100644 java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordToClassDiffTest.java diff --git a/java/java.source.base/apichanges.xml b/java/java.source.base/apichanges.xml index 98894a974bab..d17a5a703083 100644 --- a/java/java.source.base/apichanges.xml +++ b/java/java.source.base/apichanges.xml @@ -25,6 +25,18 @@ Java Source API + + + Added TreeMaker.Record + + + + + + Adding TreeMaker.Record to create a ClassTree representing a record. + + + Added TreeMaker.Yield diff --git a/java/java.source.base/nbproject/project.properties b/java/java.source.base/nbproject/project.properties index 60d4aafee63e..b0e9d0cb3753 100644 --- a/java/java.source.base/nbproject/project.properties +++ b/java/java.source.base/nbproject/project.properties @@ -23,7 +23,7 @@ javadoc.name=Java Source Base javadoc.title=Java Source Base javadoc.arch=${basedir}/arch.xml javadoc.apichanges=${basedir}/apichanges.xml -spec.version.base=2.87.0 +spec.version.base=2.88.0 test.qa-functional.cp.extra=${refactoring.java.dir}/modules/ext/nb-javac-api.jar test.unit.run.cp.extra=${o.n.core.dir}/core/core.jar:\ ${o.n.core.dir}/lib/boot.jar:\ diff --git a/java/java.source.base/src/org/netbeans/api/java/source/TreeMaker.java b/java/java.source.base/src/org/netbeans/api/java/source/TreeMaker.java index fa8a63cbb466..1569d23874f4 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/TreeMaker.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/TreeMaker.java @@ -471,13 +471,38 @@ public ClassTree AnnotationType(ModifiersTree modifiers, * empty list. * @see com.sun.source.tree.ClassTree */ - public ClassTree Enum(ModifiersTree modifiers, + public ClassTree Enum(ModifiersTree modifiers, CharSequence simpleName, List implementsClauses, List memberDecls) { return delegate.Enum(modifiers, simpleName, implementsClauses, memberDecls); } - + + /** + * Creates a new ClassTree representing a record. + *

+ * The record components must be included in {@code memberDecls} as + * {@link VariableTree}s created with {@link #RecordComponent}. + * + * @param modifiers the modifiers declaration + * @param simpleName the name of the record without its package, such + * as "Point" for the record "java.awt.Point". + * @param typeParameters the list of type parameters, or an empty list. + * @param implementsClauses the list of the interfaces this record + * implements, or an empty list. + * @param memberDecls the list of fields, methods and record + * components of this record, or an empty list. + * @see com.sun.source.tree.ClassTree + * @since 2.88 + */ + public ClassTree Record(ModifiersTree modifiers, + CharSequence simpleName, + List typeParameters, + List implementsClauses, + List memberDecls) { + return delegate.Record(modifiers, simpleName, typeParameters, implementsClauses, memberDecls); + } + /** * Creates a new CompilationUnitTree. * diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java b/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java index 991512dc9d15..de2be9a32694 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java @@ -326,13 +326,22 @@ public ClassTree AnnotationType(ModifiersTree modifiers, return Class(flags, (com.sun.tools.javac.util.List) modifiers.getAnnotations(), simpleName, Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), memberDecls); } - public ClassTree Enum(ModifiersTree modifiers, + public ClassTree Enum(ModifiersTree modifiers, CharSequence simpleName, List implementsClauses, List memberDecls) { long flags = getBitFlags(modifiers.getFlags()) | Flags.ENUM; return Class(flags, (com.sun.tools.javac.util.List) modifiers.getAnnotations(), simpleName, Collections.emptyList(), null, implementsClauses, Collections.emptyList(), memberDecls); } + + public ClassTree Record(ModifiersTree modifiers, + CharSequence simpleName, + List typeParameters, + List implementsClauses, + List memberDecls) { + long flags = getBitFlags(modifiers.getFlags()) | Flags.RECORD; + return Class(flags, (com.sun.tools.javac.util.List) modifiers.getAnnotations(), simpleName, typeParameters, null, implementsClauses, Collections.emptyList(), memberDecls); + } public CompilationUnitTree CompilationUnit(PackageTree packageDecl, List importDecls, diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java index f526c91e3a41..bbf58c91c207 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java @@ -873,6 +873,41 @@ public void visitClassDef(JCClassDecl tree) { wrapTrees(tree.implementing, cs.wrapExtendsImplementsList(), cs.alignMultilineImplements() ? out.col : out.leftMargin + cs.getContinuationIndentSize()); } + } else if ((flags & RECORD) != 0) { + print("record "); + print(tree.name); + printTypeParameters(tree.typarams); + List components = recordComponents(tree); + if (components.nonEmpty()) { + print(cs.spaceBeforeMethodDeclParen() ? " (" : "("); + if (cs.spaceWithinMethodDeclParens()) { + print(' '); + } + int oldPrec = setPrec(TreeInfo.noPrec); + wrapTrees( + components, + cs.wrapMethodParams(), + cs.alignMultilineMethodParams() + ? out.col + : out.leftMargin + cs.getContinuationIndentSize(), + true + ); + setPrec(oldPrec); + if (cs.spaceWithinMethodDeclParens()) { + needSpace(); + } + print(')'); + } + if (tree.implementing.nonEmpty()) { + wrap("implements ", cs.wrapExtendsImplementsKeyword()); + wrapTrees( + tree.implementing, + cs.wrapExtendsImplementsList(), + cs.alignMultilineImplements() + ? out.col + : out.leftMargin + cs.getContinuationIndentSize() + ); + } } else { if ((flags & ENUM) != 0) print("enum "); @@ -916,6 +951,10 @@ public void visitClassDef(JCClassDecl tree) { print('{'); printInnerCommentsAsTrailing(tree, true); java.util.List members = CasualDiff.filterHidden(diffContext, tree.defs); + if ((flags & RECORD) != 0) { + // the record components are printed in the class header: + members.removeIf(t -> t instanceof JCVariableDecl var && (var.mods.flags & RECORD) != 0); + } if (!members.isEmpty()) { blankLines(enclClass.name.isEmpty() ? cs.getBlankLinesAfterAnonymousClassHeader() : (flags & ENUM) != 0 ? cs.getBlankLinesAfterEnumHeader() : cs.getBlankLinesAfterClassHeader()); boolean firstMember = true; @@ -933,6 +972,22 @@ public void visitClassDef(JCClassDecl tree) { enclClass = enclClassPrev; } + /** + * Returns the record components of the given record class declaration. + * + * @param tree the record class declaration + * @return the list of the record components (variables with the {@code RECORD} flag) + */ + private static List recordComponents(JCClassDecl tree) { + ListBuffer components = new ListBuffer<>(); + for (JCTree t : tree.defs) { + if (t instanceof JCVariableDecl var && (var.mods.flags & RECORD) != 0) { + components.add(t); + } + } + return components.toList(); + } + private void printEnumConstants(java.util.List defs, boolean forceSemicolon, boolean printComments) { boolean first = true; boolean hasNonEnumerator = false; 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 7279a43f6ffb..61033da1d7ed 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 @@ -993,6 +993,9 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { } else if ((newT.mods.flags & Flags.INTERFACE) != 0) { copyTo(localPointer, pos); printer.print("interface"); //NOI18N + } else if ((newT.mods.flags & Flags.RECORD) != 0) { + copyTo(localPointer, pos); + printer.print("record"); //NOI18N } else { copyTo(localPointer, pos); printer.print("class"); //NOI18N @@ -1031,7 +1034,6 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { localPointer = insertHint; } } - //TODO: class to record and vice versa! if (oldT.getKind() == Kind.RECORD && newT.getKind() == Kind.RECORD) { ComponentsAndOtherMembers oldParts = splitOutRecordComponents(filteredOldTDefs); ComponentsAndOtherMembers newParts = splitOutRecordComponents(filteredNewTDefs); @@ -1071,6 +1073,41 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { moveToSrcRelevant(tokenSequence, Direction.FORWARD); // it can be > (GT) or >> (SHIFT) insertHint = tokenSequence.offset() + tokenSequence.token().length(); + } else if (newT.getKind() == Kind.RECORD) { + // class (or other non-record) -> record: print the new record + // components in the header. The old body members are diffed below; + // the ones that became components are simply deleted there. + ComponentsAndOtherMembers newParts = splitOutRecordComponents(filteredNewTDefs); + if (!newParts.components().isEmpty()) { + int posHint = insertHint; + copyTo(localPointer, posHint); + int old = printer.setPrec(TreeInfo.noPrec); + parameterPrint = true; + JCClassDecl oldEnclClass = printer.enclClass; + printer.enclClass = null; + localPointer = diffParameterList( + Collections.emptyList(), newParts.components(), new JavaTokenId[] { JavaTokenId.LPAREN, JavaTokenId.RPAREN }, posHint, Measure.MEMBER + ); + printer.enclClass = oldEnclClass; + parameterPrint = false; + printer.setPrec(old); + } + filteredNewTDefs = newParts.defs(); + } else if (oldT.getKind() == Kind.RECORD && newT.getKind() == Kind.CLASS) { + // record to class: the components in the old header become field + // declarations in the new body, where they are diffed as inserted + // members below. The old component parameter list is dropped. + ComponentsAndOtherMembers oldParts = splitOutRecordComponents(filteredOldTDefs); + filteredOldTDefs = oldParts.defs; + if (!oldParts.components().isEmpty()) { + // skip the old component parameter list '(...)': + moveFwdToToken(tokenSequence, endPos(oldParts.components.getLast()), JavaTokenId.RPAREN); + tokenSequence.moveNext(); + localPointer = tokenSequence.offset(); + } + tokenSequence.move(localPointer); + moveToSrcRelevant(tokenSequence, Direction.FORWARD); + insertHint = tokenSequence.offset() + tokenSequence.token().length(); } switch (getChangeKind(oldT.extending, newT.extending)) { case NOCHANGE: @@ -1202,7 +1239,26 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { } } } - localPointer = diffList(filteredOldTDefs, filteredNewTDefs, insertHint, est, Measure.REAL_MEMBER, printer); + if (newT.getKind() == Kind.RECORD && oldT.getKind() != Kind.RECORD && filteredNewTDefs.isEmpty() && !filteredOldTDefs.isEmpty()) { + // class -> record conversion in which every body member became + // a record component: the body is now empty, emit '{}'. + // (Skipping the member diff would otherwise leave a stray + // line break before the closing brace.) + tokenSequence.move(localPointer); + tokenSequence.moveNext(); // right after the opening '{' + int braceDepth = 0; + while (tokenSequence.moveNext()) { + JavaTokenId id = tokenSequence.token().id(); + if (id == JavaTokenId.LBRACE) { + braceDepth++; + } else if (id == JavaTokenId.RBRACE && --braceDepth < 0) { + break; + } + } + localPointer = tokenSequence.offset(); + } else { + localPointer = diffList(filteredOldTDefs, filteredNewTDefs, insertHint, est, Measure.REAL_MEMBER, printer); + } printer.enclClass = origClass; origClassName = origOuterClassName; newClassName = newOuterClassName; @@ -3498,8 +3554,8 @@ public boolean treesMatch(JCTree t1, JCTree t2, boolean deepMatch) { } private boolean kindChanged(long oldFlags, long newFlags) { - return (oldFlags & (Flags.INTERFACE | Flags.ENUM | Flags.ANNOTATION)) - != (newFlags & (Flags.INTERFACE | Flags.ENUM | Flags.ANNOTATION)); + return (oldFlags & (Flags.INTERFACE | Flags.ENUM | Flags.ANNOTATION | Flags.RECORD)) + != (newFlags & (Flags.INTERFACE | Flags.ENUM | Flags.ANNOTATION | Flags.RECORD)); } protected boolean nameChanged(Name oldName, Name newName) { diff --git a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ClassToRecordDiffTest.java b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ClassToRecordDiffTest.java new file mode 100644 index 000000000000..a76a820ae15e --- /dev/null +++ b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/ClassToRecordDiffTest.java @@ -0,0 +1,169 @@ +/* + * 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.api.java.source.gen; + +import java.io.File; +import java.io.IOException; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import java.util.EnumSet; +import java.util.List; +import javax.lang.model.element.Modifier; +import org.netbeans.api.java.source.*; +import static org.netbeans.api.java.source.JavaSource.*; +import org.netbeans.junit.NbTestSuite; + +public class ClassToRecordDiffTest extends GeneratorTestMDRCompat { + + private String sourceLevel; + + public ClassToRecordDiffTest(String testName) { + super(testName); + } + + public static NbTestSuite suite() { + NbTestSuite suite = new NbTestSuite(); + suite.addTestSuite(ClassToRecordDiffTest.class); + return suite; + } + + public void testClassToRecord() throws Exception { + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + public class R { + private int a; + private String b; + public static final int K = 1; + public int getA() { return a; } + } + """); + String golden = + """ + package hierbas.del.litoral; + public record R(int a, String b) { + public static final int K = 1; + public int getA() { return a; } + } + """; + + JavaSource src = getJavaSource(testFile); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + CompilationUnitTree cut = workingCopy.getCompilationUnit(); + TreeMaker make = workingCopy.getTreeMaker(); + + Tree classDecl = cut.getTypeDecls().get(0); + assertEquals(Kind.CLASS, classDecl.getKind()); + ClassTree classTree = (ClassTree) classDecl; + List members = classTree.getMembers(); + Tree k = null; + Tree getA = null; + for (Tree m : members) { + if (m.getKind() == Kind.VARIABLE && "K".contentEquals(((VariableTree) m).getName())) k = m; + if (m.getKind() == Kind.METHOD && "getA".contentEquals(((com.sun.source.tree.MethodTree) m).getName())) getA = m; + } + assertNotNull(k); + assertNotNull(getA); + VariableTree a = make.RecordComponent(make.Modifiers(EnumSet.noneOf(Modifier.class)), + "a", + make.Type("int")); + VariableTree b = make.RecordComponent(make.Modifiers(EnumSet.noneOf(Modifier.class)), + "b", + make.Type("java.lang.String")); + ClassTree newTree = make.Record(make.Modifiers(EnumSet.of(Modifier.PUBLIC)), + "R", + List.of(), + List.of(), + List.of(a, b, k, getA)); + workingCopy.rewrite(classTree, newTree); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + public void testClassToRecordNoBody() throws Exception { + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + public class R { + private int a; + } + """); + String golden = + """ + package hierbas.del.litoral; + public record R(int a) {} + """; + + JavaSource src = getJavaSource(testFile); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + CompilationUnitTree cut = workingCopy.getCompilationUnit(); + TreeMaker make = workingCopy.getTreeMaker(); + + Tree classDecl = cut.getTypeDecls().get(0); + assertEquals(Kind.CLASS, classDecl.getKind()); + ClassTree classTree = (ClassTree) classDecl; + VariableTree a = make.RecordComponent(make.Modifiers(EnumSet.noneOf(Modifier.class)), + "a", + make.Type("int")); + ClassTree newTree = make.Record(make.Modifiers(EnumSet.of(Modifier.PUBLIC)), + "R", + List.of(), + List.of(), + List.of(a)); + workingCopy.rewrite(classTree, newTree); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + + String getGoldenPckg() { + return ""; + } + + String getSourcePckg() { + return ""; + } + + @Override + String getSourceLevel() { + return sourceLevel; + } +} + diff --git a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordToClassDiffTest.java b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordToClassDiffTest.java new file mode 100644 index 000000000000..ff381ca51295 --- /dev/null +++ b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordToClassDiffTest.java @@ -0,0 +1,208 @@ +/* + * 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.api.java.source.gen; + +import java.io.File; +import java.io.IOException; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import java.util.EnumSet; +import java.util.List; +import javax.lang.model.element.Modifier; +import org.netbeans.api.java.source.*; +import static org.netbeans.api.java.source.JavaSource.*; +import org.netbeans.junit.NbTestSuite; + +public class RecordToClassDiffTest extends GeneratorTestMDRCompat { + + private String sourceLevel; + + public RecordToClassDiffTest(String testName) { + super(testName); + } + + public static NbTestSuite suite() { + NbTestSuite suite = new NbTestSuite(); + suite.addTestSuite(RecordToClassDiffTest.class); + return suite; + } + + public void testRecordToClass() throws Exception { + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + public record R(int a, String b) { + public static final int K = 1; + public int getA() { return a; } + } + """); + String golden = + """ + package hierbas.del.litoral; + public final class R { + + private final int a; + private final String b; + + public R(int a, String b) { + this.a = a; + this.b = b; + } + public static final int K = 1; + public int getA() { return a; } + } + """; + + JavaSource src = getJavaSource(testFile); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + CompilationUnitTree cut = workingCopy.getCompilationUnit(); + TreeMaker make = workingCopy.getTreeMaker(); + + Tree classDecl = cut.getTypeDecls().get(0); + assertEquals(Kind.RECORD, classDecl.getKind()); + ClassTree classTree = (ClassTree) classDecl; + List members = classTree.getMembers(); + Tree k = null; + Tree getA = null; + VariableTree compA = null, compB = null; + for (Tree m : members) { + if (m.getKind() == Kind.VARIABLE) { + if ("a".contentEquals(((VariableTree) m).getName())) compA = (VariableTree) m; + if ("b".contentEquals(((VariableTree) m).getName())) compB = (VariableTree) m; + if ("K".contentEquals(((VariableTree) m).getName())) k = m; + } + if (m.getKind() == Kind.METHOD && "getA".contentEquals(((com.sun.source.tree.MethodTree) m).getName())) getA = m; + } + assertNotNull(k); + assertNotNull(getA); + assertNotNull(compA); + assertNotNull(compB); + VariableTree a = make.Variable(make.Modifiers(EnumSet.of(Modifier.PRIVATE, Modifier.FINAL)), + compA.getName(), + compA.getType(), + null); + VariableTree b = make.Variable(make.Modifiers(EnumSet.of(Modifier.PRIVATE, Modifier.FINAL)), + compB.getName(), + compB.getType(), + null); + com.sun.source.tree.MethodTree ctor = make.Method(make.Modifiers(EnumSet.of(Modifier.PUBLIC)), + "", + null, + List.of(), + List.of( + make.Variable(make.Modifiers(EnumSet.noneOf(Modifier.class)), compA.getName(), compA.getType(), null), + make.Variable(make.Modifiers(EnumSet.noneOf(Modifier.class)), compB.getName(), compB.getType(), null)), + List.of(), + make.Block(List.of( + make.ExpressionStatement(make.Assignment(make.MemberSelect(make.Identifier("this"), compA.getName()), make.Identifier(compA.getName()))), + make.ExpressionStatement(make.Assignment(make.MemberSelect(make.Identifier("this"), compB.getName()), make.Identifier(compB.getName())))), + false), + null); + ClassTree newTree = make.Class(make.Modifiers(EnumSet.of(Modifier.PUBLIC, Modifier.FINAL)), + "R", + List.of(), + null, + List.of(), + List.of(), + List.of(a, b, ctor, k, getA)); + workingCopy.rewrite(classTree, newTree); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + assertEquals(golden, res); + } + + public void testRecordToClassExistingConstructor() throws Exception { + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + public record R(int a) { + public R(int a) { this.a = a; } + } + """); + String golden = + """ + package hierbas.del.litoral; + public final class R { + + private final int a; + public R(int a) { this.a = a; } + } + """; + + JavaSource src = getJavaSource(testFile); + Task task = new Task() { + + public void run(WorkingCopy workingCopy) throws IOException { + workingCopy.toPhase(Phase.RESOLVED); + CompilationUnitTree cut = workingCopy.getCompilationUnit(); + TreeMaker make = workingCopy.getTreeMaker(); + + Tree classDecl = cut.getTypeDecls().get(0); + assertEquals(Kind.RECORD, classDecl.getKind()); + ClassTree classTree = (ClassTree) classDecl; + Tree ctor = null; + for (Tree m : classTree.getMembers()) { + if (m.getKind() == Kind.METHOD) ctor = m; + } + assertNotNull(ctor); + VariableTree a = make.Variable(make.Modifiers(EnumSet.of(Modifier.PRIVATE, Modifier.FINAL)), + "a", + make.Type("int"), + null); + ClassTree newTree = make.Class(make.Modifiers(EnumSet.of(Modifier.PUBLIC, Modifier.FINAL)), + "R", + List.of(), + null, + List.of(), + List.of(), + List.of(a, ctor)); + workingCopy.rewrite(classTree, newTree); + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + assertEquals(golden, res); + } + + String getGoldenPckg() { + return ""; + } + + String getSourcePckg() { + return ""; + } + + @Override + String getSourceLevel() { + return sourceLevel; + } +} From b15a3433cec19b160d487079074059c2296b02ce Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Thu, 12 Nov 2026 16:26:54 +0000 Subject: [PATCH 2/3] Add Class To Record and Record To Class hints Assisted-by: qwen3.8-27b --- .../netbeans/modules/java/hints/Feature.java | 3 + .../java/hints/suggestions/ClassToRecord.java | 222 ++++++++++++++ .../java/hints/suggestions/RecordToClass.java | 223 +++++++++++++++ .../hints/suggestions/ClassToRecordTest.java | 270 ++++++++++++++++++ .../hints/suggestions/RecordToClassTest.java | 142 +++++++++ 5 files changed, 860 insertions(+) create mode 100644 java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java create mode 100644 java/java.hints/src/org/netbeans/modules/java/hints/suggestions/RecordToClass.java create mode 100644 java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java create mode 100644 java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/RecordToClassTest.java diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java b/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java index 4176236793e5..83f47dc2f423 100644 --- a/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java +++ b/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java @@ -38,6 +38,9 @@ public enum Feature { /// https://openjdk.org/jeps/394 INSTANCEOF_PATTERN(14, 16), + /// https://openjdk.org/jeps/395 + RECORD(14, 16), + /// https://openjdk.org/jeps/441 SWITCH_PATTERN(17, 21), diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java new file mode 100644 index 000000000000..83ea9378e88e --- /dev/null +++ b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java @@ -0,0 +1,222 @@ +/* + * 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.suggestions; + +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.Trees; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.Modifier; +import org.netbeans.api.java.source.GeneratorUtilities; +import org.netbeans.api.java.source.TreeMaker; +import org.netbeans.api.java.source.TreePathHandle; +import org.netbeans.api.java.source.TreeUtilities; +import org.netbeans.api.java.source.WorkingCopy; +import org.netbeans.modules.java.hints.Feature; +import org.netbeans.spi.editor.hints.ErrorDescription; +import org.netbeans.spi.java.hints.ErrorDescriptionFactory; +import org.netbeans.spi.java.hints.Hint; +import org.netbeans.spi.java.hints.HintContext; +import org.netbeans.spi.java.hints.JavaFix; +import org.netbeans.spi.java.hints.TriggerTreeKind; +import org.openide.util.NbBundle; +import org.openide.util.NbBundle.Messages; + +/** + * Suggests converting a class declaration into a record. + *

+ * The conversion is only offered when it is safe to do so: the class is not + * abstract, sealed or {@code strictfp}, it declares no explicit constructor, + * it does not extend another class, and its instance fields are plain (no + * initializer or annotations). + * + * @author mbien + */ +public class ClassToRecord { + + @Hint( + displayName = "#DN_convertClassToRecord", + description = "#DESC_convertClassToRecord", + category = "class_structure", + hintKind = Hint.Kind.INSPECTION, + minSourceVersion = "14", + suppressWarnings = "ClassToRecord" + ) + @Messages({ + "DN_convertClassToRecord=Convert Class to Record", + "DESC_convertClassToRecord=Reports classes which can be converted to records. The non-static fields become record components, static fields and methods remain in the body and the canonical constructor is implied.", + "MSG_ClassToRecord=Class {0} may be converted to a record", + "FIX_ConvertClassToRecord=Convert class {0} to a record" + }) + @TriggerTreeKind(Kind.CLASS) + public static ErrorDescription convertClassToRecord(HintContext context) { + if (!Feature.RECORD.isEnabled(context.getInfo())) { + return null; + } + if (testClassMayBeRecord(context.getInfo().getTrees(), context.getInfo().getTreeUtilities(), context.getPath())) { + ClassTree cls = (ClassTree) context.getPath().getLeaf(); + return ErrorDescriptionFactory.forName(context, cls, NbBundle.getMessage(ClassToRecord.class, "MSG_ClassToRecord", cls.getSimpleName()), //NOI18N + new ConvertClassToRecordFixImpl(TreePathHandle.create(context.getPath(), context.getInfo()), + NbBundle.getMessage(ClassToRecord.class, "FIX_ConvertClassToRecord", cls.getSimpleName())).toEditorFix()); //NOI18N + } + return null; + } + + private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtilities, TreePath path) { + // a record may be top level or a member of another class, + // but not a local or anonymous class + if (path.getParentPath() == null) { + return false; + } + Tree parent = path.getParentPath().getLeaf(); + if (parent.getKind() != Kind.COMPILATION_UNIT && !TreeUtilities.CLASS_TREE_KINDS.contains(parent.getKind())) { + return false; + } + Element element = trees.getElement(path); + if (element == null || element.getKind() != ElementKind.CLASS) { + return false; + } + Set classModifiers = element.getModifiers(); + if (classModifiers.contains(Modifier.SEALED) || classModifiers.contains(Modifier.NON_SEALED)) { + return false; + } + if (classModifiers.contains(Modifier.ABSTRACT) || classModifiers.contains(Modifier.STRICTFP)) { + return false; + } + ClassTree cls = (ClassTree) path.getLeaf(); + if (cls.getExtendsClause() != null) { + return false; + } + // record components are the plain non-static fields + Set componentNames = new HashSet<>(); + for (Tree member : cls.getMembers()) { + if (member.getKind() == Kind.VARIABLE && !((VariableTree) member).getModifiers().getFlags().contains(Modifier.STATIC)) { + VariableTree field = (VariableTree) member; + // a record component cannot have an initializer or annotations + if (field.getInitializer() != null || !field.getModifiers().getAnnotations().isEmpty()) { + return false; + } + componentNames.add(field.getName().toString()); + } + } + for (Tree member : cls.getMembers()) { + if (treeUtilities.isSynthetic(TreePath.getPath(path, member))) { + continue; + } + switch (member.getKind()) { + case METHOD -> { + MethodTree mth = (MethodTree) member; + if (mth.getModifiers().getFlags().contains(Modifier.ABSTRACT)) { + // records cannot be abstract + return false; + } + if (mth.getReturnType() == null) { + // an explicit constructor would be dropped in favor + // of the implicit canonical one + return false; + } + if (mth.getParameters().isEmpty() && componentNames.contains(mth.getName().toString())) { + // would collide with the implicitly declared component accessor + return false; + } + } + case VARIABLE -> { + // fields were already validated above: static fields and + // record components both stay valid in the record body + break; + } + case CLASS, ENUM, INTERFACE, ANNOTATION_TYPE -> { + // nested types stay in the record body + break; + } + default -> { + return false; + } + } + } + return true; + } + + private static final class ConvertClassToRecordFixImpl extends JavaFix { + + private final String text; + + public ConvertClassToRecordFixImpl(TreePathHandle clsHandle, String text) { + super(clsHandle); + this.text = text; + } + + @Override + public String getText() { + return text; + } + + @Override + protected void performRewrite(TransformationContext ctx) { + WorkingCopy wc = ctx.getWorkingCopy(); + GeneratorUtilities gu = GeneratorUtilities.get(wc); + TreePath path = ctx.getPath(); + ClassTree cls = (ClassTree) path.getLeaf(); + gu.importComments(cls, wc.getCompilationUnit()); + TreeMaker make = wc.getTreeMaker(); + // records are implicitly final + Set modifiers = EnumSet.copyOf(cls.getModifiers().getFlags()); + modifiers.remove(Modifier.FINAL); + ModifiersTree mods = make.Modifiers(modifiers, cls.getModifiers().getAnnotations()); + gu.copyComments(cls.getModifiers(), mods, true); + gu.copyComments(cls.getModifiers(), mods, false); + List members = new ArrayList<>(); + for (Tree member : cls.getMembers()) { + if (member.getKind() == Kind.METHOD && ((MethodTree) member).getReturnType() == null) { + // the implicit canonical constructor replaces the default one + continue; + } + if (member.getKind() == Kind.VARIABLE && !((VariableTree) member).getModifiers().getFlags().contains(Modifier.STATIC)) { + VariableTree field = (VariableTree) member; + VariableTree component = make.RecordComponent( + make.Modifiers(EnumSet.noneOf(Modifier.class)), + field.getName(), + field.getType() + ); + gu.copyComments(field, component, true); + gu.copyComments(field, component, false); + members.add(component); + } else { + members.add(member); + } + } + ClassTree nue = make.Record(mods, cls.getSimpleName(), cls.getTypeParameters(), cls.getImplementsClause(), members); + gu.copyComments(cls, nue, true); + gu.copyComments(cls, nue, false); + wc.rewrite(path.getLeaf(), nue); + } + } +} + diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/RecordToClass.java b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/RecordToClass.java new file mode 100644 index 000000000000..0696762b591d --- /dev/null +++ b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/RecordToClass.java @@ -0,0 +1,223 @@ +/* + * 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.suggestions; + +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.StatementTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.Trees; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.Modifier; +import org.netbeans.api.java.source.GeneratorUtilities; +import org.netbeans.api.java.source.TreeMaker; +import org.netbeans.api.java.source.TreePathHandle; +import org.netbeans.api.java.source.TreeUtilities; +import org.netbeans.api.java.source.WorkingCopy; +import org.netbeans.spi.editor.hints.ErrorDescription; +import org.netbeans.spi.editor.hints.Severity; +import org.netbeans.spi.java.hints.ErrorDescriptionFactory; +import org.netbeans.spi.java.hints.Hint; +import org.netbeans.spi.java.hints.HintContext; +import org.netbeans.spi.java.hints.JavaFix; +import org.netbeans.spi.java.hints.TriggerTreeKind; +import org.openide.util.NbBundle; +import org.openide.util.NbBundle.Messages; + +/** + * Suggests converting a record declaration into a class. + *

+ * The record components become {@code private final} fields, the record's + * implicit finality is preserved by a {@code final} class modifier, and an + * explicit canonical constructor is generated when the record does not + * declare one. + * + * @author mbien + */ +public class RecordToClass { + + @Hint( + displayName = "#DN_convertRecordToClass", + description = "#DESC_convertRecordToClass", + category = "class_structure", + hintKind = Hint.Kind.INSPECTION, + severity = Severity.HINT, + minSourceVersion = "14", + suppressWarnings = "RecordToClass" + ) + @Messages({ + "DN_convertRecordToClass=Convert Record to Class", + "DESC_convertRecordToClass=Reports records which can be converted to classes. The record components become private final fields, the canonical constructor is made explicit and the remaining members stay in the body.", + "MSG_RecordToClass=Record {0} can be converted to a class", + "FIX_ConvertRecordToClass=Convert record {0} to a class" + }) + @TriggerTreeKind(Kind.RECORD) + public static ErrorDescription convertRecordToClass(HintContext context) { + if (testRecordMayBeClass(context.getInfo().getTrees(), context.getPath())) { + ClassTree rec = (ClassTree) context.getPath().getLeaf(); + return ErrorDescriptionFactory.forName(context, rec, NbBundle.getMessage(RecordToClass.class, "MSG_RecordToClass", rec.getSimpleName()), //NOI18N + new ConvertRecordToClassFixImpl(TreePathHandle.create(context.getPath(), context.getInfo()), + NbBundle.getMessage(RecordToClass.class, "FIX_ConvertRecordToClass", rec.getSimpleName())).toEditorFix()); //NOI18N + } + return null; + } + + private static boolean testRecordMayBeClass(Trees trees, TreePath path) { + // a class may be top level or a member of another class, + // but not a local or anonymous class + if (path.getParentPath() == null) { + return false; + } + Tree parent = path.getParentPath().getLeaf(); + if (parent.getKind() != Kind.COMPILATION_UNIT && !TreeUtilities.CLASS_TREE_KINDS.contains(parent.getKind())) { + return false; + } + Element element = trees.getElement(path); + return element != null && element.getKind() == ElementKind.RECORD; + } + + private static final class ConvertRecordToClassFixImpl extends JavaFix { + + private final String text; + + public ConvertRecordToClassFixImpl(TreePathHandle recHandle, String text) { + super(recHandle); + this.text = text; + } + + @Override + public String getText() { + return text; + } + + @Override + protected void performRewrite(TransformationContext ctx) { + WorkingCopy wc = ctx.getWorkingCopy(); + GeneratorUtilities gu = GeneratorUtilities.get(wc); + TreePath path = ctx.getPath(); + ClassTree rec = (ClassTree) path.getLeaf(); + gu.importComments(rec, wc.getCompilationUnit()); + TreeMaker make = wc.getTreeMaker(); + TreeUtilities tu = wc.getTreeUtilities(); + List components = new ArrayList<>(); + List otherMembers = new ArrayList<>(); + for (Tree member : rec.getMembers()) { + if (tu.isSynthetic(new TreePath(path, member))) { + // skip the implicit canonical constructor the compiler + // adds to the tree - it has no source text of its own + continue; + } + if (member.getKind() == Kind.VARIABLE && tu.isRecordComponent((VariableTree) member)) { + components.add((VariableTree) member); + } else { + otherMembers.add(member); + } + } + // a record is implicitly final: keep the finality in the class + Set modifiers = EnumSet.noneOf(Modifier.class); + for (Modifier m : rec.getModifiers().getFlags()) { + if (m == Modifier.PUBLIC || m == Modifier.PROTECTED || m == Modifier.PRIVATE) { + modifiers.add(m); + } + } + modifiers.add(Modifier.FINAL); + ModifiersTree mods = make.Modifiers(modifiers, rec.getModifiers().getAnnotations()); + gu.copyComments(rec.getModifiers(), mods, true); + gu.copyComments(rec.getModifiers(), mods, false); + boolean hasCanonicalConstructor = false; + for (Tree member : otherMembers) { + if (member.getKind() == Kind.METHOD) { + MethodTree mth = (MethodTree) member; + if (mth.getReturnType() == null && isCanonicalConstructor(mth, components)) { + hasCanonicalConstructor = true; + } + } + } + List members = new ArrayList<>(); + for (VariableTree component : components) { + VariableTree field = make.Variable(make.Modifiers(EnumSet.of(Modifier.PRIVATE, Modifier.FINAL)), + component.getName(), + component.getType(), + null); + gu.copyComments(component, field, true); + gu.copyComments(component, field, false); + members.add(field); + } + if (!hasCanonicalConstructor) { + members.add(makeCanonicalConstructor(make, rec, components)); + } + members.addAll(otherMembers); + ClassTree nue = make.Class(mods, rec.getSimpleName(), rec.getTypeParameters(), null, rec.getImplementsClause(), List.of(), members); + gu.copyComments(rec, nue, true); + gu.copyComments(rec, nue, false); + wc.rewrite(path.getLeaf(), nue); + } + + // the compiler enforces that the canonical constructor's parameters + // are named after the record components, in order + private static boolean isCanonicalConstructor(MethodTree mth, List components) { + List params = mth.getParameters(); + if (params.size() != components.size()) { + return false; + } + for (int i = 0; i < params.size(); i++) { + if (!params.get(i).getName().contentEquals(components.get(i).getName())) { + return false; + } + } + return true; + } + + private static MethodTree makeCanonicalConstructor(TreeMaker make, ClassTree rec, List components) { + // the implicit canonical constructor has the record's access + Set flags = rec.getModifiers().getFlags().contains(Modifier.PUBLIC) ? + EnumSet.of(Modifier.PUBLIC) : + EnumSet.noneOf(Modifier.class); + List params = new ArrayList<>(); + List body = new ArrayList<>(); + for (VariableTree component : components) { + params.add(make.Variable(make.Modifiers(EnumSet.noneOf(Modifier.class)), + component.getName(), + component.getType(), + null)); + body.add(make.ExpressionStatement(make.Assignment( + make.MemberSelect(make.Identifier("this"), component.getName()), //NOI18N + make.Identifier(component.getName())))); + } + return make.Method(make.Modifiers(flags), + "", //NOI18N + null, + List.of(), + params, + List.of(), + make.Block(body, false), + null); + } + } +} + diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java new file mode 100644 index 000000000000..57ab58bef44f --- /dev/null +++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java @@ -0,0 +1,270 @@ +/* + * 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.suggestions; + +import org.junit.Test; +import org.netbeans.junit.NbTestCase; +import org.netbeans.modules.java.hints.test.api.HintTest; + +/** + * @author mbien + */ +public class ClassToRecordTest extends NbTestCase { + + public ClassToRecordTest(String name) { + super(name); + } + + @Test + public void testConvertClassToRecord() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + private String b; + public static final int K = 1; + public int getA() { + return a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a, String b) { + public static final int K = 1; + public int getA() { + return a; + } + } + """); + } + + @Test + public void testConvertClassToRecordNoBody() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) {} + """); + } + + @Test + public void testConvertFinalClassToRecord() throws Exception { + HintTest.create() + .input( """ + package test; + public final class Test { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:19-1:23:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) {} + """); + } + + @Test + public void testConvertClassWithImplements() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test implements Cloneable, java.io.Serializable { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) implements Cloneable, java.io.Serializable {} + """); + } + + @Test + public void testConvertGenericClassToRecord() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private T value; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(T value) {} + """); + } + + @Test + public void testClassWithConstructor() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + public Test(int a) { + this.a = a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testClassWithExtends() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test extends Object { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testAbstractClass() throws Exception { + HintTest.create() + .input( """ + package test; + public abstract class Test { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testFieldWithInitializer() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a = 1; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testFieldWithAnnotation() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + @Deprecated private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testAccessorNameCollision() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + public int a() { + return a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testSealedClass() throws Exception { + HintTest.create() + .input( """ + package test; + public sealed class Test permits Sub { + private int a; + } + final class Sub extends Test { + private final int b = 1; + } + """) + .sourceLevel("17") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testOldSourceLevel() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + } + """) + .sourceLevel("13") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } +} + diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/RecordToClassTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/RecordToClassTest.java new file mode 100644 index 000000000000..f420eac2dd76 --- /dev/null +++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/RecordToClassTest.java @@ -0,0 +1,142 @@ +/* + * 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.suggestions; + +import org.junit.Test; +import org.netbeans.junit.NbTestCase; +import org.netbeans.modules.java.hints.test.api.HintTest; + +/** + * @author mbien + */ +public class RecordToClassTest extends NbTestCase { + + public RecordToClassTest(String name) { + super(name); + } + + @Test + public void testConvertRecordToClass() throws Exception { + HintTest.create() + .input( """ + package test; + public record Test(int a, String b) { + public static final int K = 1; + public int getA() { + return a; + } + } + """) + .sourceLevel("16") + .run(RecordToClass.class) + .findWarning("1:14-1:18:hint:Record Test can be converted to a class") + .applyFix("Convert record Test to a class") + .assertCompilable() + .assertOutput( + """ + package test; + public final class Test { + + private final int a; + private final String b; + + public Test(int a, String b) { + this.a = a; + this.b = b; + } + public static final int K = 1; + public int getA() { + return a; + } + } + """); + } + + @Test + public void testConvertRecordToClassWithConstructor() throws Exception { + HintTest.create() + .input( """ + package test; + public record Test(int a, String b) { + public Test(int a, String b) { + this.a = a; + this.b = b; + } + } + """) + .sourceLevel("16") + .run(RecordToClass.class) + .findWarning("1:14-1:18:hint:Record Test can be converted to a class") + .applyFix("Convert record Test to a class") + .assertCompilable() + .assertOutput( + """ + package test; + public final class Test { + + private final int a; + private final String b; + public Test(int a, String b) { + this.a = a; + this.b = b; + } + } + """); + } + + @Test + public void testConvertRecordToClassNoBody() throws Exception { + HintTest.create() + .input( """ + package test; + public record Test(int a) {} + """) + .sourceLevel("16") + .run(RecordToClass.class) + .findWarning("1:14-1:18:hint:Record Test can be converted to a class") + .applyFix("Convert record Test to a class") + .assertCompilable() + .assertOutput( + """ + package test; + public final class Test { + + private final int a; + public Test(int a) { + this.a = a; + } + } + """); + } + + @Test + public void testNoHintForClass() throws Exception { + HintTest.create() + .input( """ + package test; + public final class Test { + private int a; + } + """) + .sourceLevel("16") + .run(RecordToClass.class) + .assertWarnings(); + } +} + From a5e463b27ef18c8603696da505bd4c6e19f2968f Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Fri, 13 Nov 2026 16:44:40 +0000 Subject: [PATCH 3/3] Improve Class To Record hint Accept record-shaped classes: a canonical constructor that only assigns the parameters to the fields and record-style accessors are dropped in favor of the implicit ones, everything else (including equals/hashCode/toString) remains in the record body. Classes with an explicit super(...) call, local and anonymous classes are rejected. Add an option to also drop equals(), hashCode() and toString() methods in favor of the ones provided by java.lang.Record; the value is read when the fix is applied, so the preference can be changed after the hint was shown. Build TreePaths by direct linking instead of rescanning with Trees.getPath(). Assisted-by: qwen3.8-27b --- .../java/hints/suggestions/Bundle.properties | 5 +- .../java/hints/suggestions/ClassToRecord.java | 302 ++++++++++++++-- .../hints/suggestions/ClassToRecordTest.java | 323 +++++++++++++++++- 3 files changed, 602 insertions(+), 28 deletions(-) diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Bundle.properties b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Bundle.properties index 9ae23b594e4c..491608fe5918 100644 --- a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Bundle.properties +++ b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Bundle.properties @@ -111,4 +111,7 @@ DN_ConvertVarLambdaParameters=Convert Lambda Parameter Type to 'var' Type DESC_ConvertVarLambdaParameters=Convert lambda explicit or implicit parameter type to 'var' type. \ The hint will be disable for mixed or error parameter type. FIX_ConvertVarLambdaParameters=Use var parameter types -ERR_ConvertVarLambdaParameters=Use var parameter types \ No newline at end of file +ERR_ConvertVarLambdaParameters=Use var parameter types + +OPT_ClassToRecord_RemoveObjectMethods=Also remove equals(), hashCode() and toString() methods +DESC_ClassToRecord_RemoveObjectMethods=Drops the methods from the record body, even when their implementation differs from the one provided by java.lang.Record diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java index 83ea9378e88e..851e85b07291 100644 --- a/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java +++ b/java/java.hints/src/org/netbeans/modules/java/hints/suggestions/ClassToRecord.java @@ -18,9 +18,18 @@ */ package org.netbeans.modules.java.hints.suggestions; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.BlockTree; import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionStatementTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.StatementTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.VariableTree; @@ -28,12 +37,19 @@ import com.sun.source.util.Trees; import java.util.ArrayList; import java.util.EnumSet; -import java.util.HashSet; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.prefs.Preferences; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; +import javax.lang.model.element.TypeElement; +import javax.lang.model.util.Elements; +import javax.lang.model.util.Types; +import org.netbeans.api.java.source.CompilationInfo; import org.netbeans.api.java.source.GeneratorUtilities; import org.netbeans.api.java.source.TreeMaker; import org.netbeans.api.java.source.TreePathHandle; @@ -41,26 +57,41 @@ import org.netbeans.api.java.source.WorkingCopy; import org.netbeans.modules.java.hints.Feature; import org.netbeans.spi.editor.hints.ErrorDescription; +import org.netbeans.spi.editor.hints.Fix; import org.netbeans.spi.java.hints.ErrorDescriptionFactory; +import org.netbeans.spi.java.hints.BooleanOption; import org.netbeans.spi.java.hints.Hint; import org.netbeans.spi.java.hints.HintContext; import org.netbeans.spi.java.hints.JavaFix; import org.netbeans.spi.java.hints.TriggerTreeKind; +import org.netbeans.spi.java.hints.UseOptions; import org.openide.util.NbBundle; import org.openide.util.NbBundle.Messages; /** * Suggests converting a class declaration into a record. *

- * The conversion is only offered when it is safe to do so: the class is not - * abstract, sealed or {@code strictfp}, it declares no explicit constructor, - * it does not extend another class, and its instance fields are plain (no - * initializer or annotations). + * The conversion is only offered when it does not change the program's + * behavior: a canonical constructor and accessor methods that are redundant + * with the record's implicit ones are dropped, everything else remains in + * the body. + *

+ * The {@link #OPTION_REMOVE_OBJECT_METHODS} option trades that guarantee for + * a slimmer record body: the class' {@code equals()}, {@code hashCode()} and + * {@code toString()} methods are dropped in favor of the ones inherited from + * {@code java.lang.Record}, even when their implementation differs. * * @author mbien */ public class ClassToRecord { + @BooleanOption( + displayName = "#OPT_ClassToRecord_RemoveObjectMethods", + tooltip = "#DESC_ClassToRecord_RemoveObjectMethods", + defaultValue = false + ) + public static final String OPTION_REMOVE_OBJECT_METHODS = "classtorecord.remove.object.methods"; //NOI18N + @Hint( displayName = "#DN_convertClassToRecord", description = "#DESC_convertClassToRecord", @@ -71,25 +102,30 @@ public class ClassToRecord { ) @Messages({ "DN_convertClassToRecord=Convert Class to Record", - "DESC_convertClassToRecord=Reports classes which can be converted to records. The non-static fields become record components, static fields and methods remain in the body and the canonical constructor is implied.", + "DESC_convertClassToRecord=Reports classes which can be converted to records. The non-static fields become record components, a redundant canonical constructor and record-style accessors are dropped and everything else remains in the body.", "MSG_ClassToRecord=Class {0} may be converted to a record", "FIX_ConvertClassToRecord=Convert class {0} to a record" }) @TriggerTreeKind(Kind.CLASS) + @UseOptions(OPTION_REMOVE_OBJECT_METHODS) public static ErrorDescription convertClassToRecord(HintContext context) { - if (!Feature.RECORD.isEnabled(context.getInfo())) { + CompilationInfo info = context.getInfo(); + if (!Feature.RECORD.isEnabled(info)) { return null; } - if (testClassMayBeRecord(context.getInfo().getTrees(), context.getInfo().getTreeUtilities(), context.getPath())) { + if (testClassMayBeRecord(info, context.getPath())) { ClassTree cls = (ClassTree) context.getPath().getLeaf(); - return ErrorDescriptionFactory.forName(context, cls, NbBundle.getMessage(ClassToRecord.class, "MSG_ClassToRecord", cls.getSimpleName()), //NOI18N - new ConvertClassToRecordFixImpl(TreePathHandle.create(context.getPath(), context.getInfo()), - NbBundle.getMessage(ClassToRecord.class, "FIX_ConvertClassToRecord", cls.getSimpleName())).toEditorFix()); //NOI18N + Fix fix = new ConvertClassToRecordFixImpl( + TreePathHandle.create(context.getPath(), info), + NbBundle.getMessage(ClassToRecord.class, "FIX_ConvertClassToRecord", cls.getSimpleName()), + context.getPreferences() + ).toEditorFix(); + return ErrorDescriptionFactory.forName(context, cls, NbBundle.getMessage(ClassToRecord.class, "MSG_ClassToRecord", cls.getSimpleName()), fix); //NOI18N } return null; } - private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtilities, TreePath path) { + private static boolean testClassMayBeRecord(CompilationInfo info, TreePath path) { // a record may be top level or a member of another class, // but not a local or anonymous class if (path.getParentPath() == null) { @@ -99,6 +135,7 @@ private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtili if (parent.getKind() != Kind.COMPILATION_UNIT && !TreeUtilities.CLASS_TREE_KINDS.contains(parent.getKind())) { return false; } + Trees trees = info.getTrees(); Element element = trees.getElement(path); if (element == null || element.getKind() != ElementKind.CLASS) { return false; @@ -115,7 +152,8 @@ private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtili return false; } // record components are the plain non-static fields - Set componentNames = new HashSet<>(); + List componentNames = new ArrayList<>(); + Map components = new HashMap<>(); for (Tree member : cls.getMembers()) { if (member.getKind() == Kind.VARIABLE && !((VariableTree) member).getModifiers().getFlags().contains(Modifier.STATIC)) { VariableTree field = (VariableTree) member; @@ -124,10 +162,13 @@ private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtili return false; } componentNames.add(field.getName().toString()); + components.put(field.getName().toString(), field); } } + TreeUtilities treeUtilities = info.getTreeUtilities(); + Types types = info.getTypes(); for (Tree member : cls.getMembers()) { - if (treeUtilities.isSynthetic(TreePath.getPath(path, member))) { + if (treeUtilities.isSynthetic(new TreePath(path, member))) { continue; } switch (member.getKind()) { @@ -138,13 +179,21 @@ private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtili return false; } if (mth.getReturnType() == null) { - // an explicit constructor would be dropped in favor - // of the implicit canonical one - return false; - } - if (mth.getParameters().isEmpty() && componentNames.contains(mth.getName().toString())) { - // would collide with the implicitly declared component accessor - return false; + // a constructor is kept as the record's explicitly + // declared canonical constructor, unless it is + // redundant with the implicit one + if (!isRedundantCanonicalConstructor(mth, componentNames, components, trees, types, treeUtilities, path) + && hasSuperCall(mth, treeUtilities, path)) { + // records extend java.lang.Record, an explicit + // super(...) call would not be safe + return false; + } + } else if (mth.getParameters().isEmpty() && components.containsKey(mth.getName().toString())) { + // a no-arg method named after a component would + // collide with the implicitly declared accessor + if (!isRedundantAccessor(mth, components.get(mth.getName().toString()), trees, types, path)) { + return false; + } } } case VARIABLE -> { @@ -164,13 +213,184 @@ private static boolean testClassMayBeRecord(Trees trees, TreeUtilities treeUtili return true; } + /** + * Checks whether the given constructor is redundant with the implicit + * canonical constructor of the record: it has one parameter per record + * component in declaration order (same name and type) and its body merely + * assigns the parameters to the components. + */ + private static boolean isRedundantCanonicalConstructor(MethodTree ctor, + List componentNames, Map components, + Trees trees, Types types, TreeUtilities treeUtilities, TreePath classPath) { + if (!ctor.getModifiers().getAnnotations().isEmpty()) { + // an annotated constructor is kept, it becomes the record's + // explicitly declared canonical constructor + return false; + } + List params = ctor.getParameters(); + if (params.size() != componentNames.size()) { + return false; + } + TreePath ctorPath = new TreePath(classPath, ctor); + for (int i = 0; i < params.size(); i++) { + VariableTree param = params.get(i); + if (!componentNames.get(i).equals(param.getName().toString()) + || !param.getModifiers().getAnnotations().isEmpty() + || !types.isSameType(trees.getElement(new TreePath(ctorPath, param)).asType(), + trees.getElement(new TreePath(classPath, components.get(componentNames.get(i)))).asType())) { + return false; + } + } + if (!(ctor.getBody() instanceof BlockTree block)) { + return false; + } + TreePath bodyPath = new TreePath(ctorPath, block); + List statements = new ArrayList<>(); + for (StatementTree statement : block.getStatements()) { + if (!treeUtilities.isSynthetic(new TreePath(bodyPath, statement))) { + // javac injects the implicit super(...) call into + // resolved constructor bodies, it is not part of the source + statements.add(statement); + } + } + if (statements.size() != params.size()) { + return false; + } + for (int i = 0; i < statements.size(); i++) { + if (!isThisAssignment(statements.get(i), componentNames.get(i))) { + return false; + } + } + return true; + } + + /** + * Checks whether the given method is a redundant record-style accessor: + * a public, non-annotated, no-argument method named after a record + * component whose body merely returns the component field. + */ + private static boolean isRedundantAccessor(MethodTree mth, VariableTree component, + Trees trees, Types types, TreePath classPath) { + if (!mth.getModifiers().getFlags().equals(EnumSet.of(Modifier.PUBLIC)) + || !mth.getModifiers().getAnnotations().isEmpty()) { + return false; + } + if (!types.isSameType(((ExecutableElement) trees.getElement(new TreePath(classPath, mth))).getReturnType(), + trees.getElement(new TreePath(classPath, component)).asType())) { + return false; + } + if (!(mth.getBody() instanceof BlockTree block) || block.getStatements().size() != 1) { + return false; + } + String name = mth.getName().toString(); + if (!(block.getStatements().getFirst() instanceof ReturnTree ret) || ret.getExpression() == null) { + return false; + } + Tree expression = ret.getExpression(); + if (expression.getKind() == Kind.IDENTIFIER) { + return name.equals(((IdentifierTree) expression).getName().toString()); + } + if (expression.getKind() == Kind.MEMBER_SELECT) { + MemberSelectTree select = (MemberSelectTree) expression; + return "this".equals(identifierName(select.getExpression())) //NOI18N + && name.equals(select.getIdentifier().toString()); + } + return false; + } + + /** + * Checks whether the given method is one of the three {@code Object} methods + * provided by {@code java.lang.Record}: {@code public [final] boolean + * equals(Object)}, {@code public [final] int hashCode()} or {@code public + * [final] String toString()}. An {@code @Override} annotation is tolerated, + * any other annotation keeps the method in the record body. + */ + private static boolean isRecordObjectMethod(MethodTree mth, Trees trees, Types types, Elements elements, TreePath classPath) { + Set flags = mth.getModifiers().getFlags(); + if (!flags.equals(EnumSet.of(Modifier.PUBLIC)) && !flags.equals(EnumSet.of(Modifier.PUBLIC, Modifier.FINAL))) { + return false; + } + List annotations = mth.getModifiers().getAnnotations(); + if (annotations.size() > 1 + || (!annotations.isEmpty() && !isOverrideAnnotation(annotations.getFirst(), trees, types, elements, classPath, mth))) { + return false; + } + List params = mth.getParameters(); + String name = mth.getName().toString(); + if ("equals".equals(name)) { //NOI18N + if (params.size() != 1) { + return false; + } + // the return type is forced to boolean by the compiler, the + // parameter type must be Object for this to be an override + return types.isSameType(trees.getElement(new TreePath(new TreePath(classPath, mth), params.getFirst())).asType(), + elements.getTypeElement("java.lang.Object").asType()); //NOI18N + } + // the return types are forced to int and String by the compiler + return params.isEmpty() && ("hashCode".equals(name) || "toString".equals(name)); //NOI18N + } + + private static boolean isOverrideAnnotation(AnnotationTree annotation, Trees trees, Types types, Elements elements, TreePath classPath, MethodTree mth) { + TypeElement override = elements.getTypeElement("java.lang.Override"); //NOI18N + if (override == null) { + return false; + } + Element element = trees.getElement(new TreePath(new TreePath(classPath, mth), annotation.getAnnotationType())); + return element != null && types.isSameType(element.asType(), override.asType()); + } + + private static boolean isThisAssignment(StatementTree statement, String name) { + if (!(statement instanceof ExpressionStatementTree es) + || !(es.getExpression() instanceof AssignmentTree assignment)) { + return false; + } + if (assignment.getVariable().getKind() != Kind.MEMBER_SELECT) { + return false; + } + MemberSelectTree select = (MemberSelectTree) assignment.getVariable(); + return "this".equals(identifierName(select.getExpression())) //NOI18N + && name.equals(select.getIdentifier().toString()) + && assignment.getExpression().getKind() == Kind.IDENTIFIER + && name.equals(((IdentifierTree) assignment.getExpression()).getName().toString()); + } + + private static boolean hasSuperCall(MethodTree ctor, TreeUtilities treeUtilities, TreePath classPath) { + if (!(ctor.getBody() instanceof BlockTree block)) { + // be conservative with unusual constructor bodies + return true; + } + TreePath bodyPath = new TreePath(new TreePath(classPath, ctor), block); + for (StatementTree statement : block.getStatements()) { + if (treeUtilities.isSynthetic(new TreePath(bodyPath, statement))) { + // the implicit super(...) call javac injects into + // resolved constructor bodies + continue; + } + if (statement instanceof ExpressionStatementTree es + && es.getExpression().getKind() == Kind.METHOD_INVOCATION + && "super".equals(identifierName(((MethodInvocationTree) es.getExpression()).getMethodSelect()))) { //NOI18N + return true; + } + } + return false; + } + + private static String identifierName(Tree tree) { + if (tree.getKind() == Kind.IDENTIFIER) { + return ((IdentifierTree) tree).getName().toString(); + } + return null; + } + private static final class ConvertClassToRecordFixImpl extends JavaFix { private final String text; + private final Preferences prefs; - public ConvertClassToRecordFixImpl(TreePathHandle clsHandle, String text) { + public ConvertClassToRecordFixImpl(TreePathHandle clsHandle, String text, Preferences prefs) { super(clsHandle); this.text = text; + this.prefs = prefs; } @Override @@ -187,16 +407,46 @@ protected void performRewrite(TransformationContext ctx) { gu.importComments(cls, wc.getCompilationUnit()); TreeMaker make = wc.getTreeMaker(); // records are implicitly final - Set modifiers = EnumSet.copyOf(cls.getModifiers().getFlags()); + Set modifiers = EnumSet.noneOf(Modifier.class); + modifiers.addAll(cls.getModifiers().getFlags()); modifiers.remove(Modifier.FINAL); ModifiersTree mods = make.Modifiers(modifiers, cls.getModifiers().getAnnotations()); gu.copyComments(cls.getModifiers(), mods, true); gu.copyComments(cls.getModifiers(), mods, false); + // record components are the plain non-static fields + List componentNames = new ArrayList<>(); + Map components = new HashMap<>(); + for (Tree member : cls.getMembers()) { + if (member.getKind() == Kind.VARIABLE && !((VariableTree) member).getModifiers().getFlags().contains(Modifier.STATIC)) { + VariableTree field = (VariableTree) member; + componentNames.add(field.getName().toString()); + components.put(field.getName().toString(), field); + } + } + Trees trees = wc.getTrees(); + Types types = wc.getTypes(); + Elements elements = wc.getElements(); + TreeUtilities treeUtilities = wc.getTreeUtilities(); List members = new ArrayList<>(); + boolean removeObjectMethods = prefs.getBoolean(OPTION_REMOVE_OBJECT_METHODS, false); for (Tree member : cls.getMembers()) { - if (member.getKind() == Kind.METHOD && ((MethodTree) member).getReturnType() == null) { - // the implicit canonical constructor replaces the default one - continue; + if (member.getKind() == Kind.METHOD) { + MethodTree mth = (MethodTree) member; + if (removeObjectMethods && isRecordObjectMethod(mth, trees, types, elements, path)) { + // replaced by the methods inherited from java.lang.Record + continue; + } + if (mth.getReturnType() == null) { + if (isRedundantCanonicalConstructor(mth, componentNames, components, trees, types, treeUtilities, path)) { + // the implicit canonical constructor replaces the redundant one + continue; + } + } else if (mth.getParameters().isEmpty() && components.containsKey(mth.getName().toString())) { + if (isRedundantAccessor(mth, components.get(mth.getName().toString()), trees, types, path)) { + // the implicit component accessor replaces the redundant one + continue; + } + } } if (member.getKind() == Kind.VARIABLE && !((VariableTree) member).getModifiers().getFlags().contains(Modifier.STATIC)) { VariableTree field = (VariableTree) member; diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java index 57ab58bef44f..40720048cbf0 100644 --- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java +++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/suggestions/ClassToRecordTest.java @@ -146,6 +146,27 @@ public record Test(T value) {} """); } + @Test + public void testPackagePrivateClass() throws Exception { + HintTest.create() + .input( """ + package test; + class Test { + private int a; + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:6-1:10:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + record Test(int a) {} + """); + } + @Test public void testClassWithConstructor() throws Exception { HintTest.create() @@ -160,6 +181,198 @@ public Test(int a) { """) .sourceLevel("16") .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) {} + """); + } + + @Test + public void testRecordLikeClass() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private final int i; + public Test(int i) { + this.i = i; + } + public int i() { + return i; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int i) {} + """); + } + + @Test + public void testRecordLikeClassWithObjectMembers() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private final int i; + public Test(int i) { + this.i = i; + } + public int i() { + return i; + } + @Override + public boolean equals(Object o) { + if (o == this) + return true; + if (!(o instanceof Test other)) + return false; + return i == other.i; + } + @Override + public int hashCode() { + return java.util.Objects.hash(i); + } + @Override + public String toString() { + return "Test[i=" + i + "]"; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int i) { + @Override + public boolean equals(Object o) { + if (o == this) + return true; + if (!(o instanceof Test other)) + return false; + return i == other.i; + } + @Override + public int hashCode() { + return java.util.Objects.hash(i); + } + @Override + public String toString() { + return "Test[i=" + i + "]"; + } + } + """); + } + + @Test + public void testCtorWithLogic() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + public Test(int a) { + if (a < 0) + throw new IllegalArgumentException(); + this.a = a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) { + public Test(int a) { + if (a < 0) + throw new IllegalArgumentException(); + this.a = a; + } + } + """); + } + + @Test + public void testAnnotatedCtorIsKept() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + @Deprecated + public Test(int a) { + this.a = a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) { + @Deprecated + public Test(int a) { + this.a = a; + } + } + """); + } + + @Test + public void testCtorWithSuperCall() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + public Test(int a) { + super(); + this.a = a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) + .assertNotContainsWarnings("Class Test may be converted to a record"); + } + + @Test + public void testAnnotatedAccessor() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + @Deprecated + public int a() { + return a; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) .assertNotContainsWarnings("Class Test may be converted to a record"); } @@ -220,7 +433,7 @@ public class Test { } @Test - public void testAccessorNameCollision() throws Exception { + public void testRecordStyleAccessor() throws Exception { HintTest.create() .input( """ package test; @@ -233,6 +446,30 @@ public int a() { """) .sourceLevel("16") .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int a) {} + """); + } + + @Test + public void testNonRedundantAccessorNameCollision() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int a; + public int a() { + return a + 1; + } + } + """) + .sourceLevel("16") + .run(ClassToRecord.class) .assertNotContainsWarnings("Class Test may be converted to a record"); } @@ -266,5 +503,89 @@ public class Test { .run(ClassToRecord.class) .assertNotContainsWarnings("Class Test may be converted to a record"); } + + @Test + public void testObjectMethodsRemoved() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private final int i; + public Test(int i) { + this.i = i; + } + @Override + public final boolean equals(Object o) { + if (o == this) + return true; + if (!(o instanceof Test other)) + return false; + return i == other.i; + } + @Override + public int hashCode() { + return java.util.Objects.hash(i); + } + @Override + public String toString() { + return "Test[i=" + i + "]"; + } + } + """) + .sourceLevel("16") + .preference(ClassToRecord.OPTION_REMOVE_OBJECT_METHODS, true) + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int i) {} + """); + } + + @Test + public void testObjectMethodsRemovedKeepsNonOverriding() throws Exception { + HintTest.create() + .input( """ + package test; + public class Test { + private int i; + @Override + public boolean equals(Object o) { + return o instanceof Test other && i == other.i; + } + public boolean equals(int other) { + return i == other; + } + @Deprecated + @Override + public String toString() { + return "Test[i=" + i + "]"; + } + } + """) + .sourceLevel("16") + .preference(ClassToRecord.OPTION_REMOVE_OBJECT_METHODS, true) + .run(ClassToRecord.class) + .findWarning("1:13-1:17:verifier:Class Test may be converted to a record") + .applyFix("Convert class Test to a record") + .assertCompilable() + .assertOutput( + """ + package test; + public record Test(int i) { + public boolean equals(int other) { + return i == other; + } + @Deprecated + @Override + public String toString() { + return "Test[i=" + i + "]"; + } + } + """); + } }