Merge pull request #194 from allevato/5.2-cherrypicks

Cherry-pick recent 5.2-compatible changes into the 5.2 branch.

Author: allevato

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -211,7 +211,7 @@ public class PrettyPrinter {
       numberToPrint = consecutiveNewlineCount == 0 ? 1 : 0
     case .soft(let count, _):
       // We add 1 to the max blank lines because it takes 2 newlines to create the first blank line.
-      numberToPrint = min(count - consecutiveNewlineCount, configuration.maximumBlankLines + 1)
+      numberToPrint = min(count, configuration.maximumBlankLines + 1) - consecutiveNewlineCount
     case .hard(let count):
       numberToPrint = count
     }
@@ -359,11 +359,14 @@ public class PrettyPrinter {
           = openCloseBreakCompensatingLineNumber != matchingOpenBreak.lineNumber
 
         if matchingOpenBreak.contributesBlockIndent {
+          // The actual line number is used, instead of the compensating line number. When the close
+          // break is at the start of a new line, the block indentation isn't carried to the new line.
+          let currentLine = lineNumber
           // When two or more open breaks are encountered on the same line, only the final open
           // break is allowed to increase the block indent, avoiding multiple block indents. As the
           // open breaks on that line are closed, the new final open break must be enabled again to
           // add a block indent.
-          if matchingOpenBreak.lineNumber == openCloseBreakCompensatingLineNumber,
+          if matchingOpenBreak.lineNumber == currentLine,
             let lastActiveOpenBreak = activeOpenBreaks.last,
             lastActiveOpenBreak.kind == .block,
             !lastActiveOpenBreak.contributesBlockIndent
@@ -651,15 +654,12 @@ public class PrettyPrinter {
         lengths.append(0)
 
       case .commaDelimitedRegionEnd:
-        // The trailing comma needs to be included in the length of the preceding break, but is not
-        // included in the length of the enclosing group. A trailing comma cannot cause the group
-        // to break onto multiple lines, because the comma isn't printed for a single line group.
-        if let index = delimIndexStack.last, case .break = tokens[index] {
-          lengths[index] += 1
-        }
-        // If the closest delimiter token is an open, instead of a break, then adding the comma's
-        // length isn't necessary. In that case, the comma is printed if the preceding break fires.
-
+        // The token's length is only necessary when a comma will be printed, but it's impossible to
+        // know at this point whether the region-start token will be on the same line as this token.
+        // Without adding this length to the total, it would be possible for this comma to be
+        // printed in column `maxLineLength`. Unfortunately, this can cause breaks to fire
+        // unnecessarily when the enclosed tokens comma would fit within `maxLineLength`.
+        total += 1
         lengths.append(1)
       }
     }

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

@@ -22,22 +22,31 @@ import SwiftSyntax
 public final class NoEmptyTrailingClosureParentheses: SyntaxFormatRule {
 
   public override func visit(_ node: FunctionCallExprSyntax) -> ExprSyntax {
-    guard node.argumentList.count == 0 else { return ExprSyntax(node) }
+    guard node.argumentList.count == 0 else { return super.visit(node) }
 
-    guard node.trailingClosure != nil && node.argumentList.isEmpty && node.leftParen != nil else {
-      return ExprSyntax(node)
+    guard let trailingClosure = node.trailingClosure,
+      node.argumentList.isEmpty && node.leftParen != nil else
+    {
+      return super.visit(node)
     }
     guard let name = node.calledExpression.lastToken?.withoutTrivia() else {
-      return ExprSyntax(node)
+      return super.visit(node)
     }
 
     diagnose(.removeEmptyTrailingParentheses(name: "\(name)"), on: node)
 
+    // Need to visit `calledExpression` before creating a new node so that the location data (column
+    // and line numbers) is available.
+    guard let rewrittenCalledExpr = ExprSyntax(visit(Syntax(node.calledExpression))) else {
+      return super.visit(node)
+    }
     let formattedExp = replaceTrivia(
-      on: node.calledExpression,
-      token: node.calledExpression.lastToken,
+      on: rewrittenCalledExpr,
+      token: rewrittenCalledExpr.lastToken,
       trailingTrivia: .spaces(1))
+    let formattedClosure = visit(trailingClosure).as(ClosureExprSyntax.self)
     let result = node.withLeftParen(nil).withRightParen(nil).withCalledExpression(formattedExp)
+      .withTrailingClosure(formattedClosure)
     return ExprSyntax(result)
   }
 }

Sources/SwiftFormatRules/NoEmptyTrailingClosureParentheses.swift

@@ -25,14 +25,82 @@ public final class ReturnVoidInsteadOfEmptyTuple: SyntaxFormatRule {
   public override func visit(_ node: FunctionTypeSyntax) -> TypeSyntax {
     guard let returnType = node.returnType.as(TupleTypeSyntax.self),
       returnType.elements.count == 0
-    else { return TypeSyntax(node) }
-    diagnose(.returnVoid, on: node.returnType)
-    let voidKeyword = SyntaxFactory.makeSimpleTypeIdentifier(
+    else {
+      return super.visit(node)
+    }
+
+    diagnose(.returnVoid, on: returnType)
+
+    // If the user has put non-whitespace trivia inside the empty tuple, like a comment, then we
+    // still diagnose it as a lint error but we don't replace it because it's not obvious where the
+    // comment should go.
+    if hasNonWhitespaceLeadingTrivia(returnType.rightParen) {
+      return super.visit(node)
+    }
+
+    // Make sure that function types nested in the argument list are also rewritten (for example,
+    // `(Int -> ()) -> ()` should become `(Int -> Void) -> Void`).
+    let arguments = visit(node.arguments).as(TupleTypeElementListSyntax.self)!
+    let voidKeyword = makeVoidIdentifierType(toReplace: returnType)
+    return TypeSyntax(node.withArguments(arguments).withReturnType(TypeSyntax(voidKeyword)))
+  }
+
+  public override func visit(_ node: ClosureSignatureSyntax) -> Syntax {
+    guard let output = node.output,
+      let returnType = output.returnType.as(TupleTypeSyntax.self),
+      returnType.elements.count == 0
+    else {
+      return super.visit(node)
+    }
+
+    diagnose(.returnVoid, on: returnType)
+
+    // If the user has put non-whitespace trivia inside the empty tuple, like a comment, then we
+    // still diagnose it as a lint error but we don't replace it because it's not obvious where the
+    // comment should go.
+    if hasNonWhitespaceLeadingTrivia(returnType.rightParen) {
+      return super.visit(node)
+    }
+
+    let input: Syntax?
+    if let parameterClause = node.input?.as(ParameterClauseSyntax.self) {
+      // If the closure input is a complete parameter clause (variables and types), make sure that
+      // nested function types are also rewritten (for example, `label: (Int -> ()) -> ()` should
+      // become `label: (Int -> Void) -> Void`).
+      input = visit(parameterClause)
+    } else {
+      // Otherwise, it's a simple signature (just variable names, no types), so there is nothing to
+      // rewrite.
+      input = node.input
+    }
+    let voidKeyword = makeVoidIdentifierType(toReplace: returnType)
+    return Syntax(node.withInput(input).withOutput(output.withReturnType(TypeSyntax(voidKeyword))))
+  }
+
+  /// Returns a value indicating whether the leading trivia of the given token contained any
+  /// non-whitespace pieces.
+  private func hasNonWhitespaceLeadingTrivia(_ token: TokenSyntax) -> Bool {
+    for piece in token.leadingTrivia {
+      switch piece {
+      case .blockComment, .docBlockComment, .docLineComment, .garbageText, .lineComment:
+        return true
+      default:
+        break
+      }
+    }
+    return false
+  }
+
+  /// Returns a type syntax node with the identifier `Void` whose leading and trailing trivia have
+  /// been copied from the tuple type syntax node it is replacing.
+  private func makeVoidIdentifierType(toReplace node: TupleTypeSyntax) -> SimpleTypeIdentifierSyntax
+  {
+    return SyntaxFactory.makeSimpleTypeIdentifier(
       name: SyntaxFactory.makeIdentifier(
         "Void",
-        trailingTrivia: returnType.rightParen.trailingTrivia),
+        leadingTrivia: node.firstToken?.leadingTrivia ?? [],
+        trailingTrivia: node.lastToken?.trailingTrivia ?? []),
       genericArgumentClause: nil)
-    return TypeSyntax(node.withReturnType(TypeSyntax(voidKeyword)))
   }
 }
 

Sources/SwiftFormatRules/ReturnVoidInsteadOfEmptyTuple.swift

@@ -73,17 +73,23 @@ public final class UseTripleSlashForDocumentationComments: SyntaxFormatRule {
   /// a docLineComment.
   private func convertDocBlockCommentToDocLineComment(_ decl: DeclSyntax) -> DeclSyntax {
     guard let commentText = decl.docComment else { return decl }
-    guard let declLeadinTrivia = decl.leadingTrivia else { return decl }
+    guard let declLeadingTrivia = decl.leadingTrivia else { return decl }
     let docComments = commentText.components(separatedBy: "\n")
     var pieces = [TriviaPiece]()
 
     // Ensures the documentation comment is a docLineComment.
     var hasFoundDocComment = false
-    for piece in declLeadinTrivia.reversed() {
+    for piece in declLeadingTrivia.reversed() {
       if case .docBlockComment(_) = piece, !hasFoundDocComment {
         hasFoundDocComment = true
         diagnose(.avoidDocBlockComment, on: decl)
         pieces.append(contentsOf: separateDocBlockIntoPieces(docComments).reversed())
+      } else if case .docLineComment(_) = piece, !hasFoundDocComment {
+        // The comment was a doc-line comment all along. Leave it alone.
+        // This intentionally only considers the comment closest to the decl. There may be other
+        // comments, including block or doc-block comments, which are left as-is because they aren't
+        // necessarily related to the decl and are unlikely part of the decl's documentation.
+        return decl
       } else {
         pieces.append(piece)
       }

Sources/SwiftFormatRules/UseTripleSlashForDocumentationComments.swift

@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import Foundation
+import SwiftFormatConfiguration
+
+/// Loads formatter configurations, caching them in memory so that multiple operations in the same
+/// directory do not repeatedly hit the file system.
+struct ConfigurationLoader {
+  /// A mapping from configuration file URLs to the loaded configuration data.
+  private var cache = [URL: Configuration]()
+
+  /// Returns the configuration associated with the configuration file at the given path.
+  ///
+  /// - Throws: If an error occurred loading the configuration.
+  mutating func configuration(atPath path: String) throws -> Configuration {
+    return try configuration(at: URL(fileURLWithPath: path))
+  }
+
+  /// Returns the configuration found by searching in the directory (and ancestor directories)
+  /// containing the given `.swift` source file.
+  ///
+  /// If no configuration file was found during the search, this method returns nil.
+  ///
+  /// - Throws: If a configuration file was found but an error occurred loading it.
+  mutating func configuration(forSwiftFileAtPath path: String) throws -> Configuration? {
+    let swiftFileURL = URL(fileURLWithPath: path)
+    guard let configurationFileURL = Configuration.url(forConfigurationFileApplyingTo: swiftFileURL)
+    else {
+      return nil
+    }
+    return try configuration(at: configurationFileURL)
+  }
+
+  /// Returns the configuration associated with the configuration file at the given URL.
+  ///
+  /// - Throws: If an error occurred loading the configuration.
+  private mutating func configuration(at url: URL) throws -> Configuration {
+    if let cachedConfiguration = cache[url] {
+      return cachedConfiguration
+    }
+
+    let configuration = try Configuration(contentsOf: url)
+    cache[url] = configuration
+    return configuration
+  }
+}

Sources/swift-format/Frontend/ConfigurationLoader.swift

@@ -0,0 +1,78 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import Foundation
+import SwiftFormat
+import SwiftFormatConfiguration
+import SwiftSyntax
+
+/// The frontend for formatting operations.
+class FormatFrontend: Frontend {
+  /// Whether or not to format the Swift file in-place.
+  private let inPlace: Bool
+
+  init(lintFormatOptions: LintFormatOptions, inPlace: Bool) {
+    self.inPlace = inPlace
+    super.init(lintFormatOptions: lintFormatOptions)
+  }
+
+  override func processFile(_ fileToProcess: FileToProcess) {
+    // Even though `diagnosticEngine` is defined, it's use is reserved for fatal messages. Pass nil
+    // to the formatter to suppress other messages since they will be fixed or can't be
+    // automatically fixed anyway.
+    let formatter = SwiftFormatter(
+      configuration: fileToProcess.configuration, diagnosticEngine: nil)
+    formatter.debugOptions = debugOptions
+
+    let path = fileToProcess.path
+    guard let source = fileToProcess.sourceText else {
+      diagnosticEngine.diagnose(
+        Diagnostic.Message(.error, "Unable to read source for formatting from \(path)."))
+      return
+    }
+
+    var stdoutStream = FileHandle.standardOutput
+    do {
+      let assumingFileURL = URL(fileURLWithPath: path)
+      if inPlace {
+        var buffer = ""
+        try formatter.format(source: source, assumingFileURL: assumingFileURL, to: &buffer)
+
+        let bufferData = buffer.data(using: .utf8)!  // Conversion to UTF-8 cannot fail
+        try bufferData.write(to: assumingFileURL, options: .atomic)
+      } else {
+        try formatter.format(source: source, assumingFileURL: assumingFileURL, to: &stdoutStream)
+      }
+    } catch SwiftFormatError.fileNotReadable {
+      diagnosticEngine.diagnose(
+        Diagnostic.Message(
+          .error, "Unable to format \(path): file is not readable or does not exist."))
+      return
+    } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
+      guard !lintFormatOptions.ignoreUnparsableFiles else {
+        guard !inPlace else {
+          // For in-place mode, nothing is expected to stdout and the file shouldn't be modified.
+          return
+        }
+        stdoutStream.write(source)
+        return
+      }
+      let location = SourceLocationConverter(file: path, source: source).location(for: position)
+      diagnosticEngine.diagnose(
+        Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
+        location: location)
+      return
+    } catch {
+      diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to format \(path): \(error)"))
+    }
+  }
+}