Skip to content

Commit

Permalink
fix: correctly sniper-print type member comments after removal of all…
Browse files Browse the repository at this point in the history
… modifiers (#3747)
  • Loading branch information
slarse authored Jan 13, 2021
1 parent e815c80 commit 498d122
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -306,17 +306,39 @@ 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;
}
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() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
22 changes: 22 additions & 0 deletions src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<CtType<?>> 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<CtType<?>, 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
Expand Down

0 comments on commit 498d122

Please sign in to comment.