From e7eea154bcd38052a5f84076e367cab27ccb14ee Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 30 Dec 2024 10:35:08 +0100 Subject: [PATCH 01/19] solves #7043, formatting error of closing brace in record definition adds tests to show the error adds inner and outer record in ui template text for setting java format spaces solves the error by moving spaces(cs.spaceWithinMethodDeclParens() ? 1 : 0, true); to the correct place. solves #7043 formatter to handle record closing braces correctly squashed commits of test and solution. --- .../modules/java/source/save/Reformatter.java | 7 +- .../source/save/RecordFormattingTest.java | 192 ++++++++++++++++++ .../modules/java/ui/Bundle.properties | 4 + 3 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/save/Reformatter.java b/java/java.source.base/src/org/netbeans/modules/java/source/save/Reformatter.java index 0727cbe743be..0e44783168e2 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/save/Reformatter.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/save/Reformatter.java @@ -947,7 +947,7 @@ public Boolean visitClass(ClassTree node, Void p) { } List perms = node.getPermitsClause(); if (perms != null && !perms.isEmpty()) { - wrapToken(cs.wrapExtendsImplementsKeyword(), 1, EXTENDS); + wrapToken(cs.wrapExtendsImplementsKeyword(), 1, EXTENDS); wrapList(cs.wrapExtendsImplementsList(), cs.alignMultilineImplements(), true, COMMA, perms); } } finally { @@ -1352,6 +1352,7 @@ private Boolean scanRecord(ClassTree node, Void p) { if (!recParams.isEmpty()) { spaces(cs.spaceWithinMethodDeclParens() ? 1 : 0, true); wrapList(cs.wrapMethodParams(), cs.alignMultilineMethodParams(), false, COMMA, recParams); + spaces(cs.spaceWithinMethodDeclParens() ? 1 : 0, true); // solves #7403 } accept(RPAREN); List impls = node.getImplementsClause(); @@ -1398,8 +1399,6 @@ private Boolean scanRecord(ClassTree node, Void p) { isFirstMember = false; } } - - spaces(cs.spaceWithinMethodDeclParens() ? 1 : 0, true); } } finally { indent = oldIndent; @@ -5425,7 +5424,7 @@ private void reformatComment() { } /** - * + * * @see for more info on inline tags check documentation here. * @return returns true if has inline tag prefix like "{+@tagname" */ diff --git a/java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java b/java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java new file mode 100644 index 000000000000..385328784609 --- /dev/null +++ b/java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java @@ -0,0 +1,192 @@ +/* + * 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.source.save; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.prefs.Preferences; +import javax.lang.model.SourceVersion; +import javax.swing.JEditorPane; +import javax.swing.text.Document; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.fail; +import org.netbeans.api.editor.mimelookup.MimeLookup; +import org.netbeans.api.java.classpath.ClassPath; +import org.netbeans.api.java.lexer.JavaTokenId; +import org.netbeans.api.java.source.SourceUtilsTestUtil; +import org.netbeans.api.java.source.TestUtilities; +import org.netbeans.api.lexer.Language; +import org.netbeans.junit.NbTestCase; +import org.netbeans.junit.NbTestSuite; +import org.netbeans.modules.editor.indent.api.Reformat; +import org.netbeans.modules.java.source.BootClassPathUtil; +import org.netbeans.modules.java.source.usages.IndexUtil; +import org.netbeans.spi.java.classpath.ClassPathProvider; +import org.netbeans.spi.java.classpath.support.ClassPathSupport; +import org.netbeans.spi.java.queries.SourceLevelQueryImplementation; +import org.openide.cookies.EditorCookie; +import org.openide.filesystems.FileObject; +import org.openide.filesystems.FileUtil; +import org.openide.loaders.DataObject; +import org.openide.util.SharedClassObject; + +/** + * Test to show the problem mentioned in #7043. + * + * For some strange reason, the setting spaceWithinMethodDeclParens affects the + * formatting (indentation) of the trailing brace (}) of a record definition, + * both as inner record and as top level record. + * + * @author homberghp + */ +public class RecordFormattingTest extends NbTestCase { + + File testFile = null; + private String sourceLevel = "1.8"; + private static final List EXTRA_OPTIONS = new ArrayList<>(); + + /** + * Creates a new instance of FormatingTest + */ + public RecordFormattingTest(String name) { + super(name); + } + + public static NbTestSuite suite() { + NbTestSuite suite = new NbTestSuite(); + suite.addTestSuite(RecordFormattingTest.class); + return suite; + } + + String content + = """ + record Student(int id,String lastname,String firstname) implements Serializable { + int m(int x){ + return id+x; + } + } // should stay flush to left margin + """; + + public void test7043NoSpaces() throws Exception { + run7043(""" + record Student(int id, String lastname, String firstname) implements Serializable { + int m(int x) { + return id + x; + } + } // should stay flush to left margin + """, false); + } + + public void test7043WithSpaces() throws Exception { + run7043(""" + record Student( int id, String lastname, String firstname ) implements Serializable { + int m( int x ) { + return id + x; + } + } // should stay flush to left margin + """, true); + } + + /** + * The java formatter indents the final brace '}' of a record to far to the + * right, when the setting spaceWithinMethodDeclParens is True AND the + * record header defines parameters. This + * + * @throws Exception for some reason + */ + // copied from testSealed + public void run7043(String golden, boolean spacesInMethodDecl) throws Exception { + try { + SourceVersion.valueOf("RELEASE_17"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_15, skip test + return; + } + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, ""); + FileObject testSourceFO = FileUtil.toFileObject(testFile); + DataObject testSourceDO = DataObject.find(testSourceFO); + EditorCookie ec = (EditorCookie)testSourceDO.getCookie(EditorCookie.class); + Preferences preferences = MimeLookup.getLookup(JavaTokenId.language().mimeType()).lookup(Preferences.class); + preferences.putBoolean("spaceWithinMethodDeclParens", spacesInMethodDecl); + preferences.putInt("blankLinesAfterClassHeader", 0); + + final Document doc = ec.openDocument(); + doc.putProperty(Language.class, JavaTokenId.language()); + reformat(doc, content, golden); + } + + private void reformat(Document doc, String content, String golden) throws Exception { + reformat(doc, content, golden, 0, content.length()); + } + + private void reformat(Document doc, String content, String golden, int startOffset, int endOffset) throws Exception { + doc.remove(0, doc.getLength()); + doc.insertString(0, content, null); + + Reformat reformat = Reformat.get(doc); + reformat.lock(); + try { + reformat.reformat(startOffset, endOffset); + } finally { + reformat.unlock(); + } + String res = doc.getText(0, doc.getLength()); + assertNoDiff(golden, res); + } + + static void assertNoDiff(String expected, String actual) { + var expectedLines = expected.trim().split("\n"); + var actualLines = actual.trim().split("\n"); + // fail if not of equal length + String assertResult = ""; + if (expectedLines.length != actualLines.length) { + assertResult + = "number of lines differ :\n expected nr of lines = " + expectedLines.length + + " actual nr of lines= " + actualLines.length; + } + + for (int i = 0; i < expectedLines.length && i < actualLines.length; i++) { + String actualLine = actualLines[i]; + String expLine = expectedLines[i]; + if (expLine.equals(actualLine)) { + continue; + } + assertResult += "\n expected at line " + (i + 1) + ": \n'" + expLine + "'\n not equal to actual \n'" + actualLine + "'"; + } + if (assertResult.equals("")) { + return; + } + // output generated only when test fails, to help finding the difference. + for (int i = 1; i <= expectedLines.length; i++) { + String line = expectedLines[i-1]; + System.err.format("%3d: %s\n", i,line); + + } + System.err.println("actual"); + for (int i = 1; i <= actualLines.length; i++) { + String line = actualLines[i-1]; + System.err.format("%3d: %s\n", i,line); + } + fail("not golden, see stderr for more info"); + + } + +} diff --git a/java/java.source/src/org/netbeans/modules/java/ui/Bundle.properties b/java/java.source/src/org/netbeans/modules/java/ui/Bundle.properties index 7589b9380dfe..eb2edc7a97b6 100644 --- a/java/java.source/src/org/netbeans/modules/java/ui/Bundle.properties +++ b/java/java.source/src/org/netbeans/modules/java/ui/Bundle.properties @@ -497,6 +497,10 @@ default\:\ public int add(int a, int b) {\ return a + b;\ }\ +public record ARecord(String name, int age){\ +}\ +}\ +record TopLevelRecord(String name, double value){\ } #do not translate From d21c95d1502e297cd50f6b18860c2d057310d883 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Thu, 2 Jan 2025 10:32:36 +0100 Subject: [PATCH 02/19] improve debug output --- .../java/test/InnerOuterRecordTest.java | 301 ++++++++++++++++++ 1 file changed, 301 insertions(+) create mode 100644 java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java new file mode 100644 index 000000000000..3cef3e04d4a8 --- /dev/null +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -0,0 +1,301 @@ +/* + * 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.refactoring.java.test; + +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.TreePath; +import java.util.Arrays; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import javax.lang.model.SourceVersion; +import org.netbeans.api.java.source.CompilationController; +import org.netbeans.api.java.source.JavaSource; +import org.netbeans.api.java.source.Task; +import org.netbeans.api.java.source.TreePathHandle; +import org.netbeans.modules.refactoring.api.Problem; +import org.netbeans.modules.refactoring.api.RefactoringSession; +import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring; +import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems; +import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.writeFilesAndWaitForScan; + +/** + * Test inner to outer refactoring for test. + * + * @author homberghp {@code } + */ +public class InnerOuterRecordTest extends RefactoringTestBase { + + public InnerOuterRecordTest(String name) { + super(name); + } + + public void test259004() throws Exception { + String source; + writeFilesAndWaitForScan(src, + new File("t/A.java", + source = """ + package t; + + import java.util.function.Consumer; + + public class A { + + public static void main(String[] args) { + Consumer c = f -> {}; + } + + public static final class F {} + }""")); + performInnerToOuterTest2(null); + verifyContent(src, + new File("t/A.java", + source = """ + package t; + + import java.util.function.Consumer; + + public class A { + + public static void main(String[] args) { + Consumer c = f -> {}; + } + + }"""), + new File("t/F.java", + """ + /* + * Refactoring License + */ + + package t; + + /** + * + * @author junit + */ + public final class F { + } + """)); + } + + public void testApacheNetbeans7044() throws Exception { + //ensure we are running on at least 17. + try { + SourceVersion.valueOf("RELEASE_17"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_17, skip test + return; + } + // initial outer has record with meaningful canonical constructor. + writeFilesAndWaitForScan(src, new File("t/A.java", + """ + package t; + + import java.time.LocalDate; + import java.util.Objects; + + public class A { + + record Student(int id, String name, LocalDate dob) { + + public Student { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + } + } + + void useStudent() { + var s = new Student(42,"Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); System.out.println("student = " + s); + } + } + + """)); + performInnerToOuterTest2(null); + verifyContent(src, + new File("t/A.java", + """ + package t; + + import java.time.LocalDate; + import java.util.Objects; + + public class A { + + void useStudent(Student s) { + System.out.println("student = " + s); + } + } + """), + new File("t/Student.java", + """ + record Student(int id, String name, LocalDate dob) { + + public Student { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + } + } + """ + )); + + } + + public void testApacheNetbeans7044a() throws Exception { + //ensure we are running on at least 17. + try { + SourceVersion.valueOf("RELEASE_17"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_17, skip test + return; + } + // initial outer has record with meaningful canonical constructor. + writeFilesAndWaitForScan(src, new File("t/A.java", + """ + package t; + + import java.time.LocalDate; + import java.util.Objects; + + public class A { + + public static class Student { + int id; + String name; + LocalDate dob + public Student(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id=id; + this.name=name; + this.dob=dob; + } + } + + void useStudent() { + var s = new Student(42,"Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); System.out.println("student = " + s); + } + } + + """.trim())); + performInnerToOuterTest2(null); + verifyContent(src, + new File("t/A.java", + """ + package t; + + + public class A { + + void useStudent(Student s) { + System.out.println("student = " + s); + } + } + """.trim()), + new File("t/Student.java", + """ + import java.time.LocalDate; + import java.util.Objects; + + record Student(int id, String name, LocalDate dob) { + + public Student { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id=id; + this.name=name; + this.dob=dob; + } + } + """.trim() + )); + + } + + // variant for record inner to outer test + private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { + final InnerToOuterRefactoring[] r = new InnerToOuterRefactoring[1]; + + JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { + + @Override + public void run(CompilationController parameter) throws Exception { + parameter.toPhase(JavaSource.Phase.RESOLVED); + CompilationUnitTree cut = parameter.getCompilationUnit(); +// System.out.println("cut class = " + cut.getClass()); + ClassTree outter = (ClassTree)cut.getTypeDecls().get(0); + printNumbered("outter", outter.toString()); + List members = outter.getMembers(); + int m = 0; + for (Tree member : members) { + printNumbered("member " + (m++), member.toString()); + } + var tps = cut.getTypeDecls(); + for (int i = 0; i < tps.size(); i++) { + var type = tps.get(i); + var kind = type.getKind(); + printNumbered("decl " + i + " " + kind + " ", type.toString()); + } + var inner = outter.getMembers().get(outter.getMembers().size() - 1); + printNumbered("inner", inner.toString()); + TreePath tp = TreePath.getPath(cut, inner); + r[0] = new InnerToOuterRefactoring(TreePathHandle.create(tp, parameter)); + } + }, true); + + r[0].setClassName("F"); + printNumbered("result ", r[0].toString()); + r[0].setReferenceName(generateOuter); + + RefactoringSession rs = RefactoringSession.create("Session"); + List problems = new LinkedList(); + + addAllProblems(problems, r[0].preCheck()); + addAllProblems(problems, r[0].prepare(rs)); + addAllProblems(problems, rs.doRefactoring(true)); + + assertProblems(Arrays.asList(expectedProblems), problems); + } + + static boolean debug = true; + + static void printNumbered(final String name, String source) { + if (!debug) { + return; + } + AtomicInteger c = new AtomicInteger(1); + source.trim().lines().forEach(l -> System.out.println("%s [%4d] %s".formatted(name, c.getAndIncrement(), l))); + } +} From 52129f6794df7b7d654e3e03cc7f357fc26bd871 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Thu, 2 Jan 2025 13:21:08 +0100 Subject: [PATCH 03/19] improved formatting of RefactoringTestBase. Add expected values for passing test in InnerOuterRecordTest. The existing javaformatting did not conform to any standard. To make it possible to work with that file, default netbeans formatting has been applied. That leads to some white space changes only, so be it. Further added method assertLinesEqual to get better readable output when comparing expected and acyla multi line string values, as in comparing or diffing files. Part of effort to find the problem described in #7044 --- .../java/test/InnerOuterRecordTest.java | 166 +++++---- .../java/test/RefactoringTestBase.java | 318 ++++++++++-------- 2 files changed, 279 insertions(+), 205 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 3cef3e04d4a8..1bdaaef24a58 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -22,6 +22,7 @@ import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; +import java.io.PrintStream; import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -151,17 +152,26 @@ void useStudent(Student s) { """), new File("t/Student.java", """ - record Student(int id, String name, LocalDate dob) { - - public Student { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - } + /* + * Refactoring License + */ + + package t; + + import java.time.LocalDate; + import java.util.Objects; + + record Student(int id, String name, LocalDate dob) { + + public Student { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); } - """ + } + """ )); } @@ -175,79 +185,101 @@ public void testApacheNetbeans7044a() throws Exception { return; } // initial outer has record with meaningful canonical constructor. - writeFilesAndWaitForScan(src, new File("t/A.java", - """ - package t; + writeFilesAndWaitForScan(src, + new File("t/A.java", + """ + package t; - import java.time.LocalDate; - import java.util.Objects; + import java.time.LocalDate; + import java.util.Objects; - public class A { + public class A { - public static class Student { - int id; - String name; - LocalDate dob - public Student(int id, String name, LocalDate dob) { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - this.id=id; - this.name=name; - this.dob=dob; - } - } + void useStudent() { + var s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); + } - void useStudent() { - var s = new Student(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); System.out.println("student = " + s); + public static class Student { + int id; + String name; + LocalDate dob + public Student(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id=id; + this.name=name; + this.dob=dob; } } - """.trim())); + } + """)); performInnerToOuterTest2(null); verifyContent(src, new File("t/A.java", """ package t; + import java.time.LocalDate; + import java.util.Objects; public class A { - void useStudent(Student s) { + void useStudent() { + var s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } + + } - """.trim()), + """), new File("t/Student.java", """ + /* + * Refactoring License + */ + + package t; + import java.time.LocalDate; import java.util.Objects; - record Student(int id, String name, LocalDate dob) { - - public Student { + /** + * + * @author junit + */ + public class Student { + + int id; + String name; + LocalDate dob; + + public Student(int id, String name, LocalDate dob) { Objects.requireNonNull(id); Objects.requireNonNull(name); Objects.requireNonNull(dob); assert !name.isEmpty() && !name.isBlank(); assert dob.isAfter(LocalDate.EPOCH); - this.id=id; - this.name=name; - this.dob=dob; + this.id = id; + this.name = name; + this.dob = dob; } + } - """.trim() + """ )); } + final boolean debug = false; + // variant for record inner to outer test private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { final InnerToOuterRefactoring[] r = new InnerToOuterRefactoring[1]; - JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { @Override @@ -255,28 +287,39 @@ public void run(CompilationController parameter) throws Exception { parameter.toPhase(JavaSource.Phase.RESOLVED); CompilationUnitTree cut = parameter.getCompilationUnit(); // System.out.println("cut class = " + cut.getClass()); - ClassTree outter = (ClassTree)cut.getTypeDecls().get(0); - printNumbered("outter", outter.toString()); - List members = outter.getMembers(); + ClassTree outer = (ClassTree)cut.getTypeDecls().get(0); + if (debug) { + printNumbered(System.err, "outer", outer.toString()); + } + List members = outer.getMembers(); int m = 0; - for (Tree member : members) { - printNumbered("member " + (m++), member.toString()); + if (debug) { + for (Tree member : members) { + printNumbered(System.err, "member " + (m++), member.toString()); + } } var tps = cut.getTypeDecls(); - for (int i = 0; i < tps.size(); i++) { - var type = tps.get(i); - var kind = type.getKind(); - printNumbered("decl " + i + " " + kind + " ", type.toString()); + if (debug) { + for (int i = 0; i < tps.size(); i++) { + var type = tps.get(i); + var kind = type.getKind(); + printNumbered(System.err, "decl " + i + " " + kind + " ", type.toString()); + } + } + // selecting the last element assumes that the inner class is the last member in the outer class. + var inner = outer.getMembers().get(outer.getMembers().size() - 1); + if (debug) { + printNumbered(System.err, "inner", inner.toString()); } - var inner = outter.getMembers().get(outter.getMembers().size() - 1); - printNumbered("inner", inner.toString()); TreePath tp = TreePath.getPath(cut, inner); r[0] = new InnerToOuterRefactoring(TreePathHandle.create(tp, parameter)); } }, true); r[0].setClassName("F"); - printNumbered("result ", r[0].toString()); + if (debug) { + printNumbered(System.err, "result ", r[0].toString()); + } r[0].setReferenceName(generateOuter); RefactoringSession rs = RefactoringSession.create("Session"); @@ -288,14 +331,5 @@ public void run(CompilationController parameter) throws Exception { assertProblems(Arrays.asList(expectedProblems), problems); } - - static boolean debug = true; - static void printNumbered(final String name, String source) { - if (!debug) { - return; - } - AtomicInteger c = new AtomicInteger(1); - source.trim().lines().forEach(l -> System.out.println("%s [%4d] %s".formatted(name, c.getAndIncrement(), l))); - } } diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java index ebc261b08126..ff798c87b499 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java @@ -21,6 +21,7 @@ import java.io.EOFException; import java.io.FileInputStream; import java.io.IOException; +import java.io.PrintStream; import java.nio.charset.Charset; import java.util.Arrays; import java.util.Collections; @@ -31,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; import javax.swing.event.ChangeListener; @@ -76,7 +78,7 @@ public class RefactoringTestBase extends NbTestCase { public RefactoringTestBase(String name) { super(name); - sourcelevel = "1.6"; + sourcelevel = "1.6";//?? } public RefactoringTestBase(String name, String sourcelevel) { @@ -115,7 +117,7 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except todo.addAll(Arrays.asList(file.getChildren())); } } - + Throwable exception = null; for (File f : files) { String fileContent = content.remove(f.filename); @@ -123,19 +125,21 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except assertNotNull(f.content); assertNotNull("Cannot find " + f.filename + " in map " + content, fileContent); try { - assertEquals(getName() ,f.content.replaceAll("[ \t\r\n\n]+", " "), fileContent.replaceAll("[ \t\r\n\n]+", " ")); +// assertEquals(getName(), f.content.replaceAll("[ \t\r\n\n]+", " "), fileContent.replaceAll("[ \t\r\n\n]+", " ")); + assertLinesEqual(f.content, fileContent); } catch (Throwable t) { - System.err.println("expected:"); - System.err.println(f.content); - System.err.println("actual:"); - System.err.println(fileContent); - throw t; + System.err.println("fail for file " + f.filename); + printNumbered(System.err, f.filename + " expected", f.content); + printNumbered(System.err, f.filename + " actual ", fileContent); + exception = t; } } - - assertTrue(content.toString(), content.isEmpty()); + if (exception != null) { + fail("file contents does not match expectation, see stderr for more details."); + } + assertTrue("not all files processeed", content.isEmpty()); } - + /** * Returns a string which contains the contents of a file. * @@ -143,7 +147,7 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except * @return the contents of the file(s). */ private static String copyFileToString(java.io.File f) throws java.io.IOException { - int s = (int) f.length(); + int s = (int)f.length(); byte[] data = new byte[s]; int len = new FileInputStream(f).read(data); if (len != s) { @@ -173,8 +177,8 @@ protected static void assertProblems(Iterable golden, Iterabl boolean goldenHasNext = g.hasNext(); boolean realHasNext = r.hasNext(); - assertFalse(goldenHasNext?"Expected: " + g.next().getMessage():"", goldenHasNext); - assertFalse(realHasNext?"Unexpected: " + r.next().getMessage():"", realHasNext); + assertFalse(goldenHasNext ? "Expected: " + g.next().getMessage() : "", goldenHasNext); + assertFalse(realHasNext ? "Unexpected: " + r.next().getMessage() : "", realHasNext); } static { @@ -182,6 +186,7 @@ protected static void assertProblems(Iterable golden, Iterabl } protected static final class File { + public final String filename; public final String content; @@ -202,144 +207,147 @@ protected void setUp() throws Exception { System.setProperty("org.netbeans.modules.java.source.usages.SourceAnalyser.fullIndex", "true"); Logger.getLogger("").setLevel(Level.SEVERE); //turn off chatty logs MimeTypes.setAllMimeTypes(new HashSet()); - SourceUtilsTestUtil.prepareTest(new String[] {"org/netbeans/modules/openide/loaders/layer.xml", - "org/netbeans/modules/java/source/resources/layer.xml", - "org/netbeans/modules/java/editor/resources/layer.xml", - "org/netbeans/modules/refactoring/java/test/resources/layer.xml", "META-INF/generated-layer.xml"}, new Object[] { - new ClassPathProvider() { - @Override - public ClassPath findClassPath(FileObject file, String type) { - if (sourcePath != null && sourcePath.contains(file)){ - if (ClassPath.BOOT.equals(type)) { - return TestUtil.getBootClassPath(); - } - if (JavaClassPathConstants.MODULE_BOOT_PATH.equals(type)) { - return BootClassPathUtil.getModuleBootPath(); - } - if (ClassPath.COMPILE.equals(type)) { - return ClassPathSupport.createClassPath(new FileObject[0]); - } - if (ClassPath.SOURCE.equals(type)) { - return sourcePath; - } - } - - return null; + SourceUtilsTestUtil.prepareTest(new String[]{"org/netbeans/modules/openide/loaders/layer.xml", + "org/netbeans/modules/java/source/resources/layer.xml", + "org/netbeans/modules/java/editor/resources/layer.xml", + "org/netbeans/modules/refactoring/java/test/resources/layer.xml", "META-INF/generated-layer.xml"}, new Object[]{ + new ClassPathProvider() { + @Override + public ClassPath findClassPath(FileObject file, String type) { + if (sourcePath != null && sourcePath.contains(file)) { + if (ClassPath.BOOT.equals(type)) { + return TestUtil.getBootClassPath(); } - }, - new ProjectFactory() { - @Override - public boolean isProject(FileObject projectDirectory) { - return src != null && src.getParent() == projectDirectory; + if (JavaClassPathConstants.MODULE_BOOT_PATH.equals(type)) { + return BootClassPathUtil.getModuleBootPath(); } - @Override - public Project loadProject(final FileObject projectDirectory, ProjectState state) throws IOException { - if (!isProject(projectDirectory)) { + if (ClassPath.COMPILE.equals(type)) { + return ClassPathSupport.createClassPath(new FileObject[0]); + } + if (ClassPath.SOURCE.equals(type)) { + return sourcePath; + } + } + return null; } - return new Project() { + }, + new ProjectFactory() { + @Override + public boolean isProject(FileObject projectDirectory) { + return src != null && src.getParent() == projectDirectory; + } + + @Override + public Project loadProject(final FileObject projectDirectory, ProjectState state) throws IOException { + if (!isProject(projectDirectory)) { + return null; + } + return new Project() { + @Override + public FileObject getProjectDirectory() { + return projectDirectory; + } + + @Override + public Lookup getLookup() { + final Project p = this; + return Lookups.singleton(new Sources() { + @Override - public FileObject getProjectDirectory() { - return projectDirectory; + public SourceGroup[] getSourceGroups(String type) { + return new SourceGroup[]{GenericSources.group(p, src.getParent(), "source", "Java Sources", null, null), + GenericSources.group(p, test, "testsources", "Test Sources", null, null)}; } + @Override - public Lookup getLookup() { - final Project p = this; - return Lookups.singleton(new Sources() { - - @Override - public SourceGroup[] getSourceGroups(String type) { - return new SourceGroup[] {GenericSources.group(p, src.getParent(), "source", "Java Sources", null, null), - GenericSources.group(p, test, "testsources", "Test Sources", null, null)}; - } - - @Override - public void addChangeListener(ChangeListener listener) { - } - - @Override - public void removeChangeListener(ChangeListener listener) { - } - }); + public void addChangeListener(ChangeListener listener) { } - }; - } - @Override - public void saveProject(Project project) throws IOException, ClassCastException {} - }, - new TestLocator() { - @Override - public boolean appliesTo(FileObject fo) { - return true; + @Override + public void removeChangeListener(ChangeListener listener) { + } + }); } + }; + } - @Override - public boolean asynchronous() { - return false; - } + @Override + public void saveProject(Project project) throws IOException, ClassCastException { + } + }, + new TestLocator() { - @Override - public LocationResult findOpposite(FileObject fo, int caretOffset) { - ClassPath srcCp; - - if ((srcCp = ClassPath.getClassPath(fo, ClassPath.SOURCE)) == null) { - return new LocationResult("File not found"); //NOI18N - } - - String baseResName = srcCp.getResourceName(fo, '/', false); - String testResName = getTestResName(baseResName, fo.getExt()); - assert testResName != null; - FileObject fileObject = test.getFileObject(testResName); - if(fileObject != null) { - return new LocationResult(fileObject, -1); - } - - return new LocationResult("File not found"); //NOI18N - } + @Override + public boolean appliesTo(FileObject fo) { + return true; + } - @Override - public void findOpposite(FileObject fo, int caretOffset, LocationListener callback) { - throw new UnsupportedOperationException("This should not be called on synchronous locators."); - } + @Override + public boolean asynchronous() { + return false; + } - @Override - public FileType getFileType(FileObject fo) { - if(FileUtil.isParentOf(test, fo)) { - return FileType.TEST; - } else if(FileUtil.isParentOf(src, fo)) { - return FileType.TESTED; - } - return FileType.NEITHER; - } + @Override + public LocationResult findOpposite(FileObject fo, int caretOffset) { + ClassPath srcCp; - private String getTestResName(String baseResName, String ext) { - StringBuilder buf - = new StringBuilder(baseResName.length() + ext.length() + 10); - buf.append(baseResName).append("Test"); //NOI18N - if (ext.length() != 0) { - buf.append('.').append(ext); - } - return buf.toString(); - } - }, - new SourceLevelQueryImplementation() { + if ((srcCp = ClassPath.getClassPath(fo, ClassPath.SOURCE)) == null) { + return new LocationResult("File not found"); //NOI18N + } - @Override - public String getSourceLevel(FileObject javaFile) { - return sourcelevel; - } - }}); + String baseResName = srcCp.getResourceName(fo, '/', false); + String testResName = getTestResName(baseResName, fo.getExt()); + assert testResName != null; + FileObject fileObject = test.getFileObject(testResName); + if (fileObject != null) { + return new LocationResult(fileObject, -1); + } + + return new LocationResult("File not found"); //NOI18N + } + + @Override + public void findOpposite(FileObject fo, int caretOffset, LocationListener callback) { + throw new UnsupportedOperationException("This should not be called on synchronous locators."); + } + + @Override + public FileType getFileType(FileObject fo) { + if (FileUtil.isParentOf(test, fo)) { + return FileType.TEST; + } else if (FileUtil.isParentOf(src, fo)) { + return FileType.TESTED; + } + return FileType.NEITHER; + } + + private String getTestResName(String baseResName, String ext) { + StringBuilder buf + = new StringBuilder(baseResName.length() + ext.length() + 10); + buf.append(baseResName).append("Test"); //NOI18N + if (ext.length() != 0) { + buf.append('.').append(ext); + } + return buf.toString(); + } + }, + new SourceLevelQueryImplementation() { + + @Override + public String getSourceLevel(FileObject javaFile) { + return sourcelevel; + } + }}); Main.initializeURLFactory(); org.netbeans.api.project.ui.OpenProjects.getDefault().getOpenProjects(); - + // org.netbeans.modules.java.source.TreeLoader.DISABLE_CONFINEMENT_TEST = true; - prepareTest(); - org.netbeans.api.project.ui.OpenProjects.getDefault().open(new Project[] {prj = ProjectManager.getDefault().findProject(src.getParent())}, false); + org.netbeans.api.project.ui.OpenProjects.getDefault().open(new Project[]{prj = ProjectManager.getDefault().findProject(src.getParent())}, false); MimeTypes.setAllMimeTypes(Collections.singleton("text/x-java")); sourcePath = ClassPathSupport.createClassPath(src, test); - GlobalPathRegistry.getDefault().register(ClassPath.SOURCE, new ClassPath[] {sourcePath}); + GlobalPathRegistry.getDefault().register(ClassPath.SOURCE, new ClassPath[]{sourcePath}); RepositoryUpdater.getDefault().start(true); super.setUp(); FileUtil.createData(FileUtil.getConfigRoot(), "Templates/Classes/Empty.java"); @@ -349,8 +357,8 @@ public String getSourceLevel(FileObject javaFile) { @Override protected void tearDown() throws Exception { super.tearDown(); - GlobalPathRegistry.getDefault().unregister(ClassPath.SOURCE, new ClassPath[] {sourcePath}); - org.netbeans.api.project.ui.OpenProjects.getDefault().close(new Project[] {prj}); + GlobalPathRegistry.getDefault().unregister(ClassPath.SOURCE, new ClassPath[]{sourcePath}); + org.netbeans.api.project.ui.OpenProjects.getDefault().close(new Project[]{prj}); CountDownLatch cdl = new CountDownLatch(1); RepositoryUpdater.getDefault().stop(() -> { cdl.countDown(); @@ -366,12 +374,12 @@ private void prepareTest() throws Exception { src = FileUtil.createFolder(projectFolder, "src"); test = FileUtil.createFolder(projectFolder, "test"); - FileObject cache = FileUtil.createFolder(workdir, "cache"); + FileObject cache = FileUtil.createFolder(workdir, "cache"); - CacheFolder.setCacheFolder(cache); - } + CacheFolder.setCacheFolder(cache); + } - @ServiceProvider(service=MimeDataProvider.class) + @ServiceProvider(service = MimeDataProvider.class) public static final class MimeDataProviderImpl implements MimeDataProvider { private static final Lookup L = Lookups.singleton(new JavaCustomIndexer.Factory()); @@ -384,7 +392,7 @@ public Lookup getLookup(MimePath mimePath) { return null; } - + } protected static boolean problemIsFatal(List problems) { @@ -411,10 +419,42 @@ protected void runTest() throws Throwable { super.runTest(); return; } catch (Throwable t) { - if (exc == null) exc = t; + if (exc == null) { + exc = t; + } } } throw exc; } -} \ No newline at end of file + void printNumbered(final PrintStream out, final String name, String source) { + AtomicInteger c = new AtomicInteger(1); + source.trim().lines().forEach(l -> out.println("%s [%4d] %s".formatted(name, c.getAndIncrement(), l))); + } + + /** + * Compare string by splitting them into lines, and remove trailing whitespace. + * When the strings differ, both expectend and actual are printed. + * @param expected to compare + * @param actual to compare + */ + public void assertLinesEqual(String expected, String actual){ + String[] linesExpected= expected.lines().toArray(String[]::new); + String[] linesActual= actual.lines().toArray(String[]::new); + int limit= Math.max(linesExpected.length,linesActual.length); + StringBuilder sb = new StringBuilder(); + boolean equals = true; + for (int i = 0; i < limit; i++) { + String e= (i < linesExpected.length? linesExpected[i]:"").trim(); + String a= (i < linesActual.length? linesActual[i]:"").trim(); + boolean same = e.equals(a); + String sep= same?" ":" | "; + equals &= same; + sb.append(String.format("[%3d] %-80s%s%-80s%n",i,e,sep,a)); + } + if (!equals){ + System.err.println(sb.toString()); + fail("lines differ, see stderr for more details."); + } + } +} From 0f4607ea47c1a005c7276b6701290ef6904f34f6 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 09:49:54 +0100 Subject: [PATCH 04/19] add full line compare to make it easier on the eyes for visual inspection the original comparison of expected and actual by collapsing all white space chars into one space is kept to keep original tests intact. --- .../java/test/RefactoringTestBase.java | 112 +++++++++++++----- 1 file changed, 83 insertions(+), 29 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java index ff798c87b499..933982ab4e0b 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java @@ -78,7 +78,7 @@ public class RefactoringTestBase extends NbTestCase { public RefactoringTestBase(String name) { super(name); - sourcelevel = "1.6";//?? + sourcelevel = "17";//"1.6";//?? } public RefactoringTestBase(String name, String sourcelevel) { @@ -87,6 +87,7 @@ public RefactoringTestBase(String name, String sourcelevel) { } protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... files) throws Exception { + long currentTimeMillis = System.currentTimeMillis(); for (FileObject c : sourceRoot.getChildren()) { c.delete(); } @@ -97,8 +98,28 @@ protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... fi } IndexingManager.getDefault().refreshIndexAndWait(sourceRoot.toURL(), null, true); + long currentTimeMillis1 = System.currentTimeMillis(); + System.err.println("writeFilesAndWaitForScan took "+(currentTimeMillis1-currentTimeMillis)+" millis"); } + protected static void writeFilesNoIndexing(FileObject sourceRoot, File... files) throws Exception { + for (FileObject c : sourceRoot.getChildren()) { + c.delete(); + } + + for (File f : files) { + FileObject fo = FileUtil.createData(sourceRoot, f.filename); + TestUtilities.copyStringToFile(fo, f.content); + } + } + + /** + * Verify that the given file(names) are present in the sourceRoot and that the files + * in said sourceRoot are equal to the given files. + * @param sourceRoot to contain generated (refactored) files + * @param files expected files + * @throws Exception well why not? + */ protected void verifyContent(FileObject sourceRoot, File... files) throws Exception { List todo = new LinkedList(); @@ -111,32 +132,27 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except while (!todo.isEmpty()) { FileObject file = todo.remove(0); - if (file.isData()) { + if (file.isData()) { // normal file content.put(FileUtil.getRelativePath(sourceRoot, file), copyFileToString(FileUtil.toFile(file))); - } else { + } else { //folder? todo.addAll(Arrays.asList(file.getChildren())); } } + // only do full line compare for InnerOuterRecorTest to not break exsiting tests, which make different assumptions. + boolean fullLineCompare = this.getClass() == InnerOuterRecordTest.class; Throwable exception = null; for (File f : files) { + // take the element from the map filled by sourceRootTraversal. String fileContent = content.remove(f.filename); - assertNotNull(f); assertNotNull(f.content); - assertNotNull("Cannot find " + f.filename + " in map " + content, fileContent); - try { -// assertEquals(getName(), f.content.replaceAll("[ \t\r\n\n]+", " "), fileContent.replaceAll("[ \t\r\n\n]+", " ")); - assertLinesEqual(f.content, fileContent); - } catch (Throwable t) { - System.err.println("fail for file " + f.filename); - printNumbered(System.err, f.filename + " expected", f.content); - printNumbered(System.err, f.filename + " actual ", fileContent); - exception = t; + assertNotNull("Cannot find expected " + f.filename + " in map filled by sourceRoot " + content, fileContent); + if (fullLineCompare) { + assertLinesEqual2(f.content, fileContent); + } else { // original tests. + assertLinesEqual1(f.content, fileContent); } } - if (exception != null) { - fail("file contents does not match expectation, see stderr for more details."); - } assertTrue("not all files processeed", content.isEmpty()); } @@ -408,7 +424,8 @@ protected static boolean problemIsFatal(List problems) { return false; } - private static final int RETRIES = 3; +// private static final int RETRIES = 3; + private static final int RETRIES = 1; @Override protected void runTest() throws Throwable { @@ -427,32 +444,69 @@ protected void runTest() throws Throwable { throw exc; } - void printNumbered(final PrintStream out, final String name, String source) { + /** + * Prints a source by splitting on the line breaks and prefixing with name and line number. + * + * @param out the stream to print to + * @param name the name as prefix to each line + * @param source the source code to print to the out stream. + */ + public void printNumbered(final PrintStream out, final String name, String source) { AtomicInteger c = new AtomicInteger(1); source.trim().lines().forEach(l -> out.println("%s [%4d] %s".formatted(name, c.getAndIncrement(), l))); } /** - * Compare string by splitting them into lines, and remove trailing whitespace. - * When the strings differ, both expectend and actual are printed. + * Compare strings by replacing all multiples of whitrespace([ \t\n\r]) with a space. + * + * The test programmer choose this to make it easier to write the input and the expected strings. + * + * @param expected to compare + * @param actual to compare + */ + public void assertLinesEqual1(String expected, String actual) { + try { + assertEquals(getName(), expected.replaceAll("[ \t\r\n\n]+", " "), actual.replaceAll("[ \t\r\n\n]+", " ")); + } catch (Throwable t) { + System.err.println("expected:"); + System.err.println(expected); + System.err.println("actual:"); + System.err.println(actual); + throw t; + } + } + + /** + * Compare strings by splitting them into lines, and remove trailing white + * space. + * + * Only when any of the lines differ, all lines are printed with the unequal + * lines flagged. + * + * Before the lines are compared, they are trimmed and the white space is + * normalized by collapsing multiple whitespace characters into one. This + * should make the tests less brittle. + * * @param expected to compare * @param actual to compare */ - public void assertLinesEqual(String expected, String actual){ - String[] linesExpected= expected.lines().toArray(String[]::new); - String[] linesActual= actual.lines().toArray(String[]::new); - int limit= Math.max(linesExpected.length,linesActual.length); + public void assertLinesEqual2(String expected, String actual) { + expected = expected.trim().replaceAll("([ \t\r\n])\\1+", "$1"); + actual = actual.trim().replaceAll("([ \t\r\n])\\1+", "$1"); + String[] linesExpected = expected.lines().toArray(String[]::new); + String[] linesActual = actual.lines().toArray(String[]::new); + int limit = Math.max(linesExpected.length, linesActual.length); StringBuilder sb = new StringBuilder(); boolean equals = true; for (int i = 0; i < limit; i++) { - String e= (i < linesExpected.length? linesExpected[i]:"").trim(); - String a= (i < linesActual.length? linesActual[i]:"").trim(); + String e = (i < linesExpected.length ? linesExpected[i] : "").trim(); + String a = (i < linesActual.length ? linesActual[i] : "").trim(); boolean same = e.equals(a); - String sep= same?" ":" | "; + String sep = same ? " " : " | "; equals &= same; - sb.append(String.format("[%3d] %-80s%s%-80s%n",i,e,sep,a)); + sb.append(String.format("[%3d] %-80s%s%-80s%n", i, e, sep, a)); } - if (!equals){ + if (!equals) { System.err.println(sb.toString()); fail("lines differ, see stderr for more details."); } From 1a41df5ff26787edc4c7d32462aeaa74290bd612 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 09:52:25 +0100 Subject: [PATCH 05/19] Add tests for record refactoring. Refactoring of records is still broken as described in #7044. These tests should help to find the error. The error might be something small as forgetting to consider a record as a class in the same way as enum. --- .../java/test/InnerOuterRecordTest.java | 185 +++++++++++++----- 1 file changed, 132 insertions(+), 53 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 1bdaaef24a58..4bc10603e6d9 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -22,12 +22,17 @@ import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; +import java.io.IOException; import java.io.PrintStream; import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import java.util.Properties; import java.util.concurrent.atomic.AtomicInteger; import javax.lang.model.SourceVersion; +import javax.lang.model.element.Name; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; import org.netbeans.api.java.source.CompilationController; import org.netbeans.api.java.source.JavaSource; import org.netbeans.api.java.source.Task; @@ -37,6 +42,7 @@ import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring; import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems; import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.writeFilesAndWaitForScan; +import org.openide.util.Exceptions; /** * Test inner to outer refactoring for test. @@ -47,11 +53,23 @@ public class InnerOuterRecordTest extends RefactoringTestBase { public InnerOuterRecordTest(String name) { super(name); - } + //ensure we are running on at least 16. + try { + SourceVersion.valueOf("RELEASE_16"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_17, skip test + return; + } +// final JavaCompiler cmp = ToolProvider.getSystemJavaCompiler(); +// for (SourceVersion sourceVersion : cmp.getSourceVersions()) { +// System.out.println("sourceVersion = " + sourceVersion); +// } + } + // for reference public void test259004() throws Exception { String source; - writeFilesAndWaitForScan(src, + writeFilesNoIndexing(src, new File("t/A.java", source = """ package t; @@ -94,20 +112,16 @@ public static void main(String[] args) { * @author junit */ public final class F { + } """)); } public void testApacheNetbeans7044() throws Exception { - //ensure we are running on at least 17. - try { - SourceVersion.valueOf("RELEASE_17"); //NOI18N - } catch (IllegalArgumentException ex) { - //OK, no RELEASE_17, skip test - return; - } + debug = true; // initial outer has record with meaningful canonical constructor. - writeFilesAndWaitForScan(src, new File("t/A.java", + // note that Inner class should be in last member for assumptions in the test. + writeFilesNoIndexing(src, new File("t/A.java", """ package t; @@ -116,9 +130,17 @@ public void testApacheNetbeans7044() throws Exception { public class A { - record Student(int id, String name, LocalDate dob) { + void useStudent() { + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); System.out.println("student = " + s); + } - public Student { + record F(int id, String name, LocalDate dob) { + + /** + * Validate stuff. + */ + public F { Objects.requireNonNull(id); Objects.requireNonNull(name); Objects.requireNonNull(dob); @@ -127,13 +149,10 @@ record Student(int id, String name, LocalDate dob) { } } - void useStudent() { - var s = new Student(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); System.out.println("student = " + s); - } } - """)); + """) + ); performInnerToOuterTest2(null); verifyContent(src, new File("t/A.java", @@ -150,7 +169,7 @@ void useStudent(Student s) { } } """), - new File("t/Student.java", + new File("t/F.java", """ /* * Refactoring License @@ -161,9 +180,12 @@ void useStudent(Student s) { import java.time.LocalDate; import java.util.Objects; - record Student(int id, String name, LocalDate dob) { + record F(int id, String name, LocalDate dob) { - public Student { + /** + * Validate stuff. + */ + public F { Objects.requireNonNull(id); Objects.requireNonNull(name); Objects.requireNonNull(dob); @@ -171,21 +193,14 @@ record Student(int id, String name, LocalDate dob) { assert dob.isAfter(LocalDate.EPOCH); } } - """ - )); + """) + ); } - public void testApacheNetbeans7044a() throws Exception { - //ensure we are running on at least 17. - try { - SourceVersion.valueOf("RELEASE_17"); //NOI18N - } catch (IllegalArgumentException ex) { - //OK, no RELEASE_17, skip test - return; - } + public void testBasicClassExample() throws Exception { // initial outer has record with meaningful canonical constructor. - writeFilesAndWaitForScan(src, + writeFilesNoIndexing(src, new File("t/A.java", """ package t; @@ -196,7 +211,7 @@ public void testApacheNetbeans7044a() throws Exception { public class A { void useStudent() { - var s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } @@ -230,11 +245,11 @@ public Student(int id, String name, LocalDate dob) { public class A { void useStudent() { - var s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } - - + + } """), new File("t/Student.java", @@ -275,7 +290,58 @@ public Student(int id, String name, LocalDate dob) { } - final boolean debug = false; + public void testBasicRecord1() throws Exception { + // initial outer has record with meaningful canonical constructor. + writeFilesAndWaitForScan(src, + new File("t/A.java", + """ + package t; + + import java.time.LocalDate; + + record A(int id, String name, LocalDate dob) { + + F f; + record F(int x, int y){} + } + """ + )); + performInnerToOuterTest2(null); + verifyContent(src, + new File("t/A.java", + """ + package t; + + import java.time.LocalDate; + + record A(int id, String name, LocalDate dob) { + + F f; + + } + """), + new File("t/F.java", + """ + /* + * Refactoring License + */ + + package t; + + + /** + * + * @author junit + */ + record F(int x, int y){ + } + + """ + )); + + } + + boolean debug = true; // variant for record inner to outer test private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { @@ -283,36 +349,49 @@ private void performInnerToOuterTest2(String generateOuter, Problem... expectedP JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { @Override - public void run(CompilationController parameter) throws Exception { - parameter.toPhase(JavaSource.Phase.RESOLVED); + public void run(CompilationController parameter) { + try { + parameter.toPhase(JavaSource.Phase.RESOLVED); + } catch (IOException ex) { + Exceptions.printStackTrace(ex); + } CompilationUnitTree cut = parameter.getCompilationUnit(); -// System.out.println("cut class = " + cut.getClass()); + if (debug) { + System.err.println("cut is of type " + cut.getClass().getCanonicalName()); + } ClassTree outer = (ClassTree)cut.getTypeDecls().get(0); if (debug) { - printNumbered(System.err, "outer", outer.toString()); + printNumbered(System.err, "start source " + outer.getKind().toString(), outer.toString()); } List members = outer.getMembers(); int m = 0; if (debug) { for (Tree member : members) { - printNumbered(System.err, "member " + (m++), member.toString()); - } - } - var tps = cut.getTypeDecls(); - if (debug) { - for (int i = 0; i < tps.size(); i++) { - var type = tps.get(i); - var kind = type.getKind(); - printNumbered(System.err, "decl " + i + " " + kind + " ", type.toString()); + printNumbered(System.err, "member %d %15s".formatted(m++, member.getKind()), member.toString()); + String toString = member.toString(); + if (member instanceof ClassTree ct) { + int n = 0; + Name simpleName = ct.getSimpleName(); + for (Tree ctm : ct.getMembers()) { + printNumbered(System.err, " ctm " + simpleName + " %d %15s".formatted(n++, ctm.getKind()), ctm.toString()); + } + } } } // selecting the last element assumes that the inner class is the last member in the outer class. - var inner = outer.getMembers().get(outer.getMembers().size() - 1); + Tree lastInnerClass = outer.getMembers().get(outer.getMembers().size() - 1); if (debug) { - printNumbered(System.err, "inner", inner.toString()); + String n = "lastInnerClass " + lastInnerClass.getKind().toString(); + printNumbered(System.err, n, lastInnerClass.toString()); + } + TreePath tp = TreePath.getPath(cut, lastInnerClass); + try { + r[0] = new InnerToOuterRefactoring(TreePathHandle.create(tp, parameter)); + } catch (Throwable t) { + System.err.println("InnerOuter refatoring failed with exception " + t); + t.printStackTrace(System.out); + throw t; } - TreePath tp = TreePath.getPath(cut, inner); - r[0] = new InnerToOuterRefactoring(TreePathHandle.create(tp, parameter)); } }, true); From 727001d9e373480fe3112e4d7f8137bde38ec60c Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 10:38:25 +0100 Subject: [PATCH 06/19] improved output of file comparison, added case labels for RECORD. Case labels for RECORD were missing in two places in WorkingCopy.java --- .../org/netbeans/api/java/source/WorkingCopy.java | 4 ++++ .../refactoring/java/test/InnerOuterRecordTest.java | 13 ++++++++----- .../refactoring/java/test/RefactoringTestBase.java | 7 ++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/java/java.source.base/src/org/netbeans/api/java/source/WorkingCopy.java b/java/java.source.base/src/org/netbeans/api/java/source/WorkingCopy.java index ff93ca70b8b5..87a76549adcb 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/WorkingCopy.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/WorkingCopy.java @@ -1221,6 +1221,7 @@ String template(ElementKind kind) { case INTERFACE: return "Templates/Classes/Interface.java"; // NOI18N case ANNOTATION_TYPE: return "Templates/Classes/AnnotationType.java"; // NOI18N case ENUM: return "Templates/Classes/Enum.java"; // NOI18N + case RECORD: return "Templates/Classes/Record.java"; // NOI18N case PACKAGE: return "Templates/Classes/package-info.java"; // NOI18N default: Logger.getLogger(WorkingCopy.class.getName()).log(Level.SEVERE, "Cannot resolve template for {0}", kind); @@ -1248,6 +1249,9 @@ FileObject doCreateFromTemplate(CompilationUnitTree cut) throws IOException { case ENUM: kind = ElementKind.ENUM; break; + case RECORD: + kind = ElementKind.RECORD; + break; default: Logger.getLogger(WorkingCopy.class.getName()).log(Level.SEVERE, "Cannot resolve template for {0}", cut.getTypeDecls().get(0).getKind()); kind = null; diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 4bc10603e6d9..6594771b04f5 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -132,7 +132,7 @@ public class A { void useStudent() { F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); System.out.println("student = " + s); + System.out.println("student = " + s); } record F(int id, String name, LocalDate dob) { @@ -164,22 +164,25 @@ record F(int id, String name, LocalDate dob) { public class A { - void useStudent(Student s) { + void useStudent() { + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } } """), new File("t/F.java", - """ + """ /* * Refactoring License */ - package t; import java.time.LocalDate; import java.util.Objects; - + /** + * + * @author junit + */ record F(int id, String name, LocalDate dob) { /** diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java index 933982ab4e0b..e418555158a2 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java @@ -148,7 +148,7 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except assertNotNull(f.content); assertNotNull("Cannot find expected " + f.filename + " in map filled by sourceRoot " + content, fileContent); if (fullLineCompare) { - assertLinesEqual2(f.content, fileContent); + assertLinesEqual2(f.filename,f.content, fileContent); } else { // original tests. assertLinesEqual1(f.content, fileContent); } @@ -490,7 +490,7 @@ public void assertLinesEqual1(String expected, String actual) { * @param expected to compare * @param actual to compare */ - public void assertLinesEqual2(String expected, String actual) { + public void assertLinesEqual2(String name,String expected, String actual) { expected = expected.trim().replaceAll("([ \t\r\n])\\1+", "$1"); actual = actual.trim().replaceAll("([ \t\r\n])\\1+", "$1"); String[] linesExpected = expected.lines().toArray(String[]::new); @@ -504,9 +504,10 @@ public void assertLinesEqual2(String expected, String actual) { boolean same = e.equals(a); String sep = same ? " " : " | "; equals &= same; - sb.append(String.format("[%3d] %-80s%s%-80s%n", i, e, sep, a)); + sb.append(String.format(name+" [%3d] %-80s%s%-80s%n", i, e, sep, a)); } if (!equals) { + System.err.println("test "+getName()+" failed"); System.err.println(sb.toString()); fail("lines differ, see stderr for more details."); } From f7ee50131fd2a05ca07319d19d720292675643bb Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 11:21:19 +0100 Subject: [PATCH 07/19] removed useless version test --- .../modules/refactoring/java/test/InnerOuterRecordTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 6594771b04f5..65798e0f446c 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -60,11 +60,6 @@ public InnerOuterRecordTest(String name) { //OK, no RELEASE_17, skip test return; } -// final JavaCompiler cmp = ToolProvider.getSystemJavaCompiler(); -// for (SourceVersion sourceVersion : cmp.getSourceVersions()) { -// System.out.println("sourceVersion = " + sourceVersion); -// } - } // for reference public void test259004() throws Exception { From 4eb02e1b11e4289986b1b8f6a73749895863894f Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 14:05:32 +0100 Subject: [PATCH 08/19] make test less chatty with debug flag. --- .../modules/refactoring/java/test/RefactoringTestBase.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java index e418555158a2..947a75962e00 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java @@ -86,6 +86,7 @@ public RefactoringTestBase(String name, String sourcelevel) { this.sourcelevel = sourcelevel; } + static boolean debug=false; protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... files) throws Exception { long currentTimeMillis = System.currentTimeMillis(); for (FileObject c : sourceRoot.getChildren()) { @@ -99,7 +100,9 @@ protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... fi IndexingManager.getDefault().refreshIndexAndWait(sourceRoot.toURL(), null, true); long currentTimeMillis1 = System.currentTimeMillis(); - System.err.println("writeFilesAndWaitForScan took "+(currentTimeMillis1-currentTimeMillis)+" millis"); + if (debug) { + System.err.println("writeFilesAndWaitForScan took " + (currentTimeMillis1 - currentTimeMillis) + " millis"); + } } protected static void writeFilesNoIndexing(FileObject sourceRoot, File... files) throws Exception { From 96ca0cfb90e4ea9055ef405db8b1c13d31e0dae9 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Mon, 6 Jan 2025 15:10:13 +0100 Subject: [PATCH 09/19] check for nullness before dereference. Faster without scan and wait --- .../java/source/transform/ImmutableTreeTranslator.java | 2 +- .../refactoring/java/test/InnerOuterRecordTest.java | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java b/java/java.source.base/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java index 4efeb9b2af6c..b38ed6ea1131 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java @@ -1385,7 +1385,7 @@ protected final ModifiersTree rewriteChildren(ModifiersTree tree) { protected final ErroneousTree rewriteChildren(ErroneousTree tree) { List oldErrs = tree.getErrorTrees(); List newErrs = translate(oldErrs); - if (!newErrs.equals(oldErrs)) { + if (null !=newErrs && !newErrs.equals(oldErrs)) { ErroneousTree n = make.Erroneous(newErrs); model.setType(n, model.getType(tree)); copyCommentTo(tree,n); diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 65798e0f446c..137cf7a9b0a8 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -61,8 +61,9 @@ public InnerOuterRecordTest(String name) { return; } } + // for reference - public void test259004() throws Exception { + public void _test259004() throws Exception { String source; writeFilesNoIndexing(src, new File("t/A.java", @@ -290,7 +291,7 @@ public Student(int id, String name, LocalDate dob) { public void testBasicRecord1() throws Exception { // initial outer has record with meaningful canonical constructor. - writeFilesAndWaitForScan(src, + writeFilesNoIndexing(src, new File("t/A.java", """ package t; @@ -323,10 +324,8 @@ record A(int id, String name, LocalDate dob) { /* * Refactoring License */ - package t; - /** * * @author junit From 88b1059eaa576b5efb9e283015076e89332f9726 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 10:21:17 +0100 Subject: [PATCH 10/19] added tests for simple and case with compact constructors New tests to separate out more issues. Current recognized issues : Compact construtor in inner is not properly moved. Record header of inner is not properly moved. Current understanding is that code refactoring to source for inner record is incomplete (Not considering RECORD and or RECORD_COMPONENT). --- .../java/test/InnerOuterRecordTest.java | 109 +++++++++++++++++- 1 file changed, 104 insertions(+), 5 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 137cf7a9b0a8..a6ec1454dabb 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -61,7 +61,7 @@ public InnerOuterRecordTest(String name) { return; } } - + // for reference public void _test259004() throws Exception { String source; @@ -176,7 +176,7 @@ void useStudent() { import java.time.LocalDate; import java.util.Objects; /** - * + * * @author junit */ record F(int id, String name, LocalDate dob) { @@ -197,7 +197,7 @@ record F(int id, String name, LocalDate dob) { } - public void testBasicClassExample() throws Exception { + public void testBasicRecordInRecord() throws Exception { // initial outer has record with meaningful canonical constructor. writeFilesNoIndexing(src, new File("t/A.java", @@ -290,7 +290,6 @@ public Student(int id, String name, LocalDate dob) { } public void testBasicRecord1() throws Exception { - // initial outer has record with meaningful canonical constructor. writeFilesNoIndexing(src, new File("t/A.java", """ @@ -316,7 +315,7 @@ record F(int x, int y){} record A(int id, String name, LocalDate dob) { F f; - + } """), new File("t/F.java", @@ -338,6 +337,106 @@ record F(int x, int y){ } + public void testOuterWithCompact() throws Exception{ + String source= + """ + package t; + import java.time.LocalDate; + record A(F f){ + + public A{ + assert f!=null; + } + record F(int id, String name, LocalDate dob){} + } + """; + String newOuter= + """ + package t; + import java.time.LocalDate; + record A(F f){ + public A{ + assert f!=null; + } + } + """; + String newInner= + """ + /* + * Refactoring License + */ + package t; + import java.time.LocalDate; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob){} + """; + writeFilesNoIndexing(src,new File("t/A.java",source)); + performInnerToOuterTest2(null); + verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + } + + public void testInnerWithCompact() throws Exception{ + String source= + """ + package t; + + import java.time.LocalDate; + + record A(F f) { + + public A { + assert f != null; + } + + record F(int id, String name, LocalDate dob) { + + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + } + } + """; + String newOuter= + """ + package t; + import java.time.LocalDate; + record A(F f) { + public A { + assert f != null; + } + } + """; + String newInner= + """ + /* + * Refactoring License + */ + package t; + import java.time.LocalDate; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob) { + + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + } + """; + writeFilesNoIndexing(src,new File("t/A.java",source)); + performInnerToOuterTest2(null); + verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + } + + boolean debug = true; // variant for record inner to outer test From d25d1de1c201626677f75a52b5f6b4a8e5c16319 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 10:26:50 +0100 Subject: [PATCH 11/19] Add RECORD and RECORD_COMPONENT as valid case labels Part of the solution ma --- .../netbeans/api/java/source/ElementHandle.java | 17 ++++++++++------- .../api/java/source/TreePathHandle.java | 5 ++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java b/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java index 076e41c446ae..c096c0c4f631 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java @@ -107,7 +107,7 @@ private ElementHandle(final ElementKind kind, String... signatures) { * Resolves an {@link Element} from the {@link ElementHandle}. * @param compilationInfo representing the {@link javax.tools.JavaCompiler.CompilationTask} * in which the {@link Element} should be resolved. - * @return resolved subclass of {@link Element} or null if the elment does not exist on + * @return resolved subclass of {@link Element} or null if the element does not exist on * the classpath/sourcepath of {@link javax.tools.JavaCompiler.CompilationTask}. */ @SuppressWarnings ("unchecked") // NOI18N @@ -143,12 +143,12 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { if (log.isLoggable(Level.FINE)) log.log(Level.FINE, "Resolving element kind: {0}", this.kind); // NOI18N ElementKind simplifiedKind = this.kind; - if (simplifiedKind.name().equals("RECORD")) { - simplifiedKind = ElementKind.CLASS; //TODO: test - } - if (simplifiedKind.name().equals("RECORD_COMPONENT")) { - simplifiedKind = ElementKind.FIELD; //TODO: test - } +// if (simplifiedKind.name().equals("RECORD")) { +// simplifiedKind = ElementKind.CLASS; //TODO: test +// } +// if (simplifiedKind.name().equals("RECORD_COMPONENT")) { +// simplifiedKind = ElementKind.FIELD; //TODO: test +// } switch (simplifiedKind) { case PACKAGE: assert signatures.length == 1; @@ -156,6 +156,7 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { case CLASS: case INTERFACE: case ENUM: + case RECORD: case ANNOTATION_TYPE: { assert signatures.length == 1; final Element type = getTypeElementByBinaryName (module, signatures[0], jt); @@ -213,6 +214,8 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { } case FIELD: case ENUM_CONSTANT: + case RECORD_COMPONENT: + { assert signatures.length == 3; final Element type = getTypeElementByBinaryName (module, signatures[0], jt); diff --git a/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java b/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java index 14b097f0afb8..90f8c5c2d69d 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java @@ -322,6 +322,7 @@ private static boolean isSupported(Element el) { case ENUM_CONSTANT: case RECORD: //TODO: record component + case RECORD_COMPONENT: return true; case PARAMETER: //only method and constructor parameters supported (not lambda): @@ -410,7 +411,7 @@ private TreeDelegate(Position position, KindPath kindPath, FileObject file, Elem case ENUM: kind = Tree.Kind.ENUM; break; case INTERFACE: kind = Tree.Kind.INTERFACE; break; case RECORD: kind = Tree.Kind.RECORD; break; - case ENUM_CONSTANT: case FIELD: kind = Tree.Kind.VARIABLE; break; + case ENUM_CONSTANT: case FIELD: case RECORD_COMPONENT: kind = Tree.Kind.VARIABLE; break; case METHOD: case CONSTRUCTOR: kind = Tree.Kind.METHOD; break; default: kind = null; break; } @@ -714,6 +715,7 @@ public Kind getKind() { case ENUM: case CLASS: + case RECORD: case ANNOTATION_TYPE: case INTERFACE: return Kind.CLASS; @@ -724,6 +726,7 @@ public Kind getKind() { case LOCAL_VARIABLE: case RESOURCE_VARIABLE: case EXCEPTION_PARAMETER: + case RECORD_COMPONENT: return Kind.VARIABLE; case METHOD: From 1e3328fff9e82e07f5b1420758266c47a9c8491a Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 10:33:19 +0100 Subject: [PATCH 12/19] mute debug info in test --- .../modules/refactoring/java/test/InnerOuterRecordTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index a6ec1454dabb..7988fa1ec032 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -114,7 +114,6 @@ public final class F { } public void testApacheNetbeans7044() throws Exception { - debug = true; // initial outer has record with meaningful canonical constructor. // note that Inner class should be in last member for assumptions in the test. writeFilesNoIndexing(src, new File("t/A.java", @@ -437,7 +436,7 @@ record F(int id, String name, LocalDate dob) { } - boolean debug = true; + boolean debug = false; // variant for record inner to outer test private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { From e8fe39108dfe1edd9e8f8ebb9115528d10c3143e Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 10:53:57 +0100 Subject: [PATCH 13/19] test code cleanup to easy visual inspaction. --- .../java/test/InnerOuterRecordTest.java | 433 +++++++++--------- 1 file changed, 211 insertions(+), 222 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 7988fa1ec032..633cdc017526 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -63,126 +63,76 @@ public InnerOuterRecordTest(String name) { } // for reference - public void _test259004() throws Exception { - String source; - writeFilesNoIndexing(src, - new File("t/A.java", - source = """ - package t; + public void test259004() throws Exception { + String source + = """ + package t; - import java.util.function.Consumer; + import java.util.function.Consumer; - public class A { + public class A { - public static void main(String[] args) { - Consumer c = f -> {}; - } + public static void main(String[] args) { + Consumer c = f -> {}; + } - public static final class F {} - }""")); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - source = """ - package t; + public static final class F {} + }"""; + String newOuter + = """ + package t; - import java.util.function.Consumer; + import java.util.function.Consumer; - public class A { + public class A { - public static void main(String[] args) { - Consumer c = f -> {}; - } + public static void main(String[] args) { + Consumer c = f -> {}; + } - }"""), - new File("t/F.java", - """ - /* - * Refactoring License - */ + }"""; + String newInner + = """ + /* + * Refactoring License + */ - package t; + package t; - /** - * - * @author junit - */ - public final class F { + /** + * + * @author junit + */ + public final class F { + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); - } - """)); } public void testApacheNetbeans7044() throws Exception { // initial outer has record with meaningful canonical constructor. // note that Inner class should be in last member for assumptions in the test. - writeFilesNoIndexing(src, new File("t/A.java", - """ - package t; - - import java.time.LocalDate; - import java.util.Objects; - - public class A { - - void useStudent() { - F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } - - record F(int id, String name, LocalDate dob) { - - /** - * Validate stuff. - */ - public F { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - } - } - - } - - """) - ); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; + String source + = """ + package t; - import java.time.LocalDate; - import java.util.Objects; + import java.time.LocalDate; + import java.util.Objects; - public class A { + public class A { - void useStudent() { - F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } + void useStudent() { + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """), - new File("t/F.java", - """ - /* - * Refactoring License - */ - package t; - import java.time.LocalDate; - import java.util.Objects; - /** - * - * @author junit - */ record F(int id, String name, LocalDate dob) { /** - * Validate stuff. - */ + * Validate stuff. + */ public F { Objects.requireNonNull(id); Objects.requireNonNull(name); @@ -191,16 +141,11 @@ record F(int id, String name, LocalDate dob) { assert dob.isAfter(LocalDate.EPOCH); } } - """) - ); - } - - public void testBasicRecordInRecord() throws Exception { - // initial outer has record with meaningful canonical constructor. - writeFilesNoIndexing(src, - new File("t/A.java", - """ + } + """; + String newOuter + = """ package t; import java.time.LocalDate; @@ -209,136 +154,179 @@ public void testBasicRecordInRecord() throws Exception { public class A { void useStudent() { - Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + package t; - public static class Student { - int id; - String name; - LocalDate dob - public Student(int id, String name, LocalDate dob) { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - this.id=id; - this.name=name; - this.dob=dob; - } + import java.time.LocalDate; + import java.util.Objects; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob) { + + /** + * Validate stuff. + */ + public F { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); } + } + """; + + innerOuterSetupAndTest(source, newOuter, newInner); + } + public void testBasicClassInClass() throws Exception { + // initial outer has record with meaningful canonical constructor. + String source + = """ + package t; + + import java.time.LocalDate; + import java.util.Objects; + + public class A { + + void useStudent() { + F s = new F(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """)); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; - import java.time.LocalDate; - import java.util.Objects; + public static class F { + int id; + String name; + LocalDate dob + public Student(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id=id; + this.name=name; + this.dob=dob; + } + } - public class A { + } + """; + String newOuter + = + """ + package t; - void useStudent() { - Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } + import java.time.LocalDate; + import java.util.Objects; + public class A { + void useStudent() { + F s = new F(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """), - new File("t/Student.java", - """ - /* - * Refactoring License - */ - package t; - import java.time.LocalDate; - import java.util.Objects; + } + """; + String newInner = + """ + /* + * Refactoring License + */ - /** - * - * @author junit - */ - public class Student { - - int id; - String name; - LocalDate dob; - - public Student(int id, String name, LocalDate dob) { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - this.id = id; - this.name = name; - this.dob = dob; - } + package t; - } - """ - )); + import java.time.LocalDate; + import java.util.Objects; + + /** + * + * @author junit + */ + public class F { + + int id; + String name; + LocalDate dob; + + public F(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id = id; + this.name = name; + this.dob = dob; + } + } + """; + + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testBasicRecord1() throws Exception { - writeFilesNoIndexing(src, - new File("t/A.java", - """ - package t; + public void testBasicRecordInRecord() throws Exception { + String source = + """ + package t; - import java.time.LocalDate; + import java.time.LocalDate; - record A(int id, String name, LocalDate dob) { + record A(int id, String name, LocalDate dob) { - F f; - record F(int x, int y){} - } - """ - )); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; + F f; + record F(int x, int y){} + } + """; + String newOuter= + """ + package t; - import java.time.LocalDate; + import java.time.LocalDate; - record A(int id, String name, LocalDate dob) { + record A(int id, String name, LocalDate dob) { - F f; + F f; - } - """), - new File("t/F.java", - """ - /* - * Refactoring License - */ - package t; + } + """; + String newInner = + """ + /* + * Refactoring License + */ + package t; - /** - * - * @author junit - */ - record F(int x, int y){ - } + /** + * + * @author junit + */ + record F(int x, int y){ + } - """ - )); + """; + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testOuterWithCompact() throws Exception{ - String source= - """ + public void testOuterWithCompact() throws Exception { + String source + = """ package t; import java.time.LocalDate; record A(F f){ @@ -349,8 +337,8 @@ record A(F f){ record F(int id, String name, LocalDate dob){} } """; - String newOuter= - """ + String newOuter + = """ package t; import java.time.LocalDate; record A(F f){ @@ -359,8 +347,8 @@ record A(F f){ } } """; - String newInner= - """ + String newInner + = """ /* * Refactoring License */ @@ -372,14 +360,12 @@ record A(F f){ */ record F(int id, String name, LocalDate dob){} """; - writeFilesNoIndexing(src,new File("t/A.java",source)); - performInnerToOuterTest2(null); - verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testInnerWithCompact() throws Exception{ - String source= - """ + public void testInnerWithCompact() throws Exception { + String source + = """ package t; import java.time.LocalDate; @@ -399,9 +385,9 @@ record F(int id, String name, LocalDate dob) { } } } - """; - String newOuter= - """ + """; + String newOuter + = """ package t; import java.time.LocalDate; record A(F f) { @@ -410,8 +396,8 @@ record A(F f) { } } """; - String newInner= - """ + String newInner + = """ /* * Refactoring License */ @@ -430,11 +416,14 @@ record F(int id, String name, LocalDate dob) { } } """; - writeFilesNoIndexing(src,new File("t/A.java",source)); - performInnerToOuterTest2(null); - verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + innerOuterSetupAndTest(source, newOuter, newInner); } + void innerOuterSetupAndTest(String source, String newOuter, String newInner) throws Exception { + writeFilesNoIndexing(src, new File("t/A.java", source)); + performInnerToOuterTest2(null); + verifyContent(src, new File("t/A.java", newOuter), new File("t/F.java", newInner)); + } boolean debug = false; From cb94b08949fcbdd884da7a037dacb4007cdf018d Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 10:26:50 +0100 Subject: [PATCH 14/19] Add RECORD and RECORD_COMPONENT as valid case labels Part of the solution ma --- .../api/java/source/ElementHandle.java | 17 +- .../api/java/source/TreePathHandle.java | 5 +- .../java/test/InnerOuterRecordTest.java | 496 ++++++++++-------- 3 files changed, 286 insertions(+), 232 deletions(-) diff --git a/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java b/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java index 076e41c446ae..c096c0c4f631 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java @@ -107,7 +107,7 @@ private ElementHandle(final ElementKind kind, String... signatures) { * Resolves an {@link Element} from the {@link ElementHandle}. * @param compilationInfo representing the {@link javax.tools.JavaCompiler.CompilationTask} * in which the {@link Element} should be resolved. - * @return resolved subclass of {@link Element} or null if the elment does not exist on + * @return resolved subclass of {@link Element} or null if the element does not exist on * the classpath/sourcepath of {@link javax.tools.JavaCompiler.CompilationTask}. */ @SuppressWarnings ("unchecked") // NOI18N @@ -143,12 +143,12 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { if (log.isLoggable(Level.FINE)) log.log(Level.FINE, "Resolving element kind: {0}", this.kind); // NOI18N ElementKind simplifiedKind = this.kind; - if (simplifiedKind.name().equals("RECORD")) { - simplifiedKind = ElementKind.CLASS; //TODO: test - } - if (simplifiedKind.name().equals("RECORD_COMPONENT")) { - simplifiedKind = ElementKind.FIELD; //TODO: test - } +// if (simplifiedKind.name().equals("RECORD")) { +// simplifiedKind = ElementKind.CLASS; //TODO: test +// } +// if (simplifiedKind.name().equals("RECORD_COMPONENT")) { +// simplifiedKind = ElementKind.FIELD; //TODO: test +// } switch (simplifiedKind) { case PACKAGE: assert signatures.length == 1; @@ -156,6 +156,7 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { case CLASS: case INTERFACE: case ENUM: + case RECORD: case ANNOTATION_TYPE: { assert signatures.length == 1; final Element type = getTypeElementByBinaryName (module, signatures[0], jt); @@ -213,6 +214,8 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) { } case FIELD: case ENUM_CONSTANT: + case RECORD_COMPONENT: + { assert signatures.length == 3; final Element type = getTypeElementByBinaryName (module, signatures[0], jt); diff --git a/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java b/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java index 14b097f0afb8..90f8c5c2d69d 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java @@ -322,6 +322,7 @@ private static boolean isSupported(Element el) { case ENUM_CONSTANT: case RECORD: //TODO: record component + case RECORD_COMPONENT: return true; case PARAMETER: //only method and constructor parameters supported (not lambda): @@ -410,7 +411,7 @@ private TreeDelegate(Position position, KindPath kindPath, FileObject file, Elem case ENUM: kind = Tree.Kind.ENUM; break; case INTERFACE: kind = Tree.Kind.INTERFACE; break; case RECORD: kind = Tree.Kind.RECORD; break; - case ENUM_CONSTANT: case FIELD: kind = Tree.Kind.VARIABLE; break; + case ENUM_CONSTANT: case FIELD: case RECORD_COMPONENT: kind = Tree.Kind.VARIABLE; break; case METHOD: case CONSTRUCTOR: kind = Tree.Kind.METHOD; break; default: kind = null; break; } @@ -714,6 +715,7 @@ public Kind getKind() { case ENUM: case CLASS: + case RECORD: case ANNOTATION_TYPE: case INTERFACE: return Kind.CLASS; @@ -724,6 +726,7 @@ public Kind getKind() { case LOCAL_VARIABLE: case RESOURCE_VARIABLE: case EXCEPTION_PARAMETER: + case RECORD_COMPONENT: return Kind.VARIABLE; case METHOD: diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index a6ec1454dabb..8760dc5a12d7 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -63,127 +63,76 @@ public InnerOuterRecordTest(String name) { } // for reference - public void _test259004() throws Exception { - String source; - writeFilesNoIndexing(src, - new File("t/A.java", - source = """ - package t; + public void test259004() throws Exception { + String source + = """ + package t; - import java.util.function.Consumer; + import java.util.function.Consumer; - public class A { + public class A { - public static void main(String[] args) { - Consumer c = f -> {}; - } + public static void main(String[] args) { + Consumer c = f -> {}; + } - public static final class F {} - }""")); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - source = """ - package t; + public static final class F {} + }"""; + String newOuter + = """ + package t; - import java.util.function.Consumer; + import java.util.function.Consumer; - public class A { + public class A { - public static void main(String[] args) { - Consumer c = f -> {}; - } + public static void main(String[] args) { + Consumer c = f -> {}; + } - }"""), - new File("t/F.java", - """ - /* - * Refactoring License - */ + }"""; + String newInner + = """ + /* + * Refactoring License + */ - package t; + package t; - /** - * - * @author junit - */ - public final class F { + /** + * + * @author junit + */ + public final class F { + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); - } - """)); } public void testApacheNetbeans7044() throws Exception { - debug = true; // initial outer has record with meaningful canonical constructor. // note that Inner class should be in last member for assumptions in the test. - writeFilesNoIndexing(src, new File("t/A.java", - """ - package t; - - import java.time.LocalDate; - import java.util.Objects; - - public class A { - - void useStudent() { - F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } - - record F(int id, String name, LocalDate dob) { - - /** - * Validate stuff. - */ - public F { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - } - } - - } - - """) - ); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; + String source + = """ + package t; - import java.time.LocalDate; - import java.util.Objects; + import java.time.LocalDate; + import java.util.Objects; - public class A { + public class A { - void useStudent() { - F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } + void useStudent() { + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """), - new File("t/F.java", - """ - /* - * Refactoring License - */ - package t; - import java.time.LocalDate; - import java.util.Objects; - /** - * - * @author junit - */ record F(int id, String name, LocalDate dob) { /** - * Validate stuff. - */ + * Validate stuff. + */ public F { Objects.requireNonNull(id); Objects.requireNonNull(name); @@ -192,16 +141,11 @@ record F(int id, String name, LocalDate dob) { assert dob.isAfter(LocalDate.EPOCH); } } - """) - ); - - } - public void testBasicRecordInRecord() throws Exception { - // initial outer has record with meaningful canonical constructor. - writeFilesNoIndexing(src, - new File("t/A.java", - """ + } + """; + String newOuter + = """ package t; import java.time.LocalDate; @@ -210,136 +154,179 @@ public void testBasicRecordInRecord() throws Exception { public class A { void useStudent() { - Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + F s = new F(42,"Jan Klaassen", LocalDate.now().minusDays(1)); System.out.println("student = " + s); } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + package t; - public static class Student { - int id; - String name; - LocalDate dob - public Student(int id, String name, LocalDate dob) { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - this.id=id; - this.name=name; - this.dob=dob; - } + import java.time.LocalDate; + import java.util.Objects; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob) { + + /** + * Validate stuff. + */ + public F { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); } + } + """; + + innerOuterSetupAndTest(source, newOuter, newInner); + } + + public void testBasicClassInClass() throws Exception { + // initial outer has record with meaningful canonical constructor. + String source + = """ + package t; + import java.time.LocalDate; + import java.util.Objects; + + public class A { + + void useStudent() { + F s = new F(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """)); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; - import java.time.LocalDate; - import java.util.Objects; + public static class F { + int id; + String name; + LocalDate dob + public Student(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id=id; + this.name=name; + this.dob=dob; + } + } - public class A { + } + """; + String newOuter + = + """ + package t; - void useStudent() { - Student s = new Student(42, "Jan Klaassen", LocalDate.now().minusDays(1)); - System.out.println("student = " + s); - } + import java.time.LocalDate; + import java.util.Objects; + public class A { + void useStudent() { + F s = new F(42, "Jan Klaassen", LocalDate.now().minusDays(1)); + System.out.println("student = " + s); } - """), - new File("t/Student.java", - """ - /* - * Refactoring License - */ - package t; - import java.time.LocalDate; - import java.util.Objects; + } + """; + String newInner = + """ + /* + * Refactoring License + */ - /** - * - * @author junit - */ - public class Student { - - int id; - String name; - LocalDate dob; - - public Student(int id, String name, LocalDate dob) { - Objects.requireNonNull(id); - Objects.requireNonNull(name); - Objects.requireNonNull(dob); - assert !name.isEmpty() && !name.isBlank(); - assert dob.isAfter(LocalDate.EPOCH); - this.id = id; - this.name = name; - this.dob = dob; - } + package t; - } - """ - )); + import java.time.LocalDate; + import java.util.Objects; + /** + * + * @author junit + */ + public class F { + + int id; + String name; + LocalDate dob; + + public F(int id, String name, LocalDate dob) { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(dob); + assert !name.isEmpty() && !name.isBlank(); + assert dob.isAfter(LocalDate.EPOCH); + this.id = id; + this.name = name; + this.dob = dob; + } + + } + """; + + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testBasicRecord1() throws Exception { - writeFilesNoIndexing(src, - new File("t/A.java", - """ - package t; + public void testBasicRecordInRecord() throws Exception { + String source = + """ + package t; - import java.time.LocalDate; + import java.time.LocalDate; - record A(int id, String name, LocalDate dob) { + record A(int id, String name, LocalDate dob) { - F f; - record F(int x, int y){} - } - """ - )); - performInnerToOuterTest2(null); - verifyContent(src, - new File("t/A.java", - """ - package t; + F f; + record F(int x, int y){} + } + """; + String newOuter= + """ + package t; - import java.time.LocalDate; + import java.time.LocalDate; - record A(int id, String name, LocalDate dob) { + record A(int id, String name, LocalDate dob) { - F f; + F f; - } - """), - new File("t/F.java", - """ - /* - * Refactoring License - */ - package t; + } + """; + String newInner = + """ + /* + * Refactoring License + */ + package t; - /** - * - * @author junit - */ - record F(int x, int y){ - } + /** + * + * @author junit + */ + record F(int x, int y){ + } - """ - )); + """; + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testOuterWithCompact() throws Exception{ - String source= - """ + public void testOuterWithCompact() throws Exception { + String source + = """ package t; import java.time.LocalDate; record A(F f){ @@ -350,8 +337,8 @@ record A(F f){ record F(int id, String name, LocalDate dob){} } """; - String newOuter= - """ + String newOuter + = """ package t; import java.time.LocalDate; record A(F f){ @@ -360,8 +347,8 @@ record A(F f){ } } """; - String newInner= - """ + String newInner + = """ /* * Refactoring License */ @@ -373,14 +360,12 @@ record A(F f){ */ record F(int id, String name, LocalDate dob){} """; - writeFilesNoIndexing(src,new File("t/A.java",source)); - performInnerToOuterTest2(null); - verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + innerOuterSetupAndTest(source, newOuter, newInner); } - public void testInnerWithCompact() throws Exception{ - String source= - """ + public void testInnerWithCompact() throws Exception { + String source + = """ package t; import java.time.LocalDate; @@ -400,9 +385,9 @@ record F(int id, String name, LocalDate dob) { } } } - """; - String newOuter= - """ + """; + String newOuter + = """ package t; import java.time.LocalDate; record A(F f) { @@ -411,8 +396,8 @@ record A(F f) { } } """; - String newInner= - """ + String newInner + = """ /* * Refactoring License */ @@ -431,13 +416,76 @@ record F(int id, String name, LocalDate dob) { } } """; - writeFilesNoIndexing(src,new File("t/A.java",source)); - performInnerToOuterTest2(null); - verifyContent(src, new File("t/A.java",newOuter),new File("t/F.java",newInner)); + innerOuterSetupAndTest(source, newOuter, newInner); } + + // outer may have effect + public void testClassWithInnerRecord() throws Exception { + String source + = """ + package t; + + import java.time.LocalDate; + + class A { + final F f; + public A(F f) { + assert f != null; + this.f=f; + } + + record F(int id, String name, LocalDate dob) { + + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + } + } + """; + String newOuter + = """ + package t; + import java.time.LocalDate; + class A { + final F f; + public A(F f) { + assert f != null; + this.f=f; + } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + package t; + import java.time.LocalDate; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob) { + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + void innerOuterSetupAndTest(String source, String newOuter, String newInner) throws Exception { + writeFilesNoIndexing(src, new File("t/A.java", source)); + performInnerToOuterTest2(null); + verifyContent(src, new File("t/A.java", newOuter), new File("t/F.java", newInner)); + } - boolean debug = true; + boolean debug = false; // variant for record inner to outer test private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { From 1b976021e6a758b664bc7552505658eb7ec069c3 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Tue, 7 Jan 2025 17:27:17 +0100 Subject: [PATCH 15/19] add test to rename inner record. Inner record has compact constructor that does a meaningful validation. Test runs without issues. Rationale: Trying to find who mishandles the inner record as described in #7044. --- .../java/plugins/InnerToOuterTransformer.java | 8 ++--- .../java/test/InnerOuterRecordTest.java | 6 ++-- .../java/test/RenameRecordTest.java | 36 +++++++++++++++++-- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java index 226e6ae0b404..a1447acaffcc 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java @@ -246,12 +246,12 @@ public Tree visitClass(ClassTree classTree, Element element) { if (currentElement!=null && currentElement == outer) { Element outerouter = outer.getEnclosingElement(); Tree superVisit = super.visitClass(classTree, element); - TreePath tp = workingCopy.getTrees().getPath(inner); - if (tp==null) { + TreePath innerTp = workingCopy.getTrees().getPath(inner); + if (innerTp==null) { //#194346 return superVisit; } - ClassTree innerClass = (ClassTree) tp.getLeaf(); + ClassTree innerClass = (ClassTree) innerTp.getLeaf(); ClassTree newInnerClass = innerClass; newInnerClass = genUtils.importComments(newInnerClass, workingCopy.getCompilationUnit()); @@ -270,7 +270,7 @@ public Tree visitClass(ClassTree classTree, Element element) { if(outerPath != null) { JavaRefactoringUtils.cacheTreePathInfo(outerPath, workingCopy); } - CompilationUnitTree compilationUnit = tp.getCompilationUnit(); + CompilationUnitTree compilationUnit = innerTp.getCompilationUnit(); String relativePath = RefactoringUtils.getPackageName(compilationUnit).replace('.', '/') + '/' + refactoring.getClassName() + ".java"; // NOI18N CompilationUnitTree newCompilation = JavaPluginUtils.createCompilationUnit(sourceRoot, relativePath, newInnerClass, workingCopy, make); rewrite(null, newCompilation); diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 8760dc5a12d7..a99078226df2 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -288,8 +288,8 @@ public void testBasicRecordInRecord() throws Exception { import java.time.LocalDate; record A(int id, String name, LocalDate dob) { - - F f; + + static F f; record F(int x, int y){} } """; @@ -301,7 +301,7 @@ record F(int x, int y){} record A(int id, String name, LocalDate dob) { - F f; + static F f; } """; diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RenameRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RenameRecordTest.java index 43fbba7faa72..da6e5ae7f020 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RenameRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RenameRecordTest.java @@ -344,7 +344,7 @@ public void testRenameRecord() throws Exception { package test; public record Te|st(int component) { public Test { - component = ""; + component = 42; } } """; @@ -367,7 +367,7 @@ private Test test() { package test; public record NewName(int component) { public NewName { - component = ""; + component = 42; } } """), @@ -381,6 +381,38 @@ private NewName test() { } """)); + } + + public void testRenameInnerRecord() throws Exception { + String testCode = """ + package test; + public record Test(Component component) { + public Test { + assert component !=null; + } + record Comp|onent(int c){ + assert c >= 0; + } + } + """; + TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); + writeFilesAndWaitForScan(src, + new File("Test.java", splitCode.code())); + JavaRenameProperties props = new JavaRenameProperties(); + performRename(src.getFileObject("Test.java"), splitCode.pos(), "Part", props, true); + verifyContent(src, new File("Test.java", + """ + package test; + public record Test(Part component) { + public Test { + assert component !=null; + } + record Part(int c){ + assert c >= 0; + } + } + """)); + } private void performRename(FileObject source, final int absPos, final String newname, final JavaRenameProperties props, final boolean searchInComments, Problem... expectedProblems) throws Exception { final RenameRefactoring[] r = new RenameRefactoring[1]; From 32588dd2dc199b15b297f1edb06e2043c3f12b77 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Wed, 8 Jan 2025 15:35:37 +0100 Subject: [PATCH 16/19] add null check before dereferencing element. --- .../modules/refactoring/java/test/InnerOuterRecordTest.java | 4 +--- .../profiler/nbimpl/providers/JavaProfilerSourceImpl.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index a99078226df2..a9bdee258cf7 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -316,9 +316,7 @@ record A(int id, String name, LocalDate dob) { * * @author junit */ - record F(int x, int y){ - } - + record F(int x, int y){} """; innerOuterSetupAndTest(source, newOuter, newInner); diff --git a/profiler/profiler.nbimpl/src/org/netbeans/modules/profiler/nbimpl/providers/JavaProfilerSourceImpl.java b/profiler/profiler.nbimpl/src/org/netbeans/modules/profiler/nbimpl/providers/JavaProfilerSourceImpl.java index 21524a96c2a3..baab6c928b79 100644 --- a/profiler/profiler.nbimpl/src/org/netbeans/modules/profiler/nbimpl/providers/JavaProfilerSourceImpl.java +++ b/profiler/profiler.nbimpl/src/org/netbeans/modules/profiler/nbimpl/providers/JavaProfilerSourceImpl.java @@ -636,7 +636,7 @@ public void run(final CompilationController cc) throws Exception { @Override public Void visitMethod(MethodTree node, Void p) { Element e = cc.getTrees().getElement(getCurrentPath()); - if (e.getKind() == ElementKind.METHOD) { + if (null != e && e.getKind() == ElementKind.METHOD) { ExecutableElement ee = (ExecutableElement)e; if (ee.getSimpleName().contentEquals("suite") && // NOI18N (ee.getReturnType().toString().equals(JUNIT_TEST) || From e28add2d69cd84364092299a6be14f297f999814 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Thu, 9 Jan 2025 15:24:11 +0100 Subject: [PATCH 17/19] add source levels up to java 17, make reindex optional since Netbeans requires 17 to build and run, it is reasonable to expect that in source levelutils too. In RefactoringTestBase the repo is always reindexed, even when not required. As per documentation this is an expensive operation and taxes your patience. remove modifiers in variable member, so the do not show up Modifiers (private final in particular) show up in record header, making the resulting source code illegal and different from expected. The removal is done by rewriting the member tree where variables occur. make sure static fields of inner records survive field removal. ensure static fields remain in inner record. normal fields are modified by removing private final, but this should not happen to static fields. This is part of making inner records work. avoid use of raw types. pass declared type further down. Original code always passes down CLASS, now RECORD is passed down as such. It should not make a difference for class, interface, or enum. spelling RECORD should be equivalent to class, interface and enum. Add RECORD_COMPONENT as allowed element types add isInnerRecord boolean field. Some reformatting. For the reformatting: sorry. Stick to the formatting rules. For the field: Record does not fit the standard class definition mold because of the record header which has parenthesis before the first curly brace and the compact constructor that needs special attention. Somehow the information that ithe inner tis a inner record must be forwardable to the processors. The author comment is modified by some part. The @author line is substituted by some yet unknown processor, so exclude it from the result comparison to make the tests less britle because of dependence of the user property. generateOuter sounds like a boolean, but it is the name of the new inner match expected to actual after formatting. This test passes, showing that the Compact constructor in the outer survives the refactoring. formatting in expected outer cleanup. Permanently remove deprecated boolean cancelRequest cancelRequest is not only deprecated but has no use at all and was already replaced (function wide) by and atomicBoolean cancelRequested This commit cleans up the tests, may add a print that needs removing. For now: I am able to refactor inner record to outer but are still lacking the understandig of how to modify a MethodTree or the equivalent source code, and how to write a compact constructor instead. remove deprecated boolean no need to handle method special here. make test helper static to make it usable in more places. --- .../refactoring/api/RefactoringSession.java | 4 +- .../netbeans/api/java/source/JavaSource.java | 5 +- .../api/java/source/ModificationResult.java | 22 +-- .../netbeans/api/java/source/TreeMaker.java | 2 +- .../java/source/base/SourceLevelUtils.java | 7 + .../api/java/source/gen/RecordTest.java | 53 +++++ .../java/api/InnerToOuterRefactoring.java | 89 +++++++-- .../plugins/CopyClassRefactoringPlugin.java | 1 - .../plugins/CopyClassesRefactoringPlugin.java | 1 - .../java/plugins/InnerToOuterTransformer.java | 182 +++++++++++------- .../java/plugins/JavaPluginUtils.java | 5 +- .../plugins/JavaWhereUsedQueryPlugin.java | 1 - .../plugins/MoveFileRefactoringPlugin.java | 1 - .../plugins/SafeDeleteRefactoringPlugin.java | 1 - .../UseSuperTypeRefactoringPlugin.java | 1 - .../refactoring/java/spi/DiffElement.java | 2 +- .../java/spi/JavaRefactoringPlugin.java | 8 - .../java/spi/RefactoringVisitor.java | 2 + .../refactoring/java/ui/UIUtilities.java | 2 +- .../java/test/InnerOuterRecordTest.java | 128 ++++++------ .../java/test/InnerToOutterTest.java | 4 +- .../java/test/RefactoringTestBase.java | 110 ++++++++--- 22 files changed, 419 insertions(+), 212 deletions(-) diff --git a/ide/refactoring.api/src/org/netbeans/modules/refactoring/api/RefactoringSession.java b/ide/refactoring.api/src/org/netbeans/modules/refactoring/api/RefactoringSession.java index 64ffcfabf4c1..d6976ba561fc 100644 --- a/ide/refactoring.api/src/org/netbeans/modules/refactoring/api/RefactoringSession.java +++ b/ide/refactoring.api/src/org/netbeans/modules/refactoring/api/RefactoringSession.java @@ -102,7 +102,7 @@ public Problem doRefactoring(final boolean saveAfterDone) { private Problem reallyDoRefactoring(boolean saveAfterDone) { long time = System.currentTimeMillis(); - Iterator it = internalList.iterator(); + Iterator it = internalList.iterator(); ArrayList commits = SPIAccessor.DEFAULT.getCommits(bag); float progressStep = (float)COMMITSTEPS / internalList.size(); float current = 0F; @@ -115,7 +115,7 @@ private Problem reallyDoRefactoring(boolean saveAfterDone) { try { try { while (it.hasNext()) { - RefactoringElementImplementation element = (RefactoringElementImplementation) it.next(); + RefactoringElementImplementation element = it.next(); if (element.isEnabled() && !((element.getStatus() == RefactoringElement.GUARDED) || (element.getStatus() == RefactoringElement.READ_ONLY))) { element.performChange(); } diff --git a/java/java.source.base/src/org/netbeans/api/java/source/JavaSource.java b/java/java.source.base/src/org/netbeans/api/java/source/JavaSource.java index c9c9c504bb33..165272125130 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/JavaSource.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/JavaSource.java @@ -644,9 +644,12 @@ else if (rootCase instanceof RuntimeException) { } - /** Runs a task which permits for modifying the sources. + /** + * Runs a task which permits for modifying the sources. * Call to this method will cancel processing of all the phase completion tasks until * this task does not finish.
+ * + * ??? what does 'until this task does not finish mean'??? * @see Task for information about implementation requirements * @param task The task which. */ diff --git a/java/java.source.base/src/org/netbeans/api/java/source/ModificationResult.java b/java/java.source.base/src/org/netbeans/api/java/source/ModificationResult.java index ee316276f130..1a786ab4118d 100644 --- a/java/java.source.base/src/org/netbeans/api/java/source/ModificationResult.java +++ b/java/java.source.base/src/org/netbeans/api/java/source/ModificationResult.java @@ -223,25 +223,25 @@ static void commit (final FileObject fo, final List differences, fin if (source != null && out == null) { final Document doc = source.getDocument(false); if (doc != null) { - final IOException[] exceptions = new IOException [1]; - LineDocumentUtils.asRequired(doc, AtomicLockDocument.class).runAtomic(new Runnable () { - public void run () { + final IOException[] exceptions = new IOException[1]; + LineDocumentUtils.asRequired(doc, AtomicLockDocument.class).runAtomic(new Runnable() { + public void run() { try { - commit2 (doc, differences, out); + commit2(doc, differences, out); } catch (IOException ex) { - exceptions [0] = ex; + exceptions[0] = ex; } } }); - if (exceptions [0] != null) { + if (exceptions[0] != null) { LOG.log(Level.INFO, "Cannot commit changes into " + fo, exceptions[0]); int s = lastCommitted.size(); for (Throwable t : lastCommitted) { LOG.log(Level.INFO, "Previous commit number " + s--, t); } - throw exceptions [0]; + throw exceptions[0]; } - return; + return; } } Reader in = null; @@ -468,11 +468,11 @@ private static void createUnit(Difference diff, Writer out) { } public static class Difference { - Kind kind; + Kind kind; final Position startPos; final Position endPos; - String oldText; - String newText; + String oldText; + String newText; final String description; private boolean excluded; private boolean ignoreGuards = false; 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 2b443de9ba75..22fea8b10dfb 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 @@ -1308,7 +1308,7 @@ public Tree BindingPattern(CharSequence name, /** * Creates a new Tree for a given VariableTree - * specication : 15.20.2 + * specification : 15.20.2 * @param vt the VariableTree of the pattern * @see com.sun.source.tree.BindingPatternTree * @return the newly created BindingPatternTree diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java b/java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java index 4399c9f67ca3..7eee6d59dbb6 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java @@ -34,6 +34,13 @@ public class SourceLevelUtils { public static final Source JDK1_9 = Source.lookup("9"); public static final Source JDK14 = Source.lookup("14"); public static final Source JDK15 = Source.lookup("15"); + public static final Source JDK16 = Source.lookup("16"); + public static final Source JDK17 = Source.lookup("17"); + // for next release: +// public static final Source JDK18 = Source.lookup("18"); +// public static final Source JDK19 = Source.lookup("19"); +// public static final Source JDK20 = Source.lookup("20"); +// public static final Source JDK21 = Source.lookup("21"); public static boolean allowDefaultMethods(Source in) { return in.compareTo(JDK1_8) >= 0; diff --git a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordTest.java b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordTest.java index bc33aa888e5d..91279a55f6ab 100644 --- a/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordTest.java +++ b/java/java.source.base/test/unit/src/org/netbeans/api/java/source/gen/RecordTest.java @@ -194,6 +194,59 @@ public void run(WorkingCopy workingCopy) throws IOException { assertEquals(golden, res); } + /** + * Is the compact constructor preserved? + * Added check for #7044 + * + */ + public void testPreserveCompact() throws Exception { + testFile = new File(getWorkDir(), "Test.java"); + TestUtilities.copyStringToFile(testFile, + """ + package hierbas.del.litoral; + public record R(String first, String component) { + public R { + assert null != first; + } + } + """); + String golden = + """ + package hierbas.del.litoral; + public record R(String first) { + public R { + assert null != first; + } + } + """; + + 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 recordDecl = cut.getTypeDecls().get(0); + assertEquals(Kind.RECORD, recordDecl.getKind()); + ClassTree classTree = (ClassTree) recordDecl; + for (Tree m : classTree.getMembers()) { + if (m.getKind() == Kind.VARIABLE && + ((VariableTree) m).getName().contentEquals("component")) { + workingCopy.rewrite(classTree, make.removeClassMember(classTree, m)); + break; + } + } + } + + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(testFile); + //System.err.println(res); + assertEquals(golden, res); + } + public void testRemoveComponent() throws Exception { testFile = new File(getWorkDir(), "Test.java"); TestUtilities.copyStringToFile(testFile, diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/api/InnerToOuterRefactoring.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/api/InnerToOuterRefactoring.java index 52d457c59b8e..a5f244185035 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/api/InnerToOuterRefactoring.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/api/InnerToOuterRefactoring.java @@ -22,10 +22,10 @@ import org.netbeans.modules.refactoring.api.AbstractRefactoring; import org.openide.util.lookup.Lookups; -/** +/** * Convert Inner to Top-Level refactoring implementation class. This refactoring * is capable of converting an inner class into a top-level class. - * + * * @see org.netbeans.modules.refactoring.spi.RefactoringPlugin * @see org.netbeans.modules.refactoring.spi.RefactoringPluginFactory * @see org.netbeans.modules.refactoring.api.AbstractRefactoring @@ -39,18 +39,23 @@ public final class InnerToOuterRefactoring extends AbstractRefactoring { // parameters of the refactoring private String className; private String referenceName; - + + private boolean innerIsRecord; + /** * Creates a new instance of InnerToOuterRefactoring. - * - * @param sourceType An inner class that should be converted to a top-level class. + * + * @param sourceType An inner class that should be converted to a top-level + * class. */ public InnerToOuterRefactoring(TreePathHandle sourceType) { super(Lookups.singleton(sourceType)); } - - /** Returns the type the members of which should be pulled up - * by this refactoring. + + /** + * Returns the type the members of which should be pulled up by this + * refactoring. + * * @return Source of the members to be pulled up. */ public TreePathHandle getSourceType() { @@ -58,34 +63,80 @@ public TreePathHandle getSourceType() { } // --- PARAMETERS ---------------------------------------------------------- - - /** Returns the name for the top-level class to be created. + /** + * Returns the name for the top-level class to be created. + * * @return Class name. */ public String getClassName() { return className; } - /** Sets name for the top-level class to be created. + /** + * Sets name for the top-level class to be created. + * * @param className Class name. */ public void setClassName(String className) { this.className = className; } - /** Returns name of the field that should be generated as a reference to the original - * outer class. If null, no field will be generated. - * @return Name of the field to be generated or null if no field will be generated. + /** + * Returns name of the field that should be generated as a reference to the + * original outer class. If null, no field will be generated. + * + * @return Name of the field to be generated or null if no field will be + * generated. */ public String getReferenceName() { return referenceName; } - - /** Sets name of the field that should be generated as a reference to the original - * outer class. Can be set to null which indicates that no field should be generated. - * @param referenceName Name of the field or null if no field should be generated. - */ + + /** + * Sets name of the field that should be generated as a reference to the + * original outer class. Can be set to null which indicates that no field + * should be generated. + * + * @param referenceName Name of the field or null if no field should be + * generated. + */ public void setReferenceName(String referenceName) { this.referenceName = referenceName; } + + /** + * Inner records need special handling because of the RecordComponents which + * are declared before the first curly brace, which differs from class, + * interface and enum. + * + * Also, the compact constructor should be considered. + * + * A compact constructor consists of the name of the Record, no parameters + * (not even the parens) and a block that does NOT assign the fields. + * + * @return the current value for this refactoring + */ + public boolean isInnerIsRecord() { + + return innerIsRecord; + } + + /** + * Inner records need special handling because of the RecordComponents which + * are declared before the first curly brace, which differs from class, + * interface and enum. + * + * Also, the compact constructor should be considered. + * + * A compact constructor consists of the name of the Record, no parameters + * (not even the parens) and a block that does NOT assign the fields. + * + * @param innerIsRecord use when inner class needs the special handling of + * an inner record. + */ + + public void setInnerIsRecord(boolean innerIsRecord) { + this.innerIsRecord = innerIsRecord; + } + } diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassRefactoringPlugin.java index fb206da1f92a..f7bb944167f4 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassRefactoringPlugin.java @@ -111,7 +111,6 @@ public Problem checkParameters() { @Override public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); return null; } diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassesRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassesRefactoringPlugin.java index 899677264961..378eb9338db3 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassesRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/CopyClassesRefactoringPlugin.java @@ -112,7 +112,6 @@ public Problem checkParameters() { @Override public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); return null; } diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java index a1447acaffcc..a00d5b6e6c98 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package org.netbeans.modules.refactoring.java.plugins; import com.sun.source.tree.*; @@ -52,7 +51,7 @@ public class InnerToOuterTransformer extends RefactoringVisitor { private InnerToOuterRefactoring refactoring; private boolean isInInnerClass = false; private Set referencedPrivateElement; - + private Element getCurrentElement() { return workingCopy.getTrees().getElement(getCurrentPath()); } @@ -78,7 +77,7 @@ public Tree visitIdentifier(IdentifierTree node, Element p) { return null; } if (inner.equals(current)) { - Tree newTree = make.setLabel(node, refactoring.getClassName()); + Tree newTree = make.setLabel(node, refactoring.getClassName()); rewrite(node, newTree); } else if (isThisReferenceToOuter() && isThisInInner()) { if (current.getModifiers().contains(Modifier.PRIVATE)) { @@ -105,7 +104,7 @@ public Tree visitIdentifier(IdentifierTree node, Element p) { TreePath elementPath = workingCopy.getTrees().getPath(current); JavaFileObject sourceFile = elementPath != null ? elementPath.getCompilationUnit().getSourceFile() : null; if ( (parent != null && parent.getKind() == Tree.Kind.CASE && ((CaseTree) parent).getExpression() == node && current.getKind() == ElementKind.ENUM_CONSTANT) - || path.getCompilationUnit().getSourceFile() == sourceFile) { + || path.getCompilationUnit().getSourceFile() == sourceFile) { rewrite(node, make.Identifier(current.getSimpleName())); } else { rewrite(node, make.QualIdent(current)); @@ -141,7 +140,7 @@ public Tree visitNewClass(NewClassTree arg0, Element arg1) { else { thisString = "this"; // NOI18N } - + } if (thisString != null && currentElement instanceof ExecutableElement) { ExecutableElement constr = (ExecutableElement) currentElement; @@ -154,7 +153,7 @@ public Tree visitNewClass(NewClassTree arg0, Element arg1) { removeEnclosingExpression = true; } } - + int index = constr.getParameters().size(); if (constr.isVarArgs()) { index--; @@ -209,7 +208,7 @@ public Tree visitNewClass(NewClassTree arg0, Element arg1) { } return super.visitNewClass(arg0, arg1); } - + private TypeElement getOuter(TypeElement element) { while (element != null && !workingCopy.getTypes().isSubtype(element.asType(),outer.asType())) { element = workingCopy.getElementUtilities().enclosingTypeElement(element); @@ -231,7 +230,7 @@ public Tree visitMethod(MethodTree constructor, Element element) { rewrite(superCall, newSuperCall); } } - + } return super.visitMethod(constructor, element); } @@ -242,7 +241,7 @@ public Tree visitClass(ClassTree classTree, Element element) { if (currentElement == null) { return super.visitClass(classTree, element); } - GeneratorUtilities genUtils = GeneratorUtilities.get(workingCopy); // helper + GeneratorUtilities genUtils = GeneratorUtilities.get(workingCopy); // helper if (currentElement!=null && currentElement == outer) { Element outerouter = outer.getEnclosingElement(); Tree superVisit = super.visitClass(classTree, element); @@ -257,11 +256,11 @@ public Tree visitClass(ClassTree classTree, Element element) { newInnerClass = genUtils.importComments(newInnerClass, workingCopy.getCompilationUnit()); newInnerClass = make.setLabel(newInnerClass, refactoring.getClassName()); - + newInnerClass = refactorInnerClass(newInnerClass); - + TreePath outerPath = workingCopy.getTrees().getPath(outer); - + if (outerouter.getKind() == ElementKind.PACKAGE) { FileObject sourceRoot=ClassPath.getClassPath(workingCopy.getFileObject(), ClassPath.SOURCE).findOwnerRoot(workingCopy.getFileObject()); ClassTree outerTree = (ClassTree) workingCopy.getTrees().getTree(outer); @@ -290,53 +289,53 @@ public Tree visitClass(ClassTree classTree, Element element) { rewrite(outerouterTree, newOuterOuter); return newOuterOuter; } - + } else if (refactoring.getReferenceName() != null && currentElement!=null && workingCopy.getTypes().isSubtype(currentElement.asType(), inner.asType()) && currentElement!=inner) { - VariableTree variable = make.Variable(make.Modifiers(Collections.emptySet()), refactoring.getReferenceName(), make.Type(outer.asType()), null); + VariableTree variable = make.Variable(make.Modifiers(Collections.emptySet()), refactoring.getReferenceName(), make.Type(outer.asType()), null); for (Tree member:classTree.getMembers()) { - if (member.getKind() == Tree.Kind.METHOD) { - MethodTree m = (MethodTree) member; + if (member.getKind() == Tree.Kind.METHOD) { + MethodTree m = (MethodTree) member; if (m.getReturnType()==null) { - + for( VariableTree var: m.getParameters() ) { if( var.getName().contentEquals(refactoring.getReferenceName()) ) { - problem = MoveTransformer.createProblem(problem, true, NbBundle.getMessage(InnerToOuterTransformer.class, "ERR_InnerToOuter_OuterNameClashSubtype", refactoring.getReferenceName(), refactoring.getClassName(), currentElement.getSimpleName())); - } + problem = MoveTransformer.createProblem(problem, true, NbBundle.getMessage(InnerToOuterTransformer.class, "ERR_InnerToOuter_OuterNameClashSubtype", refactoring.getReferenceName(), refactoring.getClassName(), currentElement.getSimpleName())); } + } - MethodInvocationTree superCall = (MethodInvocationTree) ((ExpressionStatementTree) m.getBody().getStatements().get(0)).getExpression(); - List newArgs = new ArrayList(superCall.getArguments()); - - MethodTree newConstructor = null; - ExpressionTree exprTree = (ExpressionTree)make.Identifier(variable.getName().toString()); - if (hasVarArgs(m)) { - int index = m.getParameters().size() - 1; - newArgs.add(index, exprTree); - newConstructor = make.insertMethodParameter(m, index, variable); - } else { - newArgs.add(exprTree); - newConstructor = make.addMethodParameter(m, variable); - } - MethodInvocationTree method = make.MethodInvocation( - Collections.emptyList(), - make.Identifier("super"), // NOI18N - newArgs); - - BlockTree block = make.insertBlockStatement(m.getBody(), 0, make.ExpressionStatement(method)); - block = make.removeBlockStatement(block, 1); - - newConstructor = make.Constructor( - make.Modifiers(newConstructor.getModifiers().getFlags(), newConstructor.getModifiers().getAnnotations()), - newConstructor.getTypeParameters(), - newConstructor.getParameters(), - newConstructor.getThrows(), - block); - - rewrite(m, newConstructor); + MethodInvocationTree superCall = (MethodInvocationTree) ((ExpressionStatementTree) m.getBody().getStatements().get(0)).getExpression(); + List newArgs = new ArrayList(superCall.getArguments()); + + MethodTree newConstructor = null; + ExpressionTree exprTree = (ExpressionTree) make.Identifier(variable.getName().toString()); + if (hasVarArgs(m)) { + int index = m.getParameters().size() - 1; + newArgs.add(index, exprTree); + newConstructor = make.insertMethodParameter(m, index, variable); + } else { + newArgs.add(exprTree); + newConstructor = make.addMethodParameter(m, variable); } + MethodInvocationTree method = make.MethodInvocation( + Collections.emptyList(), + make.Identifier("super"), // NOI18N + newArgs); + + BlockTree block = make.insertBlockStatement(m.getBody(), 0, make.ExpressionStatement(method)); + block = make.removeBlockStatement(block, 1); + + newConstructor = make.Constructor( + make.Modifiers(newConstructor.getModifiers().getFlags(), newConstructor.getModifiers().getAnnotations()), + newConstructor.getTypeParameters(), + newConstructor.getParameters(), + newConstructor.getThrows(), + block); + + rewrite(m, newConstructor); } - } + } } + } if (currentElement == inner) { try { @@ -346,7 +345,7 @@ public Tree visitClass(ClassTree classTree, Element element) { isInInnerClass = false; } } - + return super.visitClass(classTree, element); } @@ -359,7 +358,7 @@ public Tree visitCompilationUnit(CompilationUnitTree node, Element p) { for (Element privEl : this.referencedPrivateElement) { problem = MoveTransformer.createProblem(problem, false, NbBundle.getMessage(InnerToOuterRefactoringPlugin.class, "WRN_InnerToOuterRefToPrivate", privEl)); } - + Trees trees = workingCopy.getTrees(); CompilationUnitTree newNode = node; for (ImportTree imp : node.getImports()) { @@ -376,7 +375,7 @@ public Tree visitCompilationUnit(CompilationUnitTree node, Element p) { } return result; } - + private Problem problem; public Problem getProblem() { @@ -391,7 +390,7 @@ private boolean containsImport(String imp) { } return false; } - + @Override public Tree visitMemberSelect(MemberSelectTree memberSelect, Element element) { @@ -456,9 +455,9 @@ public Tree visitMemberSelect(MemberSelectTree memberSelect, Element element) { rewrite(variable.getModifiers(), make.removeModifiersModifier(variable.getModifiers(), Modifier.PRIVATE)); } } - + } - + return super.visitMemberSelect(memberSelect, element); } @@ -495,7 +494,7 @@ private boolean isThisReferenceToInner() { TypeElement encl = workingCopy.getElementUtilities().enclosingTypeElement(cur); return encl!=null && workingCopy.getTypes().isSubtype(encl.asType(), inner.asType()) ; } - + private boolean isThisReferenceToOuter() { Element cur = getCurrentElement(); if (cur==null || cur.getKind() == ElementKind.PACKAGE) { @@ -542,7 +541,7 @@ private boolean isIn(Element el) { } return false; } - + private boolean hasVarArgs(MethodTree mt) { List list = mt.getParameters(); if (list.isEmpty()) { @@ -551,8 +550,11 @@ private boolean hasVarArgs(MethodTree mt) { VariableTree vt = (VariableTree)list.get(list.size() - 1); return vt.toString().indexOf("...") != -1; // [NOI18N] [TODO] temporal hack, will be rewritten } - + private ClassTree refactorInnerClass(ClassTree innerClass) { + if (innerClass.getKind() == Tree.Kind.RECORD) { + return refactorInnerRecord(innerClass); + } ClassTree newInnerClass = innerClass; String referenceName = refactoring.getReferenceName(); GeneratorUtilities genUtils = GeneratorUtilities.get(workingCopy); @@ -568,11 +570,13 @@ private ClassTree refactorInnerClass(ClassTree innerClass) { } else { outerType = outer.asType(); } + // for none static inner classes, add a member to point to the 'old' outer. + // because there has to be an explicit member after refactoring. if (referenceName != null) { VariableTree variable = make.Variable(make.Modifiers(EnumSet.of(Modifier.PRIVATE, Modifier.FINAL)), refactoring.getReferenceName(), make.Type(outerType), null); newInnerClass = genUtils.insertClassMember(newInnerClass, variable); } - + ModifiersTree modifiersTree = newInnerClass.getModifiers(); ModifiersTree newModifiersTree = make.removeModifiersModifier(modifiersTree, Modifier.PRIVATE); newModifiersTree = make.removeModifiersModifier(newModifiersTree, Modifier.STATIC); @@ -582,16 +586,17 @@ private ClassTree refactorInnerClass(ClassTree innerClass) { } rewrite(modifiersTree, newModifiersTree); + // create constructor which refers with member to old outer. if (referenceName != null) { for (Tree member:newInnerClass.getMembers()) { if (member.getKind() == Tree.Kind.METHOD) { - MethodTree m = (MethodTree) member; - if (m.getName().contentEquals("") || m.getReturnType() == null) { - VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), refactoring.getReferenceName(), make.Type(outerType), null); - MethodTree newConstructor = hasVarArgs(m) ? - make.insertMethodParameter(m, m.getParameters().size() - 1, parameter) : - make.addMethodParameter(m, parameter); - + MethodTree ctor = (MethodTree) member; + if (ctor.getName().contentEquals("") || ctor.getReturnType() == null) { + VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), referenceName, make.Type(outerType), null); + MethodTree newConstructor = hasVarArgs(ctor) ? + make.insertMethodParameter(ctor, ctor.getParameters().size() - 1, parameter) : + make.addMethodParameter(ctor, parameter); + AssignmentTree assign = make.Assignment(make.Identifier("this."+referenceName), make.Identifier(referenceName)); // NOI18N BlockTree block = make.insertBlockStatement(newConstructor.getBody(), 1, make.ExpressionStatement(assign)); Set modifiers = EnumSet.noneOf(Modifier.class); @@ -599,20 +604,20 @@ private ClassTree refactorInnerClass(ClassTree innerClass) { modifiers.remove(Modifier.PRIVATE); newConstructor = make.Constructor( make.Modifiers(modifiers,newConstructor.getModifiers().getAnnotations()), - newConstructor.getTypeParameters(), + newConstructor.getTypeParameters(), newConstructor.getParameters(), newConstructor.getThrows(), block); - newInnerClass = make.removeClassMember(newInnerClass, m); - genUtils.copyComments(m, newConstructor, true); - genUtils.copyComments(m, newConstructor, false); + newInnerClass = make.removeClassMember(newInnerClass, ctor); + genUtils.copyComments(ctor, newConstructor, true); + genUtils.copyComments(ctor, newConstructor, false); newInnerClass = genUtils.insertClassMember(newInnerClass, newConstructor); } } } } - + if(innerClass != newInnerClass) { genUtils.copyComments(innerClass, newInnerClass, true); genUtils.copyComments(innerClass, newInnerClass, false); @@ -620,6 +625,41 @@ private ClassTree refactorInnerClass(ClassTree innerClass) { return newInnerClass; } + private ClassTree refactorInnerRecord(ClassTree innerRecord) { + refactoring.setInnerIsRecord(true); + ClassTree newInnerRecord = innerRecord; + List members = newInnerRecord.getMembers(); + int insertAt=0; + for (Tree member : members) { + Tree.Kind kind = member.getKind(); +// System.out.println("member = " + kind + "" + member); + + switch (kind) { + // The fields, except the static, should loose private final modifiers + // so that they appear without these modifiers in the record header + case VARIABLE: + VariableTree vt = (VariableTree) member; + ModifiersTree mt = vt.getModifiers(); + if (mt.getFlags().contains(Modifier.STATIC)) { + continue; + } + var mtype = vt.getType(); + var mName = vt.getName(); + var mods = make.Modifiers(EnumSet.noneOf(Modifier.class)); + var newMember = make.RecordComponent(mods, mName, mtype); + newInnerRecord = make.removeClassMember(newInnerRecord, member); + newInnerRecord = make.insertClassMember(newInnerRecord, insertAt, newMember); + insertAt++; + + break; + default: + } + +// if (member.getKind()) + } + return newInnerRecord; + } + private boolean isThisInInner() { TreePath t=getCurrentPath(); Tree innerTree = workingCopy.getTrees().getTree(inner); diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaPluginUtils.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaPluginUtils.java index 5377f15eb677..a0eea7f238b1 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaPluginUtils.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaPluginUtils.java @@ -523,8 +523,11 @@ private static String guessLiteralName(String str) { public static CompilationUnitTree createCompilationUnit(FileObject sourceRoot, String relativePath, Tree typeDecl, WorkingCopy workingCopy, TreeMaker make) { GeneratorUtilities genUtils = GeneratorUtilities.get(workingCopy); CompilationUnitTree newCompilation; + ElementKind clzOrRecord = ElementKind.CLASS; + if (typeDecl.getKind()== Kind.RECORD) + clzOrRecord=ElementKind.RECORD; try { - newCompilation = genUtils.createFromTemplate(sourceRoot, relativePath, ElementKind.CLASS); + newCompilation = genUtils.createFromTemplate(sourceRoot, relativePath, clzOrRecord);//ElementKind.CLASS); List typeDecls = newCompilation.getTypeDecls(); if (typeDecls.isEmpty()) { newCompilation = make.addCompUnitTypeDecl(newCompilation, typeDecl); diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaWhereUsedQueryPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaWhereUsedQueryPlugin.java index 8ea3afd99508..6f563d0d0b4b 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaWhereUsedQueryPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/JavaWhereUsedQueryPlugin.java @@ -90,7 +90,6 @@ protected JavaSource getJavaSource(Phase p) { @Override public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); TreePathHandle handle = refactoring.getRefactoringSource().lookup(TreePathHandle.class); if (!handle.getFileObject().isValid()) { diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/MoveFileRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/MoveFileRefactoringPlugin.java index faf47912afdb..2b802349264a 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/MoveFileRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/MoveFileRefactoringPlugin.java @@ -115,7 +115,6 @@ private MoveFileRefactoringPlugin(AbstractRefactoring refactoring, boolean isRen @Override public Problem preCheck() { - cancelRequest =false; cancelRequested.set(false); Problem preCheckProblem = null; for (FileObject file:filesToMove) { diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/SafeDeleteRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/SafeDeleteRefactoringPlugin.java index 345dad4f94d7..0bda6041ca06 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/SafeDeleteRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/SafeDeleteRefactoringPlugin.java @@ -199,7 +199,6 @@ public Problem fastCheckParameters() { @Override @NbBundle.Messages({"# {0} - VariableName", "ERR_VarNotInBlockOrMethod=Variable \"{0}\" is not inside a block or method declaration."}) public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); final Problem[] problem = new Problem[1]; Collection handles = refactoring.getRefactoringSource().lookupAll(TreePathHandle.class); diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/UseSuperTypeRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/UseSuperTypeRefactoringPlugin.java index 8a455f2538c7..e4c2475f91c0 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/UseSuperTypeRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/UseSuperTypeRefactoringPlugin.java @@ -100,7 +100,6 @@ protected JavaSource getJavaSource(Phase p) { */ @Override public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); // Element subType = refactoring.getTypeElement(); // if(!(subType instanceof JavaClass)){ diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/DiffElement.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/DiffElement.java index d8dd797c1057..f4ef358ffab3 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/DiffElement.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/DiffElement.java @@ -34,7 +34,7 @@ import org.openide.util.lookup.Lookups; /** - * Implementatation of RefactoringElementImplementation specific to refactoring + * Implementation of RefactoringElementImplementation specific to refactoring * in java files. * * @author Jan Becicka diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/JavaRefactoringPlugin.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/JavaRefactoringPlugin.java index 99555656184d..f21d0d87b1b6 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/JavaRefactoringPlugin.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/JavaRefactoringPlugin.java @@ -57,12 +57,6 @@ public abstract class JavaRefactoringPlugin extends ProgressProviderAdapter implements RefactoringPlugin { protected enum Phase {PRECHECK, FASTCHECKPARAMETERS, CHECKPARAMETERS, PREPARE}; - /** - * Use cancelRequested - * @deprecated - */ - @Deprecated - protected volatile boolean cancelRequest = false; /** * true if cancel was requested @@ -112,7 +106,6 @@ protected Problem fastCheckParameters(CompilationController javac) throws IOExce @Override public Problem preCheck() { - cancelRequest = false; cancelRequested.set(false); return workingTask.run(Phase.PRECHECK); } @@ -149,7 +142,6 @@ public Problem fastCheckParameters() { @Override public void cancelRequest() { - cancelRequest = true; cancelRequested.set(true); if (currentTask!=null) { currentTask.cancel(); diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/RefactoringVisitor.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/RefactoringVisitor.java index 357fb4b64047..844f77025c9e 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/RefactoringVisitor.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/spi/RefactoringVisitor.java @@ -183,6 +183,7 @@ public Tree scan(Tree tree, Element p) { case CLASS: case ENUM: case INTERFACE: + case RECORD: case VARIABLE: TreePath path = new TreePath(currentPath, tree); scanJavadoc(path, p); @@ -456,6 +457,7 @@ public DocTree visitVersion(VersionTree node, Element p) { return docScanner.visitVersion(node, p, null); } + /** * @since 1.47 */ diff --git a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/ui/UIUtilities.java b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/ui/UIUtilities.java index 6a0bbbaf59f8..8680f4bfb14e 100644 --- a/java/refactoring.java/src/org/netbeans/modules/refactoring/java/ui/UIUtilities.java +++ b/java/refactoring.java/src/org/netbeans/modules/refactoring/java/ui/UIUtilities.java @@ -65,7 +65,7 @@ public final class UIUtilities { */ public static final EnumSet allowedElementKinds = EnumSet.of(ElementKind.PACKAGE, ElementKind.CLASS, ElementKind.INTERFACE, ElementKind.RECORD, ElementKind.ENUM, ElementKind.ANNOTATION_TYPE, ElementKind.METHOD, ElementKind.CONSTRUCTOR, ElementKind.INSTANCE_INIT, - ElementKind.STATIC_INIT, ElementKind.FIELD, ElementKind.ENUM_CONSTANT, ElementKind.TYPE_PARAMETER); + ElementKind.STATIC_INIT, ElementKind.FIELD, ElementKind.ENUM_CONSTANT, ElementKind.TYPE_PARAMETER, ElementKind.RECORD_COMPONENT); // not to be instantiated private UIUtilities() { diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index f734f74f5630..f425a595cd55 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -23,16 +23,13 @@ import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; import java.io.IOException; -import java.io.PrintStream; import java.util.Arrays; import java.util.LinkedList; import java.util.List; -import java.util.Properties; -import java.util.concurrent.atomic.AtomicInteger; import javax.lang.model.SourceVersion; import javax.lang.model.element.Name; -import javax.tools.JavaCompiler; -import javax.tools.ToolProvider; +import org.junit.FixMethodOrder; +import org.junit.runners.MethodSorters; import org.netbeans.api.java.source.CompilationController; import org.netbeans.api.java.source.JavaSource; import org.netbeans.api.java.source.Task; @@ -41,29 +38,34 @@ import org.netbeans.modules.refactoring.api.RefactoringSession; import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring; import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems; -import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.writeFilesAndWaitForScan; import org.openide.util.Exceptions; /** * Test inner to outer refactoring for test. + * + * In the input files, and the expected outcomes, the indentation + * does not really matter as far as the tests are concerned because + * the indentation is stripped away before the remaining source lines + * are compared to the expected lines. * * @author homberghp {@code } */ +@FixMethodOrder(MethodSorters.NAME_ASCENDING) public class InnerOuterRecordTest extends RefactoringTestBase { - + public InnerOuterRecordTest(String name) { super(name); //ensure we are running on at least 16. try { SourceVersion.valueOf("RELEASE_16"); //NOI18N } catch (IllegalArgumentException ex) { - //OK, no RELEASE_17, skip test - return; + //OK, no RELEASE_16, skip test + throw new RuntimeException("need at least Java 16 for record"); } } // for reference - public void test259004() throws Exception { + public void test0_259004() throws Exception { String source = """ package t; @@ -108,10 +110,10 @@ public final class F { } """; innerOuterSetupAndTest(source, newOuter, newInner); - + } - - public void testApacheNetbeans7044() throws Exception { + + public void test9ApacheNetbeans7044() throws Exception { // initial outer has record with meaningful canonical constructor. // note that Inner class should be in last member for assumptions in the test. String source @@ -186,11 +188,11 @@ record F(int id, String name, LocalDate dob) { } } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - - public void testBasicClassInClass() throws Exception { + + public void test1BasicClassInClass() throws Exception { // initial outer has record with meaningful canonical constructor. String source = """ @@ -225,8 +227,7 @@ public Student(int id, String name, LocalDate dob) { } """; String newOuter - = - """ + = """ package t; import java.time.LocalDate; @@ -242,8 +243,8 @@ void useStudent() { } """; - String newInner = - """ + String newInner + = """ /* * Refactoring License */ @@ -276,24 +277,27 @@ public F(int id, String name, LocalDate dob) { } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - - public void testBasicRecordInRecord() throws Exception { - String source = - """ + + public void test2BasicRecordInRecord() throws Exception { + String source + = """ package t; import java.time.LocalDate; record A(int id, String name, LocalDate dob) { static F f; - record F(int x, int y){} + record F(int x, int y){ + /** I should be back. */ + static String code = "nix"; + } } """; - String newOuter= - """ + String newOuter + = """ package t; import java.time.LocalDate; @@ -304,8 +308,8 @@ record A(int id, String name, LocalDate dob) { } """; - String newInner = - """ + String newInner + = """ /* * Refactoring License */ @@ -313,19 +317,29 @@ record A(int id, String name, LocalDate dob) { /** * - * @author junit + * @author hom */ - record F(int x, int y){} + record F(int x, int y) { + /** I should be back. */ + static String code = "nix"; + } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - - public void testOuterWithCompact() throws Exception { + + /** + * Test to verify what happens to the compact constructor in the outer + * record. It appears to survive the refactoring. + * + * @throws Exception + */ + public void test3OuterWithCompact() throws Exception { String source = """ package t; import java.time.LocalDate; + /** Record with compact ctor. */ record A(F f){ public A{ @@ -338,10 +352,11 @@ record F(int id, String name, LocalDate dob){} = """ package t; import java.time.LocalDate; + /** Record with compact ctor. */ record A(F f){ - public A{ + public A{ assert f!=null; - } + } } """; String newInner @@ -355,12 +370,13 @@ record A(F f){ * * @author junit */ - record F(int id, String name, LocalDate dob){} + record F(int id, String name, LocalDate dob) { + } """; innerOuterSetupAndTest(source, newOuter, newInner); } - - public void testInnerWithCompact() throws Exception { + + public void test4InnerWithCompact() throws Exception { String source = """ package t; @@ -375,7 +391,7 @@ record A(F f) { record F(int id, String name, LocalDate dob) { - public F { + public F { if (dob.isBefore(LocalDate.EPOCH)) { throw new IllegalArgumentException("to old " + dob); } @@ -415,9 +431,9 @@ record F(int id, String name, LocalDate dob) { """; innerOuterSetupAndTest(source, newOuter, newInner); } - + // outer may have effect - public void testClassWithInnerRecord() throws Exception { + public void test5ClassWithInnerRecord() throws Exception { String source = """ package t; @@ -475,20 +491,20 @@ record F(int id, String name, LocalDate dob) { """; innerOuterSetupAndTest(source, newOuter, newInner); } - - void innerOuterSetupAndTest(String source, String newOuter, String newInner) throws Exception { + + void innerOuterSetupAndTest(String source, String newOuterName, String newInnerName) throws Exception { writeFilesNoIndexing(src, new File("t/A.java", source)); performInnerToOuterTest2(null); - verifyContent(src, new File("t/A.java", newOuter), new File("t/F.java", newInner)); + verifyContent(src, new File("t/A.java", newOuterName), new File("t/F.java", newInnerName)); } - + boolean debug = false; // variant for record inner to outer test - private void performInnerToOuterTest2(String generateOuter, Problem... expectedProblems) throws Exception { + private void performInnerToOuterTest2(String newOuterName, Problem... expectedProblems) throws Exception { final InnerToOuterRefactoring[] r = new InnerToOuterRefactoring[1]; JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { - + @Override public void run(CompilationController parameter) { try { @@ -500,7 +516,7 @@ public void run(CompilationController parameter) { if (debug) { System.err.println("cut is of type " + cut.getClass().getCanonicalName()); } - ClassTree outer = (ClassTree)cut.getTypeDecls().get(0); + ClassTree outer = (ClassTree) cut.getTypeDecls().get(0); if (debug) { printNumbered(System.err, "start source " + outer.getKind().toString(), outer.toString()); } @@ -535,21 +551,21 @@ public void run(CompilationController parameter) { } } }, true); - + r[0].setClassName("F"); if (debug) { printNumbered(System.err, "result ", r[0].toString()); } - r[0].setReferenceName(generateOuter); - + r[0].setReferenceName(newOuterName); + RefactoringSession rs = RefactoringSession.create("Session"); List problems = new LinkedList(); - + addAllProblems(problems, r[0].preCheck()); addAllProblems(problems, r[0].prepare(rs)); addAllProblems(problems, rs.doRefactoring(true)); - + assertProblems(Arrays.asList(expectedProblems), problems); } - + } diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerToOutterTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerToOutterTest.java index 0376b394200b..08b61677fcef 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerToOutterTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerToOutterTest.java @@ -657,7 +657,7 @@ public void testNETBEANS345() throws Exception { "} ")); } - private void performInnerToOuterTest(String generateOuter, Problem... expectedProblems) throws Exception { + private void performInnerToOuterTest(String outerNameInInner, Problem... expectedProblems) throws Exception { final InnerToOuterRefactoring[] r = new InnerToOuterRefactoring[1]; JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { @@ -676,7 +676,7 @@ public void run(CompilationController parameter) throws Exception { }, true); r[0].setClassName("F"); - r[0].setReferenceName(generateOuter); + r[0].setReferenceName(outerNameInInner); RefactoringSession rs = RefactoringSession.create("Session"); List problems = new LinkedList(); diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java index 947a75962e00..bf0cb73acdd1 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java @@ -86,8 +86,37 @@ public RefactoringTestBase(String name, String sourcelevel) { this.sourcelevel = sourcelevel; } - static boolean debug=false; + static boolean debug = false; +// static boolean skipIndexing = false; + + /** + * Write given files to sourceRoot and fully re index. + * + * First the (file) children of sourceRoot are deleted. This method can take + * a substantial time of your patience, so use wisely. See the doc in + * {@link IndexManager#refreshIndexAndWait} + * + * @param sourceRoot sic + * @param files to save + * @throws Exception whenever + */ protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... files) throws Exception { + writeFilesAndWaitForScan(true, sourceRoot, files); + } + + /** + * Write given files to sourceRoot and possibly reindex. + * + * First the (file) children of sourceRoot are deleted. This method can take + * a substantial time of your patience, so use wisely. See the doc in + * {@link IndexManager#refreshIndexAndWait} + * + * @param fulleIndex fully reindex the type repo + * @param sourceRoot sic + * @param files to save + * @throws Exception whenever + */ + protected static void writeFilesAndWaitForScan(boolean fullIndex, FileObject sourceRoot, File... files) throws Exception { long currentTimeMillis = System.currentTimeMillis(); for (FileObject c : sourceRoot.getChildren()) { c.delete(); @@ -97,28 +126,35 @@ protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... fi FileObject fo = FileUtil.createData(sourceRoot, f.filename); TestUtilities.copyStringToFile(fo, f.content); } - - IndexingManager.getDefault().refreshIndexAndWait(sourceRoot.toURL(), null, true); - long currentTimeMillis1 = System.currentTimeMillis(); - if (debug) { - System.err.println("writeFilesAndWaitForScan took " + (currentTimeMillis1 - currentTimeMillis) + " millis"); + + if (fullIndex) { + IndexingManager.getDefault().refreshIndexAndWait(sourceRoot.toURL(), null, true); + long currentTimeMillis1 = System.currentTimeMillis(); + if (debug) { + System.err.println("writeFilesAndWaitForScan took " + (currentTimeMillis1 - currentTimeMillis) + " millis"); + } } } + /** + * Save file but do not reindex. + * + * Deletes the existing files under sourceRoot, then saves the given files. + * + * Makes tests run faster. In particular single tests. + * + * @param sourceRoot sic + * @param files to save + * @throws Exception whenever + */ protected static void writeFilesNoIndexing(FileObject sourceRoot, File... files) throws Exception { - for (FileObject c : sourceRoot.getChildren()) { - c.delete(); - } - - for (File f : files) { - FileObject fo = FileUtil.createData(sourceRoot, f.filename); - TestUtilities.copyStringToFile(fo, f.content); - } + writeFilesAndWaitForScan(false, sourceRoot, files); } /** - * Verify that the given file(names) are present in the sourceRoot and that the files - * in said sourceRoot are equal to the given files. + * Verify that the given file(names) are present in the sourceRoot and that + * the files in said sourceRoot are equal to the given files. + * * @param sourceRoot to contain generated (refactored) files * @param files expected files * @throws Exception well why not? @@ -151,7 +187,7 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except assertNotNull(f.content); assertNotNull("Cannot find expected " + f.filename + " in map filled by sourceRoot " + content, fileContent); if (fullLineCompare) { - assertLinesEqual2(f.filename,f.content, fileContent); + assertLinesEqual2(f.filename, f.content, fileContent); } else { // original tests. assertLinesEqual1(f.content, fileContent); } @@ -166,7 +202,7 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except * @return the contents of the file(s). */ private static String copyFileToString(java.io.File f) throws java.io.IOException { - int s = (int)f.length(); + int s = (int) f.length(); byte[] data = new byte[s]; int len = new FileInputStream(f).read(data); if (len != s) { @@ -448,21 +484,24 @@ protected void runTest() throws Throwable { } /** - * Prints a source by splitting on the line breaks and prefixing with name and line number. - * + * Prints a source by splitting on the line breaks and prefixing with name + * and line number. + * * @param out the stream to print to - * @param name the name as prefix to each line + * @param name the name as prefix to each line * @param source the source code to print to the out stream. */ - public void printNumbered(final PrintStream out, final String name, String source) { + public static void printNumbered(final PrintStream out, final String name, String source) { AtomicInteger c = new AtomicInteger(1); source.trim().lines().forEach(l -> out.println("%s [%4d] %s".formatted(name, c.getAndIncrement(), l))); } /** - * Compare strings by replacing all multiples of whitrespace([ \t\n\r]) with a space. - * - * The test programmer choose this to make it easier to write the input and the expected strings. + * Compare strings by replacing all multiples of whitrespace([ \t\n\r]) with + * a space. + * + * The test programmer chooses this to make it easier to write the input and + * the expected strings. * * @param expected to compare * @param actual to compare @@ -480,37 +519,44 @@ public void assertLinesEqual1(String expected, String actual) { } /** - * Compare strings by splitting them into lines, and remove trailing white + * Compare strings by splitting them into lines, remove empty lines, and trim white * space. * * Only when any of the lines differ, all lines are printed with the unequal * lines flagged. * * Before the lines are compared, they are trimmed and the white space is - * normalized by collapsing multiple whitespace characters into one. This + * normalized by collapsing multiple white space characters into one. This * should make the tests less brittle. + * + * If any of the compared lines are unequal, this test fails and the + * comparison result is shown on stderr in a simplified diff format. * * @param expected to compare * @param actual to compare */ - public void assertLinesEqual2(String name,String expected, String actual) { + public void assertLinesEqual2(String name, String expected, String actual) { expected = expected.trim().replaceAll("([ \t\r\n])\\1+", "$1"); actual = actual.trim().replaceAll("([ \t\r\n])\\1+", "$1"); - String[] linesExpected = expected.lines().toArray(String[]::new); - String[] linesActual = actual.lines().toArray(String[]::new); + String[] linesExpected = expected.lines().filter(l-> !l.isEmpty()).toArray(String[]::new); + String[] linesActual = actual.lines().filter(l-> !l.isEmpty()).toArray(String[]::new); int limit = Math.max(linesExpected.length, linesActual.length); StringBuilder sb = new StringBuilder(); boolean equals = true; for (int i = 0; i < limit; i++) { String e = (i < linesExpected.length ? linesExpected[i] : "").trim(); String a = (i < linesActual.length ? linesActual[i] : "").trim(); + // somehow my user is inserted, so avoid to test those lines. + if (e.contains("@author") && a.contains("@author")){ + e=a="* @author goes here"; + } boolean same = e.equals(a); String sep = same ? " " : " | "; equals &= same; - sb.append(String.format(name+" [%3d] %-80s%s%-80s%n", i, e, sep, a)); + sb.append(String.format(name + " [%3d] %-80s%s%-80s%n", i, e, sep, a)); } if (!equals) { - System.err.println("test "+getName()+" failed"); + System.err.println("test " + getName() + " failed"); System.err.println(sb.toString()); fail("lines differ, see stderr for more details."); } From 364d6bb3e806ff46203c2ed20fa1b67f01ea389d Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Wed, 15 Jan 2025 20:13:33 +0100 Subject: [PATCH 18/19] solves #7044 The refactoring of inner record to outer now works as expected. The implementation tries to detect candidate 'compact constructor' even if the original constructor declaration was not. I consider that acceptable. --- .../java/source/pretty/VeryPretty.java | 229 +++++++++++------- .../java/test/InnerOuterRecordTest.java | 169 ++++++++++--- 2 files changed, 273 insertions(+), 125 deletions(-) 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 559d428ebb64..36549a55494a 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 @@ -104,6 +104,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; +import javax.lang.model.element.Modifier; import org.netbeans.api.java.classpath.ClassPath; import org.netbeans.api.java.lexer.JavaTokenId; @@ -231,7 +232,7 @@ public void reset(int margin, int col) { public int getIndent() { return out.leftMargin; } - + public void setIndent(int indent) { out.leftMargin = indent; } @@ -270,14 +271,14 @@ public final void print(String s) { if (s == null) return; out.append(s); - } + } public final void print(Name n) { if (n == null) return; out.append(n.toString()); } - + private void print(javax.lang.model.element.Name n) { if (n == null) return; @@ -291,11 +292,11 @@ public void print(JCTree t) { doAccept(t, true); blankLines(t, false); } - + public void print(DCTree t) { print(t, false); } - + public void print(DCTree t, boolean noMarginAfter) { if (t == null) return; blankLines(t, true, false); @@ -303,7 +304,7 @@ public void print(DCTree t, boolean noMarginAfter) { doAccept(t); blankLines(t, false, noMarginAfter); } - + private Map overrideStartPositions; private int getOldPos(JCTree oldT) { @@ -318,7 +319,7 @@ private int getOldPos(JCTree oldT) { public int endPos(JCTree t) { return TreeInfo.getEndPos(t, diffContext.origUnit.endPositions); } - + private java.util.List getStatements(Tree tree) { switch (tree.getKind()) { case BLOCK: return ((BlockTree) tree).getStatements(); @@ -326,31 +327,31 @@ private java.util.List getStatements(Tree tree) { default: return null; } } - + private java.util.List printOriginalPartOfFieldGroup(FieldGroupTree fgt) { java.util.List variables = fgt.getVariables(); TreePath tp = TreePath.getPath(diffContext.origUnit, variables.get(0)); TreePath parent = tp != null ? tp.getParentPath() : null; - + if (parent == null) return variables; - + java.util.List statements = getStatements(parent.getLeaf()); - + if (statements == null) return variables; JCVariableDecl firstDecl = fgt.getVariables().get(0); int startIndex = statements.indexOf(firstDecl); - + if (startIndex < 0) return variables; //XXX: should not happen - + int origCount = 0; int s = statements.size(); for (JCTree t : variables) { if (startIndex >= s || statements.get(startIndex++) != t) break; origCount++; } - + if (origCount < 2) return variables; - + int firstPos = getOldPos(firstDecl); int groupStart = startIndex; // find the start of the field group among the original statement. The fieldgroup is detected using @@ -381,7 +382,7 @@ private java.util.List printOriginalPartOfFieldGroup(FieldGroupT overrideStartPositions = null; return variables.subList(origCount, variables.size()); } - + public Set oldTrees = Collections.emptySet(); public SortedSet reindentRegions = new TreeSet<>(new Comparator() { @Override public int compare(int[] o1, int[] o2) { @@ -394,7 +395,7 @@ private java.util.List printOriginalPartOfFieldGroup(FieldGroupT private void doAccept(JCTree t, boolean printComments/*XXX: should ideally always print comments?*/) { if (!handlePossibleOldTrees(Collections.singletonList(t), printComments)) { if (printComments) printPrecedingComments(t, true); - + int start = out.length(); if (t instanceof FieldGroupTree) { @@ -404,7 +405,7 @@ private void doAccept(JCTree t, boolean printComments/*XXX: should ideally alway printEnumConstants(List.from(fgt.getVariables().toArray(new JCTree[0])), !fgt.isEnum() || fgt.moreElementsFollowEnum(), printComments); } else { java.util.List remainder = printOriginalPartOfFieldGroup(fgt); - + //XXX: this will unroll the field group (see FieldGroupTest.testMove187766) //XXX: copied from visitClassDef boolean firstMember = remainder.size() == fgt.getVariables().size(); @@ -430,14 +431,14 @@ private void doAccept(JCTree t, boolean printComments/*XXX: should ideally alway if (tag != null) { tag2Span.put(tag, new int[]{start + initialOffset, end + initialOffset}); } - + if (printComments) { printInnerCommentsAsTrailing(t, true); printTrailingComments(t, true); } } } - + private void doAccept(DCTree t) { // int start = toString().length(); @@ -465,36 +466,36 @@ public boolean handlePossibleOldTrees(java.util.List toPrint, CommentSet cs = commentHandler.getComments(t); if (cs.hasChanges()) return false; } - + if (toPrint.size() > 1) { //verify that all the toPrint trees belong to the same parent, and appear //in the same uninterrupted order under that parent: TreePath tp = TreePath.getPath(diffContext.mainUnit, toPrint.get(0)); TreePath parent = tp.getParentPath(); - + if (parent == null) return false; //XXX: should not happen, right? - + java.util.List statements = getStatements(parent.getLeaf()); - + if (statements == null) return false; - + int startIndex = statements.indexOf(toPrint.get(0)); - + if (startIndex < 0) return false; //XXX: should not happen - + for (JCTree t : toPrint) { if (statements.get(startIndex++) != t) return false; } } - + doPrintOriginalTree(toPrint, includeComments); - + return true; } - + private void doPrintOriginalTree(java.util.List toPrint, final boolean includeComments) { if (out.isWhitespaceLine()) toLeftMargin(); - + JCTree firstTree = toPrint.get(0); JCTree lastTree = toPrint.get(toPrint.size() - 1); @@ -507,7 +508,7 @@ private void doPrintOriginalTree(java.util.List toPrint, final } else { realStart = getOldPos(firstTree); } - + final int newStart = out.length() + initialOffset; final int[] realEnd = {endPos(lastTree)}; @@ -520,7 +521,7 @@ public Void scan(Tree node, Void p) { realEnd[0] = Math.max(realEnd[0], Math.max(CasualDiff.commentEnd(old, CommentSet.RelativePosition.INLINE), CasualDiff.commentEnd(old, CommentSet.RelativePosition.TRAILING))); trailingCommentsHandled.add(node); } - + Object tag = tree2Tag != null ? tree2Tag.get(node) : null; if (tag != null) { @@ -528,7 +529,7 @@ public Void scan(Tree node, Void p) { int e = endPos((JCTree) node); tag2Span.put(tag, new int[]{s - realStart + newStart, e - realStart + newStart}); } - + } return super.scan(node, p); } @@ -557,10 +558,10 @@ private void copyToIndented(int from, int to) { } String text = origText.substring(from, to); - + int newLine = text.indexOf("\n") + 1; boolean wasWhitespaceLine = out.isWhitespaceLine(); - + if (newLine == 0 && !wasWhitespaceLine) { print(text); } else { @@ -575,8 +576,8 @@ private void copyToIndented(int from, int to) { /** * Adjusts {@link #reindentRegions} if the char buffer conntents is * trimmed. - * - * @param limit + * + * @param limit */ @Override public void trimmed(int limit) { @@ -595,7 +596,7 @@ public void trimmed(int limit) { } } } - + /** Print a package declaration. */ @@ -640,9 +641,17 @@ public String getMethodHeader(MethodTree t, String s) { print(tree.name); s = replace(s, NAME); } - print('('); - wrapTrees(tree.params, WrapStyle.WRAP_NEVER, out.col); - print(')'); + boolean isRecord = enclClass.getKind()== Kind.RECORD; + // return type null makes this method a constructor + boolean isCandidateCompactCtor = isRecord && null == t.getReturnType(); + if (isCandidateCompactCtor) { + System.err.println("spotted compact "+ t.toString()); + } + if (!isCandidateCompactCtor) { + print('('); + wrapTrees(tree.params, WrapStyle.WRAP_NEVER, out.col); + print(')'); + } s = replace(s, PARAMETERS); if (tree.thrown.nonEmpty()) { print(" throws "); @@ -844,7 +853,7 @@ public void visitPackageDef(JCPackageDecl tree) { printPackage(tree.pid); } } - + @Override public void visitImport(JCImport tree) { print("import "); @@ -977,6 +986,9 @@ private void printEnumConstants(java.util.List defs, boolean f @Override public void visitMethodDef(JCMethodDecl tree) { + boolean methodIsConstructor = null==tree.getReturnType(); + boolean enclosingIsRecord= enclClass.getKind() == Kind.RECORD; + boolean paramsIsComponents= paramsIsComponents(tree.getParameters()); if ((tree.mods.flags & Flags.SYNTHETIC)==0 && tree.name != names.init || enclClass != null) { @@ -995,18 +1007,9 @@ public void visitMethodDef(JCMethodDecl tree) { needSpace(); print(tree.name); } - print(cs.spaceBeforeMethodDeclParen() ? " (" : "("); - if (cs.spaceWithinMethodDeclParens() && tree.params.nonEmpty()) - print(' '); - boolean oldPrintingMethodParams = printingMethodParams; - printingMethodParams = true; - wrapTrees(tree.params, cs.wrapMethodParams(), cs.alignMultilineMethodParams() - ? out.col : out.leftMargin + cs.getContinuationIndentSize(), - true); - printingMethodParams = oldPrintingMethodParams; - if (cs.spaceWithinMethodDeclParens() && tree.params.nonEmpty()) - needSpace(); - print(')'); + if (!(methodIsConstructor && enclosingIsRecord && paramsIsComponents)){ + printMethodParams(tree); + } if (tree.thrown.nonEmpty()) { wrap("throws ", cs.wrapThrowsKeyword()); wrapTrees(tree.thrown, cs.wrapThrowsList(), cs.alignMultilineThrows() @@ -1026,6 +1029,21 @@ public void visitMethodDef(JCMethodDecl tree) { } } + void printMethodParams(JCMethodDecl tree) { + print(cs.spaceBeforeMethodDeclParen() ? " (" : "("); + if (cs.spaceWithinMethodDeclParens() && tree.params.nonEmpty()) + print(' '); + boolean oldPrintingMethodParams = printingMethodParams; + printingMethodParams = true; + wrapTrees(tree.params, cs.wrapMethodParams(), cs.alignMultilineMethodParams() + ? out.col : out.leftMargin + cs.getContinuationIndentSize(), + true); + printingMethodParams = oldPrintingMethodParams; + if (cs.spaceWithinMethodDeclParens() && tree.params.nonEmpty()) + needSpace(); + print(')'); + } + @Override public void visitVarDef(JCVariableDecl tree) { boolean notEnumConst = (tree.mods.flags & Flags.ENUM) == 0; @@ -1656,7 +1674,7 @@ else if (tree.clazz.type != null) printNewClassBody(tree); } } - + public void printNewClassBody(JCNewClass tree) { JCClassDecl enclClassPrev = enclClass; enclClass = tree.def; @@ -1828,8 +1846,8 @@ public void visitTypeCast(JCTypeCast tree) { @Override public void visitTypeUnion(JCTypeUnion that) { boolean sep = cs.spaceAroundBinaryOps(); - wrapTrees(that.getTypeAlternatives(), - cs.wrapDisjunctiveCatchTypes(), + wrapTrees(that.getTypeAlternatives(), + cs.wrapDisjunctiveCatchTypes(), cs.alignMultilineDisjunctiveCatchTypes() ? out.col : out.leftMargin + cs.getContinuationIndentSize(), false, sep, sep, "|"); // NOI18N } @@ -1959,15 +1977,15 @@ private static String quote(String val, char keep, boolean charContext) { } return sb.toString(); } - + private static final String[] typeTagNames = new String[TypeTag.values().length]; - + static { for (TypeTag tt : TypeTag.values()) { typeTagNames[tt.ordinal()] = tt.name().toLowerCase(Locale.ENGLISH); } } - + /** * Workaround for defect #239258. Converts typetag names into lowercase using ENGLISH locale. */ @@ -2000,7 +2018,7 @@ public void visitAnnotatedType(JCAnnotatedType tree) { print(' '); printExpr(tree.underlyingType); } - + @Override public void visitTypeParameter(JCTypeParameter tree) { print(tree.name); @@ -2195,7 +2213,7 @@ private void blankLines(JCTree tree, boolean before) { return; } } - + private boolean isFirst(JCTree tree, List list) { for (JCTree t : list) { if (!isSynthetic(t)) { @@ -2204,7 +2222,7 @@ private boolean isFirst(JCTree tree, List list) { } return false; } - + private boolean isLast(JCTree tree, List list) { boolean b = false; for (JCTree t : list) { @@ -2214,7 +2232,7 @@ private boolean isLast(JCTree tree, List list) { } return b; } - + /** * The following tags are block-tags *
    @@ -2729,6 +2747,43 @@ public void setDocCommentKind(JavaTokenId docCommentKind) { this.docCommentKind = docCommentKind; } + /** + * Check that the given parameters are equal to the classes (record) component. + * + * return true iff parameters and components are of same length, in same name and type order + * @param parameters of the method + * @return if the parameters equal the record components. + */ + private boolean paramsIsComponents(List parameters) { + record TypeInfo(String name, String type){} + TypeInfo[] params = parameters.stream() + .map(p -> new TypeInfo(p.getName().toString(), p.getType().toString())) + .toArray(TypeInfo[]::new); + TypeInfo[] components = enclClass.getMembers().stream() + .filter(VeryPretty::isRecordComponent) + .map(x ->(VariableTree) x) + .map(p -> new TypeInfo(p.getName().toString(), p.getType().toString())) + .toArray(TypeInfo[]::new); + // short circuit on different length. + if (params.length != components.length) return false; + return Arrays.deepEquals(params, components); + } + + /** + * A member is a normal field when not a class, (or enum ...) not a method + * and not static. For record that should be a record component then. + * @param t + * @return true if fields of the class/record are of same amount, order and types + */ + static boolean isRecordComponent(JCTree t){ + if (t.getKind()==Kind.CLASS) return false; + if (t.getKind()==Kind.METHOD) return false; + if (t.getKind()==Kind.VARIABLE && t instanceof VariableTree vt){ + if (vt.getModifiers().getFlags().contains(Modifier.STATIC)) return false; + return true; + } + return false; + } private final class Linearize extends ErrorAwareTreeScanner> { @Override public Boolean scan(Tree node, java.util.List p) { @@ -2766,12 +2821,12 @@ private void adjustSpans(Iterable original, String code) { if (tree2Tag == null) { return; //nothing to copy } - + java.util.List linearized = new LinkedList(); if (!new Linearize().scan(original, linearized) != Boolean.TRUE) { return; //nothing to copy } - + ClassPath empty = ClassPathSupport.createClassPath(new URL[0]); ClasspathInfo cpInfo = ClasspathInfo.create(JavaPlatformManager.getDefault().getDefaultPlatform().getBootstrapLibraries(), empty, empty); JavacTaskImpl javacTask = JavacParser.createJavacTask(cpInfo, null, null, null, null, null, null, null, Arrays.asList(FileObjects.memoryFileObject("", "Scratch.java", code))); @@ -2798,7 +2853,7 @@ private static String whitespace(int num) { private boolean printAnnotationsFormatted(List annotations) { if (reallyPrintAnnotations) return false; - + VeryPretty del = new VeryPretty(diffContext, cs, new HashMap(), tree2Doc, new HashMap(), origText, 0); del.reallyPrintAnnotations = true; del.printingMethodParams = printingMethodParams; @@ -2807,7 +2862,7 @@ private boolean printAnnotationsFormatted(List annotations) { String str = del.out.toString(); int col = printingMethodParams ? out.leftMargin + cs.getContinuationIndentSize() : out.col; - + str = Reformatter.reformat(str + " class A{}", cs, cs.getRightMargin() - col); str = str.trim().replace("\n", "\n" + whitespace(col)); @@ -2823,7 +2878,7 @@ private boolean printAnnotationsFormatted(List annotations) { return true; } - + private void printAnnotations(List annotations) { if (annotations.isEmpty()) return ; @@ -2831,7 +2886,7 @@ private void printAnnotations(List annotations) { toColExactly(out.leftMargin); return ; } - + while (annotations.nonEmpty()) { printNoParenExpr(annotations.head); if (annotations.tail != null && annotations.tail.nonEmpty()) { @@ -2889,19 +2944,19 @@ else if (addSpace) needSpace(); } } - + private static final String[] flagLowerCaseNames = new String[Flag.values().length]; - + static { for (Flag flag : Flag.values()) { flagLowerCaseNames[flag.ordinal()] = flag.name().toLowerCase(Locale.ENGLISH); } } - + /** - * Workaround for defect #239258. Prints flag names converted to lowercase in ENGLISH locale to + * Workaround for defect #239258. Prints flag names converted to lowercase in ENGLISH locale to * avoid weird Turkish I > i-without-dot-above conversion. - * + * * @param flags flags * @return flag names, space-separated. */ @@ -3009,7 +3064,7 @@ private void printExprs(List < T > trees, String sep) { private void printStat(JCTree tree) { printStat(tree, false, false); } - + private void printStat(JCTree tree, boolean member, boolean first) { printStat(tree, member, first, false, false, false); } @@ -3017,12 +3072,12 @@ private void printStat(JCTree tree, boolean member, boolean first) { /** * Prints blank lines before, positions to the exact column (optional), prints tree and * blank lines after. And optional additional newline. - * + * * @param tree * @param member * @param first * @param col - * @param nl + * @param nl */ private void printStat(JCTree tree, boolean member, boolean first, boolean col, boolean nl, boolean printComments) { if(tree==null) { @@ -3040,13 +3095,13 @@ private void printStat(JCTree tree, boolean member, boolean first, boolean col, if (col) { toColExactly(out.leftMargin); } - // because of comment duplication + // because of comment duplication if(printComments) printPrecedingComments(tree, !member); printInnerCommentsAsTrailing(tree, !member); printExpr(tree, TreeInfo.notExpression); int tag = tree.getTag().ordinal();//XXX: comparing ordinals!!! if(JCTree.Tag.APPLY.ordinal()<=tag && tag<=JCTree.Tag.MOD_ASG.ordinal()) print(';'); - + printTrailingComments(tree, !member); blankLines(tree, false); if (nl) { @@ -3122,7 +3177,7 @@ private void printBlock(JCTree tree, List stats, BracePlacemen } public int conditionStartHack = (-1); - + private void printBlock(JCTree tree, List stats, BracePlacement bracePlacement, boolean spaceBeforeLeftBrace, boolean members, boolean printComments) { if (printComments) printPrecedingComments(tree, true); int old = indent(); @@ -3446,14 +3501,14 @@ private void wrap(String s, WrapStyle wrapStyle) { private void wrapTrees(List trees, WrapStyle wrapStyle, int wrapIndent) { wrapTrees(trees, wrapStyle, wrapIndent, false); //TODO: false for "compatibility", with the previous release, but maybe should be true for everyone? } - + private void wrapTrees(List trees, WrapStyle wrapStyle, int wrapIndent, boolean wrapFirst) { wrapTrees(trees, wrapStyle, wrapIndent, wrapFirst, cs.spaceBeforeComma(), cs.spaceAfterComma(), ","); // NOI18N } private void wrapTrees(List trees, WrapStyle wrapStyle, int wrapIndent, boolean wrapFirst, boolean spaceBeforeSeparator, boolean spaceAfterSeparator, String separator) { - + boolean first = true; for (List < T > l = trees; l.nonEmpty(); l = l.tail) { if (!first) { @@ -3462,7 +3517,7 @@ private void wrapTrees(List trees, WrapStyle wrapStyle, in } print(separator); } - + if (!first || wrapFirst) { switch(first && wrapStyle != WrapStyle.WRAP_NEVER ? WrapStyle.WRAP_IF_LONG : wrapStyle) { case WRAP_IF_LONG: @@ -3487,7 +3542,7 @@ private void wrapTrees(List trees, WrapStyle wrapStyle, in first = false; } } - + private void wrapAssignOpTree(final String operator, int col, final Runnable print) { final boolean spaceAroundAssignOps = cs.spaceAroundAssignOps(); if (cs.wrapAfterAssignOps()) { @@ -3506,7 +3561,7 @@ private void wrapAssignOpTree(final String operator, int col, final Runnable pri } }); } - + private void wrapTree(WrapStyle wrapStyle, boolean needsSpaceBefore, int colAfterWrap, Runnable print) { switch(wrapStyle) { case WRAP_NEVER: diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index f425a595cd55..05e6515e933c 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -23,6 +23,7 @@ import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; import java.io.IOException; +import java.time.LocalDate; import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -42,17 +43,17 @@ /** * Test inner to outer refactoring for test. - * - * In the input files, and the expected outcomes, the indentation - * does not really matter as far as the tests are concerned because - * the indentation is stripped away before the remaining source lines - * are compared to the expected lines. + * + * In the input files, and the expected outcomes, the indentation does not + * really matter as far as the tests are concerned because the indentation is + * stripped away before the remaining source lines are compared to the expected + * lines. * * @author homberghp {@code } */ @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class InnerOuterRecordTest extends RefactoringTestBase { - + public InnerOuterRecordTest(String name) { super(name); //ensure we are running on at least 16. @@ -110,9 +111,9 @@ public final class F { } """; innerOuterSetupAndTest(source, newOuter, newInner); - + } - + public void test9ApacheNetbeans7044() throws Exception { // initial outer has record with meaningful canonical constructor. // note that Inner class should be in last member for assumptions in the test. @@ -188,10 +189,10 @@ record F(int id, String name, LocalDate dob) { } } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - + public void test1BasicClassInClass() throws Exception { // initial outer has record with meaningful canonical constructor. String source @@ -277,10 +278,10 @@ public F(int id, String name, LocalDate dob) { } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - + public void test2BasicRecordInRecord() throws Exception { String source = """ @@ -324,15 +325,15 @@ record F(int x, int y) { static String code = "nix"; } """; - + innerOuterSetupAndTest(source, newOuter, newInner); } - + /** * Test to verify what happens to the compact constructor in the outer * record. It appears to survive the refactoring. - * - * @throws Exception + * + * @throws Exception */ public void test3OuterWithCompact() throws Exception { String source @@ -375,7 +376,7 @@ record F(int id, String name, LocalDate dob) { """; innerOuterSetupAndTest(source, newOuter, newInner); } - + public void test4InnerWithCompact() throws Exception { String source = """ @@ -492,19 +493,93 @@ record F(int id, String name, LocalDate dob) { innerOuterSetupAndTest(source, newOuter, newInner); } + public void test6InnerWithCompactAndMethodAndExtraCtor() throws Exception { + String source + = """ + package t; + + import java.time.LocalDate; + record A(F f) { + + enum Suite { + SPADE, CLUB, DIAMOND, HEART; + } + + public A { + assert f != null; + } + + record F(int id, String name, LocalDate dob) { + + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + public F(int id, String name){ + this(id,name,LocalDate.now()); + } + boolean bornBefore(LocalDate someDate) { + return dob.isBefore(someDate); + } + } + } + """; + String newOuter + = """ + package t; + import java.time.LocalDate; + record A(F f) { + enum Suite { + SPADE, CLUB, DIAMOND, HEART; + } + public A { + assert f != null; + } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + package t; + import java.time.LocalDate; + /** + * + * @author junit + */ + record F(int id, String name, LocalDate dob) { + + public F { + if (dob.isBefore(LocalDate.EPOCH)) { + throw new IllegalArgumentException("to old " + dob); + } + } + public F(int id, String name) { + this(id, name, LocalDate.now()); + } + boolean bornBefore(LocalDate someDate) { + return dob.isBefore(someDate); + } + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + void innerOuterSetupAndTest(String source, String newOuterName, String newInnerName) throws Exception { writeFilesNoIndexing(src, new File("t/A.java", source)); performInnerToOuterTest2(null); verifyContent(src, new File("t/A.java", newOuterName), new File("t/F.java", newInnerName)); } - + boolean debug = false; // variant for record inner to outer test private void performInnerToOuterTest2(String newOuterName, Problem... expectedProblems) throws Exception { final InnerToOuterRefactoring[] r = new InnerToOuterRefactoring[1]; JavaSource.forFileObject(src.getFileObject("t/A.java")).runUserActionTask(new Task() { - + @Override public void run(CompilationController parameter) { try { @@ -523,25 +598,17 @@ public void run(CompilationController parameter) { List members = outer.getMembers(); int m = 0; if (debug) { - for (Tree member : members) { - printNumbered(System.err, "member %d %15s".formatted(m++, member.getKind()), member.toString()); - String toString = member.toString(); - if (member instanceof ClassTree ct) { - int n = 0; - Name simpleName = ct.getSimpleName(); - for (Tree ctm : ct.getMembers()) { - printNumbered(System.err, " ctm " + simpleName + " %d %15s".formatted(n++, ctm.getKind()), ctm.toString()); - } - } - } + printMembers(members, m); } // selecting the last element assumes that the inner class is the last member in the outer class. Tree lastInnerClass = outer.getMembers().get(outer.getMembers().size() - 1); - if (debug) { - String n = "lastInnerClass " + lastInnerClass.getKind().toString(); - printNumbered(System.err, n, lastInnerClass.toString()); + if (debug && lastInnerClass instanceof ClassTree lct) { +// String n = "lastInnerClass " + lastInnerClass.getKind().toString(); +// printNumbered(System.err, n, lastInnerClass.toString()); + printClassTree(lct); } TreePath tp = TreePath.getPath(cut, lastInnerClass); + try { r[0] = new InnerToOuterRefactoring(TreePathHandle.create(tp, parameter)); } catch (Throwable t) { @@ -550,22 +617,48 @@ public void run(CompilationController parameter) { throw t; } } + }, true); - + r[0].setClassName("F"); if (debug) { printNumbered(System.err, "result ", r[0].toString()); } r[0].setReferenceName(newOuterName); - + RefactoringSession rs = RefactoringSession.create("Session"); List problems = new LinkedList(); - + addAllProblems(problems, r[0].preCheck()); addAllProblems(problems, r[0].prepare(rs)); addAllProblems(problems, rs.doRefactoring(true)); - + assertProblems(Arrays.asList(expectedProblems), problems); } - + + // test helper + static void printMembers(List members, int m) { + printMembers(members, m, ""); + } + + // test helper + static void printMembers(List members, int m, String indent) { + for (Tree member : members) { + printNumbered(System.err, indent + "member %d %15s".formatted(m, member.getKind()), member.toString()); + String toString = member.toString(); + if (member instanceof ClassTree ct) { + int n = 0; + Name simpleName = ct.getSimpleName(); + List members1 = ct.getMembers(); + printMembers(members1, n, indent + " " + m + " "); + } + m++; + } + } + + // test helper + static void printClassTree(ClassTree ct) { + printMembers(ct.getMembers(), 0, "class " + ct.getSimpleName() + " type " + ct.getKind() + " "); + } + } From 2288644614cd33c94adc27d51da709012ee0f22d Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Thu, 16 Jan 2025 13:16:09 +0100 Subject: [PATCH 19/19] adde generic inner record in tests. Test passes. --- .../java/test/InnerOuterRecordTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java index 05e6515e933c..eb95944292c1 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -567,6 +567,55 @@ boolean bornBefore(LocalDate someDate) { innerOuterSetupAndTest(source, newOuter, newInner); } + + public void test7Generic() throws Exception{ + String source + = """ + package t; + + record A(F f) { + public A { + assert f != null; + } + record F(P first, Q second) { + + public F { + assert null != first; + assert null != second; + } + } + } + """; + String newOuter + = """ + package t; + record A(F f) { + public A { + assert f != null; + } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + package t; + /** + * + * @author junit + */ + record F(P first, Q second) { + public F { + assert null != first; + assert null != second; + } + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + + } + void innerOuterSetupAndTest(String source, String newOuterName, String newInnerName) throws Exception { writeFilesNoIndexing(src, new File("t/A.java", source)); performInnerToOuterTest2(null);