Make the NoBlockComments rule lint-only.

After some real world use, this rule feels a bit too aggressive
as a format rule. The opportunities where the automatic fix are
known to be harmless are fairly limited—e.g., end-of-line or
single-line block comments.

When block comments spanned multiple lines, or started at the
end of a line and continued on multiple lines, determining the
appropriate amount of leading indentation to preserve for the
subsequent lines is not always trivial (consider the case of a
multi-line comment where some lines are indented less than the
first).

In other situations, block comments might be used to temporarily
comment out large blocks of code, and transforming these to
line comments is impolite, to say the least.

Author: allevato

Sources/SwiftFormat/Pipelines+Generated.swift

@@ -280,7 +280,6 @@ extension FormatPipeline {
     node = FullyIndirectEnum(context: context).visit(node)
     node = GroupNumericLiterals(context: context).visit(node)
     node = NoAccessLevelOnExtensionDeclaration(context: context).visit(node)
-    node = NoBlockComments(context: context).visit(node)
     node = NoCasesWithOnlyFallthrough(context: context).visit(node)
     node = NoEmptyTrailingClosureParentheses(context: context).visit(node)
     node = NoLabelsInCasePatterns(context: context).visit(node)

Sources/SwiftFormatCore/Syntax+Convenience.swift

@@ -72,6 +72,51 @@ extension Syntax {
 
     return startLocation.line == endLocation.line
   }
+
+  /// Returns the absolute position of the trivia piece at the given index in the receiver's leading
+  /// trivia collection.
+  ///
+  /// If the trivia piece spans multiple characters, the value returned is the position of the first
+  /// character.
+  ///
+  /// - Precondition: `index` is a valid index in the receiver's leading trivia collection.
+  ///
+  /// - Parameter index: The index of the trivia piece in the leading trivia whose position should
+  ///   be returned.
+  /// - Returns: The absolute position of the trivia piece.
+  public func position(ofLeadingTriviaAt index: Trivia.Index) -> AbsolutePosition {
+    let leadingTrivia = self.leadingTrivia ?? []
+    guard leadingTrivia.indices.contains(index) else {
+      preconditionFailure("Index was out of bounds in the node's leading trivia.")
+    }
+
+    var offset = SourceLength.zero
+    for currentIndex in leadingTrivia.startIndex..<index {
+      offset += leadingTrivia[currentIndex].sourceLength
+    }
+    return self.position + offset
+  }
+
+  /// Returns the source location of the trivia piece at the given index in the receiver's leading
+  /// trivia collection.
+  ///
+  /// If the trivia piece spans multiple characters, the value returned is the location of the first
+  /// character.
+  ///
+  /// - Precondition: `index` is a valid index in the receiver's leading trivia collection.
+  ///
+  /// - Parameters:
+  ///   - index: The index of the trivia piece in the leading trivia whose location should be
+  ///     returned.
+  ///   - converter: The `SourceLocationConverter` that was previously initialized using the root
+  ///     tree of this node.
+  /// - Returns: The source location of the trivia piece.
+  public func startLocation(
+    ofLeadingTriviaAt index: Trivia.Index,
+    converter: SourceLocationConverter
+  ) -> SourceLocation {
+    return converter.location(for: position(ofLeadingTriviaAt: index))
+  }
 }
 
 extension SyntaxCollection {

Sources/SwiftFormatCore/SyntaxLintRule.swift

@@ -30,17 +30,25 @@ extension Rule {
   ///
   /// - Parameters:
   ///   - message: The diagnostic message to emit.
-  ///   - location: The source location which the diagnostic should be attached.
+  ///   - node: The syntax node to which the diagnostic should be attached. The diagnostic will be
+  ///     placed at the start of the node (excluding leading trivia).
+  ///   - leadingTriviaIndex: If non-nil, the index of a trivia piece in the node's leading trivia
+  ///     that should be used to determine the location of the diagnostic. Otherwise, the
+  ///     diagnostic's location will be the start of the node after any leading trivia.
   ///   - actions: A set of actions to add notes, highlights, and fix-its to diagnostics.
   public func diagnose(
     _ message: Diagnostic.Message,
     on node: Syntax?,
+    leadingTriviaIndex: Trivia.Index? = nil,
     actions: ((inout Diagnostic.Builder) -> Void)? = nil
   ) {
-    context.diagnosticEngine?.diagnose(
-      message.withRule(self),
-      location: node?.startLocation(converter: context.sourceLocationConverter),
-      actions: actions
-    )
+    let location: SourceLocation?
+    if let leadingTriviaIndex = leadingTriviaIndex {
+      location = node?.startLocation(
+        ofLeadingTriviaAt: leadingTriviaIndex, converter: context.sourceLocationConverter)
+    } else {
+      location = node?.startLocation(converter: context.sourceLocationConverter)
+    }
+    context.diagnosticEngine?.diagnose(message.withRule(self), location: location, actions: actions)
   }
 }

Sources/SwiftFormatRules/NoBlockComments.swift

@@ -18,129 +18,20 @@ import SwiftSyntax
 ///
 /// Lint: If a block comment appears, a lint error is raised.
 ///
-/// Format: If a block comment appears on its own on a line, or if a block comment spans multiple
-///         lines without appearing on the same line as code, it will be replaced with multiple
-///         single-line comments.
-///         If a block comment appears inline with code, it will be removed and hoisted to the line
-///         above the code it appears on.
-///
 /// - SeeAlso: https://google.github.io/swift#non-documentation-comments
-public final class NoBlockComments: SyntaxFormatRule {
-  public override func visit(_ token: TokenSyntax) -> Syntax {
-    var pieces = [TriviaPiece]()
-    var hasBlockComment = false
-    var validToken = token
-
-    // Ensures that the comments that appear inline with code have
-    // at least 2 spaces before the `//`.
-    if let nextToken = token.nextToken,
-      containsBlockCommentInline(trivia: nextToken.leadingTrivia)
-    {
-      hasBlockComment = true
-      validToken = addSpacesBeforeComment(token)
-    }
-
-    // Ensures that all block comments are replaced with line comment,
-    // unless the comment is between tokens on the same line.
-    for piece in token.leadingTrivia {
-      if case .blockComment(let text) = piece,
-        !commentIsBetweenCode(token)
-      {
-        diagnose(.avoidBlockComment, on: token)
-        hasBlockComment = true
-        let lineCommentText = convertBlockCommentsToLineComments(text)
-        let lineComment = TriviaPiece.lineComment(lineCommentText)
-        pieces.append(lineComment)
-      } else {
-        pieces.append(piece)
+public final class NoBlockComments: SyntaxLintRule {
+  public override func visit(_ token: TokenSyntax) -> SyntaxVisitorContinueKind {
+    for triviaIndex in token.leadingTrivia.indices {
+      let piece = token.leadingTrivia[triviaIndex]
+      if case .blockComment = piece {
+        diagnose(.avoidBlockComment, on: token, leadingTriviaIndex: triviaIndex)
       }
     }
-    validToken = validToken.withLeadingTrivia(Trivia(pieces: pieces))
-    return hasBlockComment ? validToken : token
-  }
-
-  /// Returns a Boolean value indicating if the given trivia has a piece trivia
-  /// of block comment inline with code.
-  private func containsBlockCommentInline(trivia: Trivia) -> Bool {
-    // When the comment isn't inline with code, it doesn't need to
-    // to check that there are two spaces before the line comment.
-    if let firstPiece = trivia.first {
-      if case .newlines(_) = firstPiece {
-        return false
-      }
-    }
-    for piece in trivia {
-      if case .blockComment(_) = piece {
-        return true
-      }
-    }
-    return false
-  }
-
-  /// Indicates if a block comment is between tokens on the same line.
-  /// If it does, it should only raise a lint error.
-  private func commentIsBetweenCode(_ token: TokenSyntax) -> Bool {
-    let hasCommentBetweenCode = token.leadingTrivia.isBetweenTokens
-    if hasCommentBetweenCode {
-      diagnose(.avoidBlockCommentBetweenCode, on: token)
-    }
-    return hasCommentBetweenCode
-  }
-
-  /// Ensures there is always at least 2 spaces before the comment.
-  private func addSpacesBeforeComment(_ token: TokenSyntax) -> TokenSyntax {
-    let numSpaces = token.trailingTrivia.numberOfSpaces
-    if numSpaces < 2 {
-      let addSpaces = 2 - numSpaces
-      return token.withTrailingTrivia(
-        token.trailingTrivia.appending(.spaces(addSpaces)))
-    }
-    return token
-  }
-
-  /// Receives the text of a Block comment and converts it to a Line Comment format text.
-  private func convertBlockCommentsToLineComments(_ text: String) -> String {
-    // Removes the '/*', '*/', the extra spaces and newlines from the comment.
-    let textTrim = text.dropFirst(2).dropLast(2)
-      .trimmingCharacters(in: .whitespacesAndNewlines)
-
-    let splitComment = textTrim.split(separator: "\n", omittingEmptySubsequences: false)
-    var lineCommentText = [String]()
-
-    for line in splitComment {
-      let startsComment = line.starts(with: " ") || line.count == 0 ? "//" : "// "
-      lineCommentText.append(startsComment + line)
-    }
-    return lineCommentText.joined(separator: "\n")
+    return .skipChildren
   }
 }
 
 extension Diagnostic.Message {
   static let avoidBlockComment = Diagnostic.Message(
     .warning, "replace block comment with line comments")
-
-  static let avoidBlockCommentBetweenCode = Diagnostic.Message(
-    .warning, "remove block comment inline with code")
-}
-
-extension Trivia {
-  /// Indicates if the trivia is between tokens, for example
-  /// if a leading trivia that contains a comment, doesn't starts
-  /// and finishes with a new line then the comment is between tokens.
-  var isBetweenTokens: Bool {
-    var beginsNewLine = false
-    var endsNewLine = false
-
-    if let firstPiece = self.first,
-      let lastPiece = self.reversed().first
-    {
-      if case .newlines(_) = firstPiece {
-        beginsNewLine = true
-      }
-      if case .newlines(_) = lastPiece {
-        endsNewLine = true
-      }
-    }
-    return !beginsNewLine && !endsNewLine
-  }
 }

Tests/SwiftFormatRulesTests/NoBlockCommentsTests.swift

@@ -4,41 +4,34 @@ import XCTest
 @testable import SwiftFormatRules
 
 public class NoBlockCommentsTests: DiagnosingTestCase {
-  public func testRemoveBlockComments() {
-    XCTAssertFormatting(
-      NoBlockComments.self,
-      input: """
-             /*
-             Lorem ipsum dolor sit amet, at nonumes adipisci sea, natum
-             offendit vis ex. Audiam legendos expetenda ei quo, nonumes
+  public func testDiagnoseBlockComments() {
+    let input =
+      """
+      /*
+      Lorem ipsum dolor sit amet, at nonumes adipisci sea, natum
+      offendit vis ex. Audiam legendos expetenda ei quo, nonumes
 
-                 msensibus eloquentiam ex vix.
-             */
-             let a = /*ff*/10  /*ff*/ + 10
-             var b = 0/*Block Comment inline with code*/
+          msensibus eloquentiam ex vix.
+      */
+      let a = /*ff*/10  /*ff*/ + 10
+      var b = 0/*Block Comment inline with code*/
 
-             /*
+      /*
 
-             Block Comment
-             */
-             let c = a + b
-             /* This is the end
-             of a file
+      Block Comment
+      */
+      let c = a + b
+      /* This is the end
+      of a file
 
-             */
-             """,
-      expected: """
-                // Lorem ipsum dolor sit amet, at nonumes adipisci sea, natum
-                // offendit vis ex. Audiam legendos expetenda ei quo, nonumes
-                //
-                //    msensibus eloquentiam ex vix.
-                let a =  /*ff*/10  /*ff*/ + 10
-                var b = 0  // Block Comment inline with code
-
-                // Block Comment
-                let c = a + b
-                // This is the end
-                // of a file
-                """)
+      */
+      """
+    performLint(NoBlockComments.self, input: input)
+    XCTAssertDiagnosed(.avoidBlockComment)
+    XCTAssertDiagnosed(.avoidBlockComment)
+    XCTAssertDiagnosed(.avoidBlockComment)
+    XCTAssertDiagnosed(.avoidBlockComment)
+    XCTAssertDiagnosed(.avoidBlockComment)
+    XCTAssertDiagnosed(.avoidBlockComment)
   }
 }

Tests/SwiftFormatRulesTests/XCTestManifests.swift

@@ -157,7 +157,7 @@ extension NoBlockCommentsTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__NoBlockCommentsTests = [
-        ("testRemoveBlockComments", testRemoveBlockComments),
+        ("testDiagnoseBlockComments", testDiagnoseBlockComments),
     ]
 }