Skip to content

Commit

Permalink
StatementSwitchToExpressionSwitch: for return switch transformation, …
Browse files Browse the repository at this point in the history
…retain comments appearing in the switch block before the first case

PiperOrigin-RevId: 695861404
  • Loading branch information
markhbrady authored and Error Prone Team committed Nov 13, 2024
1 parent d4e5a5f commit a9213b6
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.lang.model.element.ElementKind;
Expand Down Expand Up @@ -662,7 +663,12 @@ private static SuggestedFix convertToReturnSwitch(
transformReturnOrThrowBlock(caseTree, state, getStatements(caseTree));

if (firstCaseInGroup) {
groupedCaseCommentsAccumulator = new StringBuilder();
groupedCaseCommentsAccumulator =
caseIndex == 0
? new StringBuilder(
extractCommentsBeforeFirstCase(switchTree, allSwitchComments).orElse(""))
: new StringBuilder();

replacementCodeBuilder.append("\n ");
if (!isDefaultCase) {
replacementCodeBuilder.append("case ");
Expand Down Expand Up @@ -988,6 +994,21 @@ private static int extractLhsComments(
return lhsEnd;
}

/**
* Extracts any comments appearing within the switch tree before the first case. Comments are
* merged into a single string separated by newlines. Precondition: the switch tree has at least
* one case.
*/
private static Optional<String> extractCommentsBeforeFirstCase(
SwitchTree switchTree, ImmutableList<ErrorProneComment> allSwitchComments) {
// Indexing relative to the start position of the switch statement
int switchStart = getStartPosition(switchTree);
int firstCaseStartIndex = getStartPosition(switchTree.getCases().get(0)) - switchStart;

return filterAndRenderComments(
allSwitchComments, comment -> comment.getPos() < firstCaseStartIndex);
}

/**
* Extracts any comments appearing after the specified {@code caseIndex} but before the subsequent
* case or end of the {@code switchTree}. Comments are merged into a single string separated by
Expand All @@ -1009,20 +1030,30 @@ private static Optional<String> extractCommentsAfterCase(
? state.getEndPosition(switchTree) - switchStart
: getStartPosition(switchTree.getCases().get(caseIndex + 1)) - switchStart;

String filteredComments =
allSwitchComments.stream()
// Comments after the end of the current case and before the start of the next case
.filter(
comment ->
comment.getPos() >= caseEndIndex && comment.getPos() < nextCaseStartIndex)
return filterAndRenderComments(
allSwitchComments,
comment -> comment.getPos() >= caseEndIndex && comment.getPos() < nextCaseStartIndex);
}

/**
* Filters comments according to the supplied predicate ({@code commentFilter}), removes
* fall-through and empty comments, and renders them into a single optional string. If no comments
* remain, returns {@code Optional.empty()}.
*/
private static Optional<String> filterAndRenderComments(
ImmutableList<ErrorProneComment> comments, Predicate<ErrorProneComment> commentFilter) {

String rendered =
comments.stream()
.filter(commentFilter)
.map(ErrorProneComment::getText)
// Remove "fall thru" comments
.map(commentText -> removeFallThruLines(commentText))
// Remove empty comments
.filter(commentText -> !commentText.isEmpty())
.collect(joining("\n"));

return filteredComments.isEmpty() ? Optional.empty() : Optional.of(filteredComments);
return rendered.isEmpty() ? Optional.empty() : Optional.of(rendered);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,86 @@ public int foo(Side side) {
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

@Test
public void switchByEnum_returnSwitchCommentsBeforeFirstCase_errorAndRetained() {
assume().that(Runtime.version().feature()).isAtLeast(14);

// Check correct generated code
refactoringHelper
.addInputLines(
"Test.java",
"""
class Test {
enum Side {
HEART,
SPADE,
DIAMOND,
CLUB
};
public Test(int foo) {}
public int invoke() {
return 123;
}
public int foo(Side side) {
switch (side) {
// Abracadabra
/* foo */ case HEART:
// Card trick
case DIAMOND:
return invoke();
case SPADE:
throw new RuntimeException();
case CLUB:
throw new NullPointerException();
}
// This should never happen
int z = invoke();
z++;
throw new RuntimeException("Switch was not exhaustive at runtime " + z);
}
}
""")
.addOutputLines(
"Test.java",
"""
class Test {
enum Side {
HEART,
SPADE,
DIAMOND,
CLUB
};
public Test(int foo) {}
public int invoke() {
return 123;
}
public int foo(Side side) {
return switch (side) {
case HEART, DIAMOND ->
// Abracadabra
/* foo */
// Card trick
invoke();
case SPADE -> throw new RuntimeException();
case CLUB -> throw new NullPointerException();
};
// This should never happen
}
}
""")
.setArgs(
ImmutableList.of(
"-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion"))
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

@Test
public void switchByEnum_switchInReturnSwitchWithShouldNeverHappen_error() {
assume().that(Runtime.version().feature()).isAtLeast(14);
Expand Down

0 comments on commit a9213b6

Please sign in to comment.