[SwiftRefactor] PackageManifest: Address stylistic review feedback

Author: xedin

Sources/SwiftRefactor/PackageManifest/AddPackageTarget.swift

@@ -190,7 +190,7 @@ public struct AddPackageTarget: ManifestEditRefactoringProvider {
     }
 
     let importDecls = importModuleNames.lazy.sorted().map { name in
-      DeclSyntax("import \(raw: name)").with(\.trailingTrivia, .newline)
+      DeclSyntax("import \(raw: name)\n")
     }
 
     let imports = CodeBlockItemListSyntax {
@@ -298,17 +298,17 @@ fileprivate extension PackageTarget.Dependency {
   /// Retrieve the name of the dependency
   var name: String {
     switch self {
-    case .target(name: let name),
-      .byName(name: let name),
-      .product(name: let name, package: _):
+    case .target(let name),
+      .byName(let name),
+      .product(let name, package: _):
       return name
     }
   }
 }
 
 /// The array of auxiliary files that can be added by a package editing
 /// operation.
-fileprivate typealias AuxiliaryFiles = [(String, SourceFileSyntax)]
+private typealias AuxiliaryFiles = [(String, SourceFileSyntax)]
 
 fileprivate extension AuxiliaryFiles {
   /// Add a source file to the list of auxiliary files.
@@ -322,7 +322,7 @@ fileprivate extension AuxiliaryFiles {
 
 /// The set of dependencies we need to introduce to a newly-created macro
 /// target.
-fileprivate let macroTargetDependencies: [PackageTarget.Dependency] = [
+private let macroTargetDependencies: [PackageTarget.Dependency] = [
   .product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
   .product(name: "SwiftSyntaxMacros", package: "swift-syntax"),
 ]

Sources/SwiftRefactor/PackageManifest/ManifestEditRefactoringProvider.swift

@@ -18,7 +18,7 @@ where Self.Input == SourceFileSyntax {
   static func manifestRefactor(syntax: SourceFileSyntax, in context: Context) throws -> PackageEdit
 }
 
-extension EditRefactoringProvider where Self: ManifestEditRefactoringProvider {
+extension ManifestEditRefactoringProvider {
   public static func textRefactor(syntax: Input, in context: Context) -> [SourceEdit] {
     return (try? manifestRefactor(syntax: syntax, in: context).manifestEdits) ?? []
   }

Sources/SwiftRefactor/PackageManifest/PackageDependency.swift

@@ -22,7 +22,6 @@ public enum PackageDependency: Sendable {
   case registry(Registry)
 
   public struct FileSystem: Sendable {
-    public let nameForTargetDependencyResolutionOnly: String?
     public let path: String
   }
 
@@ -75,8 +74,6 @@ extension PackageDependency.FileSystem: ManifestSyntaxRepresentable {
 
 extension PackageDependency.SourceControl: ManifestSyntaxRepresentable {
   func asSyntax() -> ExprSyntax {
-    // TODO: Not handling identity, nameForTargetDependencyResolutionOnly,
-    // or productFilter yet.
     ".package(url: \(literal: location.description), \(requirement.asSyntax()))"
   }
 }
@@ -104,7 +101,7 @@ extension PackageDependency.SourceControl.Requirement: ManifestSyntaxRepresentab
 
     case .range(let lowerBound, let upperBound):
       return LabeledExprSyntax(
-        expression: "\(lowerBound.asSyntax())..<\(upperBound.asSyntax())" as ExprSyntax
+        expression: "\(literal: lowerBound)..<\(literal: upperBound)" as ExprSyntax
       )
 
     case .revision(let revision):

Sources/SwiftRefactor/PackageManifest/PackageEdit.swift

@@ -19,5 +19,5 @@ public struct PackageEdit {
   public var manifestEdits: [SourceEdit] = []
 
   /// Auxiliary files to write.
-  public var auxiliaryFiles: [(String, SourceFileSyntax)] = []
+  public var auxiliaryFiles: [(relativePath: String, contents: SourceFileSyntax)] = []
 }

Sources/SwiftRefactor/PackageManifest/PackageTarget.swift

@@ -114,16 +114,16 @@ extension PackageTarget: ManifestSyntaxRepresentable {
 extension PackageTarget.Dependency: ManifestSyntaxRepresentable {
   func asSyntax() -> ExprSyntax {
     switch self {
-    case .byName(name: let name):
+    case .byName(let name):
       return "\(literal: name)"
 
-    case .target(name: let name):
+    case .target(let name):
       return ".target(name: \(literal: name))"
 
-    case .product(name: let name, package: nil):
+    case .product(let name, package: nil):
       return ".product(name: \(literal: name))"
 
-    case .product(name: let name, package: let package):
+    case .product(let name, let package):
       return ".product(name: \(literal: name), package: \(literal: package))"
     }
   }
@@ -132,10 +132,10 @@ extension PackageTarget.Dependency: ManifestSyntaxRepresentable {
 extension PackageTarget.PluginUsage: ManifestSyntaxRepresentable {
   func asSyntax() -> ExprSyntax {
     switch self {
-    case .plugin(name: let name, package: nil):
+    case .plugin(let name, package: nil):
       return ".plugin(name: \(literal: name))"
 
-    case .plugin(name: let name, package: let package):
+    case .plugin(let name, let package):
       return ".plugin(name: \(literal: name), package: \(literal: package))"
     }
   }

Sources/SwiftRefactor/PackageManifest/ProductDescription.swift

@@ -76,7 +76,7 @@ extension ProductDescription: ManifestSyntaxRepresentable {
   private var functionName: String {
     switch type {
     case .executable: return "executable"
-    case .library(_): return "library"
+    case .library: return "library"
     case .macro: return "macro"
     case .plugin: return "plugin"
     case .snippet: return "snippet"

Sources/SwiftRefactor/PackageManifest/StringUtils.swift

@@ -18,7 +18,7 @@ extension String {
   func mangledToC99ExtendedIdentifier() -> String {
     // Map invalid C99-invalid Unicode scalars to a replacement character.
     let replacementUnichar: UnicodeScalar = "_"
-    var mangledUnichars: [UnicodeScalar] = self.unicodeScalars.map({
+    var mangledUnichars: [UnicodeScalar] = self.unicodeScalars.map {
       switch $0.value {
       case  // A-Z
       0x0041...0x005A,
@@ -194,10 +194,10 @@ extension String {
       default:
         return replacementUnichar
       }
-    })
+    }
 
     // Apply further restrictions to the prefix.
-    loop: for (idx, c) in mangledUnichars.enumerated() {
+    LOOP: for (idx, c) in mangledUnichars.enumerated() {
       switch c.value {
       case  // 0-9
       0x0030...0x0039,
@@ -208,16 +208,16 @@ extension String {
         0x0CE6...0x0CEF, 0x0D66...0x0D6F, 0x0E50...0x0E59,
         0x0ED0...0x0ED9, 0x0F20...0x0F33:
         mangledUnichars[idx] = replacementUnichar
-        break loop
+        break LOOP
       default:
-        break loop
+        break LOOP
       }
     }
 
     // Combine the characters as a string again and return it.
     // FIXME: We should only construct a new string if anything changed.
     // FIXME: There doesn't seem to be a way to create a string from an
     //        array of Unicode scalars; but there must be a better way.
-    return mangledUnichars.reduce("") { $0 + String($1) }
+    return String(decoding: mangledUnichars.flatMap { $0.utf8 }, as: UTF8.self)
   }
 }

Sources/SwiftRefactor/PackageManifest/SyntaxEditUtils.swift

@@ -23,22 +23,12 @@ extension Trivia {
   var hasNewlines: Bool {
     contains(where: \.isNewline)
   }
-
-  /// Produce trivia from the last newline to the end, dropping anything
-  /// prior to that.
-  var onlyLastLine: Trivia {
-    guard let lastNewline = pieces.lastIndex(where: { $0.isNewline }) else {
-      return self
-    }
-
-    return Trivia(pieces: pieces[lastNewline...])
-  }
 }
 
 /// Syntax walker to find the first occurrence of a given node kind that
 /// matches a specific predicate.
 private class FirstNodeFinder<Node: SyntaxProtocol>: SyntaxAnyVisitor {
-  var predicate: (Node) -> Bool
+  let predicate: (Node) -> Bool
   var found: Node? = nil
 
   init(predicate: @escaping (Node) -> Bool) {
@@ -100,7 +90,7 @@ extension LabeledExprListSyntax {
   /// Find the index at which the one would insert a new argument given
   /// the set of argument labels that could come after the argument we
   /// want to insert.
-  func findArgumentInsertionPosition(
+  fileprivate func findArgumentInsertionPosition(
     labelsAfter: Set<String>
   ) -> SyntaxChildrenIndex {
     firstIndex {
@@ -120,7 +110,7 @@ extension LabeledExprListSyntax {
   /// created by the `generator` function, which is provided with leading
   /// trivia and trailing comma it should use to match the surrounding
   /// context.
-  func insertingArgument(
+  fileprivate func insertingArgument(
     at position: SyntaxChildrenIndex,
     generator: (_ leadingTrivia: Trivia, _ trailingComma: TokenSyntax?) -> LabeledExprSyntax
   ) -> LabeledExprListSyntax {
@@ -236,9 +226,8 @@ extension ArrayExprSyntax {
       // there.
       if last.trailingComma == nil {
         var newElements = Array(elements)
-        newElements[newElements.count - 1].trailingComma = .commaToken()
+        newElements[newElements.count - 1].trailingComma = .commaToken(trailingTrivia: last.expression.trailingTrivia)
         newElements[newElements.count - 1].expression.trailingTrivia = Trivia()
-        newElements[newElements.count - 1].trailingTrivia = last.trailingTrivia
         elements = ArrayElementListSyntax(newElements)
       }
 
@@ -289,12 +278,12 @@ extension ExprSyntax {
 }
 
 // MARK: Utilities to oeprate on arrays of array literal elements.
-extension Array<ArrayElementSyntax> {
+extension [ArrayElementSyntax] {
   /// Append a new argument expression.
   mutating func append(expression: ExprSyntax) {
     // Add a comma on the prior expression, if there is one.
     let leadingTrivia: Trivia?
-    if count > 0 {
+    if !isEmpty {
       self[count - 1].trailingComma = TokenSyntax.commaToken()
       leadingTrivia = .newline
 
@@ -317,7 +306,7 @@ extension Array<ArrayElementSyntax> {
 
 // MARK: Utilities to operate on arrays of call arguments.
 
-extension Array<LabeledExprSyntax> {
+extension [LabeledExprSyntax] {
   /// Append a potentially labeled argument with the argument expression.
   mutating func append(label: String?, expression: ExprSyntax) {
     // Add a comma on the prior expression, if there is one.
@@ -422,7 +411,7 @@ extension Array<LabeledExprSyntax> {
 }
 
 // MARK: Utilities for adding arguments into calls.
-fileprivate class ReplacingRewriter: SyntaxRewriter {
+private class ReplacingRewriter: SyntaxRewriter {
   let childNode: Syntax
   let newChildNode: Syntax