From b58e86f4f22091ed99bd0a857d5cee774b251340 Mon Sep 17 00:00:00 2001 From: "Pieter van den Hombergh (homberghp)" Date: Thu, 24 Apr 2025 15:55:57 +0200 Subject: [PATCH] Final commit for this PR, cleanup and squash * added tests to show the issues. * added junit test helper to make test output better understandable: AssertLinesEqualHelpers.java * added RECORD case label where appropriate, for instance where the record template should be used. * added factory for Record classTree in TreeFactory and TreeMaker * added/improved methods in CasualDiff and VeryPretty to pick op parameters from record in constructor, so that var args are preserved. Author: Pieter van den Hombergh (homberghp) Date: Mon Jun 23 11:59:04 2025 +0200 --- harness/nbjunit/nbproject/project.properties | 2 +- .../junit/AssertLinesEqualHelpers.java | 158 +++ .../netbeans/api/java/source/TreeMaker.java | 20 + .../netbeans/api/java/source/WorkingCopy.java | 4 + .../java/source/builder/TreeFactory.java | 9 + .../java/source/pretty/VeryPretty.java | 93 +- .../java/source/pretty/WidthEstimator.java | 1 - .../modules/java/source/save/CasualDiff.java | 404 ++++---- .../java/test/InnerOuterRecordTest.java | 944 ++++++++++++++++++ .../refactoring/java/test/PullUpTest.java | 271 ++++- .../java/test/RefactoringTestBase.java | 380 ++++--- .../java/test/RenameRecordTest.java | 244 ++++- 12 files changed, 2142 insertions(+), 388 deletions(-) create mode 100644 harness/nbjunit/src/org/netbeans/junit/AssertLinesEqualHelpers.java create mode 100644 java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java diff --git a/harness/nbjunit/nbproject/project.properties b/harness/nbjunit/nbproject/project.properties index cbe11c7123f6..567a640a2bb6 100644 --- a/harness/nbjunit/nbproject/project.properties +++ b/harness/nbjunit/nbproject/project.properties @@ -16,7 +16,7 @@ # under the License. javac.compilerargs=-Xlint:unchecked -javac.source=1.8 +javac.release=17 javadoc.arch=${basedir}/arch.xml javadoc.apichanges=${basedir}/apichanges.xml diff --git a/harness/nbjunit/src/org/netbeans/junit/AssertLinesEqualHelpers.java b/harness/nbjunit/src/org/netbeans/junit/AssertLinesEqualHelpers.java new file mode 100644 index 000000000000..23fff8cf0f2b --- /dev/null +++ b/harness/nbjunit/src/org/netbeans/junit/AssertLinesEqualHelpers.java @@ -0,0 +1,158 @@ +/* + * 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.junit; + +import java.io.PrintStream; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.fail; +import static org.netbeans.junit.AssertLinesEqualHelpers.StringsCompareMode.EXACT; + +/** + * + * @author homberghp + */ +public class AssertLinesEqualHelpers { + + public enum StringsCompareMode { + EXACT, IGNORE_DUP_SPACES, IGNORE_INDENTATION, IGNORE_WHITESPACE_DIFF + } + private static StringsCompareMode stringsCompareMode = EXACT; + private static Function linePreprocessor = s -> s; + public static boolean showOutputOnPass = false; + + /** + * Sets the string compare mode. + *
    + *
  • EXACT: exact line by line, including empty lines. + *
  • IGNORE_DUP_SPACES: ignore duplicate (inner) spacing + *
    • INGNORE_INDENTATION ignore difference in leading and trailing white + * space + * INGNORE_WHITE_SPACE_DIFF ignore difference in any white space + *
+ * + * @param mode + */ + public static void setStringCompareMode(StringsCompareMode mode) { + stringsCompareMode = mode; + linePreprocessor = switch (mode) { + case EXACT -> + s -> s; + case IGNORE_DUP_SPACES -> + s -> s.replaceAll("\\s{2,}", " "); + case IGNORE_INDENTATION -> + s -> s.trim(); + case IGNORE_WHITESPACE_DIFF -> + s -> s.trim().replaceAll("\\s{2,}", " "); + }; + + } + + /** + * 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 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 white space([ \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 + */ + public static void assertLinesEqual1(String name, String expected, String actual) { + try { + assertEquals(name, 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, 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 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 testName to print before comparison result. + * @param fileName to print before each compared line. + * @param expected to compare + * @param actual to compare + */ + public static void assertLinesEqual2(String testName,String fileName, String expected, String actual) { + if (stringsCompareMode != EXACT) { + expected = expected.trim().replaceAll("([\t\r\n])\\1+", "$1"); + actual = actual.trim().replaceAll("([\t\r\n])\\1+", "$1"); + } + String[] linesExpected; + String[] linesActual; + linesExpected = expected.lines().toArray(String[]::new); + 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 oe = (i < linesExpected.length ? linesExpected[i] : ""); + String oa = (i < linesActual.length ? linesActual[i] : ""); + String e = linePreprocessor.apply(oe); + String a = linePreprocessor.apply(oa); + // somehow my user is inserted, so avoid to test those lines. + if (e.contains("@author") && a.contains("@author")) { + e = a = "* @author goes here"; + oa = oe; + } + boolean same = e.equals(a); + String sep = same ? " " : " | "; + equals &= same; + sb.append(String.format(fileName + " [%3d] %-80s%s%-80s%n", i, oe, sep, oa)); + } + if (!equals || showOutputOnPass) { + System.err.println("test " + testName + (equals ? " PASSED" : " FAILED") + " with compare mode = " + stringsCompareMode); + System.err.print(String.format(fileName + " %-80s%s%-80s%n", "expected", " + ", "actual")); + System.err.println(sb.toString()); + System.err.flush(); + if (!equals) { + fail("lines differ, see stderr for more details."); + } + } + } + +} 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 088e3f97028a..fa1d1c293e6a 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 @@ -479,6 +479,26 @@ public ClassTree Enum(ModifiersTree modifiers, return delegate.Enum(modifiers, simpleName, implementsClauses, memberDecls); } + /** + * Creates a new ClassTree representing record. + * + * @param modifiers the modifiers declaration + * @param simpleName the name of the class without its package, such + * as "String" for the class "java.lang.String". + * @param implementsClauses the list of the interfaces this class + * implements, or an empty list. + * @param memberDecls the list of fields defined by this class, or an + * empty list. + * @see com.sun.source.tree.ClassTree + */ + public ClassTree Record(ModifiersTree modifiers, + CharSequence simpleName, + List typeParameters, + List implementsClauses, + List memberDecls) { + return delegate.Record(modifiers, simpleName, typeParameters, implementsClauses, memberDecls); + } + /** * Creates a new CompilationUnitTree. * diff --git a/java/java.source.base/src/org/netbeans/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/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java b/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java index 936f86ed55f4..e65f1b0fe284 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/builder/TreeFactory.java @@ -329,6 +329,15 @@ public ClassTree Enum(ModifiersTree modifiers, long flags = getBitFlags(modifiers.getFlags()) | Flags.ENUM; return Class(flags, (com.sun.tools.javac.util.List) modifiers.getAnnotations(), simpleName, Collections.emptyList(), null, implementsClauses, Collections.emptyList(), memberDecls); } + + public ClassTree Record(ModifiersTree modifiers, + CharSequence simpleName, + List typeParameters, + List implementsClauses, + List memberDecls) { + long flags = getBitFlags(modifiers.getFlags()) | Flags.RECORD; + return Class(flags, (com.sun.tools.javac.util.List) modifiers.getAnnotations(), simpleName, typeParameters, null, implementsClauses, Collections.emptyList(), memberDecls); + } public CompilationUnitTree CompilationUnit(PackageTree packageDecl, List importDecls, diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java index 210452c8c07a..a95c19dc4acd 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 static java.util.stream.Collectors.toCollection; import org.netbeans.api.java.classpath.ClassPath; import org.netbeans.api.java.lexer.JavaTokenId; @@ -861,6 +862,7 @@ public void visitClassDef(JCClassDecl tree) { toLeftMargin(); printAnnotations(tree.mods.annotations); long flags = tree.mods.flags; + boolean isRecord= (flags & RECORD) != 0; if ((flags & ENUM) != 0) printFlags(flags & ~(INTERFACE | FINAL)); else @@ -876,15 +878,24 @@ public void visitClassDef(JCClassDecl tree) { ? out.col : out.leftMargin + cs.getContinuationIndentSize()); } } else { - if ((flags & ENUM) != 0) + if ((flags & ENUM) != 0) { print("enum "); - else { - if ((flags & ABSTRACT) != 0) + } else if (isRecord){ + print("record "); + } else { + if ((flags & ABSTRACT) != 0) { print("abstract "); + } print("class "); } print(tree.name); printTypeParameters(tree.typarams); + if (isRecord) { + print("("); + List components = getRecordComponents(tree); + wrapTrees(components, cs.wrapMethodParams(), out.col); //TODO: read from settings(!) + print(") "); + } if (tree.extending != null) { wrap("extends ", cs.wrapExtendsImplementsKeyword()); print(tree.extending); @@ -922,6 +933,9 @@ public void visitClassDef(JCClassDecl tree) { blankLines(enclClass.name.isEmpty() ? cs.getBlankLinesAfterAnonymousClassHeader() : (flags & ENUM) != 0 ? cs.getBlankLinesAfterEnumHeader() : cs.getBlankLinesAfterClassHeader()); boolean firstMember = true; for (JCTree t : members) { + if (t.getKind()==Kind.VARIABLE && t instanceof JCVariableDecl vardecl && 0!=(vardecl.mods.flags & Flags.RECORD)) { + continue; + } printStat(t, true, firstMember, true, true, false); firstMember = false; } @@ -935,6 +949,36 @@ public void visitClassDef(JCClassDecl tree) { enclClass = enclClassPrev; } + /** + * Get the record components, either by fetching the field-members or + * the parameters from a fitting constructor + * @param tree of a record + * @return the canonical parameters for the record + */ + private List getRecordComponents(JCClassDecl tree) { + List components + = List.from(tree.defs + .stream() + .filter(member -> member.getKind() == Kind.VARIABLE) + .map(member -> (JCVariableDecl) member) + .filter(comp -> (comp.mods.flags & RECORD) != 0) + .toList()); + + final long syntOrCompact = Flags.SYNTHETIC | Flags.COMPACT_RECORD_CONSTRUCTOR; + var recordParams = tree.getMembers() + .stream() + .filter(m -> m.getKind() == Kind.METHOD && m instanceof JCMethodDecl mdecl && mdecl.getReturnType() == null) + .map(JCMethodDecl.class::cast) + .filter(met -> (met.mods.flags & syntOrCompact) != 0) + .findFirst() + .map(m -> m.params); + if (recordParams.isPresent()) { + components = recordParams.get(); + } + + return components; + } + private void printEnumConstants(java.util.List defs, boolean forceSemicolon, boolean printComments) { boolean first = true; boolean hasNonEnumerator = false; @@ -995,18 +1039,20 @@ 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 ((tree.mods.flags & Flags.COMPACT_RECORD_CONSTRUCTOR)==0L) { + 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 (tree.thrown.nonEmpty()) { wrap("throws ", cs.wrapThrowsKeyword()); wrapTrees(tree.thrown, cs.wrapThrowsList(), cs.alignMultilineThrows() @@ -1014,7 +1060,18 @@ public void visitMethodDef(JCMethodDecl tree) { true); } if (tree.body != null) { - printBlock(tree.body, tree.body.stats, cs.getMethodDeclBracePlacement(), cs.spaceBeforeMethodDeclLeftBrace(), true); + + List stats = tree.body.stats; + if ((tree.mods.flags & Flags.COMPACT_RECORD_CONSTRUCTOR) != 0L) { + ListBuffer nstats= new ListBuffer<>(); + for (JCStatement stat : stats) { + if (!stat.toString().contains("super();")){ + nstats.append(stat); + } + } + stats=nstats.toList(); + } + printBlock(tree.body, stats, cs.getMethodDeclBracePlacement(), cs.spaceBeforeMethodDeclLeftBrace(), true); } else { if (tree.defaultValue != null) { print(" default "); @@ -1029,9 +1086,10 @@ public void visitMethodDef(JCMethodDecl tree) { @Override public void visitVarDef(JCVariableDecl tree) { boolean notEnumConst = (tree.mods.flags & Flags.ENUM) == 0; + boolean isRecordComponent = (tree.mods.flags & Flags.RECORD) != 0; printAnnotations(tree.mods.annotations); if (notEnumConst) { - printFlags(tree.mods.flags); + if(!isRecordComponent) printFlags(tree.mods.flags); if (!suppressVariableType) { if ((tree.mods.flags & VARARGS) != 0) { // Variable arity method. Expecting ArrayType, print ... instead of []. @@ -2147,6 +2205,7 @@ private void blankLines(JCTree tree, boolean before) { case ANNOTATION_TYPE: case CLASS: case ENUM: + case RECORD: case INTERFACE: n = before ? cs.getBlankLinesBeforeClass() : cs.getBlankLinesAfterClass(); if (((JCClassDecl) tree).defs.nonEmpty() && !before) { diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/WidthEstimator.java b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/WidthEstimator.java index 9a4a3fedf18b..ba03c9672fc1 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/pretty/WidthEstimator.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/pretty/WidthEstimator.java @@ -85,7 +85,6 @@ private void width(JCTree tree, int prec) { } } public void visitTree(JCTree tree) { -System.err.println("Need width calc for "+tree); width = maxwidth; } public void visitParens(JCParens tree) { diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java b/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java index 59e9a018108a..cbbccb718329 100644 --- a/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java +++ b/java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java @@ -143,6 +143,7 @@ import java.util.Map.Entry; import static java.util.logging.Level.*; import java.util.logging.Logger; +import java.util.stream.Stream; import javax.swing.text.BadLocationException; import javax.swing.text.Document; import org.netbeans.api.annotations.common.NullAllowed; @@ -205,7 +206,7 @@ public class CasualDiff { // such variable should not provide new line at the end. private boolean parameterPrint = false; private boolean enumConstantPrint = false; - + /** * Aliases places in the new text with block sequence boundaries in the old text. The diff can then get information * on how guarded blocks (or other divisor of the source) is mapped into the changed text and not generate diffs across @@ -251,9 +252,9 @@ public static Collection diff(Context context, for (Tree t : oldTreePath) { if (t == oldTree) continue; - + List embeddedElements; - + if (TreeUtilities.CLASS_TREE_KINDS.contains(t.getKind())) { embeddedElements = ((ClassTree) t).getMembers(); } else if (t.getKind() == Kind.BLOCK) { @@ -261,9 +262,9 @@ public static Collection diff(Context context, } else { continue; } - + embeddedElements = td.filterHidden(NbCollections.checkedListByCopy(embeddedElements, JCTree.class, false)); - + if (embeddedElements.isEmpty()) { int indent = getOldIndent(diffContext, t); @@ -305,7 +306,7 @@ public static Collection diff(Context context, //was: // int ln = td.oldTopLevel.lineMap.getLineNumber(start); // int lineStart = td.oldTopLevel.lineMap.getStartPosition(ln); - + td.printer.setInitialOffset(lineStart); Tree current = oldTree; @@ -325,7 +326,7 @@ public static Collection diff(Context context, for (Tree p : ((LambdaExpressionTree)t).getParameters()) { if (p == current) { td.parameterPrint = true; - + } } break; @@ -364,7 +365,7 @@ public static Collection diff(Context context, } try { String toParse = origText.substring(0, start) + resultSrc + origText.substring(end); - + final Document doc = LineDocumentUtils.createDocument("text/x-java"); doc.insertString(0, toParse, null); doc.putProperty(Language.class, JavaTokenId.language()); @@ -380,7 +381,7 @@ public static Collection diff(Context context, } final Indent i = Indent.get(doc); i.lock(); - + AtomicLockDocument adoc = LineDocumentUtils.asRequired(doc, AtomicLockDocument.class); try { Runnable r = new Runnable() { @@ -425,7 +426,7 @@ public static Collection diff(Context context, String originalText = isCUT ? origText : origText.substring(start, end); userInfo.putAll(td.diffInfo); - return td.checkDiffs(DiffUtilities.diff(originalText, resultSrc, start, + return td.checkDiffs(DiffUtilities.diff(originalText, resultSrc, start, td.readSections(originalText.length(), resultSrc.length(), lineStart, start), lineStart)); } @@ -433,23 +434,23 @@ private static class SectKey { private int off; SectKey(int off) { this.off = off; } } - + /** * Reads the section map. While the printer produced the text matching the region * starting at 'start', the diff will only cover text starting and textStart, which may * be in the middle of the line. the blockSequenceMap was filled by the printer, * so offsets may need to be moved backwards. - * + * * @param l1 length of the original text * @param l2 length of the new text * @param printerStart printer start * @param diffStart - * @return + * @return */ private int[] readSections(int l1, int l2, int printerStart, int diffStart) { Map seqMap = blockSequenceMap; if (seqMap.isEmpty()) { - // must offset the lengths, they come from the origtext/resultsrc, which may be already + // must offset the lengths, they come from the origtext/resultsrc, which may be already // only substrings of the printed area. int delta = diffStart - printerStart; return new int[] { l1 + delta, l2 + delta }; @@ -466,11 +467,11 @@ private int[] readSections(int l1, int l2, int printerStart, int diffStart) { } return res; } - + private List checkDiffs(List theDiffs) { if (theDiffs != null) { for (Diff d : theDiffs) { - if (diffContext.positionConverter.getOriginalPosition(d.getPos()) > diffContext.textLength || + if (diffContext.positionConverter.getOriginalPosition(d.getPos()) > diffContext.textLength || diffContext.positionConverter.getOriginalPosition(d.getEnd()) > diffContext.textLength) { LOG.warning("Invalid diff: " + d); } @@ -495,10 +496,10 @@ public static Collection diff(Context context, // TODO: the package name actually ends at the end of the name, so the semicolon could be treated as part // of the diffed list int start = td.oldTopLevel.getPackage() != null ? td.endPos(td.oldTopLevel.getPackage()) : 0; - + //XXX: no-javac-patch: td.tokenSequence.move(start); - + while (td.tokenSequence.movePrevious()) { if (td.isNoop(td.tokenSequence.token().id())) { start = td.tokenSequence.offset(); @@ -571,7 +572,7 @@ private int endPos(List trees) { return -1; return endPos(trees.get(trees.size()-1)); } - + private int checkLocalPointer(JCTree oldT, JCTree newT, int localPointer) { // diagnostics for defect #226498: log the tres iff the localPointer is bad if (localPointer < 0 || localPointer > origText.length()) { @@ -599,20 +600,20 @@ protected void diffTopLevel(JCCompilationUnit oldT, JCCompilationUnit newT, int[ checkLocalPointer(oldT, newT, localPointer); printer.print(origText.substring(localPointer)); } - + private static int getOldIndent(DiffContext diffContext, Tree t) { int offset = (int) diffContext.trees.getSourcePositions().getStartPosition(diffContext.origUnit, t); - + if (offset < 0) return -1; - + while (offset > 0 && diffContext.origText.charAt(offset - 1) != '\n') offset--; - + int indent = 0; - + while (offset < diffContext.origText.length()) { char c = diffContext.origText.charAt(offset++); - + if (c == '\t') { indent += diffContext.style.getTabSize(); } else if (c == '\n' || !Character.isWhitespace(c)) { @@ -621,10 +622,10 @@ private static int getOldIndent(DiffContext diffContext, Tree t) { indent++; } } - + return indent; } - + private boolean needStar(int localPointer) { if (localPointer <= 0) { return false; @@ -729,7 +730,7 @@ protected int diffModuleDef(JCModuleDecl oldT, JCModuleDecl newT, int[] bounds) } return bounds[1]; } - + protected int diffRequires(JCRequires oldT, JCRequires newT, int[] bounds) { int localPointer = bounds[0]; // module name @@ -790,7 +791,7 @@ protected int diffRequires(JCRequires oldT, JCRequires newT, int[] bounds) { localPointer = diffTree(oldT.moduleName, newT.moduleName, nameBounds); copyTo(localPointer, bounds[1]); - return bounds[1]; + return bounds[1]; } protected int diffExports(JCExports oldT, JCExports newT, int[] bounds) { @@ -811,7 +812,7 @@ protected int diffExports(JCExports oldT, JCExports newT, int[] bounds) { PositionEstimator est = EstimatorFactory.exportsOpensTo(oldT.moduleNames, newT.moduleNames, diffContext); localPointer = diffList2(oldT.moduleNames, newT.moduleNames, posHint, est); copyTo(localPointer, bounds[1]); - return bounds[1]; + return bounds[1]; } protected int diffOpens(JCOpens oldT, JCOpens newT, int[] bounds) { @@ -832,7 +833,7 @@ protected int diffOpens(JCOpens oldT, JCOpens newT, int[] bounds) { PositionEstimator est = EstimatorFactory.exportsOpensTo(oldT.moduleNames, newT.moduleNames, diffContext); localPointer = diffList2(oldT.moduleNames, newT.moduleNames, posHint, est); copyTo(localPointer, bounds[1]); - return bounds[1]; + return bounds[1]; } protected int diffProvides(JCProvides oldT, JCProvides newT, int[] bounds) { @@ -941,7 +942,15 @@ protected int diffImport(JCImport oldT, JCImport newT, int[] bounds) { // TODO: should be here printer.enclClassName be used? private Name origClassName = null; private Name newClassName = null; - + /** + * Computes differences between old and new classTree and prints. + * + * The this.origText is used between bounds[0] and bounds[1]. + * @param oldT old definition of class tree + * @param newT new definition of class tree + * @param bounds the bounds of the old classtree in thge original text + * @return the end position in the origText. + */ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { int localPointer = bounds[0]; final Name origOuterClassName = origClassName; @@ -959,6 +968,7 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { moveToSrcRelevant(tokenSequence, Direction.FORWARD); insertHint = tokenSequence.offset(); localPointer = diffModifiers(oldT.mods, newT.mods, oldT, localPointer); + boolean newIsRecord = (newT.mods.flags & Flags.RECORD) != 0; if (kindChanged(oldT.mods.flags, newT.mods.flags)) { int pos = oldT.pos; if ((oldT.mods.flags & Flags.ANNOTATION) != 0) { @@ -973,6 +983,9 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { } else if ((newT.mods.flags & Flags.ENUM) != 0) { copyTo(localPointer, pos); printer.print("enum"); //NOI18N + } else if (newIsRecord) { + copyTo(localPointer, pos); + printer.print("record"); //NOI18N } else if ((newT.mods.flags & Flags.INTERFACE) != 0) { copyTo(localPointer, pos); printer.print("interface"); //NOI18N @@ -1016,8 +1029,8 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { } //TODO: class to record and vice versa! if (oldT.getKind() == Kind.RECORD && newT.getKind() == Kind.RECORD) { - ComponentsAndOtherMembers oldParts = splitOutRecordComponents(filteredOldTDefs); - ComponentsAndOtherMembers newParts = splitOutRecordComponents(filteredNewTDefs); + ComponentsAndOtherMembers oldParts = splitOutRecordComponents(filteredOldTDefs, oldT); + ComponentsAndOtherMembers newParts = splitOutRecordComponents(filteredNewTDefs,newT); int posHint; if (oldParts.components().isEmpty()) { // compute the position. Find the parameters closing ')', its @@ -1046,16 +1059,20 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { moveFwdToToken(tokenSequence, oldParts.components.isEmpty() ? posHint : endPos(oldParts.components.get(oldParts.components.size() - 1)), JavaTokenId.RPAREN); tokenSequence.moveNext(); posHint = tokenSequence.offset(); - if (localPointer < posHint) + if (localPointer < posHint) { copyTo(localPointer, localPointer = posHint); + } filteredOldTDefs = oldParts.defs; filteredNewTDefs = newParts.defs; tokenSequence.move(localPointer); moveToSrcRelevant(tokenSequence, Direction.FORWARD); // it can be > (GT) or >> (SHIFT) - insertHint = tokenSequence.offset() + tokenSequence.token().length(); - } - switch (getChangeKind(oldT.extending, newT.extending)) { + // do not print closing { too early + insertHint = tokenSequence.offset()-1;// + tokenSequence.token().length()-1; + } // end both recod + + final ChangeKind changeKind = getChangeKind(oldT.extending, newT.extending); + switch (changeKind) { case NOCHANGE: insertHint = oldT.extending != null ? endPos(oldT.extending) : insertHint; copyTo(localPointer, localPointer = insertHint); @@ -1077,33 +1094,34 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { break; } { - // TODO (#pf): there is some space for optimization. If the new list - // is also empty, we can skip this computation. - if (oldT.implementing.isEmpty()) { - // if there is not any implementing part, we need to adjust position - // from different place. Look at the examples in all if branches. - // | represents current adjustment and ! where we want to point to - if (oldT.extending != null) - // public class Yerba| extends Object! { ... - insertHint = endPos(oldT.extending); - else { - // currently no need to adjust anything here: - // public class Yerba|! { ... + // TODO (#pf): there is some space for optimization. If the new list + // is also empty, we can skip this computation. + if (oldT.implementing.isEmpty()) { + // if there is not any implementing part, we need to adjust position + // from different place. Look at the examples in all if branches. + // | represents current adjustment and ! where we want to point to + if (oldT.extending != null) // public class Yerba| extends Object! { ... + { + insertHint = endPos(oldT.extending); + } else { + // currently no need to adjust anything here: + // public class Yerba|! { ... + } + } else { + // we already have any implements, adjust position to first + // public class Yerba| implements !Mate { ... + // Note: in case of all implements classes are removed, + // diffing mechanism will solve the implements keyword. + insertHint = oldT.implementing.iterator().next().getStartPosition(); + } + long flags = oldT.sym != null ? oldT.sym.flags() : oldT.mods.flags; + PositionEstimator estimator = (flags & INTERFACE) == 0 + ? EstimatorFactory.implementz(oldT.getImplementsClause(), newT.getImplementsClause(), diffContext) + : EstimatorFactory.extendz(oldT.getImplementsClause(), newT.getImplementsClause(), diffContext); + if (!newT.implementing.isEmpty()) { + copyTo(localPointer, insertHint); } - } else { - // we already have any implements, adjust position to first - // public class Yerba| implements !Mate { ... - // Note: in case of all implements classes are removed, - // diffing mechanism will solve the implements keyword. - insertHint = oldT.implementing.iterator().next().getStartPosition(); - } - long flags = oldT.sym != null ? oldT.sym.flags() : oldT.mods.flags; - PositionEstimator estimator = (flags & INTERFACE) == 0 ? - EstimatorFactory.implementz(oldT.getImplementsClause(), newT.getImplementsClause(), diffContext) : - EstimatorFactory.extendz(oldT.getImplementsClause(), newT.getImplementsClause(), diffContext); - if (!newT.implementing.isEmpty()) - copyTo(localPointer, insertHint); - localPointer = diffList2(oldT.implementing, newT.implementing, insertHint, estimator); + localPointer = diffList2(oldT.implementing, newT.implementing, insertHint, estimator); } { @@ -1199,31 +1217,67 @@ protected int diffClassDef(JCClassDecl oldT, JCClassDecl newT, int[] bounds) { } return bounds[1]; } - - private ComponentsAndOtherMembers splitOutRecordComponents(List defs) { - ListBuffer components = new ListBuffer<>(); - ListBuffer filteredDefs = new ListBuffer<>(); + + /** + * Get and split into components and other members from a record. + * If the record declares a synthethic or compact constructor, obtain the parameters + * from that one, and see if the last parameter is a varargs parameter. If so, patch the + * flags of the last recordComponent such that it also is returned as a varargs parameter. + * + * This works because in a record there is a constructor because the 'default' constructor + * in the classTree of a record is a canonical constructor (specifying all + * record components). Same applies for the canonical constuctor, which in the classtree is normal constructor + * with all parameters equal to the record components. + * + * @param defs member declarations to consider + * @param classTree for this record + * @return a record containing the components and the non-component members such as static fields, methods etc. + */ + private ComponentsAndOtherMembers splitOutRecordComponents(List defs, JCClassDecl classTree) { + List components = new ArrayList<>(); + List filteredDefs = new ArrayList<>(); for (JCTree t : defs) { - if (t.getKind() == Kind.VARIABLE && - (((JCVariableDecl) t).mods.flags & RECORD) != 0) { - components.add(t); + if (t.getKind() == Kind.VARIABLE && t instanceof JCVariableDecl decl + && (decl.mods.flags & RECORD) != 0) { + components.add(decl); } else { filteredDefs.add(t); } } - return new ComponentsAndOtherMembers(components.toList(), - filteredDefs.toList()); + // only if last component is present and is array, we consider varargs for the record param + if (!components.isEmpty() && components.get(components.size() - 1).vartype.getKind() == Kind.ARRAY_TYPE) { + int idx = components.size() - 1; + final long syntOrCompact = Flags.SYNTHETIC | Flags.COMPACT_RECORD_CONSTRUCTOR; + var recordParams = classTree.defs + .stream() + .filter(m -> m.getKind() == Kind.METHOD && m instanceof JCMethodDecl mdecl && mdecl.getReturnType() == null) + .map(JCMethodDecl.class::cast) + .filter(met -> (met.mods.flags & syntOrCompact) != 0) + .findFirst() + .map(m -> m.params); + if (recordParams.isPresent()) { + List params = recordParams.get(); + assert params.size() == components.size(); + JCVariableDecl lastParam = params.get(idx); + boolean varargsCtor = (lastParam.mods.flags & Flags.VARARGS) != 0; + if (varargsCtor) { + JCVariableDecl lastComponent = components.get(idx); + lastComponent.mods.flags |= Flags.VARARGS; + } + } + } + return new ComponentsAndOtherMembers(components, filteredDefs); } - record ComponentsAndOtherMembers(List components, List defs) {} + record ComponentsAndOtherMembers(List components, List defs) {} /** - * When the enumeration contains just methods, it is necessary to preced them with single ;. If a constant is - * inserted, it must be inserted first; and the semicolon should be removed. This method will attempt to remove entire - * lines of whitespace around the semicolon. Preceding or following comments are preserved. - * + * When the enumeration contains just methods, it is necessary to preceed them with single ;. If a constant is + * inserted, it must be inserted first; and the semicolon should be removed. This method will attempt to remove entire + * lines of white space around the semicolon. Preceding or following comments are preserved. + * * @param insertHint the local Pointer value * @return new localPointer value */ @@ -1284,7 +1338,7 @@ private int removeExtraEnumSemicolon(int insertHint) { tokenSequence.moveNext(); return insertHint; } - + private boolean isEnum(Tree tree) { if (tree instanceof FieldGroupTree) return ((FieldGroupTree) tree).isEnum(); if (tree instanceof VariableTree) return (((JCVariableDecl) tree).getModifiers().flags & Flags.ENUM) != 0; @@ -1371,7 +1425,7 @@ protected int diffMethodDef(JCMethodDecl oldT, JCMethodDecl newT, int[] bounds) printer.print(newRestype); printer.print(" "); // print the space after return type } - + int posHint; if (oldT.typarams.isEmpty()) { posHint = oldRestype != null ? oldRestype.getStartPosition() : oldT.getStartPosition(); @@ -1398,7 +1452,7 @@ protected int diffMethodDef(JCMethodDecl oldT, JCMethodDecl newT, int[] bounds) } if (oldT.params.isEmpty()) { // compute the position. Find the parameters closing ')', its - // start position is important for us. This is used when + // start position is important for us. This is used when // there was not any parameter in original tree. int startOffset = oldT.pos; @@ -1801,8 +1855,8 @@ protected int diffBlock(JCBlock oldT, JCBlock newT, int[] blockBounds) { JCTree tree = oldstats.get(oldstats.size() - 1); localPointer = adjustLocalPointer(localPointer, comments.getComments(oldT), CommentSet.RelativePosition.INNER); CommentSet cs = comments.getComments(tree); - localPointer = adjustLocalPointer(localPointer, cs, CommentSet.RelativePosition.INLINE); - localPointer = adjustLocalPointer(localPointer, cs, CommentSet.RelativePosition.TRAILING); + localPointer = adjustLocalPointer(localPointer, cs, CommentSet.RelativePosition.INLINE); + localPointer = adjustLocalPointer(localPointer, cs, CommentSet.RelativePosition.TRAILING); */ copyTo(localPointer, localPointer = endPos); } @@ -1821,7 +1875,7 @@ private int adjustLocalPointer(int localPointer, CommentSet cs, CommentSet.Relat } return localPointer; } - + private boolean isComment(JavaTokenId tid) { switch (tid) { case LINE_COMMENT: @@ -2028,7 +2082,7 @@ protected int diffSwitch(JCSwitch oldT, JCSwitch newT, int[] bounds) { copyTo(localPointer, localPointer = tokenSequence.offset()); PositionEstimator est = EstimatorFactory.cases(oldT.getCases(), newT.getCases(), diffContext); localPointer = diffList(oldT.cases, newT.cases, localPointer, est, Measure.MEMBER, printer); - + List cases = newT.cases; if (cases.size() != 0) { String caseKind = String.valueOf(cases.get(0).getCaseKind()); @@ -2226,7 +2280,7 @@ protected int diffTry(JCTry oldT, JCTry newT, int[] bounds) { for (Tree t = l.head; t!= null; l = l.tail, t = l.head) { printer.oldTrees.remove(t); } - } + } localPointer = diffParameterList(oldT.resources, newT.resources, null, @@ -2244,7 +2298,7 @@ protected int diffTry(JCTry oldT, JCTry newT, int[] bounds) { } } } - + copyTo(localPointer, bodyPos[0]); localPointer = diffTree(oldT.body, newT.body, bodyPos); copyTo(localPointer, localPointer = bodyPos[1]); @@ -2642,7 +2696,7 @@ protected int diffAssign(JCAssign oldT, JCAssign newT, JCTree parent, int[] boun } } //#174552 end - + // rhs copyTo(localPointer, rhsBounds[0]); localPointer = diffTree(oldT.rhs, newT.rhs, rhsBounds); @@ -2849,7 +2903,7 @@ protected int diffSelect(JCFieldAccess oldT, JCFieldAccess newT, copyTo(localPointer, bounds[1]); return bounds[1]; } - + protected int diffMemberReference(JCMemberReference oldT, JCMemberReference newT, int[] bounds) { int localPointer = bounds[0]; int[] exprBounds = getBounds(oldT.expr); @@ -2961,7 +3015,7 @@ protected int diffTypeApply(JCTypeApply oldT, JCTypeApply newT, int[] bounds) { copyTo(localPointer, bounds[1]); return bounds[1]; } - + protected int diffAnnotatedType(JCAnnotatedType oldT, JCAnnotatedType newT, int[] bounds) { int localPointer = bounds[0]; if (!listsMatch(oldT.annotations, newT.annotations)) { @@ -3086,7 +3140,7 @@ protected int diffModifiers(JCModifiers oldT, JCModifiers newT, JCTree parent, i localPointer = tokenSequence.offset(); } } - + localPointer = diffAnnotationsLists(oldT.getAnnotations(), newT.getAnnotations(), startPos, localPointer); if ((oldT.flags & Flags.ANNOTATION) != 0) { @@ -3132,7 +3186,7 @@ protected int diffModifiers(JCModifiers oldT, JCModifiers newT, JCTree parent, i private int diffAnnotationsLists(com.sun.tools.javac.util.List oldAnnotations, com.sun.tools.javac.util.List newAnnotations, int startPos, int localPointer) { int annotationsEnd = oldAnnotations.nonEmpty() ? endPos(oldAnnotations) : localPointer; - + if (listsMatch(oldAnnotations, newAnnotations)) { copyTo(localPointer, localPointer = (annotationsEnd != localPointer ? annotationsEnd : startPos)); } else { @@ -3169,13 +3223,13 @@ protected int diffErroneous(JCErroneous oldT, JCErroneous newT, int[] bounds) { copyTo(localPointer, bounds[1]); return bounds[1]; } - + protected int diffLambda(JCLambda oldT, JCLambda newT, int[] bounds) { int localPointer = bounds[0]; int posHint; if (oldT.params.isEmpty()) { // compute the position. Find the parameters closing ')', its - // start position is important for us. This is used when + // start position is important for us. This is used when // there was not any parameter in original tree. int startOffset = oldT.pos; @@ -3223,7 +3277,7 @@ protected int diffLambda(JCLambda oldT, JCLambda newT, int[] bounds) { localPointer = copyUpTo(localPointer, bounds[1]); return localPointer; } - + private static final EnumSet LAMBDA_PARAM_END_TOKENS = EnumSet.of(JavaTokenId.RPAREN, JavaTokenId.ARROW); protected int diffFieldGroup(FieldGroupTree oldT, FieldGroupTree newT, int[] bounds) { @@ -3420,14 +3474,14 @@ public boolean treesMatch(JCTree t1, JCTree t2, boolean deepMatch) { SourcePositions sps = this.diffContext.trees.getSourcePositions(); int a1 = (int)sps.getStartPosition(diffContext.origUnit, t1); int a2 = (int)sps.getEndPosition(diffContext.origUnit, t1); - + int b1 = (int)sps.getStartPosition(diffContext.origUnit, t2); int b2 = (int)sps.getEndPosition(diffContext.origUnit, t2); - + if (a1 == b1 && a2 == b2) { return true; } - + if (a1 == NOPOS || a2 == NOPOS || b1 == NOPOS || b2 == NOPOS) { return false; } @@ -3436,7 +3490,7 @@ public boolean treesMatch(JCTree t1, JCTree t2, boolean deepMatch) { } String sa = diffContext.origText.substring(a1, a2); String sb = diffContext.origText.substring(b1, b2); - + return sa.equals(sb); } default: @@ -3608,7 +3662,7 @@ private int printBreakContinueTree(int[] bounds, final Name oldTLabel, final Nam copyTo(localPointer, localPointer = getOldPos(oldT)); printer.print(stmt); localPointer += stmt.length(); - + int commentStart = -1; int commentEnd = -1; if (oldTLabel != null && oldTLabel.length() > 0) { @@ -3694,12 +3748,12 @@ private int diffParameterList( { return diffParameterList(oldList, newList, null, makeAround, pos, measure, spaceBefore, spaceAfter, listType, separator); } - + /** * Suppresses print out of parameter types; used for diff of parameters of an IMPLICIT param kind lambda expression. */ private boolean suppressParameterTypes; - + private int diffParameterList( List oldList, List newList, @@ -3760,7 +3814,7 @@ private int diffParameterList( tokenSequence.moveNext(); start = Math.max(tokenSequence.offset(), pos); } - + // in case when invoked through diffFieldGroup for enums, comments are already handled. if (start < bounds[0]) { copyTo(start, bounds[0], printer); @@ -4113,7 +4167,7 @@ private boolean commaNeeded(ResultItem[] arr, ResultItem item) { private List filterHidden(List list) { return filterHidden(diffContext, list); } - + public static List filterHidden(DiffContext diffContext, List list) { LinkedList result = new LinkedList<>(); // todo (#pf): capacity? List fieldGroup = new ArrayList<>(); @@ -4247,13 +4301,13 @@ private int diffList( ? diffContext.style.getImportGroups() : null; int lastGroup = -1; int i = 0; - + // if an item will be _inserted_ at the start (= first insert after possibly some deletes, but no modifies), // the text in between localPointer and insertPos should be copied. Insert pos may differ from estimator.getPositions()[0] // the text should be only included for INSERT operation, so save the range. See also delete op for compensation int insertPos = Math.min(getCommentCorrectedOldPos(oldList.get(i)), estimator.getInsertPos(0)); int insertSaveLocalPointer = localPointer; - + if (insertPos < localPointer) { insertPos = -1; } @@ -4284,7 +4338,7 @@ private int diffList( boolean match = false; if (lastdel != null) { boolean wasInFieldGroup = false; - // PENDING - should it be tested also in the loop of *all* deleted items ? Originally both the + // PENDING - should it be tested also in the loop of *all* deleted items ? Originally both the // FieldGroup and others only cared about the lastdel Tree. if(lastdel instanceof FieldGroupTree) { FieldGroupTree fieldGroupTree = (FieldGroupTree) lastdel; @@ -4306,7 +4360,7 @@ private int diffList( } } } - + // if inserting at the start (after possible deletes), copy the saved content detected before the result cycle start. if (insertPos > -1 && i > 0) { // do not copy past the element start, diffTree will print from that pos. @@ -4352,7 +4406,7 @@ private int diffList( break; } if (LineInsertionType.BEFORE == estimator.lineInsertType()) printer.newline(); - // specific case: the item is shuffled within the same parent. It's not expected that the printer will print it with the usual + // specific case: the item is shuffled within the same parent. It's not expected that the printer will print it with the usual // codestyle-defined blanklines before/after. However the blank lines are more expected when the item shifts to another scope. if (!oldList.contains(item.element) || !printer.handlePossibleOldTrees(Collections.singletonList(item.element), true)) { printer.print(item.element); @@ -4446,10 +4500,10 @@ private static boolean isVarTypeVariable(JCVariableDecl tree){ *

* The return may be NEGATIVE to indicate, that the comment set is the same and should be retained * in the output. If the value is POSITIVE, the method has handled the copying. - * + * * @param t * @param preceding - * @return + * @return */ private CommentSet getCommentsForTree(Tree t, boolean preceding) { if (t instanceof FieldGroupTree) { @@ -4459,7 +4513,7 @@ private CommentSet getCommentsForTree(Tree t, boolean preceding) { } return comments.getComments(t); } - + protected int diffInnerComments(JCTree oldT, JCTree newT, int localPointer) { if (innerCommentsProcessed) { return localPointer; @@ -4486,7 +4540,7 @@ protected int diffInnerComments(JCTree oldT, JCTree newT, int localPointer) { false, localPointer); } - + private DocCommentTree getDocComment(JCTree t, boolean old) { if (t instanceof FieldGroupTree) { FieldGroupTree fgt = (FieldGroupTree)t; @@ -4495,7 +4549,7 @@ private DocCommentTree getDocComment(JCTree t, boolean old) { } return old ? oldTopLevel.docComments.getCommentTree(t) : tree2Doc.get(t); } - + // note: the oldTreeStartPos must be the real start, without preceding comments. protected int diffPrecedingComments(JCTree oldT, JCTree newT, int oldTreeStartPos, int localPointer, boolean doNotDelete) { if (parent instanceof FieldGroupTree) { @@ -4520,20 +4574,20 @@ protected int diffPrecedingComments(JCTree oldT, JCTree newT, int oldTreeStartPo } else { return localPointer; } - + } DocCommentTree oldD = getDocComment(oldT, true); return diffCommentLists(oldTreeStartPos, oldPrecedingComments, newPrecedingComments, oldD, newD, false, true, false, doNotDelete, localPointer); } - + protected int diffTrailingComments(JCTree oldT, JCTree newT, int localPointer, int elementEndWithComments) { CommentSet cs = getCommentsForTree(newT, false); CommentSet old = getCommentsForTree(oldT, false); List oldInlineComments = cs == old ? ((CommentSetImpl)cs).getOrigComments(CommentSet.RelativePosition.INLINE) : old.getComments(CommentSet.RelativePosition.INLINE); List newInlineComments = cs.getComments(CommentSet.RelativePosition.INLINE); - + List oldTrailingComments = cs == old ? ((CommentSetImpl)cs).getOrigComments(CommentSet.RelativePosition.TRAILING) : old.getComments(CommentSet.RelativePosition.TRAILING); List newTrailingComments = cs.getComments(CommentSet.RelativePosition.TRAILING); boolean sameInline = sameComments(oldInlineComments, newInlineComments); @@ -4596,7 +4650,7 @@ private boolean sameComments(List oldList, List newList) { Iterator newIter = newList.iterator(); Comment oldC = safeNext(oldIter); Comment newC = safeNext(newIter); - + while (oldC != null && newC != null) { if (!commentsMatch(oldC, newC)) return false; oldC = safeNext(oldIter); @@ -4605,7 +4659,7 @@ private boolean sameComments(List oldList, List newList) { return !((oldC == null) ^ (newC == null)); } - + // refactor it! make it better private int diffCommentLists(int oldTreeStartPos, List oldList, List newList, DocCommentTree oldDoc, DocCommentTree newDoc, boolean trailing, boolean preceding, boolean inner, @@ -4680,7 +4734,7 @@ private int diffCommentLists(int oldTreeStartPos, List oldList, } while (newC != null) { if (Style.WHITESPACE != newC.style()) { -// printer.print(newC.getText()); +// printer.print(newC.getText()); if (!firstNewCommentPrinted && preceding) { copyTo(localPointer, localPointer = oldTreeStartPos); } @@ -4693,13 +4747,13 @@ private int diffCommentLists(int oldTreeStartPos, List oldList, if (!firstNewCommentPrinted && preceding) { copyTo(localPointer, localPointer = oldTreeStartPos); } - // suppress potential margin after doc comment: there's a whitespace ready between the comment and the - // JCTree. + // suppress potential margin after doc comment: there's a whitespace ready between the comment and the + // JCTree. printer.print((DCTree) newDoc, firstNewCommentPrinted); } return localPointer; } - + private int diffDocTree(DCDocComment doc, DCTree oldT, DCTree newT, int[] elementBounds) { if (oldT == null && newT != null) { throw new IllegalArgumentException("Null is not allowed in parameters."); @@ -4719,9 +4773,9 @@ private int diffDocTree(DCDocComment doc, DCTree oldT, DCTree newT, int[] elemen } return tokenSequence.offset(); } - + int localpointer = elementBounds[0]; - + if (oldT.getKind() != newT.getKind()) { // different kind of trees found, print the whole new one. int[] oldBounds = getBounds(oldT, doc); @@ -4731,7 +4785,7 @@ private int diffDocTree(DCDocComment doc, DCTree oldT, DCTree newT, int[] elemen printer.print(newT); return oldBounds[1]; } - + switch(oldT.getKind()) { case ATTRIBUTE: localpointer = diffAttribute(doc, (DCAttribute) oldT, (DCAttribute) newT, elementBounds); @@ -4854,11 +4908,11 @@ private int diffDocTree(DCDocComment doc, DCTree oldT, DCTree newT, int[] elemen return localpointer; } - + private int diffAttribute(DCDocComment doc, DCAttribute oldT, DCAttribute newT, int[] elementBounds) { return elementBounds[1]; } - + private int diffDocComment(DCDocComment doc, DCDocComment oldT, DCDocComment newT, int[] elementBounds) { //set the existing token kind, to produce correct line-beginnings: int commentPos = getOldPos(oldT, oldT); @@ -4897,7 +4951,7 @@ private int diffDocComment(DCDocComment doc, DCDocComment oldT, DCDocComment new } return elementBounds[1]; } - + private int diffParam(DCDocComment doc, DCParam oldT, DCParam newT, int[] elementBounds) { int localpointer; if(oldT.isTypeParameter != newT.isTypeParameter) { @@ -4933,7 +4987,7 @@ private int diffParam(DCDocComment doc, DCParam oldT, DCParam newT, int[] elemen } return elementBounds[1]; } - + private int diffReturn(DCDocComment doc, DCReturn oldT, DCReturn newT, int[] elementBounds) { int localpointer = oldT.description.isEmpty()? elementBounds[1] : getOldPos(oldT.description.get(0), doc); copyTo(elementBounds[0], localpointer); @@ -4943,7 +4997,7 @@ private int diffReturn(DCDocComment doc, DCReturn oldT, DCReturn newT, int[] ele } return elementBounds[1]; } - + private int diffIdentifier(DCDocComment doc, DCIdentifier oldT, DCIdentifier newT, int[] elementBounds) { if(oldT.name.equals(newT.name)) { copyTo(elementBounds[0], elementBounds[1]); @@ -4952,20 +5006,20 @@ private int diffIdentifier(DCDocComment doc, DCIdentifier oldT, DCIdentifier new } return elementBounds[1]; } - + private int diffLink(DCDocComment doc, DCLink oldT, DCLink newT, int[] elementBounds) { int localpointer = getOldPos(oldT.ref, doc); copyTo(elementBounds[0], localpointer); - + localpointer = diffDocTree(doc, oldT.ref, newT.ref, new int[] {localpointer, endPos(oldT.ref, doc)}); localpointer = diffList(doc, oldT.label, newT.label, localpointer, Measure.TAGS); - + if(localpointer < elementBounds[1]) { copyTo(localpointer, elementBounds[1]); } return elementBounds[1]; } - + private int diffSee(DCDocComment doc, DCSee oldT, DCSee newT, int[] elementBounds) { int localpointer; localpointer = getOldPos(oldT.reference.get(0), doc); @@ -4976,7 +5030,7 @@ private int diffSee(DCDocComment doc, DCSee oldT, DCSee newT, int[] elementBound } return elementBounds[1]; } - + private int diffText(DCDocComment doc, DCText oldT, DCText newT, int[] elementBounds) { if(oldT.text.equals(newT.text)) { copyTo(elementBounds[0], elementBounds[1]); @@ -4985,7 +5039,7 @@ private int diffText(DCDocComment doc, DCText oldT, DCText newT, int[] elementBo } return elementBounds[1]; } - + private int diffRawText(DCDocComment doc, DCTree.DCRawText oldT, DCTree.DCRawText newT, int[] elementBounds) { if(oldT.code.equals(newT.code)) { copyTo(elementBounds[0], elementBounds[1]); @@ -5204,7 +5258,7 @@ private int diffVersion(DCDocComment doc, DCVersion oldT, DCVersion newT, int[] } return elementBounds[1]; } - + private int diffList( DCDocComment doc, List oldList, @@ -5213,7 +5267,7 @@ private int diffList( Comparator measure) { assert oldList != null && newList != null; - + if (oldList == newList || oldList.equals(newList)) { return localPointer; } @@ -5371,7 +5425,7 @@ private boolean listContains(Listlist, Comment comment) { return true; return false; } - + private int commentStartCorrect(Comment c) { tokenSequence.move(c.pos()); @@ -5413,7 +5467,7 @@ public static int commentStart(DiffContext diffContext, CommentSet comments, Com return start; } } - + public static int commentEnd(CommentSet comments, CommentSet.RelativePosition pos) { List list = comments.getComments(pos); @@ -5423,7 +5477,7 @@ public static int commentEnd(CommentSet comments, CommentSet.RelativePosition po return list.get(list.size() - 1).endPos(); } } - + private static int commentEnd(DCDocComment doc) { int length = doc.comment.getText().length(); return doc.comment.getSourcePos(length-1); @@ -5432,16 +5486,16 @@ private static int commentEnd(DCDocComment doc) { private static int getOldPos(JCTree oldT) { return TreeInfo.getStartPos(oldT); } - + private int getOldPos(DCTree oldT, DCDocComment doc) { return oldT.pos(doc).getStartPosition(); } - + public int endPos(DCTree oldT, DCDocComment doc) { DocSourcePositions sp = JavacTrees.instance(context).getSourcePositions(); return (int) sp.getEndPosition(null, doc, oldT); } - + private int endPos(List trees, DCDocComment doc) { if (trees.isEmpty()) return -1; @@ -5461,12 +5515,12 @@ private int endPos(List trees, DCDocComment doc) { protected int diffTree(JCTree oldT, JCTree newT, int[] elementBounds) { return checkLocalPointer(oldT, newT, diffTree(oldT, newT, null, elementBounds)); } - + /** * Tracks current OLD node's path, so printout can be modified according to context */ private @NullAllowed TreePath currentPath; - + int diffTree(TreePath oldPath, JCTree newT, int[] elementBounds) { JCTree oldT = (JCTree)oldPath.getLeaf(); this.currentPath = oldPath; @@ -5493,7 +5547,7 @@ protected int diffTree(JCTree oldT, JCTree newT, JCTree parent /*used only for m currentPath = savePath; return result; } - + private int getPosAfterCommentEnd(Tree t, int minPos) { CommentSet cs = getCommentsForTree(t, false); List cmm = cs.getComments(CommentSet.RelativePosition.TRAILING); @@ -5514,7 +5568,7 @@ private int getPosAfterCommentEnd(Tree t, int minPos) { } return Math.max(minPos, pos); } - + private int getPosAfterCommentStart(Tree t, int minPos) { CommentSet cs = getCommentsForTree(t, true); List cmm = cs.getComments(CommentSet.RelativePosition.PRECEDING); @@ -5529,11 +5583,11 @@ private int getPosAfterCommentStart(Tree t, int minPos) { assert pos >= 0; return Math.max(minPos, pos); } - + private int getPosAfterTreeComments(JCTree t, int end) { class Scn extends ErrorAwareTreeScanner { int max = -1; - + @Override public Void scan(Tree node, Void p) { max = Math.max(getPosAfterCommentEnd((JCTree)node, -1), max); @@ -5545,7 +5599,7 @@ public Void scan(Tree node, Void p) { scn.scan(t, null); return Math.max(scn.max, end); } - + /** * True, if inner comments have been processed by a specialized code, or they will be printed after the tree. * Inner comments are used rarely, but e.g. blocks and classes use inner comments if they have empty body. @@ -5554,7 +5608,7 @@ public Void scan(Tree node, Void p) { * diffTreeImpl will print the comments. At the end, the flag is reset to the original value */ private boolean innerCommentsProcessed; - + private JCTree parent; protected int diffTreeImpl(JCTree oldT, JCTree newT, JCTree parent /*used only for modifiers*/, int[] elementBounds) { @@ -5566,7 +5620,7 @@ protected int diffTreeImpl(JCTree oldT, JCTree newT, JCTree parent /*used only f this.parent = saveParent; return ret; } - + private int diffTreeImpl0(JCTree oldT, JCTree newT, JCTree parent /*used only for modifiers*/, int[] elementBounds) { innerCommentsProcessed = false; if (oldT == null && newT != null) @@ -5590,7 +5644,7 @@ private int diffTreeImpl0(JCTree oldT, JCTree newT, JCTree parent /*used only fo if (printer.handlePossibleOldTrees(Collections.singletonList(newT), true)) { return getCommentCorrectedEndPos(oldT); } - + boolean handleImplicitLambda = parent != null && parent.hasTag(Tag.LAMBDA) && ((JCLambda)parent).params.size() == 1 && ((JCLambda)parent).params.get(0) == oldT &&((JCLambda)parent).paramKind == JCLambda.ParameterKind.IMPLICIT && newT.hasTag(Tag.VARDEF) && ((JCVariableDecl)newT).getType() != null; @@ -5614,15 +5668,15 @@ private int diffTreeImpl0(JCTree oldT, JCTree newT, JCTree parent /*used only fo copyTo(elementBounds[0], oldBounds[0]); } printer.print(newT); - // the printer will print attached traling comments, skip in the obsolete comments in the stream, so they don't duplicate. + // the printer will print attached traling comments, skip in the obsolete comments in the stream, so they don't duplicate. return getPosAfterTreeComments(oldT, oldBounds[1]); } } - + // if comments are the same, diffPredComments will skip them so that printer.print(newT) will // not emit them from the new element. But if printer.print() won't be used (the newT will be merged in rather // than printed anew), then surviving comments have to be printed. - int predComments = diffPrecedingComments(oldT, newT, getOldPos(oldT), elementBounds[0], + int predComments = diffPrecedingComments(oldT, newT, getOldPos(oldT), elementBounds[0], oldT.getTag() == Tag.TOPLEVEL && diffContext.forceInitialComment); int retVal = -1; // if (predComments < 0 && elementBounds[0] < -predComments) { @@ -6092,7 +6146,7 @@ private boolean matchIndexed(JCArrayAccess t1, JCArrayAccess t2) { private boolean matchSelect(JCFieldAccess t1, JCFieldAccess t2) { return treesMatch(t1.selected, t2.selected) && t1.name == t2.name; } - + private boolean matchReference(JCMemberReference t1, JCMemberReference t2) { return treesMatch(t1.expr, t2.expr) && t1.name == t2.name; } @@ -6104,12 +6158,12 @@ private boolean matchLiteral(JCLiteral t1, JCLiteral t2) { private boolean possibleTextBlock(JCLiteral t1, JCLiteral t2) { return t1.getKind() == Tree.Kind.STRING_LITERAL && t2.getKind() == Tree.Kind.STRING_LITERAL; } - + private boolean matchTypeApply(JCTypeApply t1, JCTypeApply t2) { return treesMatch(t1.clazz, t2.clazz) && listsMatch(t1.arguments, t2.arguments); } - + private boolean matchAnnotatedType(JCAnnotatedType t1, JCAnnotatedType t2) { return treesMatch(t1.underlyingType, t2.underlyingType) && listsMatch(t1.annotations, t2.annotations); @@ -6139,7 +6193,7 @@ private boolean matchLetExpr(LetExpr t1, LetExpr t2) { private boolean matchLambda(JCLambda t1, JCLambda t2) { return listsMatch(t1.params, t2.params) && treesMatch(t1.body, t2.body); } - + private boolean isCommaSeparated(JCVariableDecl oldT) { if (getOldPos(oldT) <= 0 || oldT.pos <= 0) { return false; @@ -6197,11 +6251,11 @@ private int[] getCommentCorrectedBounds(JCTree tree) { private int[] getBounds(JCTree tree) { return new int[] { getOldPos(tree), endPos(tree) }; } - + private int[] getBounds(DCTree tree, DCDocComment doc) { return new int[] { getOldPos(tree, doc), endPos(tree, doc) }; } - + private int copyUpTo(int from, int to, VeryPretty printer) { if (from < to) { copyTo(from, to, printer); @@ -6210,7 +6264,7 @@ private int copyUpTo(int from, int to, VeryPretty printer) { return from; } } - + private int copyUpTo(int from, int to) { return copyUpTo(from, to, printer); } @@ -6220,7 +6274,7 @@ private void copyTo(int from, int to) { } public static boolean noInvalidCopyTos = false; - + public void copyTo(int from, int to, VeryPretty loc) { if (from == to) { return; @@ -6245,7 +6299,7 @@ public void copyTo(int from, int to, VeryPretty loc) { } if (nextBlockBoundary == -1 && boundaries.hasNext()) { nextBlockBoundary = boundaries.next(); - } + } while (nextBlockBoundary != -1 && nextBlockBoundary < from) { if (boundaries.hasNext()) { nextBlockBoundary = boundaries.next(); @@ -6255,14 +6309,14 @@ public void copyTo(int from, int to, VeryPretty loc) { } } // map the boundary if the copied text starts at OR ends at the boundary. E.g. the after-boundary text might be - // generated, but the boundary itself is still preserved. + // generated, but the boundary itself is still preserved. while (from <= nextBlockBoundary && to >= nextBlockBoundary) { int off = nextBlockBoundary - from; int mapped = loc.out.length() + (from < nextBlockBoundary ? off : 0); - + Integer prev = blockSequenceMap.put(nextBlockBoundary, mapped); if (prev != null) { - // the first recorded value holds. + // the first recorded value holds. blockSequenceMap.put(nextBlockBoundary, prev); } nextBlockBoundary = boundaries.hasNext() ? boundaries.next() : -1; @@ -6274,13 +6328,13 @@ public void copyTo(int from, int to, VeryPretty loc) { private int diffTree(JCTree oldT, JCTree newT, int[] elementBounds, Kind parentKind) { return diffTree(oldT, newT, elementBounds, parentKind, true); } - + /** * This form contains a special hack for if, so that `else' can be placed on the same * line as the statement block end. * If a block statement is generated instead of a single stat, a newline is appended unless `retainNewline' * is false. If statement print passes false when else part of the if is present. - * + * * @param oldT old tree * @param newT new tree * @param elementBounds the old element bounds @@ -6464,7 +6518,7 @@ private void addDiffString(StringBuffer sb, Object o1, Object o2) { } } } - + private static String printCodeStyle(CodeStyle style) { if (style == null) { return ""; // NOI18N @@ -6486,12 +6540,12 @@ private static String printCodeStyle(CodeStyle style) { val = Arrays.asList((Object[])val); } sb.append(s).append(":").append(val).append("\n"); // NOI18N - } + } } } catch (Exception ex) {} return sb.toString(); } - + private int findVar(int start, int end) { tokenSequence.move(end); while (tokenSequence.movePrevious() && tokenSequence.offset() >= start) { 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..8da0a98f4eee --- /dev/null +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java @@ -0,0 +1,944 @@ +/* + * 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.io.IOException; +import java.util.Arrays; +import java.util.LinkedList; +import java.util.List; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.Name; +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; +import org.netbeans.api.java.source.TreePathHandle; +import org.netbeans.junit.AssertLinesEqualHelpers; +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.junit.AssertLinesEqualHelpers.*; +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, "16"); + //ensure we are running on at least 16. + try { + SourceVersion.valueOf("RELEASE_16"); //NOI18N + } catch (IllegalArgumentException ex) { + //OK, no RELEASE_16, skip test + throw new RuntimeException("need at least Java 16 for record"); + } + sideBySideCompare = true; + showOutputOnPass=true; + } + + 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 + = """ + 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); + } + + public void method() { + } + } + + } + """; + String newOuter + = """ + 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); + } + + } + """; + String newInner + = """ + /* + * Refactoring License + */ + + package t; + + 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); + } + + public void method() { + } + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + public void test1BasicClassInClass() 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); + } + 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; + } + } + } + """; + String newOuter + = """ + 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); + } + } + """; + String newInner + = """ + /* + * Refactoring License + */ + + 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 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){ + /** I should be back. */ + static String code = "nix"; + } + } + """; + String newOuter + = + """ + package t; + import java.time.LocalDate; + record A(int id, String name, LocalDate dob) { + static F f; + } + """; + String newInner + = """ + /* + * Refactoring License + */ + + package t; + + /** + * + * @author hom + */ + record F(int x, int y) { + /** I should be back. */ + 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 + */ + public void test3OuterWithCompact() throws Exception { + String source + = """ + package t; + + import java.time.LocalDate; + + /** Record with compact ctor. */ + 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 with compact ctor. */ + 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) {} + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + public void test4InnerWithCompact() 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); + } + } + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // outer may have effect + public void test5ClassWithInnerRecord() 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); + } + + public void test6InnerWithCompactAndMethodAndExtraCtor() throws Exception { + AssertLinesEqualHelpers.setStringCompareMode(StringsCompareMode.IGNORE_WHITESPACE_DIFF); + 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); + } + + public void test7Generic() throws Exception { + AssertLinesEqualHelpers.setStringCompareMode(StringsCompareMode.IGNORE_WHITESPACE_DIFF); + 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; + } + + @Override + public int compare(Q o){ + return this.second.compareTo(o.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; + } + + @Override + public int compare(Q o) { + return this.second.compareTo(o.second); + } + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // disable for the time being, varargs not yet implemented + public void test8Varargs() throws Exception { + AssertLinesEqualHelpers.setStringCompareMode(StringsCompareMode.IGNORE_INDENTATION); + String source = + """ + package t; + record A(F f, String... params) { + public A { + assert f != null; + } + record F

(P first, String... second) { + public F { + assert null != first; + assert null != second && second.length > 0; + } + } + } + """; + String newOuter = + """ + package t; + record A(F f, String... params) { + public A { + assert f != null; + } + } + """; + String newInner = + """ + /* + * Refactoring License + */ + + package t; + + /** + * + * @author junit + */ + record F

(P first, String... second) { + public F { + assert null != first; + assert null != second && second.length > 0; + } + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // disable for the time being, varargs not yet implemented + public void test8VarargsWithGen() throws Exception { + String source = + """ + package t; + record A(F f, Q... params) { + public A { + assert f != null; + } + record F

(P first, P... second) { + public F { + assert null != first; + assert null != second && second.length > 0; + } + } + } + """; + String newOuter = + """ + package t; + record A(F f, Q... params) { + public A { + assert f != null; + } + } + """; + String newInner = + """ + /* + * Refactoring License + */ + + package t; + + /** + * + * @author junit + */ + record F

(P first, P... second) { + + public F { + assert null != first; + assert null != second && second.length > 0; + } + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // show that brace comes too early in inner record. + public void test8RecordImplements1() throws Exception { + sideBySideCompare = true; + String source + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public class A implements Cloneable, Serializable { + static F f; + public record F(int x, int y) implements Cloneable, Serializable { + /** I should be back. */ + static String code = "nix"; + } + } + """; + String newOuter + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public class A implements Cloneable, Serializable { + static F f; + } + """; + String newInner + = """ + /* + * Refactoring License + */ + + package t; + + import java.io.Serializable; + + /** + * + * @author hom + */ + public record F(int x, int y) implements Cloneable, Serializable { + /** I should be back. */ + static String code = "nix"; + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // show that outer record survives casual diff. + public void test8RecordImplements2() throws Exception { + sideBySideCompare = true; + String source + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public record A(int a, String b) implements Cloneable, Serializable { + static F f; + enum F { + F1, F2; + /** I should be back. */ + static String code = "nix"; + } + } + """; + String newOuter + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public record A(int a, String b) implements Cloneable, Serializable { + static F f; + } + """; + String newInner + = + """ + /* + * Refactoring License + */ + + package t; + + /** + * + * @author hom + */ + enum F { + + F1, F2; + /** I should be back. */ + static String code = "nix"; + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + // Test shows that inner enum does not have the too early brace problem. + public void test8EnumImplements() throws Exception { + sideBySideCompare = true; + String source + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public class A implements Cloneable, Serializable { + static F f; + enum F implements Cloneable, Serializable { + F1,F2; + /** I should be back. */ + static String code = "nix"; + } + } + """; + String newOuter + = """ + package t; + import java.time.LocalDate; + import java.io.Serializable; + public class A implements Cloneable, Serializable { + static F f; + } + """; + String newInner + = + """ + /* + * Refactoring License + */ + + package t; + + import java.io.Serializable; + + /** + * + * @author hom + */ + enum F implements Cloneable, Serializable { + + F1, F2; + /** I should be back. */ + static String code = "nix"; + + } + """; + innerOuterSetupAndTest(source, newOuter, newInner); + } + + void innerOuterSetupAndTest(String source, String newOuterText, String newInnerText) throws Exception { + writeFilesNoIndexing(src, new File("t/A.java", source)); + performInnerToOuterTest2(null); + verifyContent(src, new File("t/A.java", newOuterText), new File("t/F.java", newInnerText)); + } + 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 { + parameter.toPhase(JavaSource.Phase.RESOLVED); + } catch (IOException ex) { + Exceptions.printStackTrace(ex); + } + CompilationUnitTree cut = parameter.getCompilationUnit(); + if (debug) { + System.err.println("cut is of type " + cut.getClass().getCanonicalName()); + } + ClassTree outer = (ClassTree) cut.getTypeDecls().get(0); + if (debug) { + printNumbered(System.err, "start source " + outer.getKind().toString(), outer.toString()); + } + List members = outer.getMembers(); + int m = 0; + if (debug) { + 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 && 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) { + System.err.println("InnerOuter refatoring failed with exception " + t); + t.printStackTrace(System.out); + 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() + " "); + } +} diff --git a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/PullUpTest.java b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/PullUpTest.java index fb7cde7eda01..815a60858b85 100644 --- a/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/PullUpTest.java +++ b/java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/PullUpTest.java @@ -34,6 +34,8 @@ import org.netbeans.api.java.source.JavaSource; import org.netbeans.api.java.source.Task; import org.netbeans.api.java.source.TreePathHandle; +import org.netbeans.junit.AssertLinesEqualHelpers; +import static org.netbeans.junit.AssertLinesEqualHelpers.*; import org.netbeans.modules.java.source.parsing.JavacParser; import org.netbeans.modules.refactoring.api.Problem; import org.netbeans.modules.refactoring.api.RefactoringSession; @@ -52,11 +54,11 @@ public class PullUpTest extends RefactoringTestBase { public PullUpTest(String name) { super(name, "1.8"); } - + static { JavacParser.DISABLE_SOURCE_LEVEL_DOWNGRADE = true; } - + public void test241514a() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public interface A { void x(); }"), @@ -66,7 +68,7 @@ public void test241514a() throws Exception { new File("pullup/A.java", "package pullup; public interface A { void x(); void y(); }"), new File("pullup/B.java", "package pullup; public interface B extends A { default void y() { } }")); } - + public void test241514b() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public interface A { void x(); }"), @@ -76,7 +78,7 @@ public void test241514b() throws Exception { new File("pullup/A.java", "package pullup; public interface A { void x(); default void y() { } }"), new File("pullup/B.java", "package pullup; public interface B extends A {}")); } - + public void testPullUpOverridingMethod() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A extends B implements C { @Override public void i() { } }"), @@ -87,7 +89,7 @@ public void testPullUpOverridingMethod() throws Exception { new File("pullup/A.java", "package pullup; public class A extends B implements C {}"), new File("pullup/B.java", "package pullup; public class B { public void i() { } }"), new File("pullup/C.java", "package pullup; public interface C { void i(); }")); - + writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A extends B { @Override public void i() { } }"), new File("pullup/B.java", "package pullup; public class B extends C { }"), @@ -98,7 +100,7 @@ public void testPullUpOverridingMethod() throws Exception { new File("pullup/B.java", "package pullup; public class B extends C { @Override public void i() { } }"), new File("pullup/C.java", "package pullup; public class C { public void i() { } }")); } - + public void test230719() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public interface A {\n" @@ -127,14 +129,14 @@ public void test230719() throws Exception { + " }\n" + "}")); } - + public void test230930() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public interface A { }"), new File("pullup/B.java", "package pullup; public class B implements A { static void y(); }")); performPullUpIface(src.getFileObject("pullup/B.java"), 0, 0, true); } - + public void test229061() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public interface A { void x(); }"), @@ -154,7 +156,7 @@ public void test134034() throws Exception { new File("pullup/A.java", "package pullup; public class A { void x() { } void y() { x(); } }"), new File("pullup/B.java", "package pullup; public class B extends A {}")); } - + public void test212934() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A { }"), @@ -181,7 +183,7 @@ public void testPullUpField() throws Exception { new File("pullup/A.java", "package pullup; public class A extends B {}"), new File("pullup/B.java", "package pullup; public class B { public int i; }")); } - + public void testPullUpGenMethoda() throws Exception { // #147508 - [Pull Up][Push down] Remap generic names writeFilesAndWaitForScan(src, new File("pullup/PullUpBaseClass.java", "package pullup;\n" @@ -303,7 +305,7 @@ public void testPullUpGenMethodb() throws Exception { new File("pullup/B.java", "package pullup; public class B extends C { }"), new File("pullup/C.java", "package pullup; public class C { void method(String x) { } }")); } - + public void testPullUpGenMethodc() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A extends B { X method() { } }"), @@ -325,7 +327,7 @@ public void testPullUpGenMethodc() throws Exception { new File("pullup/B.java", "package pullup; public class B extends C { }"), new File("pullup/C.java", "package pullup; public class C { void method(String x) { } }")); } - + public void testPullUpGenField() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A extends B { X x; }"), @@ -447,7 +449,7 @@ public void testPullUpMethod() throws Exception { + "\n" + "}")); } - + public void testPullUpMethodUndo() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/PullUpBaseClass.java", "package pullup;\n" @@ -1166,7 +1168,7 @@ public void testPullUpInterface() throws Exception { new File("pullup/A.java", "package pullup; public class A extends B { public void run() { } }"), new File("pullup/B.java", "package pullup; public class B implements Runnable { }")); } - + public void testPullUpInterface2() throws Exception { writeFilesAndWaitForScan(src, new File("pullup/A.java", "package pullup; public class A implements B { }"), @@ -1206,6 +1208,247 @@ public void testPullUpLocalyReferenced() throws Exception { new File("pullup/B.java", "package pullup; public class B { protected void foo() { } }")); } + public void testPullUpInnerTypeEnum() throws Exception { + + writeFilesAndWaitForScan(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + private void foo() { + } + enum Suite implements I{ Heart, Diamond, Club, Spade; } + private void method(Suite s) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { } + """ + ), + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + ); + performPullUp(src.getFileObject("pullup/A.java"), 2, Boolean.FALSE); + verifyContent(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + private void foo() { + } + private void method(Suite s) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { + + enum Suite implements I { + Heart, Diamond, Club, Spade + } + } + """ + ) , + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + + ); + } + + public void testPullUpInnerSimpleRecord() throws Exception { + sideBySideCompare=true; + showOutputOnPass=true; + setStringCompareMode(StringsCompareMode.IGNORE_INDENTATION); + writeFilesAndWaitForScan(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + record R( int i, String name ) {} + private void foo() { + } + private void method(R r) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { } + """ + ), + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + ); + performPullUp(src.getFileObject("pullup/A.java"), 1, Boolean.FALSE); + verifyContent(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + private void foo() { + } + private void method(R r) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { + + record R(int i, String name) { + } + } + """ + ) , + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + + ); + } + + public void testPullUpVarargRecord() throws Exception { + sideBySideCompare = true; + showOutputOnPass = true; + setStringCompareMode(StringsCompareMode.IGNORE_INDENTATION); + + writeFilesAndWaitForScan(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + record R( int i, String... name ) {} + private void foo() { + } + private void method(R r) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { } + """ + ), + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + ); + performPullUp(src.getFileObject("pullup/A.java"), 1, Boolean.FALSE); + verifyContent(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + private void foo() { + } + private void method(R r) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { + + record R(int i, String... name) { + } + } + """ + ) , + new File("pullup/I.java", + """ + package pullup; + public interface I{ } + """ + ) + + ); + } + + // disable because implements part is broken + public void testPullUpInnerRecord() throws Exception { + sideBySideCompare=true; + writeFilesAndWaitForScan(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + record R( int age, String name ) {} + private void foo() { + } + private void method(R r) { + foo(); + } + } + """ + ), + new File("pullup/B.java", + """ + package pullup; + public class B {} + """ + ) + ); + String asText = src.getFileObject("pullup/A.java").asText(); +// System.out.println("asText = " + asText); + performPullUp(src.getFileObject("pullup/A.java"), 1, Boolean.FALSE); + verifyContent(src, + new File("pullup/A.java", + """ + package pullup; + public class A extends B { + private void foo() { + } + private void method(R r) { + foo(); + } + }""" + ), + new File("pullup/B.java", + """ + package pullup; + public class B { + + record R(int age, String name) { + } + } + """ + ) + ); + } + private void performPullUpImplements(FileObject source, final int position, final int supertype, Problem... expectedProblems) throws IOException, IllegalArgumentException, InterruptedException { final PullUpRefactoring[] r = new PullUpRefactoring[1]; JavaSource.forFileObject(source).runUserActionTask(new Task() { 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..fe7feca8c65a 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,7 +21,9 @@ import java.io.EOFException; import java.io.FileInputStream; import java.io.IOException; +import java.io.PrintStream; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -31,6 +33,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; @@ -46,6 +49,7 @@ import org.netbeans.api.project.Sources; import org.netbeans.core.startup.Main; import org.netbeans.junit.NbTestCase; +import static org.netbeans.junit.AssertLinesEqualHelpers.*; import org.netbeans.modules.java.source.BootClassPathUtil; import org.netbeans.modules.java.source.TestUtil; import org.netbeans.modules.java.source.indexing.JavaCustomIndexer; @@ -75,8 +79,7 @@ public class RefactoringTestBase extends NbTestCase { public RefactoringTestBase(String name) { - super(name); - sourcelevel = "1.6"; + this(name,"17"); } public RefactoringTestBase(String name, String sourcelevel) { @@ -84,7 +87,38 @@ public RefactoringTestBase(String name, String sourcelevel) { this.sourcelevel = sourcelevel; } + 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(); } @@ -94,9 +128,38 @@ protected static void writeFilesAndWaitForScan(FileObject sourceRoot, File... fi TestUtilities.copyStringToFile(fo, f.content); } - IndexingManager.getDefault().refreshIndexAndWait(sourceRoot.toURL(), null, true); + 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 { + 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. + * + * @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(); @@ -109,33 +172,41 @@ 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 { // it is a folder todo.addAll(Arrays.asList(file.getChildren())); } } + List exc = new ArrayList<>(); for (File f : files) { - String fileContent = content.remove(f.filename); - - assertNotNull(f); - assertNotNull(f.content); - assertNotNull("Cannot find " + f.filename + " in map " + content, fileContent); + // take the element from the map filled by sourceRootTraversal. try { - assertEquals(getName() ,f.content.replaceAll("[ \t\r\n\n]+", " "), fileContent.replaceAll("[ \t\r\n\n]+", " ")); - } catch (Throwable t) { - System.err.println("expected:"); - System.err.println(f.content); - System.err.println("actual:"); - System.err.println(fileContent); - throw t; + String fileContent = content.remove(f.filename); + assertNotNull(f); + assertNotNull(f.content); + assertNotNull("Cannot find expected " + f.filename + " in map filled by sourceRoot " + content, fileContent); + if (sideBySideCompare) { + assertLinesEqual2(getName(),f.filename, f.content, fileContent); + } else { // original tests. + assertLinesEqual1(f.filename, f.content, fileContent); + } + } catch (AssertionError | Exception t) { + exc.add(t); } } + if (exc.size() > 0) { + Throwable x = exc.get(0); + if (x instanceof AssertionError ae) throw ae; + throw (Exception) x; + } - assertTrue(content.toString(), content.isEmpty()); + assertTrue("not all files processeed", content.isEmpty()); } - + + protected boolean sideBySideCompare = false; + /** * Returns a string which contains the contents of a file. * @@ -167,14 +238,14 @@ protected static void assertProblems(Iterable golden, Iterabl Problem gp = g.next(); Problem rp = r.next(); - assertEquals(gp.isFatal(), rp.isFatal()); - assertEquals(gp.getMessage(), rp.getMessage()); + assertEquals("fatality differs",gp.isFatal(), rp.isFatal()); + assertEquals("expected message differs", gp.getMessage(), rp.getMessage()); } 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 +253,7 @@ protected static void assertProblems(Iterable golden, Iterabl } protected static final class File { + public final String filename; public final String content; @@ -202,144 +274,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 +424,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 +441,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 +459,7 @@ public Lookup getLookup(MimePath mimePath) { return null; } - + } protected static boolean problemIsFatal(List problems) { @@ -400,7 +475,7 @@ protected static boolean problemIsFatal(List problems) { return false; } - private static final int RETRIES = 3; + protected int RETRIES = 3; @Override protected void runTest() throws Throwable { @@ -411,10 +486,13 @@ protected void runTest() throws Throwable { super.runTest(); return; } catch (Throwable t) { - if (exc == null) exc = t; + if (exc == null) { + exc = t; + } } } - throw exc; + if (exc != null) { + throw exc; + } } - -} \ No newline at end of file +} 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..2b950bcc5d55 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 @@ -29,6 +29,7 @@ import org.netbeans.api.java.source.TestUtilities; import org.netbeans.api.java.source.TestUtilities.TestInput; import org.netbeans.api.java.source.TreePathHandle; +import static org.netbeans.junit.AssertLinesEqualHelpers.*; import org.netbeans.modules.refactoring.api.Problem; import org.netbeans.modules.refactoring.api.RefactoringSession; import org.netbeans.modules.refactoring.api.RenameRefactoring; @@ -40,13 +41,53 @@ public class RenameRecordTest extends RefactoringTestBase { public RenameRecordTest(String name) { super(name, "17"); + sideBySideCompare=true; + showOutputOnPass=true; } public void testRenameComponent1() throws Exception { String testCode = """ - package test; - public record Test(int compo|nent) {} - """; + package test; + public record Test(int compo|nent, int y) {} + """; + TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); + writeFilesAndWaitForScan(src, + new File("Test.java", splitCode.code()), + new File("Use.java", + """ + package test; + public class Use { + private void test(Test t) { + int i = t.component(); + } + } + """)); + JavaRenameProperties props = new JavaRenameProperties(); + performRename(src.getFileObject("Test.java"), splitCode.pos(), "newName", props, true); + verifyContent(src, new File("Test.java", + """ + package test; + public record Test(int newName, int y) {} + """), + new File("Use.java", + """ + package test; + public class Use { + private void test(Test t) { + int i = t.newName(); + } + } + """)); + + } + + // changes in refactoring or javs.source.base breaks rename + // when there is somthing before the 'com|ponent'. + public void testRenameComponent1a() throws Exception { + String testCode = """ + package test; + public record Test(int x, int compo|nent) {} + """; TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); writeFilesAndWaitForScan(src, new File("Test.java", splitCode.code()), @@ -64,7 +105,7 @@ private void test(Test t) { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) {} + public record Test(int x, int newName) {} """), new File("Use.java", """ @@ -78,21 +119,64 @@ private void test(Test t) { } + // disabled, somehow having int x before to be renamed component + // breaks list diff + public void testRenameComponent1b() throws Exception { + String testCode = """ + package test; + public record Test(int x, int compo|nent) {} + """; + TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); + writeFilesAndWaitForScan(src, + new File("Test.java", splitCode.code()), + new File("Use.java", + """ + package test; + public class Use { + private void test(Test t) { + int i = t.component(); + } + } + """)); + JavaRenameProperties props = new JavaRenameProperties(); + performRename(src.getFileObject("Test.java"), splitCode.pos(), "newName", props, true); + verifyContent(src, new File("Test.java", + """ + package test; + public record Test(int x, int newName) {} + """), + new File("Use.java", + """ + package test; + public class Use { + private void test(Test t) { + int i = t.newName(); + } + } + """)); + + } + + // this test has an explicit accessor. + // this appears to break on potential compact constructor not being compact. public void testRenameComponent2() throws Exception { String testCode = """ - package test; - public record Test(int compo|nent) { - public Test(int component) { - component = -1; - } - public int component() { - return component; - } - public int hashCode() { - return component; - } - } - """; + package test; + public record Test(int compo|nent, int y) { + public Test { + component = -1; + } + public int component() { + return component; + } + public int hashCode() { + return component; + } + public Test(int someInt) { + this(someInt, 0); + } + } + """; TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); writeFilesAndWaitForScan(src, new File("Test.java", splitCode.code()), @@ -110,8 +194,8 @@ private void test(Test t) { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) { - public Test(int newName) { + public record Test(int newName, int y) { + public Test { newName = -1; } public int newName() { @@ -120,6 +204,9 @@ public int newName() { public int hashCode() { return newName; } + public Test(int someInt) { + this(someInt, 0); + } } """), new File("Use.java", @@ -134,11 +221,14 @@ private void test(Test t) { } + /* + * Show that with compact constructor behaves. + */ public void testRenameComponent3() throws Exception { String testCode = """ package test; - public record Test(int compo|nent) { - public Test { + public record Test(int compo|nent, int y) { + public Test { //compact component = -1; } public int component() { @@ -166,8 +256,8 @@ private void test(Test t) { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) { - public Test { + public record Test(int newName, int y) { + public Test { //compact newName = -1; } public int newName() { @@ -204,7 +294,7 @@ private void test(Test t) { new File("Test.java", """ package test; - public record Test(int component) { + public record Test(int component, int y) { public Test { component = -1; } @@ -222,7 +312,7 @@ public int hashCode() { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) { + public record Test(int newName, int y) { public Test { newName = -1; } @@ -260,7 +350,7 @@ private void test(Test t) { new File("Test.java", """ package test; - public record Test(int component) { + public record Test(int component, int y) { } """), new File("Use.java", splitCode.code())); @@ -269,7 +359,7 @@ public record Test(int component) { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) { + public record Test(int newName, int y) { } """), new File("Use.java", @@ -287,7 +377,7 @@ private void test(Test t) { public void testRenameComponentStartFromConstructorArg() throws Exception { String testCode = """ package test; - public record Test(int component) { + public record Test(int component, int y) { public Test { compo|nent = -1; } @@ -316,7 +406,7 @@ private void test(Test t) { verifyContent(src, new File("Test.java", """ package test; - public record Test(int newName) { + public record Test(int newName, int y) { public Test { newName = -1; } @@ -382,6 +472,102 @@ private NewName test() { """)); } + + // test for varargs and generic. + public void testRenameRecordGenVar() throws Exception { + sideBySideCompare=true; + showOutputOnPass=true; + + String testCode = """ + package test; + public record Te|st(G... component) { + public Test { + assert 0 < component.length; + } + } + """; + TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); + writeFilesAndWaitForScan(src, + new File("Test.java", splitCode.code()), + new File("Use.java", + """ + package test; + public class Use { + private Test test() { + return new Test(1, 2); + } + } + """)); + JavaRenameProperties props = new JavaRenameProperties(); + performRename(src.getFileObject("Test.java"), splitCode.pos(), "NewName", props, true); + verifyContent(src, new File("Test.java", + """ + package test; + public record NewName(G... component) { + public NewName { + assert 0 < component.length; + } + } + """), + new File("Use.java", + """ + package test; + public class Use { + private NewName test() { + return new NewName(1, 2); + } + } + """)); + + } + // test for varargs and generic. + public void testRenameRecordGenVarComponent() throws Exception { + sideBySideCompare=true; + showOutputOnPass=true; + + String testCode = """ + package test; + public record Test(G... comp|onent) { + public Test { + assert 0 < component.length; + } + } + """; + TestInput splitCode = TestUtilities.splitCodeAndPos(testCode); + writeFilesAndWaitForScan(src, + new File("Test.java", splitCode.code()), + new File("Use.java", + """ + package test; + public class Use { + private Test test() { + return new Test(1, 2); + } + } + """)); + JavaRenameProperties props = new JavaRenameProperties(); + performRename(src.getFileObject("Test.java"), splitCode.pos(), "parts", props, true); + verifyContent(src, new File("Test.java", + """ + package test; + public record Test(G... parts) { + public Test { + assert 0 < parts.length; + } + } + """), + new File("Use.java", + """ + package test; + public class Use { + private Test test() { + return new Test(1, 2); + } + } + """)); + + } + 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]; JavaSource.forFileObject(source).runUserActionTask(new Task() {