From 498d1228fea209ef9d5b9e25d8739498109ed178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 13 Jan 2021 12:51:29 +0100 Subject: [PATCH] fix: correctly sniper-print type member comments after removal of all modifiers (#3747) --- .../AbstractSourceFragmentPrinter.java | 28 +++++++++++++++++-- .../internal/ElementSourceFragment.java | 7 +++++ .../test/prettyprinter/TestSniperPrinter.java | 22 +++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java b/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java index 5dead371257..8619151b319 100644 --- a/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java +++ b/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java @@ -58,7 +58,7 @@ public void print(PrinterEvent event) { if (index != -1) { // means we have found a source code fragment corresponding to this event // we print all spaces and comments before this fragment - printSpaces(getLastNonSpaceNonCommentBefore(index), index); + printSpaces(getLastNonSpaceNonCommentBefore(index, prevIndex), index); SourceFragment fragment = childFragments.get(index); event.printSourceFragment(fragment, isFragmentModified(fragment)); @@ -306,10 +306,12 @@ protected void printStandardSpaces() { separatorActions.clear(); } - private int getLastNonSpaceNonCommentBefore(int index) { + private int getLastNonSpaceNonCommentBefore(int index, int prevIndex) { for (int i = index - 1; i >= 0; i--) { SourceFragment fragment = childFragments.get(i); - if (isSpaceFragment(fragment) || isCommentFragment(fragment)) { + if (isSpaceFragment(fragment) + || isCommentFragment(fragment) + || isRecentlySkippedModifierCollectionFragment(i, prevIndex)) { continue; } return i + 1; @@ -317,6 +319,26 @@ private int getLastNonSpaceNonCommentBefore(int index) { return 0; } + /** + * Determines if the fragment at index is a "recently skipped" collection fragment + * containing modifiers. "Recently skipped" entails that the modifier fragment has not been + * printed, and that the last printed fragment occurs before the modifier fragment. + * + * It is necessary to detect such fragments as whitespace and comments may otherwise be lost + * when completely removing modifier lists. See issue #3732 for details. + * + * @param index Index of the fragment. + * @param prevIndex Index of the last printed fragment. + * @return true if the fragment is a recently skipped collection fragment with modifiers. + */ + private boolean isRecentlySkippedModifierCollectionFragment(int index, int prevIndex) { + SourceFragment fragment = childFragments.get(index); + return prevIndex < index + && fragment instanceof CollectionSourceFragment + && ((CollectionSourceFragment) fragment).getItems().stream() + .anyMatch(ElementSourceFragment::isModifierFragment); + } + @Override public void onPush() { } diff --git a/src/main/java/spoon/support/sniper/internal/ElementSourceFragment.java b/src/main/java/spoon/support/sniper/internal/ElementSourceFragment.java index c2a723df958..76db601a5b9 100644 --- a/src/main/java/spoon/support/sniper/internal/ElementSourceFragment.java +++ b/src/main/java/spoon/support/sniper/internal/ElementSourceFragment.java @@ -910,4 +910,11 @@ static boolean isCommentFragment(SourceFragment fragment) { return fragment instanceof ElementSourceFragment && ((ElementSourceFragment) fragment).getElement() instanceof CtComment; } + /** + * @return true if {@link SourceFragment} represents a modifier + */ + static boolean isModifierFragment(SourceFragment fragment) { + return fragment instanceof ElementSourceFragment + && ((ElementSourceFragment) fragment).getRoleInParent() == CtRole.MODIFIER; + } } diff --git a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java index 750cefec1ad..d65ef09f5c8 100644 --- a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java +++ b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java @@ -61,6 +61,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.MatcherAssert.assertThat; @@ -447,6 +448,27 @@ public void testNewlineInsertedBetweenModifiedCommentAndTypeMemberWithAddedModif testSniper("TypeMemberComments", enactModifications, assertCommentCorrectlyPrinted); } + @Test + public void testTypeMemberCommentDoesNotDisappearWhenAllModifiersAreRemoved() { + // contract: A comment on a field should not disappear when all of its modifiers are removed. + + Consumer> removeTypeMemberModifiers = type -> { + type.getField("NON_FINAL_FIELD").setModifiers(Collections.emptySet()); + type.getMethodsByName("nonStaticMethod").get(0).setModifiers(Collections.emptySet()); + type.getNestedType("NonStaticInnerClass").setModifiers(Collections.emptySet()); + }; + + BiConsumer, String> assertFieldCommentPrinted = (type, result) -> + assertThat(result, allOf( + containsString("// field comment\n int NON_FINAL_FIELD"), + containsString("// method comment\n void nonStaticMethod"), + containsString("// nested type comment\n class NonStaticInnerClass") + ) + ); + + testSniper("TypeMemberComments", removeTypeMemberModifiers, assertFieldCommentPrinted); + } + @Test public void testAddedImportStatementPlacedOnSeparateLineInFileWithoutPackageStatement() { assumeNotWindows(); // FIXME Make test case pass on Windows