diff --git a/java.source/apichanges.xml b/java.source/apichanges.xml --- a/java.source/apichanges.xml +++ b/java.source/apichanges.xml @@ -108,6 +108,18 @@ + + + Added API to preserve comments and whitespaces in transformations + + + + + + If a transformation moves a Tree node elsewhere, or produces a replacement of it, it can now mark the new tree as a 'replacement' for the old one, + which causes whitespaces (most notably comments) to attach to the replacement node. + + Added utility methods to find span of a Label's name in the source. diff --git a/java.source/nbproject/project.properties b/java.source/nbproject/project.properties --- a/java.source/nbproject/project.properties +++ b/java.source/nbproject/project.properties @@ -46,7 +46,7 @@ javadoc.title=Java Source javadoc.arch=${basedir}/arch.xml javadoc.apichanges=${basedir}/apichanges.xml -spec.version.base=0.136.0 +spec.version.base=0.137.0 test.qa-functional.cp.extra=${refactoring.java.dir}/modules/ext/nb-javac-api.jar test.unit.run.cp.extra=${o.n.core.dir}/core/core.jar:\ ${o.n.core.dir}/lib/boot.jar:\ diff --git a/java.source/src/org/netbeans/api/java/source/GeneratorUtilities.java b/java.source/src/org/netbeans/api/java/source/GeneratorUtilities.java --- a/java.source/src/org/netbeans/api/java/source/GeneratorUtilities.java +++ b/java.source/src/org/netbeans/api/java/source/GeneratorUtilities.java @@ -961,11 +961,11 @@ CommentSetImpl t = handler.getComments(target); if (preceding) { - t.addComments(RelativePosition.PRECEDING, s.getComments(RelativePosition.PRECEDING)); - t.addComments(RelativePosition.INNER, s.getComments(RelativePosition.INNER)); + t.addComments(RelativePosition.PRECEDING, copy.useComments(s.getComments(RelativePosition.PRECEDING))); + t.addComments(RelativePosition.INNER, copy.useComments(s.getComments(RelativePosition.INNER))); } else { - t.addComments(RelativePosition.INLINE, s.getComments(RelativePosition.INLINE)); - t.addComments(RelativePosition.TRAILING, s.getComments(RelativePosition.TRAILING)); + t.addComments(RelativePosition.INLINE, copy.useComments(s.getComments(RelativePosition.INLINE))); + t.addComments(RelativePosition.TRAILING, copy.useComments(s.getComments(RelativePosition.TRAILING))); } } diff --git a/java.source/src/org/netbeans/api/java/source/TreeMaker.java b/java.source/src/org/netbeans/api/java/source/TreeMaker.java --- a/java.source/src/org/netbeans/api/java/source/TreeMaker.java +++ b/java.source/src/org/netbeans/api/java/source/TreeMaker.java @@ -1500,7 +1500,7 @@ * @return class tree with modified implements. */ public ClassTree removeClassImplementsClause(ClassTree clazz, Tree implementsClause) { - return delegate.removeClassImplementsClause(clazz, implementsClause); + return delegate.removeClassImplementsClause(clazz, asRemoved(implementsClause)); } /** @@ -2677,7 +2677,7 @@ */ public N setLabel(final N node, final CharSequence aLabel) throws IllegalArgumentException { - N result = setLabelImpl(node, aLabel); + N result = asReplacementOf(setLabelImpl(node, aLabel), node, true); GeneratorUtilities gu = GeneratorUtilities.get(copy); @@ -3041,7 +3041,90 @@ throw new IllegalArgumentException("Index out of bounds, index=" + index + ", length=" + comments.size()); } } - + + /** + * Marks the tree as new. This information serves as a hint to code generator, which may choose appropriate + * formatting for the tree, or suppress comments which might get associated to the tree. + * + * @param + * @param tree Tree instance + * @return the tree itself + */ + public T asNew(T tree) { + if (handler == null) { + throw new IllegalStateException("Cannot work with comments outside runModificationTask."); + } + if (tree == null) { + throw new NullPointerException("tree"); + } + copy.associateTree(tree, null, true); + return tree; + } + + /** + * Marks a tree as a replacement of some old one. The hint may cause surrounding whitespace to be + * carried over to the new tree and comments to be attached to the same (similar) positions + * as in the old tree. Prevous hints are removed. + * + * @param + * @param treeNew the new tree instance + * @param treeOld the old tree instance + * @return the new tree instance unchanged + * @see #asReplacementOf(com.sun.source.tree.Tree, com.sun.source.tree.Tree, boolean) + * @since 0.137 + */ + public T asReplacementOf(T treeNew, Tree treeOld) { + return asReplacementOf(treeNew, treeOld, false); + } + + /** + * Marks a tree as a replacement of some old one. The hint may cause surrounding whitespace to be + * carried over to the new tree and comments to be attached to the same (similar) positions + * as in the old tree. + *

+ * If 'defaultOnly' is true, the hint is only added if no previous hint exists. You generally want + * to force the hint, in code manipulation operations. Bulk tree transformers should preserve existing + * hints - the {@link TreeUtilities#translate} preserves existing relationships. + * + * @param + * @param treeNew + * @param treeOld + * @param defaultOnly + * @return a tree that corresponds to treeNew. + * @since 0.137 + */ + public T asReplacementOf(T treeNew, Tree treeOld, boolean defaultOnly) { + if (handler == null) { + throw new IllegalStateException("Cannot work with comments outside runModificationTask."); + } + if (treeOld == null) { + throw new NullPointerException("treeOld"); + } + if (treeNew != null) { + copy.associateTree(treeNew, treeOld, !defaultOnly); + } + return treeNew; + } + + /** + * Marks the tree as removed. The mark may affect whitespace and comment handling. For example, if the tree was + * rewritten by a different construct, and there's no direct replacement for the tree, the tree node should be marked + * as removed, so that comments do not end up with the replacement. The caller is then responsible for preserving + * comments or other whitespace in the removed node. + * + * @param + * @param tree tree that has been removed. + * @return the tree. + * @since 0.137 + */ + public T asRemoved(T tree) { + if (tree == null) { + throw new NullPointerException("tree"); + } + copy.associateTree(tree, null, true); + return tree; + } + /** * Creates a new BlockTree for provided bodyText. * diff --git a/java.source/src/org/netbeans/api/java/source/WorkingCopy.java b/java.source/src/org/netbeans/api/java/source/WorkingCopy.java --- a/java.source/src/org/netbeans/api/java/source/WorkingCopy.java +++ b/java.source/src/org/netbeans/api/java/source/WorkingCopy.java @@ -48,12 +48,17 @@ import com.sun.source.doctree.DocTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.EnhancedForLoopTree; import com.sun.source.tree.ExpressionStatementTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.ForLoopTree; import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.TreeVisitor; +import com.sun.source.tree.TryTree; +import com.sun.source.util.DocTrees; import com.sun.source.util.TreePath; import com.sun.source.util.TreePathScanner; import com.sun.source.util.TreeScanner; @@ -79,28 +84,23 @@ import javax.lang.model.element.ElementKind; import javax.swing.text.BadLocationException; import javax.tools.JavaFileObject; - -import com.sun.source.tree.EnhancedForLoopTree; -import com.sun.source.tree.ForLoopTree; -import com.sun.source.tree.TreeVisitor; -import com.sun.source.tree.TryTree; -import com.sun.source.util.DocTrees; import org.netbeans.api.annotations.common.NonNull; import org.netbeans.api.annotations.common.NullAllowed; import org.netbeans.api.annotations.common.NullUnknown; import org.netbeans.api.java.lexer.JavaTokenId; -import org.netbeans.modules.java.source.parsing.JavacParserResult; -import org.netbeans.modules.parsing.spi.Parser; -import org.openide.util.Exceptions; -import org.openide.util.Parameters; -import static org.netbeans.api.java.source.ModificationResult.*; +import org.netbeans.api.java.source.ModificationResult.CreateChange; +import org.netbeans.api.java.source.ModificationResult.Difference; import org.netbeans.api.lexer.TokenSequence; -import org.netbeans.modules.java.source.save.CasualDiff.Diff; +import org.netbeans.modules.java.source.builder.CommentHandlerService; +import org.netbeans.modules.java.source.builder.CommentSetImpl; import org.netbeans.modules.java.source.builder.TreeFactory; import org.netbeans.modules.java.source.parsing.CompilationInfoImpl; import org.netbeans.modules.java.source.parsing.FileObjects; +import org.netbeans.modules.java.source.parsing.JavacParserResult; import org.netbeans.modules.java.source.pretty.ImportAnalysis2; +import org.netbeans.modules.java.source.query.CommentSet.RelativePosition; import org.netbeans.modules.java.source.save.CasualDiff; +import org.netbeans.modules.java.source.save.CasualDiff.Diff; import org.netbeans.modules.java.source.save.DiffContext; import org.netbeans.modules.java.source.save.DiffUtilities; import org.netbeans.modules.java.source.save.ElementOverlay; @@ -109,14 +109,18 @@ import org.netbeans.modules.java.source.transform.ImmutableDocTreeTranslator; import org.netbeans.modules.java.source.transform.ImmutableTreeTranslator; import org.netbeans.modules.java.source.transform.TreeDuplicator; +import org.netbeans.modules.parsing.spi.Parser; import org.openide.filesystems.FileObject; import org.openide.filesystems.FileUtil; import org.openide.loaders.DataFolder; import org.openide.loaders.DataObject; +import org.openide.util.Exceptions; import org.openide.util.NbBundle; -import org.openide.util.Pair; +import org.openide.util.Parameters; import org.openide.util.Utilities; +import static org.netbeans.api.java.source.ModificationResult.*; + /**XXX: extends CompilationController now, finish method delegation * * @author Dusan Balek, Petr Hrebejk, Tomas Zezul @@ -139,6 +143,13 @@ */ private Map introducedTrees; + /** + * Hint information on rewrites done. Can mark a Tree as `new' which means comments will not be copied for it + * and it will not assume other tree's formatting on output. Can link a Tree to one or more other Trees, which + * causes the old trees comments to be preserved (copied) should the old trees be removed from the source. + */ + private Map rewriteHints = new HashMap(); + WorkingCopy(final CompilationInfoImpl impl, ElementOverlay overlay) { super(impl); this.overlay = overlay; @@ -226,6 +237,9 @@ * {@link TreeMaker} for tree element removal. If oldTree is * null, newTree must be of kind * {@link Kind#COMPILATION_UNIT COMPILATION_UNIT}. + *

+ * Since 0.137, comments in the rewritten node will be automatically assigned to the newTree + * node. Use {@link TreeMaker#asRemoved} to discard comments from the oldTree explicitly. * * * @param oldTree tree to be replaced, use tree already represented in @@ -239,6 +253,7 @@ * method. * @see GeneratorUtilities#createFromTemplate * @see TreeMaker + * @since 0.137 */ public synchronized void rewrite(@NullAllowed Tree oldTree, @NonNull Tree newTree) { checkConfinement(); @@ -255,6 +270,11 @@ } if (oldTree == null || newTree == null) throw new IllegalArgumentException("Null values are not allowed."); + Tree t = rewriteHints.get(oldTree); + if (t == null) { + // if the old tree does not have any association to a new generated node, make an implicit association + associateTree(newTree, oldTree, false); + } // Perf: trees are collected just when asserts are enabled. assert new TreeCollector(introducedTrees, true).scan(newTree, null) != null; changes.put(oldTree, newTree); @@ -433,6 +453,47 @@ // Package private methods ------------------------------------------------- +// static final Collection NOT_LINKED = new ArrayList(0); + static final Tree NOT_LINKED = new Tree() { + + @Override + public Kind getKind() { + return Kind.OTHER; + } + + @Override + public R accept(TreeVisitor visitor, D data) { + return visitor.visitOther(this, data); + } + }; + + /** + * Associates a new tree with the original. Only one association is supported, + * the last association wins. + * + * @param nue the new tree + * @param original the to-be-replaced original + */ + synchronized void associateTree(Tree nue, Tree original, boolean force) { + if (rewriteHints == null) { + rewriteHints = new HashMap(7); + } + + if (original == null) { + Tree ex = rewriteHints.get(nue); + if (ex == null || force) { + rewriteHints.put(nue, NOT_LINKED); + } + } else if (original == nue) { + return; + } else { + Tree ex = rewriteHints.get(original); + if (ex == null || force) { + rewriteHints.put(original, nue); + } + } + } + private static String codeForCompilationUnit(CompilationUnitTree topLevel) throws IOException { return ((JCTree.JCCompilationUnit) topLevel).sourcefile.getCharContent(true).toString(); } @@ -500,6 +561,7 @@ Map userInfo = new HashMap(); final Set oldTrees = new HashSet(); + final Map presentInResult = new IdentityHashMap(); if (CasualDiff.OLD_TREES_VERBATIM) { new TreeScanner() { private boolean synthetic = false; @@ -552,6 +614,7 @@ @Override public Void scan(Tree node, Void p) { addSyntheticTrees(diffContext, node); + addPresentInResult(presentInResult, node, true); return super.scan(node, p); } }.scan(diffContext.origUnit, null); @@ -603,18 +666,29 @@ parent2Rewrites.put(currentParent, new IdentityHashMap()); } } - if(changes.containsKey(tree)) { + Tree rev = changes.get(tree); + Tree hint = resolveRewriteHint(tree); + if(rev != null) { Map rewrites = parent2Rewrites.get(currentParent); + + changes.remove(tree); - Tree rev = changes.remove(tree); - + if (hint == null) { + addPresentInResult(presentInResult, rev, false); + } + //presentInResult.remove(tree); rewrites.put(tree, rev); scan(rev, p); } else { + if (hint == null) { + addPresentInResult(presentInResult, rev, false); + } + addPresentInResult(presentInResult, tree, true); super.scan(tree, p); } } else { + addPresentInResult(presentInResult, tree, true); super.scan(tree, p); } if (currentParent != null && currentParent.getLeaf() == tree) { @@ -763,6 +837,7 @@ //tagging debug //System.err.println("brandNew=" + brandNew); + new CommentReplicator(presentInResult.keySet()).scan(diffContext.origUnit, null); for (ClassTree ct : classes) { ia.classLeft(); @@ -798,6 +873,161 @@ } } + private void addPresentInResult(Map present, Tree t, boolean mark) { + present.put(t, Boolean.valueOf(mark)); + CommentSetImpl csi = CommentHandlerService.instance(impl.getJavacTask().getContext()).getComments(t); + for (RelativePosition pos : RelativePosition.values()) { + useComments(csi.getComments(pos)); + } + } + + private Set usedComments; + + /* package-private */ List useComments(List comments) { + if (usedComments == null) { + usedComments = new HashSet<>(); + } + usedComments.addAll(comments); + return comments; + } + + /** + * Copies comments according to rewrite hints:

    + *
  • copies comments from a tree to an associated tree + *
  • + */ + private class CommentReplicator extends TreePathScanner { + private final Set stillPresent; + + private boolean collectCommentsFromRemovedNodes; + private Map copyTo = new IdentityHashMap(); + private final CommentHandlerService commentHandler; + private Tree parentToCopy; + + CommentReplicator(Set presentNodes) { + this.stillPresent = presentNodes; + this.commentHandler = CommentHandlerService.instance(impl.getJavacTask().getContext()); + } + + private Object scanAndReduce(Tree node, Object p, Object r) { + return reduce(scan(node, p), r); + } + + private RelativePosition collectToPosition; + + /** Scan a list of nodes. + */ + @Override + public Object scan(Iterable nodes, Object p) { + Object r = null; + if (nodes != null) { + boolean first = true; + Tree saveParent = parentToCopy; + RelativePosition savePosition = this.collectToPosition; + collectToPosition = RelativePosition.INNER; + if (collectCommentsFromRemovedNodes) { + // find first such node that it either survived, or has a mapping to the new state. Join all + // comments to PRECEDING of such node. + for (Tree node : nodes) { + Tree target = resolveRewriteHint(node); + if (target != null) { + if (target != NOT_LINKED) { + parentToCopy = target; + collectToPosition = RelativePosition.PRECEDING; + break; + } + } else if (stillPresent.contains(node)) { + parentToCopy = node; + collectToPosition = RelativePosition.PRECEDING; + break; + } + } + } + boolean reset = false; + for (Tree node : nodes) { + if (collectCommentsFromRemovedNodes) { + Tree target = resolveRewriteHint(node); + if (target != null && target != NOT_LINKED) { + parentToCopy = target; + this.collectToPosition = RelativePosition.INNER; + reset = true; + } else if (stillPresent.contains(node)) { + parentToCopy = node; + this.collectToPosition = RelativePosition.INNER; + reset = true; + } + } + r = (first ? scan(node, p) : scanAndReduce(node, p, r)); + first = false; + // reset to trailing, output goes to the anchor node. + if (reset) { + this.collectToPosition = RelativePosition.TRAILING; + } + } + parentToCopy = saveParent; + collectToPosition = savePosition; + } + return r; + } + + @Override + public Object scan(Tree l, Object p) { + boolean collectChildren = false; + Tree newParentCopy = null; + + boolean saveCollect = this.collectCommentsFromRemovedNodes; + Tree target = resolveRewriteHint(l); + if (target == NOT_LINKED) { + // do not copy anything from this node and its children + collectChildren = false; + } else if (target != null) { + if (!commentHandler.getComments(target).hasComments()) { + if (!stillPresent.contains(l)) { + commentHandler.copyComments(l, target, null, usedComments); + newParentCopy = target; + collectChildren = true; + } + } + } else if (!stillPresent.contains(l)) { // target == null, node removed + collectChildren = collectCommentsFromRemovedNodes; + if (collectCommentsFromRemovedNodes) { + if (parentToCopy != null) { + commentHandler.copyComments(l, parentToCopy, collectToPosition, usedComments); + } + } + } + if (stillPresent.contains(l)) { + newParentCopy = l; + } + Tree saveParent = parentToCopy; + this.collectCommentsFromRemovedNodes = collectChildren; + if (newParentCopy != null) { + parentToCopy = newParentCopy; + } + Object v = super.scan(l, p); + this.parentToCopy = saveParent; + this.collectCommentsFromRemovedNodes = saveCollect; + return v; + } + + } + + private Tree resolveRewriteHint(Tree orig) { + Tree last; + Tree target = null; + Tree from = orig; + do { + last = target; + target = from; + target = rewriteHints.get(target); + if (target == NOT_LINKED) { + return target; + } + from = target; + } while (target != null); + return last; + } + private List processExternalCUs(Map tag2Span, Set syntheticTrees) { if (externalChanges == null) { return Collections.emptyList(); diff --git a/java.source/src/org/netbeans/modules/java/source/builder/CommentHandlerService.java b/java.source/src/org/netbeans/modules/java/source/builder/CommentHandlerService.java --- a/java.source/src/org/netbeans/modules/java/source/builder/CommentHandlerService.java +++ b/java.source/src/org/netbeans/modules/java/source/builder/CommentHandlerService.java @@ -108,6 +108,10 @@ * appending the new entries to the existing comment lists. */ public void copyComments(Tree fromTree, Tree toTree) { + copyComments(fromTree, toTree, null, null); + } + + public void copyComments(Tree fromTree, Tree toTree, RelativePosition copyToPos, Collection copied) { if (fromTree == toTree) { return; } @@ -119,7 +123,12 @@ map.put(toTree, to = new CommentSetImpl()); } for (RelativePosition pos : RelativePosition.values()) { - to.addComments(pos, from.getComments(pos)); + for (Comment c : from.getComments(pos)) { + if (copied != null && copied.contains(c)) { + continue; + } + to.addComment(copyToPos == null ? pos : copyToPos, c, true); + } } } } diff --git a/java.source/src/org/netbeans/modules/java/source/pretty/VeryPretty.java b/java.source/src/org/netbeans/modules/java/source/pretty/VeryPretty.java --- a/java.source/src/org/netbeans/modules/java/source/pretty/VeryPretty.java +++ b/java.source/src/org/netbeans/modules/java/source/pretty/VeryPretty.java @@ -394,7 +394,9 @@ return o1[0] - o2[0]; } }); + private boolean commentsEnabled; private final Set trailingCommentsHandled = Collections.newSetFromMap(new IdentityHashMap()); + private final Set innerCommentsHandled = Collections.newSetFromMap(new IdentityHashMap()); private void doAccept(JCTree t, boolean printComments/*XXX: should ideally always print comments?*/) { if (!handlePossibleOldTrees(Collections.singletonList(t), printComments)) { if (printComments) printPrecedingComments(t, true); @@ -421,7 +423,10 @@ } } } else { + boolean saveComments = this.commentsEnabled; + this.commentsEnabled = printComments; t.accept(this); + this.commentsEnabled = saveComments; } int end = toString().length(); @@ -432,7 +437,10 @@ tag2Span.put(tag, new int[]{start + initialOffset, end + initialOffset}); } - if (printComments) printTrailingComments(t, true); + if (printComments) { + printInnerCommentsAsTrailing(t, true); + printTrailingComments(t, true); + } } } @@ -488,7 +496,7 @@ return true; } - private void doPrintOriginalTree(java.util.List toPrint, boolean includeComments) { + private void doPrintOriginalTree(java.util.List toPrint, final boolean includeComments) { if (out.isWhitespaceLine()) toLeftMargin(); JCTree firstTree = toPrint.get(0); @@ -512,8 +520,10 @@ public Void scan(Tree node, Void p) { if (node != null) { CommentSetImpl old = comments.getComments(node); - realEnd[0] = Math.max(realEnd[0], Math.max(CasualDiff.commentEnd(old, CommentSet.RelativePosition.INLINE), CasualDiff.commentEnd(old, CommentSet.RelativePosition.TRAILING))); - trailingCommentsHandled.add(node); + if (includeComments) { + realEnd[0] = Math.max(realEnd[0], Math.max(CasualDiff.commentEnd(old, CommentSet.RelativePosition.INLINE), CasualDiff.commentEnd(old, CommentSet.RelativePosition.TRAILING))); + trailingCommentsHandled.add(node); + } Object tag = tree2Tag != null ? tree2Tag.get(node) : null; @@ -2579,7 +2589,7 @@ } else { int prevPrec = this.prec; this.prec = prec; - doAccept(tree, false); + doAccept(tree, commentsEnabled); this.prec = prevPrec; } } @@ -2633,10 +2643,13 @@ if (col) { toColExactly(out.leftMargin); } - printPrecedingComments(tree, !member); + // because of comment duplication +// printPrecedingComments(tree, !member); + printInnerCommentsAsTrailing(tree, !member); printExpr(tree, TreeInfo.notExpression); int tag = tree.getTag().ordinal();//XXX: comparing ordinals!!! if(JCTree.Tag.APPLY.ordinal()<=tag && tag<=JCTree.Tag.MOD_ASG.ordinal()) print(';'); + printTrailingComments(tree, !member); blankLines(tree, false); if (nl) { @@ -2761,6 +2774,11 @@ if (emptyBlock) { printEmptyBlockComments(tree, members); } else { + innerCommentsHandled.add(tree); + java.util.List comments = commentHandler.getComments(tree).getComments(CommentSet.RelativePosition.INNER); + for (Comment c : comments) { + printComment(c, false, members); + } if (members) blankLines(enclClassName.isEmpty() ? cs.getBlankLinesAfterAnonymousClassHeader() : cs.getBlankLinesAfterClassHeader()); else @@ -2788,8 +2806,13 @@ print('>'); } } + + private Set precedingCommentsHandled = new HashSet(); private void printPrecedingComments(JCTree tree, boolean printWhitespace) { + if (!precedingCommentsHandled.add(tree)) { + return; + } CommentSet commentSet = commentHandler.getComments(tree); java.util.List pc = commentSet.getComments(CommentSet.RelativePosition.PRECEDING); DocCommentTree doc = tree2Doc.get(tree); @@ -2814,20 +2837,32 @@ } } + private void printInnerCommentsAsTrailing(JCTree tree, boolean printWhitespace) { + if (innerCommentsHandled.contains(tree)) return ; + CommentSet commentSet = commentHandler.getComments(tree); + java.util.List cl = commentSet.getComments(CommentSet.RelativePosition.INNER); + innerCommentsHandled.add(tree); + for (Comment comment : cl) { + printComment(comment, true, printWhitespace); + } + } + private void printTrailingComments(JCTree tree, boolean printWhitespace) { if (trailingCommentsHandled.contains(tree)) return ; CommentSet commentSet = commentHandler.getComments(tree); java.util.List cl = commentSet.getComments(CommentSet.RelativePosition.INLINE); for (Comment comment : cl) { + trailingCommentsHandled.add(tree); printComment(comment, true, printWhitespace); } java.util.List tc = commentSet.getComments(CommentSet.RelativePosition.TRAILING); if (!tc.isEmpty()) { + trailingCommentsHandled.add(tree); for (Comment c : tc) printComment(c, false, printWhitespace); } } - + private void printEmptyBlockComments(JCTree tree, boolean printWhitespace) { // LinkedList comments = new LinkedList(); // if (cInfo != null) { @@ -2869,6 +2904,7 @@ // } // } // } + innerCommentsHandled.add(tree); java.util.List comments = commentHandler.getComments(tree).getComments(CommentSet.RelativePosition.INNER); for (Comment c : comments) printComment(c, false, printWhitespace); diff --git a/java.source/src/org/netbeans/modules/java/source/save/CasualDiff.java b/java.source/src/org/netbeans/modules/java/source/save/CasualDiff.java --- a/java.source/src/org/netbeans/modules/java/source/save/CasualDiff.java +++ b/java.source/src/org/netbeans/modules/java/source/save/CasualDiff.java @@ -1341,6 +1341,7 @@ diffContext ); int old = printer.indent(); + localPointer = diffInnerComments(oldT, newT, localPointer); Name oldEnclosing = printer.enclClassName; printer.enclClassName = null; List oldstats = filterHidden(oldT.stats); @@ -1872,7 +1873,11 @@ } if (!listsMatch(oldT.args, newT.args)) { if (oldT.args.nonEmpty()) { - copyTo(localPointer, localPointer = getCommentCorrectedOldPos(oldT.args.head)); + int startArg1 = getCommentCorrectedOldPos(oldT.args.head); + tokenSequence.move(startArg1); + moveToSrcRelevant(tokenSequence, Direction.BACKWARD); + tokenSequence.moveNext(); + copyTo(localPointer, localPointer = tokenSequence.offset()); } else { copyTo(localPointer, localPointer = methBounds[1]); tokenSequence.move(localPointer); @@ -1998,7 +2003,7 @@ protected int diffParens(JCParens oldT, JCParens newT, int[] bounds) { int localPointer = bounds[0]; - copyTo(localPointer, getOldPos(oldT.expr)); + copyTo(localPointer, getCommentCorrectedOldPos(oldT.expr)); localPointer = diffTree(oldT.expr, newT.expr, getBounds(oldT.expr)); copyTo(localPointer, bounds[1]); return bounds[1]; @@ -3094,11 +3099,19 @@ JCTree tree = oldList.get(oldIndex++); int[] bounds = getCommentCorrectedBounds(tree); tokenSequence.move(bounds[0]); + int start = -1; if (oldIndex != 1 && !separator.isEmpty()) { - moveToSrcRelevant(tokenSequence, Direction.BACKWARD); + if (wasLeadingDelete) { + start = Math.max(offsetToSrcWiteOnLine(tokenSequence, Direction.BACKWARD), pos); + } else { + moveToSrcRelevant(tokenSequence, Direction.BACKWARD); + } } - tokenSequence.moveNext(); - int start = Math.max(tokenSequence.offset(), pos); + if (start == -1) { + 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); @@ -3137,7 +3150,7 @@ moveToSrcRelevant(tokenSequence, Direction.FORWARD); if (tokenSequence.token().id() == JavaTokenId.COMMA) { if (tokenSequence.moveNext()) { - moveToSrcRelevant(tokenSequence, Direction.FORWARD); +// moveToSrcRelevant(tokenSequence, Direction.FORWARD); } } pos = Math.max(tokenSequence.offset(), endPos); @@ -3151,11 +3164,18 @@ } int[] bounds = getCommentCorrectedBounds(item.element); tokenSequence.move(bounds[0]); - if (oldIndex != 1 && !wasLeadingDelete && !separator.isEmpty()) { - moveToSrcRelevant(tokenSequence, Direction.BACKWARD); + int start = -1; + if (oldIndex != 1 && !separator.isEmpty()) { + if (wasLeadingDelete) { + start = offsetToSrcWiteOnLine(tokenSequence, Direction.BACKWARD); + } else { + moveToSrcRelevant(tokenSequence, Direction.BACKWARD); + } } - tokenSequence.moveNext(); - int start = tokenSequence.offset(); + if (start == -1) { + tokenSequence.moveNext(); + start = tokenSequence.offset(); + } tokenSequence.move(bounds[1]); moveToSrcRelevant(tokenSequence, Direction.FORWARD); int end; @@ -3718,6 +3738,24 @@ return comments.getComments(t); } + protected int diffInnerComments(JCTree oldT, JCTree newT, int localPointer) { + CommentSet cs = getCommentsForTree(newT, true); + CommentSet old = getCommentsForTree(oldT, true); + List oldPrecedingComments = old.getComments(CommentSet.RelativePosition.INNER); + List newPrecedingComments = cs.getComments(CommentSet.RelativePosition.INNER); + if (sameComments(oldPrecedingComments, newPrecedingComments)) { + if (oldPrecedingComments.isEmpty()) { + return localPointer; + } + int newP = oldPrecedingComments.get(oldPrecedingComments.size() - 1).endPos(); + copyTo(localPointer, newP); + return newP; + } + return diffCommentLists(-1, oldPrecedingComments, newPrecedingComments, null, null, true, false, true, + false, + localPointer); + } + protected int diffPrecedingComments(JCTree oldT, JCTree newT, int oldTreeStartPos, int localPointer, boolean doNotDelete) { CommentSet cs = getCommentsForTree(newT, true); CommentSet old = getCommentsForTree(oldT, true); @@ -3728,23 +3766,36 @@ if (oldPrecedingComments.isEmpty()) { return localPointer; } - return -oldPrecedingComments.get(oldPrecedingComments.size() - 1).endPos(); + int newP = oldPrecedingComments.get(oldPrecedingComments.size() - 1).endPos(); + if (newP > localPointer) { + copyTo(localPointer, newP); + return newP; + } else { + return localPointer; + } + } DocCommentTree oldD = oldTopLevel.docComments.getCommentTree(oldT); - return diffCommentLists(oldTreeStartPos, oldPrecedingComments, newPrecedingComments, oldD, newD, false, true, + return diffCommentLists(oldTreeStartPos, oldPrecedingComments, newPrecedingComments, oldD, newD, false, true, false, doNotDelete, localPointer); } - protected int diffTrailingComments(JCTree oldT, JCTree newT, int localPointer) { + protected int diffTrailingComments(JCTree oldT, JCTree newT, int localPointer, int elementEndWithComments) { CommentSet cs = getCommentsForTree(newT, false); CommentSet old = getCommentsForTree(oldT, false); List oldInlineComments = old.getComments(CommentSet.RelativePosition.INLINE); List newInlineComments = cs.getComments(CommentSet.RelativePosition.INLINE); List oldTrailingComments = old.getComments(CommentSet.RelativePosition.TRAILING); List newTrailingComments = cs.getComments(CommentSet.RelativePosition.TRAILING); - if (sameComments(oldInlineComments, newInlineComments) && sameComments(oldTrailingComments, newTrailingComments)) + if (sameComments(oldInlineComments, newInlineComments) && sameComments(oldTrailingComments, newTrailingComments)) { + // copy the comments + if (oldInlineComments.isEmpty() && oldTrailingComments.isEmpty()) { + return localPointer; + } + copyTo(localPointer, localPointer = elementEndWithComments); return localPointer; + } //XXX: hack: the upper diff might already add '\n' to the result, need to skip it if diffing inline comments if (!sameComments(oldInlineComments, newInlineComments)) { @@ -3752,7 +3803,7 @@ printer.eatChars(1); } - localPointer = diffCommentLists(getOldPos(oldT), oldInlineComments, newInlineComments, null, null, false, false, false, localPointer); + localPointer = diffCommentLists(getOldPos(oldT), oldInlineComments, newInlineComments, null, null, false, false, false, false, localPointer); boolean containedEmbeddedNewLine = false; boolean containsEmbeddedNewLine = false; @@ -3769,7 +3820,7 @@ printer.print("\n"); } - return diffCommentLists(getOldPos(oldT), oldTrailingComments, newTrailingComments, null, null, true, false, false, localPointer); + return diffCommentLists(getOldPos(oldT), oldTrailingComments, newTrailingComments, null, null, true, false, false, false, localPointer); } private boolean sameComments(List oldList, List newList) { @@ -3789,7 +3840,7 @@ // refactor it! make it better private int diffCommentLists(int oldTreeStartPos, List oldList, - List newList, DocCommentTree oldDoc, DocCommentTree newDoc, boolean trailing, boolean preceding, + List newList, DocCommentTree oldDoc, DocCommentTree newDoc, boolean trailing, boolean preceding, boolean inner, boolean doNotDeleteIfMissing, int localPointer) { Comment javadoc = null; @@ -3865,7 +3916,7 @@ printer.printComment(newC, !trailing, false, !preceding && !trailing); firstNewCommentPrinted = true; } - newC = safeNext(oldIter); + newC = safeNext(newIter); } if(preceding && javadoc == null && newDoc != null) { if (!firstNewCommentPrinted && preceding) { @@ -4721,11 +4772,11 @@ int predComments = diffPrecedingComments(oldT, newT, getOldPos(oldT), elementBounds[0], oldT.getTag() == Tag.TOPLEVEL && diffContext.forceInitialComment); int retVal = -1; - if (predComments < 0 && elementBounds[0] < -predComments) { - copyTo(elementBounds[0], -predComments); - } +// if (predComments < 0 && elementBounds[0] < -predComments) { +// copyTo(elementBounds[0], -predComments); +// } elementBounds[0] = Math.abs(predComments); - + int elementEnd = elementBounds[1]; int commentsStart = Math.min(commentStart(diffContext, comments.getComments(oldT), CommentSet.RelativePosition.INLINE, endPos(oldT)), commentStart(diffContext, comments.getComments(oldT), CommentSet.RelativePosition.TRAILING, endPos(oldT))); if (commentsStart < elementBounds[1]) { int lastIndex; @@ -4950,7 +5001,7 @@ if (handleImplicitLambda) { printer.print(")"); } - return diffTrailingComments(oldT, newT, retVal); + return diffTrailingComments(oldT, newT, retVal, elementEnd); } /** diff --git a/java.source/src/org/netbeans/modules/java/source/save/PositionEstimator.java b/java.source/src/org/netbeans/modules/java/source/save/PositionEstimator.java --- a/java.source/src/org/netbeans/modules/java/source/save/PositionEstimator.java +++ b/java.source/src/org/netbeans/modules/java/source/save/PositionEstimator.java @@ -1666,6 +1666,58 @@ return moveToDifferentThan(seq, dir, nonRelevant); } + /** + * + * @param seq the token sequence + * @param dir direction + * @return + */ + public static int offsetToSrcWiteOnLine(TokenSequence seq, + Direction dir) + { + boolean notBound = false; + seq.moveNext(); + int savePos = seq.offset(); + switch (dir) { + case BACKWARD: + while ((notBound = seq.movePrevious())) { + JavaTokenId tid = seq.token().id(); + if (tid == WHITESPACE) { + int nl = seq.token().text().toString().indexOf('\n'); + if (nl > -1) { + // return the position after newline: + return seq.offset() + nl + 1; + } + } else if (!nonRelevant.contains(tid)) { + break; + } else { + savePos = seq.offset(); + } + } + break; + case FORWARD: + while ((notBound = seq.moveNext())) { + JavaTokenId tid = seq.token().id(); + if (tid == WHITESPACE) { + int nl = seq.token().text().toString().indexOf('\n'); + if (nl > -1) { + // return the position after newline: + return seq.offset() + nl; + } + } else if (!nonRelevant.contains(tid)) { + break; + } else { + savePos = seq.offset() + seq.token().length(); + } + } + break; + } + if (!notBound) { + return -1; + } + return savePos; + } + @SuppressWarnings("empty-statement") public static JavaTokenId moveToDifferentThan( TokenSequence seq, diff --git a/java.source/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java b/java.source/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java --- a/java.source/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java +++ b/java.source/src/org/netbeans/modules/java/source/transform/ImmutableTreeTranslator.java @@ -45,8 +45,6 @@ package org.netbeans.modules.java.source.transform; -import javax.lang.model.util.Elements; -import org.netbeans.modules.java.source.query.CommentHandler; import com.sun.source.tree.*; import com.sun.source.tree.Tree.Kind; @@ -54,21 +52,23 @@ import com.sun.tools.javac.model.JavacElements; import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.util.Context; -import javax.lang.model.element.Element; - import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; +import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.QualifiedNameable; +import javax.lang.model.util.Elements; import org.netbeans.api.java.source.GeneratorUtilities; +import org.netbeans.api.java.source.TreeMaker; import org.netbeans.api.java.source.WorkingCopy; import org.netbeans.modules.java.source.builder.ASTService; import org.netbeans.modules.java.source.builder.CommentHandlerService; import org.netbeans.modules.java.source.builder.QualIdentTree; import org.netbeans.modules.java.source.builder.TreeFactory; import org.netbeans.modules.java.source.pretty.ImportAnalysis2; +import org.netbeans.modules.java.source.query.CommentHandler; import org.netbeans.modules.java.source.save.ElementOverlay; import static org.netbeans.modules.java.source.save.PositionEstimator.*; @@ -90,6 +90,9 @@ *
      return new_tree_to_replace_old *
      // (returning the original tree leaves it unchanged) *
    } + *

    + * To help code formatting and comment preservation, translated nodes are marked + * as replacements for their originals. */ public class ImmutableTreeTranslator implements TreeVisitor { @@ -102,6 +105,10 @@ private ElementOverlay overlay; private ImportAnalysis2 importAnalysis; private Map tree2Tag; + /** + * Newly created nodes will be mareked as replacements for the old ones + */ + private TreeMaker tmaker; private WorkingCopy copy; @@ -115,6 +122,7 @@ comments = CommentHandlerService.instance(context); model = ASTService.instance(context); overlay = context.get(ElementOverlay.class); + tmaker = copy.getTreeMaker(); this.importAnalysis = importAnalysis; this.tree2Tag = tree2Tag; } @@ -136,6 +144,7 @@ Tree t = tree.accept(this, null); if (tree2Tag != null && tree != t) { + t = tmaker.asReplacementOf(t, tree, true); tree2Tag.put(t, tree2Tag.get(tree)); } diff --git a/java.source/test/unit/src/org/netbeans/api/java/source/gen/CommentsTest.java b/java.source/test/unit/src/org/netbeans/api/java/source/gen/CommentsTest.java --- a/java.source/test/unit/src/org/netbeans/api/java/source/gen/CommentsTest.java +++ b/java.source/test/unit/src/org/netbeans/api/java/source/gen/CommentsTest.java @@ -1218,7 +1218,7 @@ " * f\n" + " */\n" + " int f;\n" + - "\n" + + " \n" + " /**\n" + " * run\n" + " */\n" + @@ -1340,6 +1340,10 @@ "import java.io.FileNotFoundException;\n" + "public abstract class Test {\n" + " public void test() {\n" + + " //t1\n" + + " //t2\n" + + " //t3\n" + + " \n" + " name();\n" + " }\n" + "}\n"; @@ -1391,6 +1395,11 @@ "import java.io.FileNotFoundException;\n" + "public abstract class Test {\n" + " public void test() {\n" + + " //t1\n" + + " //t2\n" + + " //t3\n" + + " //t4\n" + + " \n" + " name();\n" + " }\n" + "}\n"; @@ -1442,6 +1451,12 @@ "import java.io.FileNotFoundException;\n" + "public abstract class Test {\n" + " public void test() {\n" + + " //t1\n" + + " //t2\n" + + " //test1\n" + + " //t3\n" + + " //t4\n" + + " \n" + " name();\n" + " }\n" + "}\n"; diff --git a/java.source/test/unit/src/org/netbeans/api/java/source/gen/Method1Test.java b/java.source/test/unit/src/org/netbeans/api/java/source/gen/Method1Test.java --- a/java.source/test/unit/src/org/netbeans/api/java/source/gen/Method1Test.java +++ b/java.source/test/unit/src/org/netbeans/api/java/source/gen/Method1Test.java @@ -70,6 +70,7 @@ public static NbTestSuite suite() { NbTestSuite suite = new NbTestSuite(); + suite.addTest(new Method1Test("testMethodParametersWithComments")); suite.addTest(new Method1Test("testMethodModifiers")); suite.addTest(new Method1Test("testMethodName")); suite.addTest(new Method1Test("testMethodParameters")); @@ -421,7 +422,7 @@ " return x + y;\n" + " }\n" + " void m2() {\n" + - " plus( /*bar*/ plus(2, 3), /*foo*/ 1 );\n" + + " plus(/*bar*/ plus(2, 3), /*foo*/ 1 );\n" + " }\n" + "}"; testFile = new File(getWorkDir(), "Test.java"); @@ -661,6 +662,54 @@ // }); // assertFiles("testAddRemoveInOneTrans.pass"); // } + + public void testMethodParametersWithComments() throws Exception { + // XXX should also test annotations + // XXX expected whitespace might not be correct in golden + String test = + "class Test {\n" + + " void m() {\n" + + " plus(|/*foo*/ Math.abs(1), /*bar*/ Math.abs(2));\n" + + " }\n" + + "}"; + String golden = + "class Test {\n" + + " void m() {\n" + + " plus(/*bar*/ Math.abs(2), /*foo*/ Math.abs(1));\n" + + " }\n" + + "}"; + File file = new File(getWorkDir(), "Test.java"); + final int indexA = test.indexOf("|"); + assertTrue(indexA != -1); + TestUtilities.copyStringToFile(file, test.replace("|", "")); + JavaSource src = getJavaSource(file); + Task task = new Task() { + + public void run(WorkingCopy copy) throws Exception { + if (copy.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0) { + return; + } + Tree node = copy.getTreeUtilities().pathFor(indexA).getLeaf(); + GeneratorUtilities.get(copy).importComments(node, copy.getCompilationUnit()); + assertEquals(Kind.METHOD_INVOCATION, node.getKind()); + TreeMaker make = copy.getTreeMaker(); + MethodInvocationTree original = (MethodInvocationTree) node; + List oldArgs = original.getArguments(); + List newArgs = new ArrayList(); + newArgs.add(oldArgs.get(1)); + newArgs.add(oldArgs.get(0)); + @SuppressWarnings("unchecked") + MethodInvocationTree modified = make.MethodInvocation( + (List) original.getTypeArguments(), + original.getMethodSelect(), newArgs); + System.out.println("original: " + node); + System.out.println("modified: " + modified); + copy.rewrite(node, modified); } + }; + src.runModificationTask(task).commit(); + String res = TestUtilities.copyFileToString(file); + assertEquals(golden, res); + } //////////////////////////////////////////////////////////////////////////// /**