Skip to content

Commit a19bcbc

Browse files
markhbradyError Prone Team
markhbrady
authored and
Error Prone Team
committed
StatementSwitchToExpressionSwitch: Enhance code comment handling for direct conversion, including retaining comments after the final statement in a case
PiperOrigin-RevId: 653720761
1 parent 5c26d0d commit a19bcbc

2 files changed

Lines changed: 256 additions & 26 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java‎

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static java.util.stream.Collectors.joining;
3434

3535
import com.google.auto.value.AutoValue;
36+
import com.google.common.base.Joiner;
3637
import com.google.common.collect.ImmutableList;
3738
import com.google.common.collect.ImmutableSet;
3839
import com.google.common.collect.Iterables;
@@ -45,6 +46,7 @@
4546
import com.google.errorprone.matchers.Description;
4647
import com.google.errorprone.matchers.Matchers;
4748
import com.google.errorprone.util.ASTHelpers;
49+
import com.google.errorprone.util.ErrorProneComment;
4850
import com.google.errorprone.util.Reachability;
4951
import com.google.errorprone.util.SourceVersion;
5052
import com.sun.source.tree.AssignmentTree;
@@ -513,6 +515,11 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
513515
SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) {
514516

515517
List<? extends CaseTree> cases = switchTree.getCases();
518+
ImmutableList<ErrorProneComment> allSwitchComments =
519+
state.getTokensForNode(switchTree).stream()
520+
.flatMap(errorProneToken -> errorProneToken.comments().stream())
521+
.collect(toImmutableList());
522+
516523
StringBuilder replacementCodeBuilder = new StringBuilder();
517524
replacementCodeBuilder
518525
.append("switch ")
@@ -540,19 +547,49 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
540547
replacementCodeBuilder.append(
541548
isDefaultCase ? "default" : printCaseExpressions(caseTree, state));
542549

550+
Optional<String> commentsAfterCaseOptional =
551+
extractCommentsAfterCase(switchTree, allSwitchComments, state, caseIndex);
543552
if (analysisResult.groupedWithNextCase().get(caseIndex)) {
544553
firstCaseInGroup = false;
545554
replacementCodeBuilder.append(", ");
546555
// Capture comments from this case so they can be added to the group's transformed case
547556
if (!transformedBlockSource.trim().isEmpty()) {
548-
groupedCaseCommentsAccumulator.append(removeFallThruLines(transformedBlockSource));
557+
String commentsToAppend = removeFallThruLines(transformedBlockSource);
558+
if (groupedCaseCommentsAccumulator.length() > 0) {
559+
groupedCaseCommentsAccumulator.append("\n");
560+
}
561+
groupedCaseCommentsAccumulator.append(commentsToAppend);
562+
}
563+
564+
if (commentsAfterCaseOptional.isPresent()) {
565+
if (groupedCaseCommentsAccumulator.length() > 0) {
566+
groupedCaseCommentsAccumulator.append("\n");
567+
}
568+
groupedCaseCommentsAccumulator.append(commentsAfterCaseOptional.get());
549569
}
570+
550571
// Add additional cases to the list on the lhs of the arrow
551572
continue;
552573
} else {
553-
// This case is the last case in its group, so insert the collected comments from the lhs of
554-
// the colon here
555-
transformedBlockSource = groupedCaseCommentsAccumulator + transformedBlockSource;
574+
// Extract comments (if any) preceding break that was removed as redundant
575+
Optional<String> commentsBeforeRemovedBreak =
576+
filteredStatements.isEmpty()
577+
? Optional.empty()
578+
: extractCommentsBeforeRemovedBreak(caseTree, state, filteredStatements);
579+
580+
// Join together all comments and code, separating with newlines
581+
transformedBlockSource =
582+
Joiner.on("\n")
583+
.skipNulls()
584+
.join(
585+
// This case is the last case in its group, so insert any comments from prior
586+
// grouped cases first
587+
groupedCaseCommentsAccumulator.length() == 0
588+
? null
589+
: groupedCaseCommentsAccumulator.toString(),
590+
transformedBlockSource.isEmpty() ? null : transformedBlockSource,
591+
commentsBeforeRemovedBreak.orElse(null),
592+
commentsAfterCaseOptional.orElse(null));
556593
}
557594

558595
replacementCodeBuilder.append(" -> ");
@@ -570,17 +607,10 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
570607
}
571608
} else {
572609
// Transformed block has code
573-
// Extract comments (if any) for break that was removed as redundant
574-
Optional<String> commentsBeforeRemovedBreak =
575-
extractCommentsBeforeRemovedBreak(caseTree, state, filteredStatements);
576-
if (commentsBeforeRemovedBreak.isPresent()) {
577-
transformedBlockSource = transformedBlockSource + "\n" + commentsBeforeRemovedBreak.get();
578-
}
579-
580610
// To improve readability, don't use braces on the rhs if not needed
581611
if (shouldTransformCaseWithoutBraces(filteredStatements)) {
582-
// Single statement with no comments - no braces needed
583-
replacementCodeBuilder.append(transformedBlockSource);
612+
// No braces needed
613+
replacementCodeBuilder.append("\n").append(transformedBlockSource);
584614
} else {
585615
// Use braces on the rhs
586616
replacementCodeBuilder.append("{\n").append(transformedBlockSource).append("\n}");
@@ -599,7 +629,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
599629
/**
600630
* Transforms the supplied statement switch into a {@code return switch ...} style of expression
601631
* switch. In this conversion, each nontrivial statement block is mapped one-to-one to a new
602-
* expression on the right-hand side of the arrow. Comments are presevered where possible.
632+
* expression on the right-hand side of the arrow. Comments are preserved where possible.
603633
* Precondition: the {@code AnalysisResult} for the {@code SwitchTree} must have deduced that this
604634
* conversion is possible.
605635
*/
@@ -903,6 +933,43 @@ private static int extractLhsComments(
903933
return lhsEnd;
904934
}
905935

936+
/**
937+
* Extracts any comments appearing after the specified {@code caseIndex} but before the subsequent
938+
* case or end of the {@code switchTree}. Comments are merged into a single string separated by
939+
* newlines.
940+
*/
941+
private static Optional<String> extractCommentsAfterCase(
942+
SwitchTree switchTree,
943+
ImmutableList<ErrorProneComment> allSwitchComments,
944+
VisitorState state,
945+
int caseIndex) {
946+
947+
// Indexing relative to the start position of the switch statement
948+
int switchStart = getStartPosition(switchTree);
949+
// Invariant: caseEndIndex >= 0
950+
int caseEndIndex = state.getEndPosition(switchTree.getCases().get(caseIndex)) - switchStart;
951+
// Invariant: nextCaseStartIndex >= caseEndIndex
952+
int nextCaseStartIndex =
953+
caseIndex == switchTree.getCases().size() - 1
954+
? state.getEndPosition(switchTree) - switchStart
955+
: getStartPosition(switchTree.getCases().get(caseIndex + 1)) - switchStart;
956+
957+
String filteredComments =
958+
allSwitchComments.stream()
959+
// Comments after the end of the current case and before the start of the next case
960+
.filter(
961+
comment ->
962+
comment.getPos() >= caseEndIndex && comment.getPos() < nextCaseStartIndex)
963+
.map(ErrorProneComment::getText)
964+
// Remove "fall thru" comments
965+
.map(commentText -> removeFallThruLines(commentText))
966+
// Remove empty comments
967+
.filter(commentText -> !commentText.isEmpty())
968+
.collect(joining("\n"));
969+
970+
return filteredComments.isEmpty() ? Optional.empty() : Optional.of(filteredComments);
971+
}
972+
906973
/**
907974
* Finds the position in source corresponding to the end of the code block of the supplied {@code
908975
* caseIndex} within all {@code cases}.

0 commit comments

Comments
 (0)