Merge pull request #556 from allevato/5.9-cherrypicks

[5.9] Various cherry-picks since the branch cut.

Author: allevato

Package.swift

@@ -143,7 +143,6 @@ let package = Package(
         .product(name: "ArgumentParser", package: "swift-argument-parser"),
         .product(name: "SwiftSyntax", package: "swift-syntax"),
         .product(name: "SwiftParser", package: "swift-syntax"),
-        .product(name: "TSCBasic", package: "swift-tools-support-core"),
       ]
     ),
 
@@ -214,20 +213,17 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
   // Building standalone.
   package.dependencies += [
     .package(
-      url: "https://github.com/apple/swift-argument-parser.git", from: "1.2.2"),
+      url: "https://github.com/apple/swift-argument-parser.git", 
+      from: "1.2.2"
+    ),
     .package(
       url: "https://github.com/apple/swift-syntax.git",
       branch: "release/5.9"
     ),
-    .package(
-      url: "https://github.com/apple/swift-tools-support-core.git",
-      exact: Version("0.4.0")
-    ),
   ]
 } else {
   package.dependencies += [
     .package(path: "../swift-argument-parser"),
     .package(path: "../swift-syntax"),
-    .package(path: "../swift-tools-support-core"),
   ]
 }

README.md

@@ -13,14 +13,26 @@ invoked via an [API](#api-usage).
 > and the code is provided so that it can be tested on real-world code and
 > experiments can be made by modifying it.
 
-## Matching swift-format to Your Swift Version (Swift 5.7 and earlier)
+## Matching swift-format to Your Swift Version
 
-> NOTE: `swift-format` on the `main` branch now uses a version of
-> [SwiftSyntax](https://github.com/apple/swift-syntax) whose parser has been
-> rewritten in Swift and no longer has dependencies on libraries in the
-> Swift toolchain. This allows `swift-format` to be built, developed, and
-> run using any version of Swift that can compile it, decoupling it from
-> the version that supported a particular syntax.
+### Swift 5.8 and later
+
+As of Swift 5.8, swift-format depends on the version of
+[SwiftSyntax](https://github.com/apple/swift-syntax) whose parser has been
+rewritten in Swift and no longer has dependencies on libraries in the
+Swift toolchain.
+
+This change allows `swift-format` to be built, developed, and run using
+any version of Swift that can compile it, decoupling it from the version
+that supported a particular syntax. However, earlier versions of swift-format
+will still not be able to recognize new syntax added in later versions of the
+language and parser.
+
+Note also that the version numbering scheme has changed to match
+SwiftSyntax; the 5.8 release of swift-format is `508.0.0`, not `0.50800.0`,
+and future versions are also expressed this way.
+
+### Swift 5.7 and earlier
 
 `swift-format` versions 0.50700.0 and earlier depend on versions of
 [SwiftSyntax](https://github.com/apple/swift-syntax) that used a standalone
@@ -54,7 +66,7 @@ then once you have identified the version you need, you can check out the
 source and build it using the following commands:
 
 ```sh
-VERSION=0.50700.0  # replace this with the version you need
+VERSION=509.0.0  # replace this with the version you need
 git clone https://github.com/apple/swift-format.git
 cd swift-format
 git checkout "tags/$VERSION"

Sources/SwiftFormat/Parsing.swift

@@ -43,18 +43,53 @@ func parseAndEmitDiagnostics(
     operatorTable.foldAll(Parser.parse(source: source)) { _ in }.as(SourceFileSyntax.self)!
 
   let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: sourceFile)
+  var hasErrors = false
   if let parsingDiagnosticHandler = parsingDiagnosticHandler {
     let expectedConverter =
       SourceLocationConverter(file: url?.path ?? "<unknown>", tree: sourceFile)
     for diagnostic in diagnostics {
       let location = diagnostic.location(converter: expectedConverter)
-      parsingDiagnosticHandler(diagnostic, location)
+
+      // Downgrade editor placeholders to warnings, because it is useful to support formatting
+      // in-progress files that contain those.
+      if diagnostic.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
+        parsingDiagnosticHandler(downgradedToWarning(diagnostic), location)
+      } else {
+        parsingDiagnosticHandler(diagnostic, location)
+        hasErrors = true
+      }
     }
   }
 
-  guard diagnostics.isEmpty else {
+  guard !hasErrors else {
     throw SwiftFormatError.fileContainsInvalidSyntax
   }
 
   return restoringLegacyTriviaBehavior(sourceFile)
 }
+
+// Wraps a `DiagnosticMessage` but forces its severity to be that of a warning instead of an error.
+struct DowngradedDiagnosticMessage: DiagnosticMessage {
+  var originalDiagnostic: DiagnosticMessage
+
+  var message: String { originalDiagnostic.message }
+
+  var diagnosticID: SwiftDiagnostics.MessageID { originalDiagnostic.diagnosticID }
+
+  var severity: DiagnosticSeverity { .warning }
+}
+
+/// Returns a new `Diagnostic` that is identical to the given diagnostic, except that its severity
+/// has been downgraded to a warning.
+func downgradedToWarning(_ diagnostic: Diagnostic) -> Diagnostic {
+  // `Diagnostic` is immutable, so create a new one with the same values except for the
+  // severity-downgraded message.
+  return Diagnostic(
+    node: diagnostic.node,
+    position: diagnostic.position,
+    message: DowngradedDiagnosticMessage(originalDiagnostic: diagnostic.diagMessage),
+    highlights: diagnostic.highlights,
+    notes: diagnostic.notes,
+    fixIts: diagnostic.fixIts
+  )
+}

Sources/SwiftFormatConfiguration/Configuration.swift

@@ -36,6 +36,7 @@ public struct Configuration: Codable, Equatable {
     case indentSwitchCaseLabels
     case rules
     case spacesAroundRangeFormationOperators
+    case noAssignmentInExpressions
   }
 
   /// The version of this configuration.
@@ -147,6 +148,9 @@ public struct Configuration: Codable, Equatable {
   /// `...` and `..<`.
   public var spacesAroundRangeFormationOperators = false
 
+  /// Contains exceptions for the `NoAssignmentInExpressions` rule.
+  public var noAssignmentInExpressions = NoAssignmentInExpressionsConfiguration()
+
   /// Constructs a Configuration with all default values.
   public init() {
     self.version = highestSupportedConfigurationVersion
@@ -208,6 +212,10 @@ public struct Configuration: Codable, Equatable {
       ?? FileScopedDeclarationPrivacyConfiguration()
     self.indentSwitchCaseLabels
       = try container.decodeIfPresent(Bool.self, forKey: .indentSwitchCaseLabels) ?? false
+    self.noAssignmentInExpressions =
+      try container.decodeIfPresent(
+        NoAssignmentInExpressionsConfiguration.self, forKey: .noAssignmentInExpressions)
+      ?? NoAssignmentInExpressionsConfiguration()
 
     // If the `rules` key is not present at all, default it to the built-in set
     // so that the behavior is the same as if the configuration had been
@@ -238,6 +246,7 @@ public struct Configuration: Codable, Equatable {
       spacesAroundRangeFormationOperators, forKey: .spacesAroundRangeFormationOperators)
     try container.encode(fileScopedDeclarationPrivacy, forKey: .fileScopedDeclarationPrivacy)
     try container.encode(indentSwitchCaseLabels, forKey: .indentSwitchCaseLabels)
+    try container.encode(noAssignmentInExpressions, forKey: .noAssignmentInExpressions)
     try container.encode(rules, forKey: .rules)
   }
 
@@ -287,3 +296,15 @@ public struct FileScopedDeclarationPrivacyConfiguration: Codable, Equatable {
   /// private access.
   public var accessLevel: AccessLevel = .private
 }
+
+/// Configuration for the `NoAssignmentInExpressions` rule.
+public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable {
+  /// A list of function names where assignments are allowed to be embedded in expressions that are
+  /// passed as parameters to that function.
+  public var allowedFunctions: [String] = [
+    // Allow `XCTAssertNoThrow` because `XCTAssertNoThrow(x = try ...)` is clearer about intent than
+    // `x = try XCTUnwrap(try? ...)` or force-unwrapped if you need to use the value `x` later on
+    // in the test.
+    "XCTAssertNoThrow"
+  ]
+}

Sources/SwiftFormatCore/LegacyTriviaBehavior.swift

@@ -20,7 +20,7 @@ private final class LegacyTriviaBehaviorRewriter: SyntaxRewriter {
       token = token.with(\.leadingTrivia, pendingLeadingTrivia + token.leadingTrivia)
       self.pendingLeadingTrivia = nil
     }
-    if token.nextToken != nil,
+    if token.nextToken(viewMode: .sourceAccurate) != nil,
       let firstIndexToMove = token.trailingTrivia.firstIndex(where: shouldTriviaPieceBeMoved)
     {
       pendingLeadingTrivia = Trivia(pieces: Array(token.trailingTrivia[firstIndexToMove...]))

Sources/SwiftFormatCore/RuleMask.swift

@@ -136,7 +136,7 @@ fileprivate class RuleStatusCollectionVisitor: SyntaxVisitor {
   // MARK: - Syntax Visitation Methods
 
   override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
-    guard let firstToken = node.firstToken else {
+    guard let firstToken = node.firstToken(viewMode: .sourceAccurate) else {
       return .visitChildren
     }
     let comments = loneLineComments(in: firstToken.leadingTrivia, isFirstToken: true)
@@ -159,14 +159,14 @@ fileprivate class RuleStatusCollectionVisitor: SyntaxVisitor {
   }
 
   override func visit(_ node: CodeBlockItemSyntax) -> SyntaxVisitorContinueKind {
-    guard let firstToken = node.firstToken else {
+    guard let firstToken = node.firstToken(viewMode: .sourceAccurate) else {
       return .visitChildren
     }
     return appendRuleStatusDirectives(from: firstToken, of: Syntax(node))
   }
 
   override func visit(_ node: MemberDeclListItemSyntax) -> SyntaxVisitorContinueKind {
-    guard let firstToken = node.firstToken else {
+    guard let firstToken = node.firstToken(viewMode: .sourceAccurate) else {
       return .visitChildren
     }
     return appendRuleStatusDirectives(from: firstToken, of: Syntax(node))
@@ -183,7 +183,7 @@ fileprivate class RuleStatusCollectionVisitor: SyntaxVisitor {
   private func appendRuleStatusDirectives(from token: TokenSyntax, of node: Syntax)
     -> SyntaxVisitorContinueKind
   {
-    let isFirstInFile = token.previousToken == nil
+    let isFirstInFile = token.previousToken(viewMode: .sourceAccurate) == nil
     let matches = loneLineComments(in: token.leadingTrivia, isFirstToken: isFirstInFile)
       .compactMap(ruleStatusDirectiveMatch)
     let sourceRange = node.sourceRange(converter: sourceLocationConverter)

Sources/SwiftFormatPrettyPrint/PrettyPrint.swift

@@ -129,11 +129,16 @@ public class PrettyPrinter {
   private var activeBreakSuppressionCount = 0
 
   /// Whether breaks are supressed from firing. When true, no breaks should fire and the only way to
-  /// move to a new line is an explicit new line token.
-  private var isBreakingSupressed: Bool {
+  /// move to a new line is an explicit new line token. Discretionary breaks aren't suppressed
+  /// if ``allowSuppressedDiscretionaryBreaks`` is true.
+  private var isBreakingSuppressed: Bool {
     return activeBreakSuppressionCount > 0
   }
 
+  /// Indicates whether discretionary breaks should still be included even if break suppression is
+  /// enabled (see ``isBreakingSuppressed``).
+  private var allowSuppressedDiscretionaryBreaks = false
+
   /// The computed indentation level, as a number of spaces, based on the state of any unclosed
   /// delimiters and whether or not the current line is a continuation line.
   private var currentIndentation: [Indent] {
@@ -469,15 +474,15 @@ public class PrettyPrinter {
       case .soft(_, let discretionary):
         // A discretionary newline (i.e. from the source) should create a line break even if the
         // rules for breaking are disabled.
-        overrideBreakingSuppressed = discretionary
+        overrideBreakingSuppressed = discretionary && allowSuppressedDiscretionaryBreaks
         mustBreak = true
       case .hard:
         // A hard newline must always create a line break, regardless of the context.
         overrideBreakingSuppressed = true
         mustBreak = true
       }
 
-      let suppressBreaking = isBreakingSupressed && !overrideBreakingSuppressed
+      let suppressBreaking = isBreakingSuppressed && !overrideBreakingSuppressed
       if !suppressBreaking && ((!isAtStartOfLine && length > spaceRemaining) || mustBreak) {
         currentLineIsContinuation = isContinuationIfBreakFires
         writeNewlines(newline)
@@ -527,8 +532,14 @@ public class PrettyPrinter {
 
     case .printerControl(let kind):
       switch kind {
-      case .disableBreaking:
+      case .disableBreaking(let allowDiscretionary):
         activeBreakSuppressionCount += 1
+        // Override the supression of discretionary breaks if we're at the top level or
+        // discretionary breaks are currently allowed (false should override true, but not the other
+        // way around).
+        if activeBreakSuppressionCount == 1 || allowSuppressedDiscretionaryBreaks {
+          allowSuppressedDiscretionaryBreaks = allowDiscretionary
+        }
       case .enableBreaking:
         activeBreakSuppressionCount -= 1
       }

Sources/SwiftFormatPrettyPrint/Token.swift

@@ -162,8 +162,10 @@ enum PrinterControlKind {
   /// control token is encountered.
   ///
   /// It's valid to nest `disableBreaking` and `enableBreaking` tokens. Breaks will be suppressed
-  /// long as there is at least 1 unmatched disable token.
-  case disableBreaking
+  /// long as there is at least 1 unmatched disable token. If `allowDiscretionary` is `true`, then
+  /// discretionary breaks aren't effected. An `allowDiscretionary` value of true never overrides a
+  /// value of false. Hard breaks are always inserted no matter what.
+  case disableBreaking(allowDiscretionary: Bool)
 
   /// A signal that break tokens should be allowed to fire following this token, as long as there
   /// are no other unmatched disable tokens.