Merge pull request swiftlang#123 from allevato/lint-block-comments

Make the `NoBlockComments` rule lint-only.

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),
     ]
 }