Merge pull request #39224 from hamishknight/comment-etiquette-5.5

[5.5] [Aync Refactoring] Better handle comment trailing whitespace

Author: akyrtzi

lib/IDE/Refactoring.cpp

@@ -6196,26 +6196,30 @@ class AsyncConverter : private SourceEntityWalker {
     return OutputStr;
   }
 
+  /// Retrieves the SourceRange of the preceding comment, or an invalid range if
+  /// there is no preceding comment.
+  CharSourceRange getPrecedingCommentRange(SourceLoc Loc) {
+    auto Tokens = SF->getAllTokens();
+    auto TokenIter = token_lower_bound(Tokens, Loc);
+    if (TokenIter == Tokens.end() || !TokenIter->hasComment())
+      return CharSourceRange();
+    return TokenIter->getCommentRange();
+  }
+
   /// Retrieves the location for the start of a comment attached to the token
   /// at the provided location, or the location itself if there is no comment.
   SourceLoc getLocIncludingPrecedingComment(SourceLoc Loc) {
-    auto Tokens = SF->getAllTokens();
-    auto TokenIter = token_lower_bound(Tokens, Loc);
-    if (TokenIter != Tokens.end() && TokenIter->hasComment())
-      return TokenIter->getCommentStart();
-    return Loc;
+    auto CommentRange = getPrecedingCommentRange(Loc);
+    if (CommentRange.isInvalid())
+      return Loc;
+    return CommentRange.getStart();
   }
 
-  /// If the provided SourceLoc has a preceding comment, print it out. Returns
-  /// true if a comment was printed, false otherwise.
-  bool printCommentIfNeeded(SourceLoc Loc, bool AddNewline = false) {
-    auto PrecedingLoc = getLocIncludingPrecedingComment(Loc);
-    if (Loc == PrecedingLoc)
-      return false;
-    if (AddNewline)
-      OS << "\n";
-    OS << CharSourceRange(SM, PrecedingLoc, Loc).str();
-    return true;
+  /// If the provided SourceLoc has a preceding comment, print it out.
+  void printCommentIfNeeded(SourceLoc Loc) {
+    auto CommentRange = getPrecedingCommentRange(Loc);
+    if (CommentRange.isValid())
+      OS << "\n" << CommentRange.str();
   }
 
   void convertNodes(const NodesToPrint &ToPrint) {
@@ -6230,8 +6234,6 @@ class AsyncConverter : private SourceEntityWalker {
 
     // First print the nodes we've been asked to print.
     for (auto Node : ToPrint.getNodes()) {
-      OS << "\n";
-
       // If we need to print comments, do so now.
       while (!CommentLocs.empty()) {
         auto CommentLoc = CommentLocs.back().getOpaquePointerValue();
@@ -6246,16 +6248,13 @@ class AsyncConverter : private SourceEntityWalker {
 
         printCommentIfNeeded(CommentLocs.pop_back_val());
       }
+      OS << "\n";
       convertNode(Node);
     }
 
     // We're done printing nodes. Make sure to output the remaining comments.
-    bool HasPrintedComment = false;
-    while (!CommentLocs.empty()) {
-      HasPrintedComment |=
-          printCommentIfNeeded(CommentLocs.pop_back_val(),
-                               /*AddNewline*/ !HasPrintedComment);
-    }
+    while (!CommentLocs.empty())
+      printCommentIfNeeded(CommentLocs.pop_back_val());
   }
 
   void convertNode(ASTNode Node, SourceLoc StartOverride = {},

test/refactoring/ConvertAsync/basic.swift

@@ -835,7 +835,6 @@ func testPreserveComments2() {
 // PRESERVE-COMMENTS-ERROR-NEXT:   // f
 // PRESERVE-COMMENTS-ERROR-NEXT:   print("fun")
 // PRESERVE-COMMENTS-ERROR-NEXT:   // g
-// PRESERVE-COMMENTS-ERROR-NEXT:  {{ }}
 // PRESERVE-COMMENTS-ERROR-NEXT: }
 
 // RUN: %refactor -convert-to-async -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=PRESERVE-TRAILING-COMMENT-FN %s

test/refactoring/ConvertAsync/convert_function.swift

@@ -226,7 +226,6 @@ func callNonAsyncInAsyncComment(_ completion: @escaping (String) -> Void) {
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:       // i
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:     }
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:     // j
-// CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:     {{ }}
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:     // k
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT:   }
 // CALL-NON-ASYNC-IN-ASYNC-COMMENT-NEXT: }

test/refactoring/ConvertAsync/convert_params_single.swift

@@ -87,14 +87,12 @@ func testParamsSingle() async throws {
   // BOUND-COMMENT-NEXT: // l
   // BOUND-COMMENT-NEXT: print("after")
   // BOUND-COMMENT-NEXT: // m
-  // BOUND-COMMENT-NEXT: {{ }}
   // BOUND-COMMENT-NEXT: } catch let bad {
   // BOUND-COMMENT-NEXT: // d
   // BOUND-COMMENT-NEXT: // e
   // BOUND-COMMENT-NEXT: print("got error \(bad)")
   // BOUND-COMMENT-NEXT: // f
   // BOUND-COMMENT-NEXT: // g
-  // BOUND-COMMENT-NEXT: {{ }}
   // BOUND-COMMENT-NEXT: }
 
 

test/refactoring/ConvertAsync/convert_result.swift

@@ -381,7 +381,6 @@ func testResultConversion() async throws {
   // NESTEDBREAK-COMMENT-NEXT: // l
   // NESTEDBREAK-COMMENT-NEXT: print("after")
   // NESTEDBREAK-COMMENT-NEXT: // m
-  // NESTEDBREAK-COMMENT-NEXT: {{ }}
   // NESTEDBREAK-COMMENT-NOT: }
 
   // RUN: %refactor-check-compiles -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+1):3 | %FileCheck -check-prefix=ERROR-BLOCK-COMMENT %s
@@ -416,13 +415,11 @@ func testResultConversion() async throws {
   // ERROR-BLOCK-COMMENT-NEXT:   // h
   // ERROR-BLOCK-COMMENT-NEXT:   print("after")
   // ERROR-BLOCK-COMMENT-NEXT:   // i
-  // ERROR-BLOCK-COMMENT-NEXT:   {{ }}
   // ERROR-BLOCK-COMMENT-NEXT: } catch {
   // ERROR-BLOCK-COMMENT-NEXT:   // e
   // ERROR-BLOCK-COMMENT-NEXT:   print("fail")
   // ERROR-BLOCK-COMMENT-NEXT:   // f
   // ERROR-BLOCK-COMMENT-NEXT:   // g
-  // ERROR-BLOCK-COMMENT-NEXT:   {{ }}
   // ERROR-BLOCK-COMMENT-NEXT: }
   // ERROR-BLOCK-COMMENT-NOT: }