From 62d8e5508313adac617fafe3c081dbae1ddde427 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 17 Apr 2024 14:29:44 -0700 Subject: [PATCH 01/22] Add a PackageModelSyntax library that manipulates the source of a manifest file Package manifest files are Swift source code, so editing them means working with the source code directly. Introduce a new library based on swift-syntax that allows us to perform targeted manipulations of the manifest file programmatically, making suggested edits to the file without having to understand everything in it. Introduce one editing operation that adds a particular package dependency to the manifest file. For example, it takes the programmatic representation of a package dependency in the model (`PackageModel.PackageDependency`) and the syntax for a manifest file, then it will produce a set of edits that extend (or add) the dependencies in the `Package` instance to the manifest file, e.g., .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), We make an attempt to match the surrounding trivia so that we don't make an ugly mess of the resulting manifest. I have tests for a number of cases to make sure they look nice, but I expect we'll have to refine the heuristics over time. --- Package.swift | 24 ++ .../ManifestSourceGeneration.swift | 4 +- .../AddPackageDependency.swift | 111 +++++++ .../ManifestSyntaxRepresentable.swift | 40 +++ .../PackageDependency+Syntax.swift | 23 ++ .../PackageModelSyntax/SyntaxEditUtils.swift | 225 +++++++++++++ .../ManifestEditTests.swift | 312 ++++++++++++++++++ Utilities/bootstrap | 2 + 8 files changed, 739 insertions(+), 2 deletions(-) create mode 100644 Sources/PackageModelSyntax/AddPackageDependency.swift create mode 100644 Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift create mode 100644 Sources/PackageModelSyntax/PackageDependency+Syntax.swift create mode 100644 Sources/PackageModelSyntax/SyntaxEditUtils.swift create mode 100644 Tests/PackageModelSyntaxTests/ManifestEditTests.swift diff --git a/Package.swift b/Package.swift index 95a74ccc320..905b564b25c 100644 --- a/Package.swift +++ b/Package.swift @@ -49,6 +49,7 @@ let swiftPMDataModelProduct = ( "PackageLoading", "PackageMetadata", "PackageModel", + "PackageModelSyntax", "SourceControl", "Workspace", ] @@ -246,6 +247,19 @@ let package = Package( swiftSettings: packageModelResourcesSettings ), + .target( + /** Primary Package model objects relationship to SwiftSyntax */ + name: "PackageModelSyntax", + dependencies: [ + "Basics", + "PackageModel", + .product(name: "SwiftDiagnostics", package: "swift-syntax"), + .product(name: "SwiftParser", package: "swift-syntax"), + .product(name: "SwiftSyntax", package: "swift-syntax"), + .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), + ] + ), + .target( /** Package model conventions and loading support */ name: "PackageLoading", @@ -635,6 +649,14 @@ let package = Package( name: "PackageModelTests", dependencies: ["PackageModel", "SPMTestSupport"] ), + .testTarget( + name: "PackageModelSyntaxTests", + dependencies: [ + "PackageModelSyntax", + "SPMTestSupport", + .product(name: "SwiftIDEUtils", package: "swift-syntax"), + ] + ), .testTarget( name: "PackageGraphTests", dependencies: ["PackageGraph", "SPMTestSupport"] @@ -785,6 +807,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil { .package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "1.2.2")), .package(url: "https://github.com/apple/swift-driver.git", branch: relatedDependenciesBranch), .package(url: "https://github.com/apple/swift-crypto.git", .upToNextMinor(from: "3.0.0")), + .package(url: "https://github.com/apple/swift-syntax.git", branch: relatedDependenciesBranch), .package(url: "https://github.com/apple/swift-system.git", .upToNextMinor(from: "1.1.1")), .package(url: "https://github.com/apple/swift-collections.git", .upToNextMinor(from: "1.0.1")), .package(url: "https://github.com/apple/swift-certificates.git", .upToNextMinor(from: "1.0.1")), @@ -795,6 +818,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil { .package(path: "../swift-argument-parser"), .package(path: "../swift-driver"), .package(path: "../swift-crypto"), + .package(path: "../swift-syntax"), .package(path: "../swift-system"), .package(path: "../swift-collections"), .package(path: "../swift-certificates"), diff --git a/Sources/PackageModel/ManifestSourceGeneration.swift b/Sources/PackageModel/ManifestSourceGeneration.swift index f2a31e932fd..03a0d82abbc 100644 --- a/Sources/PackageModel/ManifestSourceGeneration.swift +++ b/Sources/PackageModel/ManifestSourceGeneration.swift @@ -62,7 +62,7 @@ public typealias ManifestCustomProductTypeSourceGenerator = (ProductDescription) /// Convenience initializers for package manifest structures. -fileprivate extension SourceCodeFragment { +public extension SourceCodeFragment { /// Instantiates a SourceCodeFragment to represent an entire manifest. init( @@ -633,7 +633,7 @@ public struct SourceCodeFragment { self.subnodes = subnodes } - func generateSourceCode(indent: String = "") -> String { + public func generateSourceCode(indent: String = "") -> String { var string = literal if let subnodes { switch delimiters { diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift new file mode 100644 index 00000000000..411a84dc4ca --- /dev/null +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -0,0 +1,111 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import PackageModel +import SwiftParser +import SwiftSyntax +import SwiftSyntaxBuilder + +/// Add a package dependency to a manifest's source code. +public struct AddPackageDependency { + /// The set of argument labels that can occur after the "dependencies" + /// argument in the Package initializers. + /// + /// TODO: Could we generate this from the the PackageDescription module, so + /// we don't have keep it up-to-date manually? + private static let argumentLabelsAfterDependencies: Set = [ + "targets", + "swiftLanguageVersions", + "cLanguageStandard", + "cxxLanguageStandard" + ] + + /// Produce the set of source edits needed to add the given package + /// dependency to the given manifest file. + public static func addPackageDependency( + _ dependency: PackageDependency, + to manifest: SourceFileSyntax, + manifestDirectory: AbsolutePath + ) -> [SourceEdit] { + guard let packageCall = manifest.findCall(calleeName: "Package") else { + return [] + } + + let dependencySyntax = dependency.asSyntax(manifestDirectory: manifestDirectory) + + // If there is already a "dependencies" argument, append to the array + // literal in there. + if let dependenciesArg = packageCall.findArgument(labeled: "dependencies") { + guard let argArray = dependenciesArg.expression.as(ArrayExprSyntax.self) else { + return [] + } + + let updatedArgArray = argArray.appending( + element: dependencySyntax, + outerLeadingTrivia: dependenciesArg.leadingTrivia + ) + return [ .replace(argArray, with: updatedArgArray.description) ] + } + + // There was no "dependencies" argument, so we need to create one. + + // Insert the new argument at the appropriate place in the call. + let insertionPos = packageCall.arguments.findArgumentInsertionPosition( + labelsAfter: Self.argumentLabelsAfterDependencies + ) + let newArguments = packageCall.arguments.insertingArgument( + at: insertionPos + ) { (leadingTrivia, trailingComma) in + // The argument is always [ element ], but if we have any newlines + // in the leading trivia, then we really want to split it across + // multiple lines, like this: + // [ + // element + // ] + let newArgument: ExprSyntax + if !leadingTrivia.hasNewlines { + newArgument = " [ \(dependencySyntax), ]" + } else { + let innerTrivia = leadingTrivia.appending(defaultIndent) + let arrayExpr = ArrayExprSyntax( + leadingTrivia: .space, + elements: [ + ArrayElementSyntax( + leadingTrivia: innerTrivia, + expression: dependencySyntax, + trailingComma: .commaToken() + ) + ], + rightSquare: .rightSquareToken() + .with(\.leadingTrivia, leadingTrivia) + ) + newArgument = ExprSyntax(arrayExpr) + } + + return LabeledExprSyntax( + leadingTrivia: leadingTrivia, + label: "dependencies", + colon: .colonToken(), + expression: newArgument, + trailingComma: trailingComma + ) + } + + return [ + SourceEdit.replace( + packageCall.arguments, + with: newArguments.description + ) + ] + } +} diff --git a/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift new file mode 100644 index 00000000000..45686e59bf2 --- /dev/null +++ b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift @@ -0,0 +1,40 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import SwiftSyntax + +/// Describes an entity in the package model that can be represented as +/// a syntax node. +protocol ManifestSyntaxRepresentable { + /// The most specific kind of syntax node that best describes this entity + /// in the manifest. + /// + /// There might be other kinds of syntax nodes that can also represent + /// the syntax, but this is the one that a canonical manifest will use. + /// As an example, a package dependency is usually expressed as, e.g., + /// .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") + /// + /// However, there could be other forms, e.g., this is also valid: + /// Package.Dependency.package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") + associatedtype PreferredSyntax: SyntaxProtocol + + /// Provides a suitable syntax node to describe this entity in the package + /// model. + /// + /// The resulting syntax is a fragment that describes just this entity, + /// and it's enclosing entity will need to understand how to fit it in. + /// For example, a `PackageDependency` entity would map to syntax for + /// something like + /// .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") + func asSyntax(manifestDirectory: AbsolutePath) -> PreferredSyntax +} diff --git a/Sources/PackageModelSyntax/PackageDependency+Syntax.swift b/Sources/PackageModelSyntax/PackageDependency+Syntax.swift new file mode 100644 index 00000000000..d48c7c2fb85 --- /dev/null +++ b/Sources/PackageModelSyntax/PackageDependency+Syntax.swift @@ -0,0 +1,23 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import PackageModel +import SwiftSyntax +import SwiftParser + +extension PackageDependency: ManifestSyntaxRepresentable { + func asSyntax(manifestDirectory: AbsolutePath) -> ExprSyntax { + let fragment = SourceCodeFragment(from: self, pathAnchor: manifestDirectory) + return "\(raw: fragment.generateSourceCode())" + } +} diff --git a/Sources/PackageModelSyntax/SyntaxEditUtils.swift b/Sources/PackageModelSyntax/SyntaxEditUtils.swift new file mode 100644 index 00000000000..6c5d10d14de --- /dev/null +++ b/Sources/PackageModelSyntax/SyntaxEditUtils.swift @@ -0,0 +1,225 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import PackageModel +import SwiftSyntax +import SwiftParser + +/// Default indent when we have to introduce indentation but have no context +/// to get it right. +let defaultIndent = TriviaPiece.spaces(4) + +extension Trivia { + /// Determine whether this trivia has newlines or not. + var hasNewlines: Bool { + contains { piece in + if case .newlines = piece { + return true + } else { + return false + } + } + } +} + +/// Syntax walker to find the first occurrence of a given node kind that +/// matches a specific predicate. +private class FirstNodeFinder: SyntaxAnyVisitor { + var predicate: (Node) -> Bool + var found: Node? = nil + + init(predicate: @escaping (Node) -> Bool) { + self.predicate = predicate + super.init(viewMode: .sourceAccurate) + } + + override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind { + if found != nil { + return .skipChildren + } + + if let matchedNode = node.as(Node.self), predicate(matchedNode) { + found = matchedNode + return .skipChildren + } + + return .visitChildren + } +} + +extension SyntaxProtocol { + /// Find the first node of the Self type that matches the given predicate. + static func findFirst( + in node: some SyntaxProtocol, + matching predicate: (Self) -> Bool + ) -> Self? { + withoutActuallyEscaping(predicate) { escapingPredicate in + let visitor = FirstNodeFinder(predicate: escapingPredicate) + visitor.walk(node) + return visitor.found + } + } +} + +extension FunctionCallExprSyntax { + /// Check whether this call expression has a callee that is a reference + /// to a declaration with the given name. + func hasCallee(named name: String) -> Bool { + guard let calleeDeclRef = calledExpression.as(DeclReferenceExprSyntax.self) else { + return false + } + + return calleeDeclRef.baseName.text == name + } + + /// Find a call argument based on its label. + func findArgument(labeled label: String) -> LabeledExprSyntax? { + arguments.first { $0.label?.text == label } + } +} + +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( + labelsAfter: Set + ) -> SyntaxChildrenIndex { + firstIndex { + guard let label = $0.label else { + return false + } + + return labelsAfter.contains(label.text) + } ?? endIndex + } + + /// Form a new argument list that inserts a new argument at the specified + /// position in this argument list. + /// + /// This operation will attempt to introduce trivia to match the + /// surrounding context where possible. The actual argument will be + /// created by the `generator` function, which is provided with leading + /// trivia and trailing comma it should use to match the surrounding + /// context. + func insertingArgument( + at position: SyntaxChildrenIndex, + generator: (Trivia, TokenSyntax?) -> LabeledExprSyntax + ) -> LabeledExprListSyntax { + // Turn the arguments into an array so we can manipulate them. + var arguments = Array(self) + + let positionIdx = distance(from: startIndex, to: position) + + let commaToken = TokenSyntax(.comma, presence: .present) + + // Figure out leading trivia and adjust the prior argument (if there is + // one) by adding a comma, if necessary. + let leadingTrivia: Trivia + if position > startIndex { + let priorArgument = arguments[positionIdx - 1] + + // Our leading trivia will be based on the prior argument's leading + // trivia. + leadingTrivia = priorArgument.leadingTrivia + + // If the prior argument is missing a trailing comma, add one. + if priorArgument.trailingComma == nil { + arguments[positionIdx - 1].trailingComma = commaToken + } + } else if positionIdx + 1 < count { + leadingTrivia = arguments[positionIdx + 1].leadingTrivia + } else { + leadingTrivia = Trivia() + } + + // Determine whether we need a trailing comma on this argument. + let trailingComma: TokenSyntax? + if position < endIndex { + trailingComma = commaToken + } else { + trailingComma = nil + } + + // Create the argument and insert it into the argument list. + let argument = generator(leadingTrivia, trailingComma) + arguments.insert(argument, at: positionIdx) + + return LabeledExprListSyntax(arguments) + } +} + +extension SyntaxProtocol { + /// Look for a call expression to a callee with the given name. + func findCall(calleeName: String) -> FunctionCallExprSyntax? { + return FunctionCallExprSyntax.findFirst(in: self) { call in + return call.hasCallee(named: calleeName) + } + } +} + +extension ArrayExprSyntax { + /// Produce a new array literal expression that appends the given + /// element, while trying to maintain similar indentation. + func appending( + element: ExprSyntax, + outerLeadingTrivia: Trivia + ) -> ArrayExprSyntax { + var elements = self.elements + + let commaToken = TokenSyntax(.comma, presence: .present) + + // If there are already elements, tack it on. + let leadingTrivia: Trivia + let trailingTrivia: Trivia + let leftSquareTrailingTrivia: Trivia + if let last = elements.last { + // The leading trivia of the new element should match that of the + // last element. + leadingTrivia = last.leadingTrivia + + // Add a trailing comma to the last element if it isn't already + // there. + if last.trailingComma == nil { + var newElements = Array(elements) + newElements[newElements.count-1] = last.with(\.trailingComma, commaToken) + elements = ArrayElementListSyntax(newElements) + } + + trailingTrivia = Trivia() + leftSquareTrailingTrivia = leftSquare.trailingTrivia + } else { + leadingTrivia = outerLeadingTrivia.appending(defaultIndent) + trailingTrivia = outerLeadingTrivia + if leftSquare.trailingTrivia.hasNewlines { + leftSquareTrailingTrivia = leftSquare.trailingTrivia + } else { + leftSquareTrailingTrivia = Trivia() + } + } + + elements.append( + ArrayElementSyntax( + expression: element.with(\.leadingTrivia, leadingTrivia), + trailingComma: commaToken.with(\.trailingTrivia, trailingTrivia) + ) + ) + + let newLeftSquare = leftSquare.with( + \.trailingTrivia, + leftSquareTrailingTrivia + ) + + return with(\.elements, elements).with(\.leftSquare, newLeftSquare) + } +} diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift new file mode 100644 index 00000000000..5bfb9881494 --- /dev/null +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -0,0 +1,312 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +import Basics +import PackageModel +import PackageModelSyntax +@_spi(FixItApplier) import SwiftIDEUtils +import SwiftParser +import SwiftSyntax + +import XCTest + +func assertManifestRefactor( + _ originalManifest: SourceFileSyntax, + expectedManifest: SourceFileSyntax, + file: StaticString = #filePath, + line: UInt = #line, + operation: (SourceFileSyntax) -> [SourceEdit] +) { + let edits = operation(originalManifest) + let editedManifestSource = FixItApplier.apply(edits: edits, to: originalManifest) + + let editedManifest = Parser.parse(source: editedManifestSource) + assertStringsEqualWithDiff( + editedManifest.description, + expectedManifest.description, + file: file, + line: line + ) +} + +class ManifestEditTests: XCTestCase { + static let swiftSystemURL: SourceControlURL = "https://github.com/apple/swift-system.git" + static let swiftSystemPackageDependency = PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: swiftSystemURL, + requirement: .branch("main"), productFilter: .nothing + ) + + func testAddPackageDependencyExistingComma() { + assertManifestRefactor(""" + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), + ] + ) + """, expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } + + func testAddPackageDependencyExistingNoComma() { + assertManifestRefactor(""" + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") + ] + ) + """, expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } + + func testAddPackageDependencyExistingEmpty() { + assertManifestRefactor(""" + let package = Package( + name: "packages", + dependencies: [ ] + ) + """, + expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } + + func testAddPackageDependencyNoExistingAtEnd() { + assertManifestRefactor(""" + let package = Package( + name: "packages" + ) + """, + expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } + + func testAddPackageDependencyNoExistingMiddle() { + assertManifestRefactor(""" + let package = Package( + name: "packages", + targets: [] + ) + """, + expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ], + targets: [] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } +} + + +// FIXME: Copy-paste from _SwiftSyntaxTestSupport + +/// Asserts that the two strings are equal, providing Unix `diff`-style output if they are not. +/// +/// - Parameters: +/// - actual: The actual string. +/// - expected: The expected string. +/// - message: An optional description of the failure. +/// - additionalInfo: Additional information about the failed test case that will be printed after the diff +/// - file: The file in which failure occurred. Defaults to the file name of the test case in +/// which this function was called. +/// - line: The line number on which failure occurred. Defaults to the line number on which this +/// function was called. +public func assertStringsEqualWithDiff( + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + if actual == expected { + return + } + failStringsEqualWithDiff( + actual, + expected, + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) +} + +/// Asserts that the two data are equal, providing Unix `diff`-style output if they are not. +/// +/// - Parameters: +/// - actual: The actual string. +/// - expected: The expected string. +/// - message: An optional description of the failure. +/// - additionalInfo: Additional information about the failed test case that will be printed after the diff +/// - file: The file in which failure occurred. Defaults to the file name of the test case in +/// which this function was called. +/// - line: The line number on which failure occurred. Defaults to the line number on which this +/// function was called. +public func assertDataEqualWithDiff( + _ actual: Data, + _ expected: Data, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + if actual == expected { + return + } + + // NOTE: Converting to `Stirng` here looses invalid UTF8 sequence difference, + // but at least we can see something is different. + failStringsEqualWithDiff( + String(decoding: actual, as: UTF8.self), + String(decoding: expected, as: UTF8.self), + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) +} + +/// `XCTFail` with `diff`-style output. +public func failStringsEqualWithDiff( + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + let stringComparison: String + + // Use `CollectionDifference` on supported platforms to get `diff`-like line-based output. On + // older platforms, fall back to simple string comparison. + if #available(macOS 10.15, *) { + let actualLines = actual.components(separatedBy: .newlines) + let expectedLines = expected.components(separatedBy: .newlines) + + let difference = actualLines.difference(from: expectedLines) + + var result = "" + + var insertions = [Int: String]() + var removals = [Int: String]() + + for change in difference { + switch change { + case .insert(let offset, let element, _): + insertions[offset] = element + case .remove(let offset, let element, _): + removals[offset] = element + } + } + + var expectedLine = 0 + var actualLine = 0 + + while expectedLine < expectedLines.count || actualLine < actualLines.count { + if let removal = removals[expectedLine] { + result += "–\(removal)\n" + expectedLine += 1 + } else if let insertion = insertions[actualLine] { + result += "+\(insertion)\n" + actualLine += 1 + } else { + result += " \(expectedLines[expectedLine])\n" + expectedLine += 1 + actualLine += 1 + } + } + + stringComparison = result + } else { + // Fall back to simple message on platforms that don't support CollectionDifference. + stringComparison = """ + Expected: + \(expected) + + Actual: + \(actual) + """ + } + + var fullMessage = """ + \(message.isEmpty ? "Actual output does not match the expected" : message) + \(stringComparison) + """ + if let additional = additionalInfo() { + fullMessage = """ + \(fullMessage) + \(additional) + """ + } + XCTFail(fullMessage, file: file, line: line) +} diff --git a/Utilities/bootstrap b/Utilities/bootstrap index 2da5c06d725..318e3941941 100755 --- a/Utilities/bootstrap +++ b/Utilities/bootstrap @@ -345,6 +345,7 @@ def build(args): build_dependency(args, "swift-driver", swift_driver_cmake_flags) build_dependency(args, "swift-collections") build_dependency(args, "swift-crypto") + build_dependency(args, "swift-syntax") build_dependency(args, "swift-asn1") build_dependency(args, "swift-certificates", ["-DSwiftASN1_DIR=" + os.path.join(args.build_dirs["swift-asn1"], "cmake/modules"), @@ -758,6 +759,7 @@ def get_swiftpm_env_cmd(args): os.path.join(args.build_dirs["swift-argument-parser"], "lib"), os.path.join(args.build_dirs["swift-crypto"], "lib"), os.path.join(args.build_dirs["swift-driver"], "lib"), + os.path.join(args.build_dirs["swift-syntax"], "lib"), os.path.join(args.build_dirs["swift-system"], "lib"), os.path.join(args.build_dirs["swift-collections"], "lib"), os.path.join(args.build_dirs["swift-asn1"], "lib"), From 828a50610bc2c5d3df33dc497cd1bfcb64304a7a Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 17 Apr 2024 16:36:56 -0700 Subject: [PATCH 02/22] Add `swift package add` command to add a package dependency to the manifest Introduce a new `package add` command that adds a package dependency to your manifest file, given command-line arguments containing the URL and the version requiements (branch, from-version, etc.). This utilizes the new infrastructure for swift-syntax based package editing, putting a command-line interface over it. Here's the help output: OVERVIEW: Add a dependency to the package manifest USAGE: swift package add [--branch ] [--from-version ] [--exact-version ] [--revision ] ARGUMENTS: The URL or directory of the package to add OPTIONS: --branch The branch to depend on --from-version The minimum version requirement --exact-version The exact version requirement --revision A specific revision requirement --version Show the version. -h, -help, --help Show help information. --- Package.swift | 2 + Sources/Commands/PackageCommands/Add.swift | 123 ++++++++++++++++++ .../PackageCommands/SwiftPackageCommand.swift | 1 + Sources/Workspace/Workspace.swift | 2 +- Tests/CommandsTests/PackageCommandTests.swift | 27 ++++ 5 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 Sources/Commands/PackageCommands/Add.swift diff --git a/Package.swift b/Package.swift index 905b564b25c..5d2138e12d1 100644 --- a/Package.swift +++ b/Package.swift @@ -428,10 +428,12 @@ let package = Package( dependencies: [ .product(name: "ArgumentParser", package: "swift-argument-parser"), .product(name: "OrderedCollections", package: "swift-collections"), + .product(name: "SwiftIDEUtils", package: "swift-syntax"), "Basics", "Build", "CoreCommands", "PackageGraph", + "PackageModelSyntax", "SourceControl", "Workspace", "XCBuildSupport", diff --git a/Sources/Commands/PackageCommands/Add.swift b/Sources/Commands/PackageCommands/Add.swift new file mode 100644 index 00000000000..eac6b207fee --- /dev/null +++ b/Sources/Commands/PackageCommands/Add.swift @@ -0,0 +1,123 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import ArgumentParser +import Basics +import CoreCommands +import PackageModel +import PackageModelSyntax +@_spi(FixItApplier) import SwiftIDEUtils +import SwiftParser +import SwiftSyntax +import TSCBasic +import TSCUtility +import Workspace + +extension SwiftPackageCommand { + struct Add: SwiftCommand { + package static let configuration = CommandConfiguration( + abstract: "Add a dependency to the package manifest") + + @OptionGroup(visibility: .hidden) + var globalOptions: GlobalOptions + + @Option(help: "The branch to depend on") + var branch: String? + + @Option(help: "The minimum version requirement") + var fromVersion: String? + + @Option(help: "The exact version requirement") + var exactVersion: String? + + @Option(help: "A specific revision requirement") + var revision: String? + + // FIXME: range option + + @Argument(help: "The URL or directory of the package to add") + var url: String + + func run(_ swiftCommandState: SwiftCommandState) throws { + let workspace = try swiftCommandState.getActiveWorkspace() + + guard let packagePath = try swiftCommandState.getWorkspaceRoot().packages.first else { + throw StringError("unknown package") + } + + // Load the manifest file + let fileSystem = workspace.fileSystem + let manifestPath = packagePath.appending("Package.swift") + let manifestContents: ByteString + do { + manifestContents = try fileSystem.readFileContents(manifestPath) + } catch { + throw StringError("cannot find package manifest in \(manifestPath)") + } + + // Parse the manifest. + let manifestSyntax = manifestContents.withData { data in + data.withUnsafeBytes { buffer in + buffer.withMemoryRebound(to: UInt8.self) { buffer in + Parser.parse(source: buffer) + } + } + } + + let identity = PackageIdentity(url: .init(url)) + + // Figure out the version requirement. + let requirement: PackageDependency.SourceControl.Requirement + if let branch { + requirement = .branch(branch) + } else if let fromVersion { + requirement = .revision(fromVersion) + } else if let exactVersion { + requirement = .exact(try Version(versionString: exactVersion)) + } else { + throw StringError("must specify one of --branch, --from-version, or --exact-version") + } + + // Figure out the location of the package. + let location: PackageDependency.SourceControl.Location + if let path = try? Basics.AbsolutePath(validating: url) { + location = .local(path) + } else { + location = .remote(.init(url)) + } + + let packageDependency: PackageDependency = .sourceControl( + identity: identity, + nameForTargetDependencyResolutionOnly: nil, + location: location, + requirement: requirement, + productFilter: .everything + ) + + let edits = AddPackageDependency.addPackageDependency( + packageDependency, + to: manifestSyntax, + manifestDirectory: packagePath.parentDirectory + ) + + if edits.isEmpty { + throw StringError("Unable to add package to manifest file") + } + + let updatedManifestSource = FixItApplier.apply(edits: edits, to: manifestSyntax) + try fileSystem.writeFileContents( + manifestPath, + string: updatedManifestSource + ) + } + } +} diff --git a/Sources/Commands/PackageCommands/SwiftPackageCommand.swift b/Sources/Commands/PackageCommands/SwiftPackageCommand.swift index ddedbaf7046..62b2a6398e0 100644 --- a/Sources/Commands/PackageCommands/SwiftPackageCommand.swift +++ b/Sources/Commands/PackageCommands/SwiftPackageCommand.swift @@ -35,6 +35,7 @@ package struct SwiftPackageCommand: AsyncParsableCommand { discussion: "SEE ALSO: swift build, swift run, swift test", version: SwiftVersion.current.completeDisplayString, subcommands: [ + Add.self, Clean.self, PurgeCache.self, Reset.self, diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 38af1b7362b..f96bb976761 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -96,7 +96,7 @@ public class Workspace { public let pinsStore: LoadableResult /// The file system on which the workspace will operate. - let fileSystem: any FileSystem + public let fileSystem: any FileSystem /// The host toolchain to use. private let hostToolchain: UserToolchain diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index d34e4c898fa..e6353ced4e0 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -793,6 +793,33 @@ final class PackageCommandTests: CommandsTestCase { } } + func testPackageAddVersion() throws { + try testWithTemporaryDirectory { tmpPath in + let fs = localFileSystem + let path = tmpPath.appending("PackageB") + try fs.createDirectory(path) + + try fs.writeFileContents(path.appending("Package.swift"), string: + """ + // swift-tools-version: 5.9 + import PackageDescription + let package = Package( + name: "client", + targets: [ .target(name: "client", dependencies: [ "library" ]) ] + ) + """ + ) + + _ = try execute(["add", "--branch", "main", "https://github.com/apple/swift-syntax.git"], packagePath: path) + + let manifest = path.appending("Package.swift") + XCTAssertFileExists(manifest) + let contents: String = try fs.readFileContents(manifest) + + XCTAssertMatch(contents, .contains(#".package(url: "https://github.com/apple/swift-syntax.git", .branch("main")),"#)) + } + } + func testPackageEditAndUnedit() throws { try fixture(name: "Miscellaneous/PackageEdit") { fixturePath in let fooPath = fixturePath.appending("foo") From 7a4f41478550b7a43b94101af814c661e19df7dd Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 09:36:32 -0700 Subject: [PATCH 03/22] Handle insertion into a dependencies array like "[] + a" --- .../AddPackageDependency.swift | 2 +- .../PackageModelSyntax/SyntaxEditUtils.swift | 18 +++++++++++++ .../ManifestEditTests.swift | 25 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index 411a84dc4ca..8facf1466f0 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -46,7 +46,7 @@ public struct AddPackageDependency { // If there is already a "dependencies" argument, append to the array // literal in there. if let dependenciesArg = packageCall.findArgument(labeled: "dependencies") { - guard let argArray = dependenciesArg.expression.as(ArrayExprSyntax.self) else { + guard let argArray = dependenciesArg.expression.findArrayArgument() else { return [] } diff --git a/Sources/PackageModelSyntax/SyntaxEditUtils.swift b/Sources/PackageModelSyntax/SyntaxEditUtils.swift index 6c5d10d14de..772b33d6330 100644 --- a/Sources/PackageModelSyntax/SyntaxEditUtils.swift +++ b/Sources/PackageModelSyntax/SyntaxEditUtils.swift @@ -223,3 +223,21 @@ extension ArrayExprSyntax { return with(\.elements, elements).with(\.leftSquare, newLeftSquare) } } + +extension ExprSyntax { + /// Find an array argument either at the top level or within a sequence + /// expression. + func findArrayArgument() -> ArrayExprSyntax? { + if let arrayExpr = self.as(ArrayExprSyntax.self) { + return arrayExpr + } + + if let sequenceExpr = self.as(SequenceExprSyntax.self) { + return sequenceExpr.elements.lazy.compactMap { + $0.findArrayArgument() + }.first + } + + return nil + } +} diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index 5bfb9881494..b8d1f166e51 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -96,6 +96,31 @@ class ManifestEditTests: XCTestCase { } } + func testAddPackageDependencyExistingAppended() { + assertManifestRefactor(""" + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") + ] + [] + ) + """, expectedManifest: """ + let package = Package( + name: "packages", + dependencies: [ + .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), + .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + ] + [] + ) + """) { manifest in + AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } + func testAddPackageDependencyExistingEmpty() { assertManifestRefactor(""" let package = Package( From f1491d2bdc6bb205aa764d9e199da171e6f3c15c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 11:16:12 -0700 Subject: [PATCH 04/22] Improve error handling for adding a package dependency to a manifest --- Sources/Commands/PackageCommands/Add.swift | 2 +- .../AddPackageDependency.swift | 9 +- .../ManifestEditError.swift | 31 ++ .../ManifestEditTests.swift | 304 ++++++++++-------- 4 files changed, 210 insertions(+), 136 deletions(-) create mode 100644 Sources/PackageModelSyntax/ManifestEditError.swift diff --git a/Sources/Commands/PackageCommands/Add.swift b/Sources/Commands/PackageCommands/Add.swift index eac6b207fee..73f66da4392 100644 --- a/Sources/Commands/PackageCommands/Add.swift +++ b/Sources/Commands/PackageCommands/Add.swift @@ -103,7 +103,7 @@ extension SwiftPackageCommand { productFilter: .everything ) - let edits = AddPackageDependency.addPackageDependency( + let edits = try AddPackageDependency.addPackageDependency( packageDependency, to: manifestSyntax, manifestDirectory: packagePath.parentDirectory diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index 8facf1466f0..fe33f6c3f50 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -36,9 +36,9 @@ public struct AddPackageDependency { _ dependency: PackageDependency, to manifest: SourceFileSyntax, manifestDirectory: AbsolutePath - ) -> [SourceEdit] { + ) throws -> [SourceEdit] { guard let packageCall = manifest.findCall(calleeName: "Package") else { - return [] + throw ManifestEditError.cannotFindPackage } let dependencySyntax = dependency.asSyntax(manifestDirectory: manifestDirectory) @@ -47,7 +47,10 @@ public struct AddPackageDependency { // literal in there. if let dependenciesArg = packageCall.findArgument(labeled: "dependencies") { guard let argArray = dependenciesArg.expression.findArrayArgument() else { - return [] + throw ManifestEditError.cannotFindArrayLiteralArgument( + argumentName: "dependencies", + node: Syntax(dependenciesArg.expression) + ) } let updatedArgArray = argArray.appending( diff --git a/Sources/PackageModelSyntax/ManifestEditError.swift b/Sources/PackageModelSyntax/ManifestEditError.swift new file mode 100644 index 00000000000..9ec028bbbab --- /dev/null +++ b/Sources/PackageModelSyntax/ManifestEditError.swift @@ -0,0 +1,31 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftSyntax + +/// An error describing problems that can occur when attempting to edit a +/// package manifest programattically. +package enum ManifestEditError: Error { + case cannotFindPackage + case cannotFindArrayLiteralArgument(argumentName: String, node: Syntax) +} + +extension ManifestEditError: CustomStringConvertible { + package var description: String { + switch self { + case .cannotFindPackage: + "invalid manifest: unable to find 'Package' declaration" + case .cannotFindArrayLiteralArgument(argumentName: let name, node: _): + "unable to find array literal for '\(name)' argument" + } + } +} diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index b8d1f166e51..cb1ce36c628 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -12,10 +12,10 @@ import Basics import PackageModel import PackageModelSyntax +import SPMTestSupport @_spi(FixItApplier) import SwiftIDEUtils import SwiftParser import SwiftSyntax - import XCTest func assertManifestRefactor( @@ -23,9 +23,9 @@ func assertManifestRefactor( expectedManifest: SourceFileSyntax, file: StaticString = #filePath, line: UInt = #line, - operation: (SourceFileSyntax) -> [SourceEdit] -) { - let edits = operation(originalManifest) + operation: (SourceFileSyntax) throws -> [SourceEdit] +) rethrows { + let edits = try operation(originalManifest) let editedManifestSource = FixItApplier.apply(edits: edits, to: originalManifest) let editedManifest = Parser.parse(source: editedManifestSource) @@ -46,8 +46,8 @@ class ManifestEditTests: XCTestCase { requirement: .branch("main"), productFilter: .nothing ) - func testAddPackageDependencyExistingComma() { - assertManifestRefactor(""" + func testAddPackageDependencyExistingComma() throws { + try assertManifestRefactor(""" let package = Package( name: "packages", dependencies: [ @@ -63,16 +63,16 @@ class ManifestEditTests: XCTestCase { ] ) """) { manifest in - AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") - ) - } + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } } - func testAddPackageDependencyExistingNoComma() { - assertManifestRefactor(""" + func testAddPackageDependencyExistingNoComma() throws { + try assertManifestRefactor(""" let package = Package( name: "packages", dependencies: [ @@ -88,16 +88,16 @@ class ManifestEditTests: XCTestCase { ] ) """) { manifest in - AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") - ) - } + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } } - func testAddPackageDependencyExistingAppended() { - assertManifestRefactor(""" + func testAddPackageDependencyExistingAppended() throws { + try assertManifestRefactor(""" let package = Package( name: "packages", dependencies: [ @@ -113,16 +113,16 @@ class ManifestEditTests: XCTestCase { ] + [] ) """) { manifest in - AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") - ) + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) } } - func testAddPackageDependencyExistingEmpty() { - assertManifestRefactor(""" + func testAddPackageDependencyExistingEmpty() throws { + try assertManifestRefactor(""" let package = Package( name: "packages", dependencies: [ ] @@ -136,7 +136,7 @@ class ManifestEditTests: XCTestCase { ] ) """) { manifest in - AddPackageDependency.addPackageDependency( + try AddPackageDependency.addPackageDependency( Self.swiftSystemPackageDependency, to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") @@ -144,8 +144,8 @@ class ManifestEditTests: XCTestCase { } } - func testAddPackageDependencyNoExistingAtEnd() { - assertManifestRefactor(""" + func testAddPackageDependencyNoExistingAtEnd() throws { + try assertManifestRefactor(""" let package = Package( name: "packages" ) @@ -158,7 +158,7 @@ class ManifestEditTests: XCTestCase { ] ) """) { manifest in - AddPackageDependency.addPackageDependency( + try AddPackageDependency.addPackageDependency( Self.swiftSystemPackageDependency, to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") @@ -166,8 +166,8 @@ class ManifestEditTests: XCTestCase { } } - func testAddPackageDependencyNoExistingMiddle() { - assertManifestRefactor(""" + func testAddPackageDependencyNoExistingMiddle() throws { + try assertManifestRefactor(""" let package = Package( name: "packages", targets: [] @@ -182,13 +182,52 @@ class ManifestEditTests: XCTestCase { targets: [] ) """) { manifest in - AddPackageDependency.addPackageDependency( + try AddPackageDependency.addPackageDependency( Self.swiftSystemPackageDependency, to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") ) } } + + func testAddPackageDependencyErrors() { + XCTAssertThrows( + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: """ + let package: Package = .init( + name: "packages" + ) + """, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + ) { (error: ManifestEditError) in + if case .cannotFindPackage = error { + return true + } else { + return false + } + } + + XCTAssertThrows( + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: """ + let package = Package( + name: "packages", + dependencies: blah + ) + """, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + ) { (error: ManifestEditError) in + if case .cannotFindArrayLiteralArgument(argumentName: "dependencies", node: _) = error { + return true + } else { + return false + } + } + } } @@ -206,24 +245,25 @@ class ManifestEditTests: XCTestCase { /// - line: The line number on which failure occurred. Defaults to the line number on which this /// function was called. public func assertStringsEqualWithDiff( - _ actual: String, - _ expected: String, - _ message: String = "", - additionalInfo: @autoclosure () -> String? = nil, - file: StaticString = #filePath, - line: UInt = #line + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line ) { - if actual == expected { - return - } - failStringsEqualWithDiff( - actual, - expected, - message, - additionalInfo: additionalInfo(), - file: file, - line: line - ) + if actual == expected { + return + } + + failStringsEqualWithDiff( + actual, + expected, + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) } /// Asserts that the two data are equal, providing Unix `diff`-style output if they are not. @@ -238,100 +278,100 @@ public func assertStringsEqualWithDiff( /// - line: The line number on which failure occurred. Defaults to the line number on which this /// function was called. public func assertDataEqualWithDiff( - _ actual: Data, - _ expected: Data, - _ message: String = "", - additionalInfo: @autoclosure () -> String? = nil, - file: StaticString = #filePath, - line: UInt = #line + _ actual: Data, + _ expected: Data, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line ) { - if actual == expected { - return - } + if actual == expected { + return + } - // NOTE: Converting to `Stirng` here looses invalid UTF8 sequence difference, - // but at least we can see something is different. - failStringsEqualWithDiff( - String(decoding: actual, as: UTF8.self), - String(decoding: expected, as: UTF8.self), - message, - additionalInfo: additionalInfo(), - file: file, - line: line - ) + // NOTE: Converting to `Stirng` here looses invalid UTF8 sequence difference, + // but at least we can see something is different. + failStringsEqualWithDiff( + String(decoding: actual, as: UTF8.self), + String(decoding: expected, as: UTF8.self), + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) } /// `XCTFail` with `diff`-style output. public func failStringsEqualWithDiff( - _ actual: String, - _ expected: String, - _ message: String = "", - additionalInfo: @autoclosure () -> String? = nil, - file: StaticString = #filePath, - line: UInt = #line + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line ) { - let stringComparison: String + let stringComparison: String - // Use `CollectionDifference` on supported platforms to get `diff`-like line-based output. On - // older platforms, fall back to simple string comparison. - if #available(macOS 10.15, *) { - let actualLines = actual.components(separatedBy: .newlines) - let expectedLines = expected.components(separatedBy: .newlines) + // Use `CollectionDifference` on supported platforms to get `diff`-like line-based output. On + // older platforms, fall back to simple string comparison. + if #available(macOS 10.15, *) { + let actualLines = actual.components(separatedBy: .newlines) + let expectedLines = expected.components(separatedBy: .newlines) - let difference = actualLines.difference(from: expectedLines) + let difference = actualLines.difference(from: expectedLines) - var result = "" + var result = "" - var insertions = [Int: String]() - var removals = [Int: String]() + var insertions = [Int: String]() + var removals = [Int: String]() - for change in difference { - switch change { - case .insert(let offset, let element, _): - insertions[offset] = element - case .remove(let offset, let element, _): - removals[offset] = element - } - } + for change in difference { + switch change { + case .insert(let offset, let element, _): + insertions[offset] = element + case .remove(let offset, let element, _): + removals[offset] = element + } + } - var expectedLine = 0 - var actualLine = 0 + var expectedLine = 0 + var actualLine = 0 - while expectedLine < expectedLines.count || actualLine < actualLines.count { - if let removal = removals[expectedLine] { - result += "–\(removal)\n" - expectedLine += 1 - } else if let insertion = insertions[actualLine] { - result += "+\(insertion)\n" - actualLine += 1 - } else { - result += " \(expectedLines[expectedLine])\n" - expectedLine += 1 - actualLine += 1 - } - } + while expectedLine < expectedLines.count || actualLine < actualLines.count { + if let removal = removals[expectedLine] { + result += "–\(removal)\n" + expectedLine += 1 + } else if let insertion = insertions[actualLine] { + result += "+\(insertion)\n" + actualLine += 1 + } else { + result += " \(expectedLines[expectedLine])\n" + expectedLine += 1 + actualLine += 1 + } + } - stringComparison = result - } else { - // Fall back to simple message on platforms that don't support CollectionDifference. - stringComparison = """ - Expected: - \(expected) + stringComparison = result + } else { + // Fall back to simple message on platforms that don't support CollectionDifference. + stringComparison = """ + Expected: + \(expected) - Actual: - \(actual) - """ - } + Actual: + \(actual) + """ + } - var fullMessage = """ - \(message.isEmpty ? "Actual output does not match the expected" : message) - \(stringComparison) - """ - if let additional = additionalInfo() { - fullMessage = """ - \(fullMessage) - \(additional) - """ - } - XCTFail(fullMessage, file: file, line: line) + var fullMessage = """ + \(message.isEmpty ? "Actual output does not match the expected" : message) + \(stringComparison) + """ + if let additional = additionalInfo() { + fullMessage = """ + \(fullMessage) + \(additional) + """ + } + XCTFail(fullMessage, file: file, line: line) } From 3b401f4b85abc43c810ffd9d9928712bebc08ef7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 11:34:54 -0700 Subject: [PATCH 05/22] Don't allow programmatic editing of manifest files for manifests < 5.5 The manifest format has changed a bit since then, and conditionalizing out processing here for older manifests is not worth the effort. Thanks @owenv! --- Package.swift | 1 + .../AddPackageDependency.swift | 7 ++++ .../ManifestEditError.swift | 10 ++++++ .../ManifestEditTests.swift | 33 +++++++++++++++++++ 4 files changed, 51 insertions(+) diff --git a/Package.swift b/Package.swift index 5d2138e12d1..e630489cdd7 100644 --- a/Package.swift +++ b/Package.swift @@ -252,6 +252,7 @@ let package = Package( name: "PackageModelSyntax", dependencies: [ "Basics", + "PackageLoading", "PackageModel", .product(name: "SwiftDiagnostics", package: "swift-syntax"), .product(name: "SwiftParser", package: "swift-syntax"), diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index fe33f6c3f50..ea9dfa5ac67 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// import Basics +import PackageLoading import PackageModel import SwiftParser import SwiftSyntax @@ -37,6 +38,12 @@ public struct AddPackageDependency { to manifest: SourceFileSyntax, manifestDirectory: AbsolutePath ) throws -> [SourceEdit] { + // Make sure we have tools version 5.5 or greater, + let toolsVersion = try ToolsVersionParser.parse(utf8String: manifest.description) + if toolsVersion < ToolsVersion.minimumManifestEditVersion { + throw ManifestEditError.oldManifest(toolsVersion) + } + guard let packageCall = manifest.findCall(calleeName: "Package") else { throw ManifestEditError.cannotFindPackage } diff --git a/Sources/PackageModelSyntax/ManifestEditError.swift b/Sources/PackageModelSyntax/ManifestEditError.swift index 9ec028bbbab..bca0b35b2cc 100644 --- a/Sources/PackageModelSyntax/ManifestEditError.swift +++ b/Sources/PackageModelSyntax/ManifestEditError.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import PackageModel import SwiftSyntax /// An error describing problems that can occur when attempting to edit a @@ -17,6 +18,13 @@ import SwiftSyntax package enum ManifestEditError: Error { case cannotFindPackage case cannotFindArrayLiteralArgument(argumentName: String, node: Syntax) + case oldManifest(ToolsVersion) +} + +extension ToolsVersion { + /// The minimum tools version of the manifest file that we support edit + /// operations on. + static let minimumManifestEditVersion = v5_5 } extension ManifestEditError: CustomStringConvertible { @@ -26,6 +34,8 @@ extension ManifestEditError: CustomStringConvertible { "invalid manifest: unable to find 'Package' declaration" case .cannotFindArrayLiteralArgument(argumentName: let name, node: _): "unable to find array literal for '\(name)' argument" + case .oldManifest(let version): + "package manifest version \(version) is too old: please update to manifest version \(ToolsVersion.minimumManifestEditVersion) or newer" } } } diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index cb1ce36c628..caa82b156d9 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -48,6 +48,7 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyExistingComma() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -55,6 +56,7 @@ class ManifestEditTests: XCTestCase { ] ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -73,6 +75,7 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyExistingNoComma() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -80,6 +83,7 @@ class ManifestEditTests: XCTestCase { ] ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -98,6 +102,7 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyExistingAppended() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -105,6 +110,7 @@ class ManifestEditTests: XCTestCase { ] + [] ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -123,12 +129,14 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyExistingEmpty() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ ] ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -146,11 +154,13 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyNoExistingAtEnd() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages" ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -168,12 +178,14 @@ class ManifestEditTests: XCTestCase { func testAddPackageDependencyNoExistingMiddle() throws { try assertManifestRefactor(""" + // swift-tools-version: 5.5 let package = Package( name: "packages", targets: [] ) """, expectedManifest: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: [ @@ -195,6 +207,7 @@ class ManifestEditTests: XCTestCase { try AddPackageDependency.addPackageDependency( Self.swiftSystemPackageDependency, to: """ + // swift-tools-version: 5.5 let package: Package = .init( name: "packages" ) @@ -213,6 +226,7 @@ class ManifestEditTests: XCTestCase { try AddPackageDependency.addPackageDependency( Self.swiftSystemPackageDependency, to: """ + // swift-tools-version: 5.5 let package = Package( name: "packages", dependencies: blah @@ -227,6 +241,25 @@ class ManifestEditTests: XCTestCase { return false } } + + XCTAssertThrows( + try AddPackageDependency.addPackageDependency( + Self.swiftSystemPackageDependency, + to: """ + // swift-tools-version: 5.4 + let package = Package( + name: "packages" + ) + """, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + ) { (error: ManifestEditError) in + if case .oldManifest(.v5_4) = error { + return true + } else { + return false + } + } } } From d8cb0c03428cedffad89a6b88fcc0f8aac391c33 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 11:38:07 -0700 Subject: [PATCH 06/22] Use 'package' access for newly-promoted-to-public APIs --- Sources/PackageModel/ManifestSourceGeneration.swift | 6 +++--- Sources/Workspace/Workspace.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/PackageModel/ManifestSourceGeneration.swift b/Sources/PackageModel/ManifestSourceGeneration.swift index 03a0d82abbc..7f13e34aae9 100644 --- a/Sources/PackageModel/ManifestSourceGeneration.swift +++ b/Sources/PackageModel/ManifestSourceGeneration.swift @@ -62,8 +62,8 @@ public typealias ManifestCustomProductTypeSourceGenerator = (ProductDescription) /// Convenience initializers for package manifest structures. -public extension SourceCodeFragment { - +package extension SourceCodeFragment { + /// Instantiates a SourceCodeFragment to represent an entire manifest. init( from manifest: Manifest, @@ -633,7 +633,7 @@ public struct SourceCodeFragment { self.subnodes = subnodes } - public func generateSourceCode(indent: String = "") -> String { + package func generateSourceCode(indent: String = "") -> String { var string = literal if let subnodes { switch delimiters { diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index f96bb976761..e6016e3d554 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -96,7 +96,7 @@ public class Workspace { public let pinsStore: LoadableResult /// The file system on which the workspace will operate. - public let fileSystem: any FileSystem + package let fileSystem: any FileSystem /// The host toolchain to use. private let hostToolchain: UserToolchain From a10878393e65b77922830afcd332d8fd6616371a Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 13:22:34 -0700 Subject: [PATCH 07/22] Align "add dependency" command line with SE-0301 --- .../{Add.swift => AddDependency.swift} | 85 +++++++++++++------ .../PackageCommands/SwiftPackageCommand.swift | 2 +- Tests/CommandsTests/PackageCommandTests.swift | 4 +- 3 files changed, 64 insertions(+), 27 deletions(-) rename Sources/Commands/PackageCommands/{Add.swift => AddDependency.swift} (58%) diff --git a/Sources/Commands/PackageCommands/Add.swift b/Sources/Commands/PackageCommands/AddDependency.swift similarity index 58% rename from Sources/Commands/PackageCommands/Add.swift rename to Sources/Commands/PackageCommands/AddDependency.swift index 73f66da4392..ef91d4db0c9 100644 --- a/Sources/Commands/PackageCommands/Add.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -23,29 +23,33 @@ import TSCUtility import Workspace extension SwiftPackageCommand { - struct Add: SwiftCommand { + struct AddDependency: SwiftCommand { package static let configuration = CommandConfiguration( - abstract: "Add a dependency to the package manifest") + abstract: "Add a package dependency to the manifest") + + @Argument(help: "The URL or directory of the package to add") + var dependency: String @OptionGroup(visibility: .hidden) var globalOptions: GlobalOptions - @Option(help: "The branch to depend on") - var branch: String? + @Option(help: "The exact package version to depend on") + var exact: Version? - @Option(help: "The minimum version requirement") - var fromVersion: String? + @Option(help: "The specific package revision to depend on") + var revision: String? - @Option(help: "The exact version requirement") - var exactVersion: String? + @Option(help: "The branch of the package to depend on") + var branch: String? - @Option(help: "A specific revision requirement") - var revision: String? + @Option(help: "The package version to depend on (up to the next major version)") + var from: Version? - // FIXME: range option + @Option(help: "The package version to depend on (up to the next minor version)") + var upToNextMinorFrom: Version? - @Argument(help: "The URL or directory of the package to add") - var url: String + @Option(help: "Specify upper bound on the package version range (exclusive)") + var to: Version? func run(_ swiftCommandState: SwiftCommandState) throws { let workspace = try swiftCommandState.getActiveWorkspace() @@ -73,26 +77,59 @@ extension SwiftPackageCommand { } } - let identity = PackageIdentity(url: .init(url)) + let identity = PackageIdentity(url: .init(dependency)) + + // Collect all of the possible version requirements. + var requirements: [PackageDependency.SourceControl.Requirement] = [] + if let exact { + requirements.append(.exact(exact)) + } - // Figure out the version requirement. - let requirement: PackageDependency.SourceControl.Requirement if let branch { - requirement = .branch(branch) - } else if let fromVersion { - requirement = .revision(fromVersion) - } else if let exactVersion { - requirement = .exact(try Version(versionString: exactVersion)) + requirements.append(.branch(branch)) + } + + if let revision { + requirements.append(.revision(revision)) + } + + if let from { + requirements.append(.range(.upToNextMajor(from: from))) + } + + if let upToNextMinorFrom { + requirements.append(.range(.upToNextMinor(from: upToNextMinorFrom))) + } + + if requirements.count > 1 { + throw StringError("must specify at most one of --exact, --branch, --revision, --from, or --up-to-next-minor-from") + } + + guard let firstRequirement = requirements.first else { + throw StringError("must specify one of --exact, --branch, --revision, --from, or --up-to-next-minor-from") + } + + let requirement: PackageDependency.SourceControl.Requirement + if case .range(let range) = firstRequirement { + if let to { + requirement = .range(range.lowerBound.. Date: Thu, 18 Apr 2024 14:21:37 -0700 Subject: [PATCH 08/22] Render package dependencies with the SwiftPM 5.5 syntax Modern SwiftPM implementations warn about the .package(url: "https://github.com/apple/swift-system.git", .branch("main")) syntax that's being rendered into the package manifest, because this syntax was replaced in SwiftPM 5.5 with something that uses argument labels: .package(url: "https://github.com/apple/swift-system.git", branch: "main") Since generating code with warnings is bad, and we've already stated that we won't edit packages for tools version < 5.5, switch to this new style. This does mean that we drop the (internal) dependency on ManifestSourceGeneration, which is producing the older format, because I don't have a strong sense of whether we can modernize that code. Instead, we have a new path that generates swift-syntax nodes directly for the types we generate. --- .../ManifestSourceGeneration.swift | 4 +- .../AddPackageDependency.swift | 2 +- .../ManifestSyntaxRepresentable.swift | 2 +- .../PackageDependency+Syntax.swift | 76 ++++++++++++++++++- Tests/CommandsTests/PackageCommandTests.swift | 2 +- .../ManifestEditTests.swift | 48 +++++++++--- 6 files changed, 115 insertions(+), 19 deletions(-) diff --git a/Sources/PackageModel/ManifestSourceGeneration.swift b/Sources/PackageModel/ManifestSourceGeneration.swift index 7f13e34aae9..f2b0d65c45c 100644 --- a/Sources/PackageModel/ManifestSourceGeneration.swift +++ b/Sources/PackageModel/ManifestSourceGeneration.swift @@ -62,7 +62,7 @@ public typealias ManifestCustomProductTypeSourceGenerator = (ProductDescription) /// Convenience initializers for package manifest structures. -package extension SourceCodeFragment { +fileprivate extension SourceCodeFragment { /// Instantiates a SourceCodeFragment to represent an entire manifest. init( @@ -633,7 +633,7 @@ public struct SourceCodeFragment { self.subnodes = subnodes } - package func generateSourceCode(indent: String = "") -> String { + func generateSourceCode(indent: String = "") -> String { var string = literal if let subnodes { switch delimiters { diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index ea9dfa5ac67..b1935cfe68b 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -48,7 +48,7 @@ public struct AddPackageDependency { throw ManifestEditError.cannotFindPackage } - let dependencySyntax = dependency.asSyntax(manifestDirectory: manifestDirectory) + let dependencySyntax = dependency.asSyntax() // If there is already a "dependencies" argument, append to the array // literal in there. diff --git a/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift index 45686e59bf2..1e3e8ee5d3f 100644 --- a/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift +++ b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift @@ -36,5 +36,5 @@ protocol ManifestSyntaxRepresentable { /// For example, a `PackageDependency` entity would map to syntax for /// something like /// .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") - func asSyntax(manifestDirectory: AbsolutePath) -> PreferredSyntax + func asSyntax() -> PreferredSyntax } diff --git a/Sources/PackageModelSyntax/PackageDependency+Syntax.swift b/Sources/PackageModelSyntax/PackageDependency+Syntax.swift index d48c7c2fb85..cf870669903 100644 --- a/Sources/PackageModelSyntax/PackageDependency+Syntax.swift +++ b/Sources/PackageModelSyntax/PackageDependency+Syntax.swift @@ -14,10 +14,80 @@ import Basics import PackageModel import SwiftSyntax import SwiftParser +import struct TSCUtility.Version extension PackageDependency: ManifestSyntaxRepresentable { - func asSyntax(manifestDirectory: AbsolutePath) -> ExprSyntax { - let fragment = SourceCodeFragment(from: self, pathAnchor: manifestDirectory) - return "\(raw: fragment.generateSourceCode())" + func asSyntax() -> ExprSyntax { + switch self { + case .fileSystem(let filesystem): filesystem.asSyntax() + case .sourceControl(let sourceControl): sourceControl.asSyntax() + case .registry(let registry): registry.asSyntax() + } + } +} + +extension PackageDependency.FileSystem: ManifestSyntaxRepresentable { + func asSyntax() -> ExprSyntax { + fatalError() + } +} + +extension PackageDependency.SourceControl: ManifestSyntaxRepresentable { + func asSyntax() -> ExprSyntax { + // TODO: Not handling identity, nameForTargetDependencyResolutionOnly, + // or productFilter yet. + switch location { + case .local(let path): + ".package(path: \(literal: path.description), \(requirement.asSyntax()))" + case .remote(let url): + ".package(url: \(literal: url.description), \(requirement.asSyntax()))" + } + } +} + +extension PackageDependency.Registry: ManifestSyntaxRepresentable { + func asSyntax() -> ExprSyntax { + fatalError() + } +} + +extension PackageDependency.SourceControl.Requirement: ManifestSyntaxRepresentable { + func asSyntax() -> LabeledExprSyntax { + switch self { + case .exact(let version): + LabeledExprSyntax( + label: "exact", + expression: version.asSyntax() + ) + + case .range(let range) where range == .upToNextMajor(from: range.lowerBound): + LabeledExprSyntax( + label: "from", + expression: range.lowerBound.asSyntax() + ) + + case .range(let range): + LabeledExprSyntax( + expression: "\(range.lowerBound.asSyntax())..<\(range.upperBound.asSyntax())" as ExprSyntax + ) + + case .revision(let revision): + LabeledExprSyntax( + label: "revision", + expression: "\(literal: revision)" as ExprSyntax + ) + + case .branch(let branch): + LabeledExprSyntax( + label: "branch", + expression: "\(literal: branch)" as ExprSyntax + ) + } + } +} + +extension Version: ManifestSyntaxRepresentable { + func asSyntax() -> ExprSyntax { + return "\(literal: description)" } } diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index 2eecfa24c1d..25a81b9ddce 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -816,7 +816,7 @@ final class PackageCommandTests: CommandsTestCase { XCTAssertFileExists(manifest) let contents: String = try fs.readFileContents(manifest) - XCTAssertMatch(contents, .contains(#".package(url: "https://github.com/apple/swift-syntax.git", .branch("main")),"#)) + XCTAssertMatch(contents, .contains(#".package(url: "https://github.com/apple/swift-syntax.git", branch: "main"),"#)) } } diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index caa82b156d9..d096bd560fb 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -16,6 +16,7 @@ import SPMTestSupport @_spi(FixItApplier) import SwiftIDEUtils import SwiftParser import SwiftSyntax +import struct TSCUtility.Version import XCTest func assertManifestRefactor( @@ -61,12 +62,17 @@ class ManifestEditTests: XCTestCase { name: "packages", dependencies: [ .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), - .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + .package(url: "https://github.com/apple/swift-system.git", branch: "main"), ] ) """) { manifest in try AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, + PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: Self.swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: Self.swiftSystemURL, + requirement: .branch("main"), productFilter: .nothing + ), to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") ) @@ -88,12 +94,18 @@ class ManifestEditTests: XCTestCase { name: "packages", dependencies: [ .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), - .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + .package(url: "https://github.com/apple/swift-system.git", exact: "510.0.0"), ] ) """) { manifest in try AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, + PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: Self.swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: Self.swiftSystemURL, + requirement: .exact("510.0.0"), + productFilter: .nothing + ), to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") ) @@ -115,12 +127,20 @@ class ManifestEditTests: XCTestCase { name: "packages", dependencies: [ .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), - .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + .package(url: "https://github.com/apple/swift-system.git", from: "510.0.0"), ] + [] ) """) { manifest in - try AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, + let versionRange = Range.upToNextMajor(from: Version(510, 0, 0)) + + return try AddPackageDependency.addPackageDependency( + PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: Self.swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: Self.swiftSystemURL, + requirement: .range(versionRange), + productFilter: .nothing + ), to: manifest, manifestDirectory: try! AbsolutePath(validating: "/") ) @@ -140,12 +160,18 @@ class ManifestEditTests: XCTestCase { let package = Package( name: "packages", dependencies: [ - .package(url: "https://github.com/apple/swift-system.git", .branch("main")), + .package(url: "https://github.com/apple/swift-system.git", "508.0.0"..<"510.0.0"), ] ) """) { manifest in try AddPackageDependency.addPackageDependency( - Self.swiftSystemPackageDependency, + PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: Self.swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: Self.swiftSystemURL, + requirement: .range(Version(508,0,0).. Date: Thu, 18 Apr 2024 15:05:26 -0700 Subject: [PATCH 09/22] Address code review comments and fix some trivia issues --- .../AddPackageDependency.swift | 3 +- .../PackageModelSyntax/SyntaxEditUtils.swift | 17 +++++------ .../ManifestEditTests.swift | 29 +++++++++++++++++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index b1935cfe68b..119702b86c0 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -96,8 +96,7 @@ public struct AddPackageDependency { trailingComma: .commaToken() ) ], - rightSquare: .rightSquareToken() - .with(\.leadingTrivia, leadingTrivia) + rightSquare: .rightSquareToken(leadingTrivia: leadingTrivia) ) newArgument = ExprSyntax(arrayExpr) } diff --git a/Sources/PackageModelSyntax/SyntaxEditUtils.swift b/Sources/PackageModelSyntax/SyntaxEditUtils.swift index 772b33d6330..b5ee5917619 100644 --- a/Sources/PackageModelSyntax/SyntaxEditUtils.swift +++ b/Sources/PackageModelSyntax/SyntaxEditUtils.swift @@ -22,13 +22,7 @@ let defaultIndent = TriviaPiece.spaces(4) extension Trivia { /// Determine whether this trivia has newlines or not. var hasNewlines: Bool { - contains { piece in - if case .newlines = piece { - return true - } else { - return false - } - } + contains(where: \.isNewline) } } @@ -121,7 +115,7 @@ extension LabeledExprListSyntax { let positionIdx = distance(from: startIndex, to: position) - let commaToken = TokenSyntax(.comma, presence: .present) + let commaToken = TokenSyntax.commaToken() // Figure out leading trivia and adjust the prior argument (if there is // one) by adding a comma, if necessary. @@ -177,7 +171,7 @@ extension ArrayExprSyntax { ) -> ArrayExprSyntax { var elements = self.elements - let commaToken = TokenSyntax(.comma, presence: .present) + let commaToken = TokenSyntax.commaToken() // If there are already elements, tack it on. let leadingTrivia: Trivia @@ -192,7 +186,10 @@ extension ArrayExprSyntax { // there. if last.trailingComma == nil { var newElements = Array(elements) - newElements[newElements.count-1] = last.with(\.trailingComma, commaToken) + newElements[newElements.count - 1].trailingComma = commaToken + newElements[newElements.count - 1].expression.trailingTrivia = + Trivia() + newElements[newElements.count - 1].trailingTrivia = last.trailingTrivia elements = ArrayElementListSyntax(newElements) } diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index d096bd560fb..0c053e8ee94 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -147,6 +147,35 @@ class ManifestEditTests: XCTestCase { } } + func testAddPackageDependencyExistingOneLine() throws { + try assertManifestRefactor(""" + // swift-tools-version: 5.5 + let package = Package( + name: "packages", + dependencies: [ .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") ] + ) + """, expectedManifest: """ + // swift-tools-version: 5.5 + let package = Package( + name: "packages", + dependencies: [ .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1"), .package(url: "https://github.com/apple/swift-system.git", from: "510.0.0"),] + ) + """) { manifest in + let versionRange = Range.upToNextMajor(from: Version(510, 0, 0)) + + return try AddPackageDependency.addPackageDependency( + PackageDependency.remoteSourceControl( + identity: PackageIdentity(url: Self.swiftSystemURL), + nameForTargetDependencyResolutionOnly: nil, + url: Self.swiftSystemURL, + requirement: .range(versionRange), + productFilter: .nothing + ), + to: manifest, + manifestDirectory: try! AbsolutePath(validating: "/") + ) + } + } func testAddPackageDependencyExistingEmpty() throws { try assertManifestRefactor(""" // swift-tools-version: 5.5 From 9894c438945858eb478d576f1016920da4b6a7ed Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 15:35:18 -0700 Subject: [PATCH 10/22] Update CMake build and bootstrap script for PackageModelSyntax module --- Sources/Commands/CMakeLists.txt | 1 + Sources/PackageModelSyntax/CMakeLists.txt | 32 +++++++++++++++++++++++ Utilities/bootstrap | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 Sources/PackageModelSyntax/CMakeLists.txt diff --git a/Sources/Commands/CMakeLists.txt b/Sources/Commands/CMakeLists.txt index e74414c60bf..1493348f453 100644 --- a/Sources/Commands/CMakeLists.txt +++ b/Sources/Commands/CMakeLists.txt @@ -56,6 +56,7 @@ target_link_libraries(Commands PUBLIC CoreCommands LLBuildManifest PackageGraph + PackageModelSyntax SourceControl TSCBasic TSCUtility diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt new file mode 100644 index 00000000000..f509fd6946a --- /dev/null +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -0,0 +1,32 @@ +# This source file is part of the Swift open source project +# +# Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +# Licensed under Apache License v2.0 with Runtime Library Exception +# +# See http://swift.org/LICENSE.txt for license information +# See http://swift.org/CONTRIBUTORS.txt for Swift project authors + +add_library(PackageModelSyntax + AddPackageDependency.swift + ManifestEditError.swift + ManifestSyntaxRepresentable.swift + PackageDependency+Syntax.swift + SyntaxEditUtils.swift) +target_link_libraries(PackageModelSyntax PUBLIC + Basics + PackageLoading + PackageModel + + SwiftDiagnostics + SwiftParser + SwiftSyntax + SwiftSyntaxBuilder) +# NOTE(compnerd) workaround for CMake not setting up include flags yet +set_target_properties(PackageModelSyntax PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY}) + +install(TARGETS PackageModelSyntax + ARCHIVE DESTINATION lib + LIBRARY DESTINATION lib + RUNTIME DESTINATION bin) +set_property(GLOBAL APPEND PROPERTY SwiftPM_EXPORTS PackageModelSyntax) diff --git a/Utilities/bootstrap b/Utilities/bootstrap index 318e3941941..7605c4d5bf2 100755 --- a/Utilities/bootstrap +++ b/Utilities/bootstrap @@ -198,6 +198,7 @@ def parse_global_args(args): args.source_dirs["swift-argument-parser"] = os.path.join(args.project_root, "..", "swift-argument-parser") args.source_dirs["swift-crypto"] = os.path.join(args.project_root, "..", "swift-crypto") args.source_dirs["swift-driver"] = os.path.join(args.project_root, "..", "swift-driver") + args.source_dirs["swift-syntax"] = os.path.join(args.project_root, "..", "swift-syntax") args.source_dirs["swift-system"] = os.path.join(args.project_root, "..", "swift-system") args.source_dirs["swift-collections"] = os.path.join(args.project_root, "..", "swift-collections") args.source_dirs["swift-certificates"] = os.path.join(args.project_root, "..", "swift-certificates") @@ -625,6 +626,7 @@ def build_swiftpm_with_cmake(args): add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-argument-parser"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-crypto"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-driver"], "lib")) + add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-syntax"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-system"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-collections"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-asn1"], "lib")) From 8cc44417c2261924285ed225d4d17d87ea30c6e3 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 21:39:25 -0700 Subject: [PATCH 11/22] HACK to try to debug CI failure --- Tests/FunctionalTests/ResourcesTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/FunctionalTests/ResourcesTests.swift b/Tests/FunctionalTests/ResourcesTests.swift index 85c10611032..20d252dd8dc 100644 --- a/Tests/FunctionalTests/ResourcesTests.swift +++ b/Tests/FunctionalTests/ResourcesTests.swift @@ -157,7 +157,7 @@ class ResourcesTests: XCTestCase { try localFileSystem.createDirectory(resource.parentDirectory, recursive: true) try localFileSystem.writeFileContents(resource, string: "best") - let (_, stderr) = try executeSwiftBuild(packageDir) + let (_, stderr) = try executeSwiftBuild(packageDir, env: ["DYLD_PRINT_SEARCHING": "1"]) // Filter some unrelated output that could show up on stderr. let filteredStderr = stderr.components(separatedBy: "\n").filter { !$0.contains("[logging]") }.joined(separator: "\n") XCTAssertEqual(filteredStderr, "", "unexpectedly received error output: \(stderr)") From 3f1bb3521255c7b007f2db72cf49cd92b877f251 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 21:46:57 -0700 Subject: [PATCH 12/22] Make sure to exclude CMakeLists.txt from manifest --- Package.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index e630489cdd7..04991312560 100644 --- a/Package.swift +++ b/Package.swift @@ -258,7 +258,8 @@ let package = Package( .product(name: "SwiftParser", package: "swift-syntax"), .product(name: "SwiftSyntax", package: "swift-syntax"), .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), - ] + ], + exclude: ["CMakeLists.txt"] ), .target( From 3d7cac5449f5e77a271c5ddd0623f5a21118035e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 21:49:25 -0700 Subject: [PATCH 13/22] Add missing source file to CMakeLists.txt --- Sources/Commands/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Commands/CMakeLists.txt b/Sources/Commands/CMakeLists.txt index 1493348f453..6c4b6345e58 100644 --- a/Sources/Commands/CMakeLists.txt +++ b/Sources/Commands/CMakeLists.txt @@ -7,6 +7,7 @@ # See http://swift.org/CONTRIBUTORS.txt for Swift project authors add_library(Commands + PackageCommands/AddDependency.swift PackageCommands/APIDiff.swift PackageCommands/ArchiveSource.swift PackageCommands/CompletionCommand.swift From de8eb4f8250e271759f9157596ebd3d8b2ea57d2 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 23:00:57 -0700 Subject: [PATCH 14/22] Introduce an egregious hack to work around older CI toolchains Blame Owen for the evil genius of this hack, me for trying to put it in --- Tests/CommandsTests/PackageCommandTests.swift | 16 ++++++++-------- Tests/FunctionalTests/MiscellaneousTests.swift | 2 +- Tests/FunctionalTests/PluginTests.swift | 4 ++-- Tests/FunctionalTests/ResourcesTests.swift | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index 25a81b9ddce..577ac73c00f 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -1953,13 +1953,13 @@ final class PackageCommandTests: CommandsTestCase { try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in func runPlugin(flags: [String], diagnostics: [String], completion: (String, String) -> Void) throws { - let (stdout, stderr) = try SwiftPM.Package.execute(flags + ["print-diagnostics"] + diagnostics, packagePath: fixturePath) + let (stdout, stderr) = try SwiftPM.Package.execute(flags + ["print-diagnostics"] + diagnostics, packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) completion(stdout, stderr) } // Diagnostics.error causes SwiftPM to return a non-zero exit code, but we still need to check stdout and stderr func runPluginWithError(flags: [String], diagnostics: [String], completion: (String, String) -> Void) throws { - XCTAssertThrowsError(try SwiftPM.Package.execute(flags + ["print-diagnostics"] + diagnostics, packagePath: fixturePath)) { error in + XCTAssertThrowsError(try SwiftPM.Package.execute(flags + ["print-diagnostics"] + diagnostics, packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"])) { error in guard case SwiftPMError.executionFailure(_, let stdout, let stderr) = error else { return XCTFail("invalid error \(error)") } @@ -2151,28 +2151,28 @@ final class PackageCommandTests: CommandsTestCase { // Check than nothing is echoed when echoLogs is false try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in - let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build"], packagePath: fixturePath) + let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build"], packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, isEmpty) XCTAssertMatch(stderr, isEmpty) } // Check that logs are returned to the plugin when echoLogs is false try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in - let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "printlogs"], packagePath: fixturePath) + let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "printlogs"], packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, containsLogtext) XCTAssertMatch(stderr, isEmpty) } // Check that logs echoed to the console (on stderr) when echoLogs is true try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in - let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "echologs"], packagePath: fixturePath) + let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "echologs"], packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, isEmpty) XCTAssertMatch(stderr, containsLogecho) } // Check that logs are returned to the plugin and echoed to the console (on stderr) when echoLogs is true try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in - let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "printlogs", "echologs"], packagePath: fixturePath) + let (stdout, stderr) = try SwiftPM.Package.execute(["print-diagnostics", "build", "printlogs", "echologs"], packagePath: fixturePath, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, containsLogtext) XCTAssertMatch(stderr, containsLogecho) } @@ -2465,14 +2465,14 @@ final class PackageCommandTests: CommandsTestCase { // Check arguments do { - let (stdout, stderr) = try SwiftPM.Package.execute(["plugin", "MyPlugin", "--foo", "--help", "--version", "--verbose"], packagePath: packageDir) + let (stdout, stderr) = try SwiftPM.Package.execute(["plugin", "MyPlugin", "--foo", "--help", "--version", "--verbose"], packagePath: packageDir, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, .contains("success")) XCTAssertEqual(stderr, "") } // Check default command arguments do { - let (stdout, stderr) = try SwiftPM.Package.execute(["MyPlugin", "--foo", "--help", "--version", "--verbose"], packagePath: packageDir) + let (stdout, stderr) = try SwiftPM.Package.execute(["MyPlugin", "--foo", "--help", "--version", "--verbose"], packagePath: packageDir, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertMatch(stdout, .contains("success")) XCTAssertEqual(stderr, "") } diff --git a/Tests/FunctionalTests/MiscellaneousTests.swift b/Tests/FunctionalTests/MiscellaneousTests.swift index b5e5dc3e28f..badae071951 100644 --- a/Tests/FunctionalTests/MiscellaneousTests.swift +++ b/Tests/FunctionalTests/MiscellaneousTests.swift @@ -665,7 +665,7 @@ class MiscellaneousTestCase: XCTestCase { func testRootPackageWithConditionals() throws { try fixture(name: "Miscellaneous/RootPackageWithConditionals") { path in - let (_, stderr) = try SwiftPM.Build.execute(packagePath: path) + let (_, stderr) = try SwiftPM.Build.execute(packagePath: path, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) let errors = stderr.components(separatedBy: .newlines).filter { !$0.contains("[logging] misuse") && !$0.isEmpty } XCTAssertEqual(errors, [], "unexpected errors: \(errors)") } diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index c9bb4794df6..47218b62df3 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -227,12 +227,12 @@ final class PluginTests: XCTestCase { let pathOfGeneratedFile = packageDir.appending(components: [".build", "plugins", "outputs", "mypackage", "SomeTarget", "Plugin", "best.txt"]) try createPackageUnderTest(packageDir: packageDir, toolsVersion: .v5_9) - let (_, stderr) = try executeSwiftBuild(packageDir) + let (_, stderr) = try executeSwiftBuild(packageDir, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertTrue(stderr.contains("warning: Build tool command 'empty' (applied to target 'SomeTarget') does not declare any output files"), "expected warning not emitted") XCTAssertFalse(localFileSystem.exists(pathOfGeneratedFile), "plugin generated file unexpectedly exists at \(pathOfGeneratedFile.pathString)") try createPackageUnderTest(packageDir: packageDir, toolsVersion: .v6_0) - let (_, stderr2) = try executeSwiftBuild(packageDir) + let (_, stderr2) = try executeSwiftBuild(packageDir, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) XCTAssertEqual("", stderr2) XCTAssertTrue(localFileSystem.exists(pathOfGeneratedFile), "plugin did not run, generated file does not exist at \(pathOfGeneratedFile.pathString)") } diff --git a/Tests/FunctionalTests/ResourcesTests.swift b/Tests/FunctionalTests/ResourcesTests.swift index 20d252dd8dc..95e6a192bb4 100644 --- a/Tests/FunctionalTests/ResourcesTests.swift +++ b/Tests/FunctionalTests/ResourcesTests.swift @@ -157,7 +157,7 @@ class ResourcesTests: XCTestCase { try localFileSystem.createDirectory(resource.parentDirectory, recursive: true) try localFileSystem.writeFileContents(resource, string: "best") - let (_, stderr) = try executeSwiftBuild(packageDir, env: ["DYLD_PRINT_SEARCHING": "1"]) + let (_, stderr) = try executeSwiftBuild(packageDir, env: ["SWIFT_DRIVER_SWIFTSCAN_LIB" : "/this/is/a/bad/path"]) // Filter some unrelated output that could show up on stderr. let filteredStderr = stderr.components(separatedBy: "\n").filter { !$0.contains("[logging]") }.joined(separator: "\n") XCTAssertEqual(filteredStderr, "", "unexpectedly received error output: \(stderr)") From 086cd4d253bd5442f715f1e020d972e4a03d2e02 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 19 Apr 2024 07:01:43 -0700 Subject: [PATCH 15/22] Add PackageModelSyntax to CMake --- Sources/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/CMakeLists.txt b/Sources/CMakeLists.txt index 5af06570360..1f4226f88e6 100644 --- a/Sources/CMakeLists.txt +++ b/Sources/CMakeLists.txt @@ -21,6 +21,7 @@ add_subdirectory(PackageFingerprint) add_subdirectory(PackageGraph) add_subdirectory(PackageLoading) add_subdirectory(PackageModel) +add_subdirectory(PackageModelSyntax) add_subdirectory(PackagePlugin) add_subdirectory(PackageRegistry) add_subdirectory(PackageSigning) From 0b171f6b123a6170f69203667775f8e5e59aa91e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2024 21:35:09 -0700 Subject: [PATCH 16/22] Add a new package manifest "edit" operation for adding a new target The formatting is a mess, but the functionality is sound. --- Package.swift | 1 + .../PackageCommands/AddDependency.swift | 3 +- .../AddPackageDependency.swift | 81 +-------- Sources/PackageModelSyntax/AddTarget.swift | 49 +++++ Sources/PackageModelSyntax/CMakeLists.txt | 5 +- .../ManifestEditError.swift | 12 ++ .../ManifestSyntaxRepresentable.swift | 6 + .../PackageModelSyntax/SyntaxEditUtils.swift | 170 ++++++++++++++++++ .../TargetDescription+Syntax.swift | 97 ++++++++++ .../ManifestEditTests.swift | 81 ++++++--- 10 files changed, 408 insertions(+), 97 deletions(-) create mode 100644 Sources/PackageModelSyntax/AddTarget.swift create mode 100644 Sources/PackageModelSyntax/TargetDescription+Syntax.swift diff --git a/Package.swift b/Package.swift index 04991312560..94df241e7d3 100644 --- a/Package.swift +++ b/Package.swift @@ -254,6 +254,7 @@ let package = Package( "Basics", "PackageLoading", "PackageModel", + .product(name: "SwiftBasicFormat", package: "swift-syntax"), .product(name: "SwiftDiagnostics", package: "swift-syntax"), .product(name: "SwiftParser", package: "swift-syntax"), .product(name: "SwiftSyntax", package: "swift-syntax"), diff --git a/Sources/Commands/PackageCommands/AddDependency.swift b/Sources/Commands/PackageCommands/AddDependency.swift index ef91d4db0c9..2eba6370c5a 100644 --- a/Sources/Commands/PackageCommands/AddDependency.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -142,8 +142,7 @@ extension SwiftPackageCommand { let edits = try AddPackageDependency.addPackageDependency( packageDependency, - to: manifestSyntax, - manifestDirectory: packagePath.parentDirectory + to: manifestSyntax ) if edits.isEmpty { diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index 119702b86c0..094de688e58 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -35,86 +35,19 @@ public struct AddPackageDependency { /// dependency to the given manifest file. public static func addPackageDependency( _ dependency: PackageDependency, - to manifest: SourceFileSyntax, - manifestDirectory: AbsolutePath + to manifest: SourceFileSyntax ) throws -> [SourceEdit] { - // Make sure we have tools version 5.5 or greater, - let toolsVersion = try ToolsVersionParser.parse(utf8String: manifest.description) - if toolsVersion < ToolsVersion.minimumManifestEditVersion { - throw ManifestEditError.oldManifest(toolsVersion) - } + // Make sure we have a suitable tools version in the manifest. + try manifest.checkEditManifestToolsVersion() guard let packageCall = manifest.findCall(calleeName: "Package") else { throw ManifestEditError.cannotFindPackage } - let dependencySyntax = dependency.asSyntax() - - // If there is already a "dependencies" argument, append to the array - // literal in there. - if let dependenciesArg = packageCall.findArgument(labeled: "dependencies") { - guard let argArray = dependenciesArg.expression.findArrayArgument() else { - throw ManifestEditError.cannotFindArrayLiteralArgument( - argumentName: "dependencies", - node: Syntax(dependenciesArg.expression) - ) - } - - let updatedArgArray = argArray.appending( - element: dependencySyntax, - outerLeadingTrivia: dependenciesArg.leadingTrivia - ) - return [ .replace(argArray, with: updatedArgArray.description) ] - } - - // There was no "dependencies" argument, so we need to create one. - - // Insert the new argument at the appropriate place in the call. - let insertionPos = packageCall.arguments.findArgumentInsertionPosition( - labelsAfter: Self.argumentLabelsAfterDependencies + return try packageCall.appendingToArrayArgument( + label: "dependencies", + trailingLabels: Self.argumentLabelsAfterDependencies, + newElement: dependency.asSyntax() ) - let newArguments = packageCall.arguments.insertingArgument( - at: insertionPos - ) { (leadingTrivia, trailingComma) in - // The argument is always [ element ], but if we have any newlines - // in the leading trivia, then we really want to split it across - // multiple lines, like this: - // [ - // element - // ] - let newArgument: ExprSyntax - if !leadingTrivia.hasNewlines { - newArgument = " [ \(dependencySyntax), ]" - } else { - let innerTrivia = leadingTrivia.appending(defaultIndent) - let arrayExpr = ArrayExprSyntax( - leadingTrivia: .space, - elements: [ - ArrayElementSyntax( - leadingTrivia: innerTrivia, - expression: dependencySyntax, - trailingComma: .commaToken() - ) - ], - rightSquare: .rightSquareToken(leadingTrivia: leadingTrivia) - ) - newArgument = ExprSyntax(arrayExpr) - } - - return LabeledExprSyntax( - leadingTrivia: leadingTrivia, - label: "dependencies", - colon: .colonToken(), - expression: newArgument, - trailingComma: trailingComma - ) - } - - return [ - SourceEdit.replace( - packageCall.arguments, - with: newArguments.description - ) - ] } } diff --git a/Sources/PackageModelSyntax/AddTarget.swift b/Sources/PackageModelSyntax/AddTarget.swift new file mode 100644 index 00000000000..7c7dae46945 --- /dev/null +++ b/Sources/PackageModelSyntax/AddTarget.swift @@ -0,0 +1,49 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import PackageModel +import SwiftParser +import SwiftSyntax +import SwiftSyntaxBuilder + +/// Add a target to a manifest's source code. +public struct AddTarget { + /// The set of argument labels that can occur after the "targets" + /// argument in the Package initializers. + /// + /// TODO: Could we generate this from the the PackageDescription module, so + /// we don't have keep it up-to-date manually? + private static let argumentLabelsAfterTargets: Set = [ + "swiftLanguageVersions", + "cLanguageStandard", + "cxxLanguageStandard" + ] + + public static func addTarget( + _ target: TargetDescription, + to manifest: SourceFileSyntax + ) throws -> [SourceEdit] { + // Make sure we have a suitable tools version in the manifest. + try manifest.checkEditManifestToolsVersion() + + guard let packageCall = manifest.findCall(calleeName: "Package") else { + throw ManifestEditError.cannotFindPackage + } + + return try packageCall.appendingToArrayArgument( + label: "targets", + trailingLabels: Self.argumentLabelsAfterTargets, + newElement: target.asSyntax() + ) + } +} diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index f509fd6946a..7e544d14158 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -8,10 +8,13 @@ add_library(PackageModelSyntax AddPackageDependency.swift + AddTarget.swift ManifestEditError.swift ManifestSyntaxRepresentable.swift PackageDependency+Syntax.swift - SyntaxEditUtils.swift) + SyntaxEditUtils.swift + TargetDescription+Syntax.swift +) target_link_libraries(PackageModelSyntax PUBLIC Basics PackageLoading diff --git a/Sources/PackageModelSyntax/ManifestEditError.swift b/Sources/PackageModelSyntax/ManifestEditError.swift index bca0b35b2cc..cba8eb520dd 100644 --- a/Sources/PackageModelSyntax/ManifestEditError.swift +++ b/Sources/PackageModelSyntax/ManifestEditError.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import PackageLoading import PackageModel import SwiftSyntax @@ -39,3 +40,14 @@ extension ManifestEditError: CustomStringConvertible { } } } + +extension SourceFileSyntax { + /// Check that the manifest described by this source file meets the minimum + /// tools version requirements for editing the manifest. + func checkEditManifestToolsVersion() throws { + let toolsVersion = try ToolsVersionParser.parse(utf8String: description) + if toolsVersion < ToolsVersion.minimumManifestEditVersion { + throw ManifestEditError.oldManifest(toolsVersion) + } + } +} diff --git a/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift index 1e3e8ee5d3f..8af1d379c18 100644 --- a/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift +++ b/Sources/PackageModelSyntax/ManifestSyntaxRepresentable.swift @@ -38,3 +38,9 @@ protocol ManifestSyntaxRepresentable { /// .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.1") func asSyntax() -> PreferredSyntax } + +extension String: ManifestSyntaxRepresentable { + typealias PreferredSyntax = ExprSyntax + + func asSyntax() -> ExprSyntax { "\(literal: self)" } +} diff --git a/Sources/PackageModelSyntax/SyntaxEditUtils.swift b/Sources/PackageModelSyntax/SyntaxEditUtils.swift index b5ee5917619..250848d35cb 100644 --- a/Sources/PackageModelSyntax/SyntaxEditUtils.swift +++ b/Sources/PackageModelSyntax/SyntaxEditUtils.swift @@ -238,3 +238,173 @@ extension ExprSyntax { return nil } } + +// MARK: Utilities to oeprate on arrays of array literal elements. +extension Array { + /// Append a new argument expression. + mutating func append(expression: ExprSyntax) { + // Add a comma on the prior expression, if there is one. + if count > 0 { + self[count - 1].trailingComma = TokenSyntax.commaToken(trailingTrivia: .space) + } + + append( + ArrayElementSyntax( + expression: expression + ) + ) + } +} + +// MARK: Utilities to operate on arrays of call arguments. + +extension Array { + /// 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. + if count > 0 { + self[count - 1].trailingComma = TokenSyntax.commaToken(trailingTrivia: .space) + } + + // Add the new expression. + append(LabeledExprSyntax(label: label, expression: expression)) + } + + /// Append a potentially labeled argument with a string literal. + mutating func append(label: String?, stringLiteral: String) { + append(label: label, expression: "\(literal: stringLiteral)") + } + + /// Append a potentially labeled argument with a string literal, but only + /// when the string literal is not nil. + mutating func appendIf(label: String?, stringLiteral: String?) { + if let stringLiteral { + append(label: label, stringLiteral: stringLiteral) + } + } + + /// Append an array literal containing elements that can be rendered + /// into expression syntax nodes. + mutating func append( + label: String?, + arrayLiteral: [T] + ) where T: ManifestSyntaxRepresentable, T.PreferredSyntax == ExprSyntax { + var elements: [ArrayElementSyntax] = [] + for element in arrayLiteral { + elements.append(expression: element.asSyntax()) + } + + let array = ArrayExprSyntax(elements: ArrayElementListSyntax(elements)) + append(label: label, expression: ExprSyntax(array)) + } + + /// Append an array literal containing elements that can be rendered + /// into expression syntax nodes. + mutating func appendIf( + label: String?, + arrayLiteral: [T]? + ) where T: ManifestSyntaxRepresentable, T.PreferredSyntax == ExprSyntax { + guard let arrayLiteral else { return } + append(label: label, arrayLiteral: arrayLiteral) + } + + /// Append an array literal containing elements that can be rendered + /// into expression syntax nodes, but only if it's not empty. + mutating func appendIfNonEmpty( + label: String?, + arrayLiteral: [T] + ) where T: ManifestSyntaxRepresentable, T.PreferredSyntax == ExprSyntax { + if arrayLiteral.isEmpty { return } + + append(label: label, arrayLiteral: arrayLiteral) + } +} + +// MARK: Utilities for adding arguments into calls. +extension FunctionCallExprSyntax { + /// Produce source edits that will add the given new element to the + /// array for an argument with the given label (if there is one), or + /// introduce a new argument with an array literal containing only the + /// new element. + /// + /// - Parameters: + /// - label: The argument label for the argument whose array will be + /// added or modified. + /// - trailingLabels: The argument labels that could follow the label, + /// which helps determine where the argument should be inserted if + /// it doesn't exist yet. + /// - newElement: The new element. + /// - Returns: the resulting source edits to make this change. + func appendingToArrayArgument( + label: String, + trailingLabels: Set, + newElement: ExprSyntax + ) throws -> [SourceEdit] { + // If there is already an argument with this name, append to the array + // literal in there. + if let arg = findArgument(labeled: label) { + guard let argArray = arg.expression.findArrayArgument() else { + throw ManifestEditError.cannotFindArrayLiteralArgument( + argumentName: label, + node: Syntax(arg.expression) + ) + } + + let updatedArgArray = argArray.appending( + element: newElement, + outerLeadingTrivia: arg.leadingTrivia + ) + return [ .replace(argArray, with: updatedArgArray.description) ] + } + + // There was no argument, so we need to create one. + + // Insert the new argument at the appropriate place in the call. + let insertionPos = arguments.findArgumentInsertionPosition( + labelsAfter: trailingLabels + ) + let newArguments = arguments.insertingArgument( + at: insertionPos + ) { (leadingTrivia, trailingComma) in + // The argument is always [ element ], but if we have any newlines + // in the leading trivia, then we really want to split it across + // multiple lines, like this: + // [ + // element + // ] + let newArgument: ExprSyntax + if !leadingTrivia.hasNewlines { + newArgument = " [ \(newElement), ]" + } else { + let innerTrivia = leadingTrivia.appending(defaultIndent) + let arrayExpr = ArrayExprSyntax( + leadingTrivia: .space, + elements: [ + ArrayElementSyntax( + leadingTrivia: innerTrivia, + expression: newElement, + trailingComma: .commaToken() + ) + ], + rightSquare: .rightSquareToken(leadingTrivia: leadingTrivia) + ) + newArgument = ExprSyntax(arrayExpr) + } + + return LabeledExprSyntax( + leadingTrivia: leadingTrivia, + label: "\(raw: label)", + colon: .colonToken(), + expression: newArgument, + trailingComma: trailingComma + ) + } + + return [ + SourceEdit.replace( + arguments, + with: newArguments.description + ) + ] + } +} diff --git a/Sources/PackageModelSyntax/TargetDescription+Syntax.swift b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift new file mode 100644 index 00000000000..740d55f4822 --- /dev/null +++ b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift @@ -0,0 +1,97 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +import PackageModel +import SwiftSyntax +import SwiftParser + + +extension TargetDescription: ManifestSyntaxRepresentable { + /// The function name in the package manifest. + private var functionName: String { + switch type { + case .binary: "binaryTarget" + case .executable: "executableTarget" + case .macro: "macro" + case .plugin: "plugin" + case .regular: "target" + case .system: "systemLibrary" + case .test: "testTarget" + } + } + + func asSyntax() -> ExprSyntax { + var arguments: [LabeledExprSyntax] = [] + arguments.append(label: "name", stringLiteral: name) + // FIXME: pluginCapability + + arguments.appendIfNonEmpty( + label: "dependencies", + arrayLiteral: dependencies + ) + + arguments.appendIf(label: "path", stringLiteral: path) + arguments.appendIf(label: "url", stringLiteral: url) + arguments.appendIfNonEmpty(label: "exclude", arrayLiteral: exclude) + arguments.appendIf(label: "sources", arrayLiteral: sources) + + // FIXME: resources + + arguments.appendIf( + label: "publicHeadersPath", + stringLiteral: publicHeadersPath + ) + + if !packageAccess { + arguments.append( + label: "packageAccess", + expression: "false" + ) + } + + // FIXME: cSettings + // FIXME: cxxSettings + // FIXME: swiftSettings + // FIXME: linkerSettings + // FIXME: plugins + + arguments.appendIf(label: "pkgConfig", stringLiteral: pkgConfig) + // FIXME: providers + + // Only for plugins + arguments.appendIf(label: "checksum", stringLiteral: checksum) + + return ".\(raw: functionName)(\(LabeledExprListSyntax(arguments)))" + } +} + +extension TargetDescription.Dependency: ManifestSyntaxRepresentable { + func asSyntax() -> ExprSyntax { + switch self { + case .byName(name: let name, condition: nil): + "\(literal: name)" + + case .target(name: let name, condition: nil): + ".target(name: \(literal: name))" + + case .product(name: let name, package: nil, moduleAliases: nil, condition: nil): + ".product(name: \(literal: name))" + + case .product(name: let name, package: let package, moduleAliases: nil, condition: nil): + ".product(name: \(literal: name), package: \(literal: package))" + + default: + fatalError() + } + } +} diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index 0c053e8ee94..c0ce03b387b 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -73,8 +73,7 @@ class ManifestEditTests: XCTestCase { url: Self.swiftSystemURL, requirement: .branch("main"), productFilter: .nothing ), - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") + to: manifest ) } } @@ -106,8 +105,7 @@ class ManifestEditTests: XCTestCase { requirement: .exact("510.0.0"), productFilter: .nothing ), - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") + to: manifest ) } } @@ -141,8 +139,7 @@ class ManifestEditTests: XCTestCase { requirement: .range(versionRange), productFilter: .nothing ), - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") + to: manifest ) } } @@ -171,8 +168,7 @@ class ManifestEditTests: XCTestCase { requirement: .range(versionRange), productFilter: .nothing ), - to: manifest, - manifestDirectory: try! AbsolutePath(validating: "/") + to: manifest ) } } @@ -201,8 +197,7 @@ class ManifestEditTests: XCTestCase { requirement: .range(Version(508,0,0).. Date: Thu, 18 Apr 2024 23:29:27 -0700 Subject: [PATCH 17/22] [SE-0301] Introduce `swift package add-target` support to add targets to a package Add a new package command to add a target with the given name, type, and dependencies to the package. This includes adding the target to the package manifest as well as creating stub source files for the target to guide the user. For example, this command: swift package add-target SocialGraphClientTests --dependencies SocialGraphClient --type test adds the following to the targets in the package manifest: .testTarget(name: "SocialGraphClientTests", dependencies: ["SocialGraphClient"]), as well as creating the file `Tests/SocialGraphClientTests/SocialGraphClientTests.swift`, which looks like this: import SocialGraphClient import XCTest class SocialGraphClientTests: XCTestCase { func testSocialGraphClientTests() { XCTAssertEqual(42, 17 + 25) } } There is, undoubtedly, some tuning to do to clean this up. Here's the command-line interface, which mostly aligns with SE-0301: OVERVIEW: Add a new target to the manifest USAGE: swift package add-target [--type ] [--dependencies ...] [--url ] [--path ] [--checksum ] ARGUMENTS: The name of the new target OPTIONS: --type The type of target to add, which can be one of (default: library) --dependencies A list of target dependency names --url The URL for a remote binary target --path The path to a local binary target --checksum The checksum for a remote binary target --version Show the version. -h, -help, --help Show help information. --- Package.swift | 1 + Sources/Commands/CMakeLists.txt | 1 + .../PackageCommands/AddDependency.swift | 16 +-- .../Commands/PackageCommands/AddTarget.swift | 128 ++++++++++++++++++ .../PackageCommands/SwiftPackageCommand.swift | 1 + .../AddPackageDependency.swift | 6 +- Sources/PackageModelSyntax/AddTarget.swift | 88 +++++++++++- Sources/PackageModelSyntax/CMakeLists.txt | 1 + .../PackageEditResult.swift | 97 +++++++++++++ Tests/CommandsTests/PackageCommandTests.swift | 31 +++++ .../ManifestEditTests.swift | 49 ++++++- 11 files changed, 401 insertions(+), 18 deletions(-) create mode 100644 Sources/Commands/PackageCommands/AddTarget.swift create mode 100644 Sources/PackageModelSyntax/PackageEditResult.swift diff --git a/Package.swift b/Package.swift index 94df241e7d3..b48a7debc47 100644 --- a/Package.swift +++ b/Package.swift @@ -256,6 +256,7 @@ let package = Package( "PackageModel", .product(name: "SwiftBasicFormat", package: "swift-syntax"), .product(name: "SwiftDiagnostics", package: "swift-syntax"), + .product(name: "SwiftIDEUtils", package: "swift-syntax"), .product(name: "SwiftParser", package: "swift-syntax"), .product(name: "SwiftSyntax", package: "swift-syntax"), .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), diff --git a/Sources/Commands/CMakeLists.txt b/Sources/Commands/CMakeLists.txt index 6c4b6345e58..f7978b6d84e 100644 --- a/Sources/Commands/CMakeLists.txt +++ b/Sources/Commands/CMakeLists.txt @@ -8,6 +8,7 @@ add_library(Commands PackageCommands/AddDependency.swift + PackageCommands/AddTarget.swift PackageCommands/APIDiff.swift PackageCommands/ArchiveSource.swift PackageCommands/CompletionCommand.swift diff --git a/Sources/Commands/PackageCommands/AddDependency.swift b/Sources/Commands/PackageCommands/AddDependency.swift index 2eba6370c5a..f0b21c8b5c7 100644 --- a/Sources/Commands/PackageCommands/AddDependency.swift +++ b/Sources/Commands/PackageCommands/AddDependency.swift @@ -15,7 +15,6 @@ import Basics import CoreCommands import PackageModel import PackageModelSyntax -@_spi(FixItApplier) import SwiftIDEUtils import SwiftParser import SwiftSyntax import TSCBasic @@ -140,19 +139,16 @@ extension SwiftPackageCommand { productFilter: .everything ) - let edits = try AddPackageDependency.addPackageDependency( + let editResult = try AddPackageDependency.addPackageDependency( packageDependency, to: manifestSyntax ) - if edits.isEmpty { - throw StringError("Unable to add package to manifest file") - } - - let updatedManifestSource = FixItApplier.apply(edits: edits, to: manifestSyntax) - try fileSystem.writeFileContents( - manifestPath, - string: updatedManifestSource + try editResult.applyEdits( + to: fileSystem, + manifest: manifestSyntax, + manifestPath: manifestPath, + verbose: !globalOptions.logging.quiet ) } } diff --git a/Sources/Commands/PackageCommands/AddTarget.swift b/Sources/Commands/PackageCommands/AddTarget.swift new file mode 100644 index 00000000000..0cf0b388820 --- /dev/null +++ b/Sources/Commands/PackageCommands/AddTarget.swift @@ -0,0 +1,128 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import ArgumentParser +import Basics +import CoreCommands +import PackageModel +import PackageModelSyntax +import SwiftParser +import SwiftSyntax +import TSCBasic +import TSCUtility +import Workspace + +extension SwiftPackageCommand { + struct AddTarget: SwiftCommand { + /// The type of target that can be specified on the command line. + enum TargetType: String, Codable, ExpressibleByArgument { + case library + case executable + case test + case binary + case plugin + case macro + } + + package static let configuration = CommandConfiguration( + abstract: "Add a new target to the manifest") + + @OptionGroup(visibility: .hidden) + var globalOptions: GlobalOptions + + @Argument(help: "The name of the new target") + var name: String + + @Option(help: "The type of target to add, which can be one of ") + var type: TargetType = .library + + @Option( + parsing: .upToNextOption, + help: "A list of target dependency names" + ) + var dependencies: [String] = [] + + @Option(help: "The URL for a remote binary target") + var url: String? + + @Option(help: "The path to a local binary target") + var path: String? + + @Option(help: "The checksum for a remote binary target") + var checksum: String? + + func run(_ swiftCommandState: SwiftCommandState) throws { + let workspace = try swiftCommandState.getActiveWorkspace() + + guard let packagePath = try swiftCommandState.getWorkspaceRoot().packages.first else { + throw StringError("unknown package") + } + + // Load the manifest file + let fileSystem = workspace.fileSystem + let manifestPath = packagePath.appending("Package.swift") + let manifestContents: ByteString + do { + manifestContents = try fileSystem.readFileContents(manifestPath) + } catch { + throw StringError("cannot find package manifest in \(manifestPath)") + } + + // Parse the manifest. + let manifestSyntax = manifestContents.withData { data in + data.withUnsafeBytes { buffer in + buffer.withMemoryRebound(to: UInt8.self) { buffer in + Parser.parse(source: buffer) + } + } + } + + // Map the target type. + let type: TargetDescription.TargetType = switch self.type { + case .library: .regular + case .executable: .executable + case .test: .test + case .binary: .binary + case .plugin: .plugin + case .macro: .macro + } + + // Map dependencies + let dependencies: [TargetDescription.Dependency] = + self.dependencies.map { + .byName(name: $0, condition: nil) + } + + let target = try TargetDescription( + name: name, + dependencies: dependencies, + path: path, + url: url, + type: type, + checksum: checksum + ) + + let editResult = try PackageModelSyntax.AddTarget.addTarget( + target, + to: manifestSyntax + ) + + try editResult.applyEdits( + to: fileSystem, + manifest: manifestSyntax, + manifestPath: manifestPath, + verbose: !globalOptions.logging.quiet + ) + } + } +} + diff --git a/Sources/Commands/PackageCommands/SwiftPackageCommand.swift b/Sources/Commands/PackageCommands/SwiftPackageCommand.swift index 73734df14c4..d2ca89f5b3c 100644 --- a/Sources/Commands/PackageCommands/SwiftPackageCommand.swift +++ b/Sources/Commands/PackageCommands/SwiftPackageCommand.swift @@ -36,6 +36,7 @@ package struct SwiftPackageCommand: AsyncParsableCommand { version: SwiftVersion.current.completeDisplayString, subcommands: [ AddDependency.self, + AddTarget.self, Clean.self, PurgeCache.self, Reset.self, diff --git a/Sources/PackageModelSyntax/AddPackageDependency.swift b/Sources/PackageModelSyntax/AddPackageDependency.swift index 094de688e58..7ba1889ffe4 100644 --- a/Sources/PackageModelSyntax/AddPackageDependency.swift +++ b/Sources/PackageModelSyntax/AddPackageDependency.swift @@ -36,7 +36,7 @@ public struct AddPackageDependency { public static func addPackageDependency( _ dependency: PackageDependency, to manifest: SourceFileSyntax - ) throws -> [SourceEdit] { + ) throws -> PackageEditResult { // Make sure we have a suitable tools version in the manifest. try manifest.checkEditManifestToolsVersion() @@ -44,10 +44,12 @@ public struct AddPackageDependency { throw ManifestEditError.cannotFindPackage } - return try packageCall.appendingToArrayArgument( + let edits = try packageCall.appendingToArrayArgument( label: "dependencies", trailingLabels: Self.argumentLabelsAfterDependencies, newElement: dependency.asSyntax() ) + + return PackageEditResult(manifestEdits: edits) } } diff --git a/Sources/PackageModelSyntax/AddTarget.swift b/Sources/PackageModelSyntax/AddTarget.swift index 7c7dae46945..74f7d068f89 100644 --- a/Sources/PackageModelSyntax/AddTarget.swift +++ b/Sources/PackageModelSyntax/AddTarget.swift @@ -29,10 +29,13 @@ public struct AddTarget { "cxxLanguageStandard" ] + /// Add the given target to the manifest, producing a set of edit results + /// that updates the manifest and adds some source files to stub out the + /// new target. public static func addTarget( _ target: TargetDescription, to manifest: SourceFileSyntax - ) throws -> [SourceEdit] { + ) throws -> PackageEditResult { // Make sure we have a suitable tools version in the manifest. try manifest.checkEditManifestToolsVersion() @@ -40,10 +43,91 @@ public struct AddTarget { throw ManifestEditError.cannotFindPackage } - return try packageCall.appendingToArrayArgument( + let manifestEdits = try packageCall.appendingToArrayArgument( label: "targets", trailingLabels: Self.argumentLabelsAfterTargets, newElement: target.asSyntax() ) + + let outerDirectory: String? = switch target.type { + case .binary, .macro, .plugin, .system: nil + case .executable, .regular: "Sources" + case .test: "Tests" + } + + guard let outerDirectory else { + return PackageEditResult(manifestEdits: manifestEdits) + } + + let sourceFilePath = try RelativePath(validating: outerDirectory) + .appending(components: [target.name, "\(target.name).swift"]) + + // Introduce imports for each of the dependencies that were specified. + var importModuleNames = target.dependencies.map { + $0.name + } + + // Add appropriate test module dependencies. + if target.type == .test { + importModuleNames.append("XCTest") + } + + let importDecls = importModuleNames.lazy.sorted().map { name in + DeclSyntax("import \(raw: name)").with(\.trailingTrivia, .newline) + } + + let imports = CodeBlockItemListSyntax { + for importDecl in importDecls { + importDecl + } + } + + let sourceFileText: SourceFileSyntax = switch target.type { + case .binary, .macro, .plugin, .system: + fatalError("should have exited above") + + case .test: + """ + \(imports) + class \(raw: target.name): XCTestCase { + func test\(raw: target.name)() { + XCTAssertEqual(42, 17 + 25) + } + } + """ + + case .regular: + """ + \(imports) + """ + + case .executable: + """ + \(imports) + @main + struct \(raw: target.name)Main { + static func main() { + print("Hello, world") + } + } + """ + } + + return PackageEditResult( + manifestEdits: manifestEdits, + auxiliaryFiles: [(sourceFilePath, sourceFileText)] + ) + } +} + +fileprivate extension TargetDescription.Dependency { + /// Retrieve the name of the dependency + var name: String { + switch self { + case .target(name: let name, condition: _), + .byName(name: let name, condition: _), + .product(name: let name, package: _, moduleAliases: _, condition: _): + name + } } } diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index 7e544d14158..949bbeae055 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -12,6 +12,7 @@ add_library(PackageModelSyntax ManifestEditError.swift ManifestSyntaxRepresentable.swift PackageDependency+Syntax.swift + PackageEditResult.swift SyntaxEditUtils.swift TargetDescription+Syntax.swift ) diff --git a/Sources/PackageModelSyntax/PackageEditResult.swift b/Sources/PackageModelSyntax/PackageEditResult.swift new file mode 100644 index 00000000000..6de70765eeb --- /dev/null +++ b/Sources/PackageModelSyntax/PackageEditResult.swift @@ -0,0 +1,97 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Basics +@_spi(FixItApplier) import SwiftIDEUtils +import SwiftSyntax + +/// The result of editing a package, including any edits to the package +/// manifest and any new files that are introduced. +public struct PackageEditResult { + /// Edits to perform to the package manifest. + public var manifestEdits: [SourceEdit] = [] + + /// Auxiliary files to write. + public var auxiliaryFiles: [(RelativePath, SourceFileSyntax)] = [] +} + +extension PackageEditResult { + /// Apply the edits for the given manifest to the specified file system, + /// updating the manifest to the given manifest + public func applyEdits( + to filesystem: any FileSystem, + manifest: SourceFileSyntax, + manifestPath: AbsolutePath, + verbose: Bool + ) throws { + let rootPath = manifestPath.parentDirectory + + // Update the manifest + if verbose { + print("Updating package manifest at \(manifestPath.relative(to: rootPath))...", terminator: "") + } + + let updatedManifestSource = FixItApplier.apply( + edits: manifestEdits, + to: manifest + ) + try filesystem.writeFileContents( + manifestPath, + string: updatedManifestSource + ) + if verbose { + print(" done.") + } + + // Write all of the auxiliary files. + for (auxiliaryFileRelPath, auxiliaryFileSyntax) in auxiliaryFiles { + // If the file already exists, skip it. + let filePath = rootPath.appending(auxiliaryFileRelPath) + if filesystem.exists(filePath) { + if verbose { + print("Skipping \(filePath.relative(to: rootPath)) because it already exists.") + } + + continue + } + + // If the directory does not exist yet, create it. + let fileDir = filePath.parentDirectory + if !filesystem.exists(fileDir) { + if verbose { + print("Creating directory \(fileDir.relative(to: rootPath))...", terminator: "") + } + + try filesystem.createDirectory(fileDir, recursive: true) + + if verbose { + print(" done.") + } + } + + // Write the file. + if verbose { + print("Writing \(filePath.relative(to: rootPath))...", terminator: "") + } + + try filesystem.writeFileContents( + filePath, + string: auxiliaryFileSyntax.description + ) + + if verbose { + print(" done.") + } + } + } + +} diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index 577ac73c00f..82c4810ef1d 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -820,6 +820,37 @@ final class PackageCommandTests: CommandsTestCase { } } + func testPackageAddTarget() throws { + try testWithTemporaryDirectory { tmpPath in + let fs = localFileSystem + let path = tmpPath.appending("PackageB") + try fs.createDirectory(path) + + try fs.writeFileContents(path.appending("Package.swift"), string: + """ + // swift-tools-version: 5.9 + import PackageDescription + let package = Package( + name: "client" + ) + """ + ) + + _ = try execute(["add-target", "client", "--dependencies", "MyLib", "OtherLib", "--type", "executable"], packagePath: path) + + let manifest = path.appending("Package.swift") + XCTAssertFileExists(manifest) + let contents: String = try fs.readFileContents(manifest) + + XCTAssertMatch(contents, .contains(#"targets:"#)) + XCTAssertMatch(contents, .contains(#".executableTarget"#)) + XCTAssertMatch(contents, .contains(#"name: "client""#)) + XCTAssertMatch(contents, .contains(#"dependencies:"#)) + XCTAssertMatch(contents, .contains(#""MyLib""#)) + XCTAssertMatch(contents, .contains(#""OtherLib""#)) + } + } + func testPackageEditAndUnedit() throws { try fixture(name: "Miscellaneous/PackageEdit") { fixturePath in let fooPath = fixturePath.appending("foo") diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index c0ce03b387b..2ea3bf9f1e1 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -19,15 +19,22 @@ import SwiftSyntax import struct TSCUtility.Version import XCTest +/// Assert that applying the given edit/refactor operation to the manifest +/// produces the expected manifest source file and the expected auxiliary +/// files. func assertManifestRefactor( _ originalManifest: SourceFileSyntax, expectedManifest: SourceFileSyntax, + expectedAuxiliarySources: [RelativePath: SourceFileSyntax] = [:], file: StaticString = #filePath, line: UInt = #line, - operation: (SourceFileSyntax) throws -> [SourceEdit] + operation: (SourceFileSyntax) throws -> PackageEditResult ) rethrows { let edits = try operation(originalManifest) - let editedManifestSource = FixItApplier.apply(edits: edits, to: originalManifest) + let editedManifestSource = FixItApplier.apply( + edits: edits.manifestEdits, + to: originalManifest + ) let editedManifest = Parser.parse(source: editedManifestSource) assertStringsEqualWithDiff( @@ -36,6 +43,27 @@ func assertManifestRefactor( file: file, line: line ) + + // Check all of the auxiliary sources. + for (auxSourcePath, auxSourceSyntax) in edits.auxiliaryFiles { + guard let expectedSyntax = expectedAuxiliarySources[auxSourcePath] else { + XCTFail("unexpected auxiliary source file \(auxSourcePath)") + return + } + + assertStringsEqualWithDiff( + auxSourceSyntax.description, + expectedSyntax.description, + file: file, + line: line + ) + } + + XCTAssertEqual( + edits.auxiliaryFiles.count, + expectedAuxiliarySources.count, + "didn't get all of the auxiliary files we expected" + ) } class ManifestEditTests: XCTestCase { @@ -322,7 +350,12 @@ class ManifestEditTests: XCTestCase { .target(name: "MyLib"), ] ) - """) { manifest in + """, + expectedAuxiliarySources: [ + RelativePath("Sources/MyLib/MyLib.swift") : """ + + """ + ]) { manifest in try AddTarget.addTarget( TargetDescription(name: "MyLib"), to: manifest @@ -345,7 +378,15 @@ class ManifestEditTests: XCTestCase { .target(name: "MyLib", dependencies: ["OtherLib", .product(name: "SwiftSyntax", package: "swift-syntax"), .target(name: "TargetLib")]), ] ) - """) { manifest in + """, + expectedAuxiliarySources: [ + RelativePath("Sources/MyLib/MyLib.swift") : """ + import OtherLib + import SwiftSyntax + import TargetLib + + """ + ]) { manifest in try AddTarget.addTarget( TargetDescription(name: "MyLib", dependencies: [ From 79459fe3dc58ad312259ea13144cfb9fbb747b8c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 21 Apr 2024 08:18:06 -0700 Subject: [PATCH 18/22] Fix CMake syntax again, I think, maybe --- CMakeLists.txt | 1 + Sources/PackageModelSyntax/CMakeLists.txt | 9 +++++---- Utilities/bootstrap | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 029f3ffa337..02431a09d49 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,6 +45,7 @@ if(FIND_PM_DEPS) find_package(SwiftASN1 CONFIG REQUIRED) find_package(SwiftCertificates CONFIG REQUIRED) find_package(SwiftCrypto CONFIG REQUIRED) + find_package(SwiftSyntax CONFIG REQUIRED) endif() find_package(dispatch QUIET) diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index 949bbeae055..2a052117885 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -21,10 +21,11 @@ target_link_libraries(PackageModelSyntax PUBLIC PackageLoading PackageModel - SwiftDiagnostics - SwiftParser - SwiftSyntax - SwiftSyntaxBuilder) + SwiftSyntax::SwiftDiagnostics + SwiftSyntax::SwiftIDEUtils + SwiftSyntax::SwiftParser + SwiftSyntax::SwiftSyntax + SwiftSyntax::SwiftSyntaxBuilder) # NOTE(compnerd) workaround for CMake not setting up include flags yet set_target_properties(PackageModelSyntax PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY}) diff --git a/Utilities/bootstrap b/Utilities/bootstrap index 7605c4d5bf2..c17aa5a4b7c 100755 --- a/Utilities/bootstrap +++ b/Utilities/bootstrap @@ -605,6 +605,7 @@ def build_swiftpm_with_cmake(args): "-DTSC_DIR=" + os.path.join(args.build_dirs["tsc"], "cmake/modules"), "-DArgumentParser_DIR=" + os.path.join(args.build_dirs["swift-argument-parser"], "cmake/modules"), "-DSwiftDriver_DIR=" + os.path.join(args.build_dirs["swift-driver"], "cmake/modules"), + "-DSwiftSyntax_DIR=" + os.path.join(args.build_dirs["swift-syntax"], "cmake/modules"), "-DSwiftSystem_DIR=" + os.path.join(args.build_dirs["swift-system"], "cmake/modules"), "-DSwiftCollections_DIR=" + os.path.join(args.build_dirs["swift-collections"], "cmake/modules"), "-DSwiftCrypto_DIR=" + os.path.join(args.build_dirs["swift-crypto"], "cmake/modules"), From 1c1c2fcdb2a8d241fe6d9337f5b9598d43961c84 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 21 Apr 2024 11:04:57 -0700 Subject: [PATCH 19/22] Use SwiftBasicFormat to improve formatting of edited manifest Rather than trying to thread through trivia everywhere, apply BasicFormat to arguments. --- Sources/PackageModelSyntax/CMakeLists.txt | 1 + .../PackageModelSyntax/SyntaxEditUtils.swift | 104 +++++++++++++----- .../TargetDescription+Syntax.swift | 4 +- .../ManifestEditTests.swift | 67 ++++++++++- 4 files changed, 147 insertions(+), 29 deletions(-) diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index 2a052117885..a57e30b0ebe 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -21,6 +21,7 @@ target_link_libraries(PackageModelSyntax PUBLIC PackageLoading PackageModel + SwiftSyntax::SwiftBasicFormat SwiftSyntax::SwiftDiagnostics SwiftSyntax::SwiftIDEUtils SwiftSyntax::SwiftParser diff --git a/Sources/PackageModelSyntax/SyntaxEditUtils.swift b/Sources/PackageModelSyntax/SyntaxEditUtils.swift index 250848d35cb..774853a46f7 100644 --- a/Sources/PackageModelSyntax/SyntaxEditUtils.swift +++ b/Sources/PackageModelSyntax/SyntaxEditUtils.swift @@ -12,6 +12,7 @@ import Basics import PackageModel +import SwiftBasicFormat import SwiftSyntax import SwiftParser @@ -244,12 +245,22 @@ extension Array { /// 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 { - self[count - 1].trailingComma = TokenSyntax.commaToken(trailingTrivia: .space) + self[count - 1].trailingComma = TokenSyntax.commaToken() + leadingTrivia = .newline + + // Adjust the first element to start with a newline + if count == 1 { + self[0].leadingTrivia = .newline + } + } else { + leadingTrivia = nil } append( ArrayElementSyntax( + leadingTrivia: leadingTrivia, expression: expression ) ) @@ -262,12 +273,26 @@ extension Array { /// 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. + let leadingTrivia: Trivia if count > 0 { - self[count - 1].trailingComma = TokenSyntax.commaToken(trailingTrivia: .space) + self[count - 1].trailingComma = TokenSyntax.commaToken() + leadingTrivia = .newline + + // Adjust the first element to start with a newline + if count == 1 { + self[0].leadingTrivia = .newline + } + } else { + leadingTrivia = Trivia() } // Add the new expression. - append(LabeledExprSyntax(label: label, expression: expression)) + append( + LabeledExprSyntax( + label: label, + expression: expression + ).with(\.leadingTrivia, leadingTrivia) + ) } /// Append a potentially labeled argument with a string literal. @@ -294,7 +319,19 @@ extension Array { elements.append(expression: element.asSyntax()) } - let array = ArrayExprSyntax(elements: ArrayElementListSyntax(elements)) + // When we have more than one element in the array literal, we add + // newlines at the beginning of each element. Do the same for the + // right square bracket. + let rightSquareLeadingTrivia: Trivia = elements.count > 0 + ? .newline + : Trivia() + + let array = ArrayExprSyntax( + elements: ArrayElementListSyntax(elements), + rightSquare: .rightSquareToken( + leadingTrivia: rightSquareLeadingTrivia + ) + ) append(label: label, expression: ExprSyntax(array)) } @@ -350,8 +387,19 @@ extension FunctionCallExprSyntax { ) } + // Format the element appropriately for the context. + let indentation = Trivia( + pieces: arg.leadingTrivia.filter { $0.isSpaceOrTab } + ) + let format = BasicFormat( + indentationWidth: [ defaultIndent ], + initialIndentation: indentation.appending(defaultIndent) + ) + let formattedElement = newElement.formatted(using: format) + .cast(ExprSyntax.self) + let updatedArgArray = argArray.appending( - element: newElement, + element: formattedElement, outerLeadingTrivia: arg.leadingTrivia ) return [ .replace(argArray, with: updatedArgArray.description) ] @@ -366,36 +414,40 @@ extension FunctionCallExprSyntax { let newArguments = arguments.insertingArgument( at: insertionPos ) { (leadingTrivia, trailingComma) in - // The argument is always [ element ], but if we have any newlines - // in the leading trivia, then we really want to split it across - // multiple lines, like this: - // [ - // element - // ] - let newArgument: ExprSyntax - if !leadingTrivia.hasNewlines { - newArgument = " [ \(newElement), ]" - } else { - let innerTrivia = leadingTrivia.appending(defaultIndent) - let arrayExpr = ArrayExprSyntax( - leadingTrivia: .space, - elements: [ + // Format the element appropriately for the context. + let indentation = Trivia(pieces: leadingTrivia.filter { $0.isSpaceOrTab }) + let format = BasicFormat( + indentationWidth: [ defaultIndent ], + initialIndentation: indentation.appending(defaultIndent) + ) + let formattedElement = newElement.formatted(using: format) + .cast(ExprSyntax.self) + + // Form the array. + let newArgument = ArrayExprSyntax( + leadingTrivia: .space, + leftSquare: .leftSquareToken( + trailingTrivia: .newline + ), + elements: ArrayElementListSyntax( + [ ArrayElementSyntax( - leadingTrivia: innerTrivia, - expression: newElement, + expression: formattedElement, trailingComma: .commaToken() ) - ], - rightSquare: .rightSquareToken(leadingTrivia: leadingTrivia) + ] + ), + rightSquare: .rightSquareToken( + leadingTrivia: leadingTrivia ) - newArgument = ExprSyntax(arrayExpr) - } + ) + // Create the labeled argument for the array. return LabeledExprSyntax( leadingTrivia: leadingTrivia, label: "\(raw: label)", colon: .colonToken(), - expression: newArgument, + expression: ExprSyntax(newArgument), trailingComma: trailingComma ) } diff --git a/Sources/PackageModelSyntax/TargetDescription+Syntax.swift b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift index 740d55f4822..f47f6590f06 100644 --- a/Sources/PackageModelSyntax/TargetDescription+Syntax.swift +++ b/Sources/PackageModelSyntax/TargetDescription+Syntax.swift @@ -71,7 +71,9 @@ extension TargetDescription: ManifestSyntaxRepresentable { // Only for plugins arguments.appendIf(label: "checksum", stringLiteral: checksum) - return ".\(raw: functionName)(\(LabeledExprListSyntax(arguments)))" + let separateParen: String = arguments.count > 1 ? "\n" : "" + let argumentsSyntax = LabeledExprListSyntax(arguments) + return ".\(raw: functionName)(\(argumentsSyntax)\(raw: separateParen))" } } diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index 2ea3bf9f1e1..726fcf116ce 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -213,7 +213,7 @@ class ManifestEditTests: XCTestCase { let package = Package( name: "packages", dependencies: [ - .package(url: "https://github.com/apple/swift-system.git", "508.0.0"..<"510.0.0"), + .package(url: "https://github.com/apple/swift-system.git", "508.0.0" ..< "510.0.0"), ] ) """) { manifest in @@ -375,7 +375,14 @@ class ManifestEditTests: XCTestCase { let package = Package( name: "packages", targets: [ - .target(name: "MyLib", dependencies: ["OtherLib", .product(name: "SwiftSyntax", package: "swift-syntax"), .target(name: "TargetLib")]), + .target( + name: "MyLib", + dependencies: [ + "OtherLib", + .product(name: "SwiftSyntax", package: "swift-syntax"), + .target(name: "TargetLib") + ] + ), ] ) """, @@ -398,6 +405,62 @@ class ManifestEditTests: XCTestCase { ) } } + + func testAddExecutableTargetWithDependencies() throws { + try assertManifestRefactor(""" + // swift-tools-version: 5.5 + let package = Package( + name: "packages", + targets: [ + .target(name: "MyLib") + ] + ) + """, + expectedManifest: """ + // swift-tools-version: 5.5 + let package = Package( + name: "packages", + targets: [ + .target(name: "MyLib"), + .executableTarget( + name: "MyProgram", + dependencies: [ + .product(name: "SwiftSyntax", package: "swift-syntax"), + .target(name: "TargetLib"), + "MyLib" + ] + ), + ] + ) + """, + expectedAuxiliarySources: [ + RelativePath("Sources/MyProgram/MyProgram.swift") : """ + import MyLib + import SwiftSyntax + import TargetLib + + @main + struct MyProgramMain { + static func main() { + print("Hello, world") + } + } + """ + ]) { manifest in + try AddTarget.addTarget( + TargetDescription( + name: "MyProgram", + dependencies: [ + .product(name: "SwiftSyntax", package: "swift-syntax"), + .target(name: "TargetLib", condition: nil), + .byName(name: "MyLib", condition: nil) + ], + type: .executable + ), + to: manifest + ) + } + } } From bf7eaef18fe071a6b73697a7f48e7c84a16f575c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 21 Apr 2024 15:21:12 -0700 Subject: [PATCH 20/22] [CMake] Use FetchContent for building swift-syntax --- BuildSupport/SwiftSyntax/CMakeLists.txt | 17 +++++++++++++++++ CMakeLists.txt | 2 +- Sources/PackageModelSyntax/CMakeLists.txt | 16 ++++++++++------ Utilities/bootstrap | 5 ----- 4 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 BuildSupport/SwiftSyntax/CMakeLists.txt diff --git a/BuildSupport/SwiftSyntax/CMakeLists.txt b/BuildSupport/SwiftSyntax/CMakeLists.txt new file mode 100644 index 00000000000..7a3cfbfd4de --- /dev/null +++ b/BuildSupport/SwiftSyntax/CMakeLists.txt @@ -0,0 +1,17 @@ +SET(SWIFTPM_PATH_TO_SWIFT_SYNTAX_SOURCE ${CMAKE_SOURCE_DIR}/../swift-syntax) +message(STATUS "swift-syntax path: ${SWIFTPM_PATH_TO_SWIFT_SYNTAX_SOURCE}") + +include(FetchContent) + +if(NOT EXISTS "${SWIFTPM_PATH_TO_SWIFT_SYNTAX_SOURCE}") + message(SEND_ERROR "swift-syntax is required to build SwiftPM. Please run update-checkout or specify SWIFTPM_PATH_TO_SWIFT_SYNTAX_SOURCE") + return() +endif() + +# Build swift-syntax libraries with FetchContent. +# set(CMAKE_Swift_COMPILER_TARGET ${SWIFT_HOST_TRIPLE}) +set(BUILD_SHARED_LIBS OFF) + +file(TO_CMAKE_PATH "${SWIFTPM_PATH_TO_SWIFT_SYNTAX_SOURCE}" swift_syntax_path) +FetchContent_Declare(SwiftSyntax SOURCE_DIR "${swift_syntax_path}") +FetchContent_MakeAvailable(SwiftSyntax) diff --git a/CMakeLists.txt b/CMakeLists.txt index 02431a09d49..1a9153193ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,7 +45,6 @@ if(FIND_PM_DEPS) find_package(SwiftASN1 CONFIG REQUIRED) find_package(SwiftCertificates CONFIG REQUIRED) find_package(SwiftCrypto CONFIG REQUIRED) - find_package(SwiftSyntax CONFIG REQUIRED) endif() find_package(dispatch QUIET) @@ -55,5 +54,6 @@ find_package(SQLite3 REQUIRED) # Enable `package` modifier for the whole package. add_compile_options("$<$:-package-name;SwiftPM>") +add_subdirectory(BuildSupport/SwiftSyntax) add_subdirectory(Sources) add_subdirectory(cmake/modules) diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index a57e30b0ebe..a5d51c760e2 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -20,13 +20,17 @@ target_link_libraries(PackageModelSyntax PUBLIC Basics PackageLoading PackageModel +) + +target_link_libraries(PackageModelSyntax PRIVATE + SwiftBasicFormat + SwiftDiagnostics + SwiftIDEUtils + SwiftParser + SwiftSyntax + SwiftSyntaxBuilder +) - SwiftSyntax::SwiftBasicFormat - SwiftSyntax::SwiftDiagnostics - SwiftSyntax::SwiftIDEUtils - SwiftSyntax::SwiftParser - SwiftSyntax::SwiftSyntax - SwiftSyntax::SwiftSyntaxBuilder) # NOTE(compnerd) workaround for CMake not setting up include flags yet set_target_properties(PackageModelSyntax PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY}) diff --git a/Utilities/bootstrap b/Utilities/bootstrap index c17aa5a4b7c..2da5c06d725 100755 --- a/Utilities/bootstrap +++ b/Utilities/bootstrap @@ -198,7 +198,6 @@ def parse_global_args(args): args.source_dirs["swift-argument-parser"] = os.path.join(args.project_root, "..", "swift-argument-parser") args.source_dirs["swift-crypto"] = os.path.join(args.project_root, "..", "swift-crypto") args.source_dirs["swift-driver"] = os.path.join(args.project_root, "..", "swift-driver") - args.source_dirs["swift-syntax"] = os.path.join(args.project_root, "..", "swift-syntax") args.source_dirs["swift-system"] = os.path.join(args.project_root, "..", "swift-system") args.source_dirs["swift-collections"] = os.path.join(args.project_root, "..", "swift-collections") args.source_dirs["swift-certificates"] = os.path.join(args.project_root, "..", "swift-certificates") @@ -346,7 +345,6 @@ def build(args): build_dependency(args, "swift-driver", swift_driver_cmake_flags) build_dependency(args, "swift-collections") build_dependency(args, "swift-crypto") - build_dependency(args, "swift-syntax") build_dependency(args, "swift-asn1") build_dependency(args, "swift-certificates", ["-DSwiftASN1_DIR=" + os.path.join(args.build_dirs["swift-asn1"], "cmake/modules"), @@ -605,7 +603,6 @@ def build_swiftpm_with_cmake(args): "-DTSC_DIR=" + os.path.join(args.build_dirs["tsc"], "cmake/modules"), "-DArgumentParser_DIR=" + os.path.join(args.build_dirs["swift-argument-parser"], "cmake/modules"), "-DSwiftDriver_DIR=" + os.path.join(args.build_dirs["swift-driver"], "cmake/modules"), - "-DSwiftSyntax_DIR=" + os.path.join(args.build_dirs["swift-syntax"], "cmake/modules"), "-DSwiftSystem_DIR=" + os.path.join(args.build_dirs["swift-system"], "cmake/modules"), "-DSwiftCollections_DIR=" + os.path.join(args.build_dirs["swift-collections"], "cmake/modules"), "-DSwiftCrypto_DIR=" + os.path.join(args.build_dirs["swift-crypto"], "cmake/modules"), @@ -627,7 +624,6 @@ def build_swiftpm_with_cmake(args): add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-argument-parser"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-crypto"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-driver"], "lib")) - add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-syntax"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-system"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-collections"], "lib")) add_rpath_for_cmake_build(args, os.path.join(args.build_dirs["swift-asn1"], "lib")) @@ -762,7 +758,6 @@ def get_swiftpm_env_cmd(args): os.path.join(args.build_dirs["swift-argument-parser"], "lib"), os.path.join(args.build_dirs["swift-crypto"], "lib"), os.path.join(args.build_dirs["swift-driver"], "lib"), - os.path.join(args.build_dirs["swift-syntax"], "lib"), os.path.join(args.build_dirs["swift-system"], "lib"), os.path.join(args.build_dirs["swift-collections"], "lib"), os.path.join(args.build_dirs["swift-asn1"], "lib"), From 2a42d47385f4406a2ba8f3446b1cc5e8867e5790 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 21 Apr 2024 17:30:34 -0700 Subject: [PATCH 21/22] Implement add-target support for macro targets For macro targets, we need to add two new files: one with a macro definition, and another with the list of provided macros for the macro plugin. Reshuffle some code to make that easier. --- .../Manifest/TargetDescription.swift | 2 +- Sources/PackageModelSyntax/AddTarget.swift | 110 ++++++++++++++++-- .../ManifestEditTests.swift | 56 +++++++++ 3 files changed, 159 insertions(+), 9 deletions(-) diff --git a/Sources/PackageModel/Manifest/TargetDescription.swift b/Sources/PackageModel/Manifest/TargetDescription.swift index d131c99f37e..57df7f6d37a 100644 --- a/Sources/PackageModel/Manifest/TargetDescription.swift +++ b/Sources/PackageModel/Manifest/TargetDescription.swift @@ -92,7 +92,7 @@ public struct TargetDescription: Hashable, Encodable, Sendable { } /// The declared target dependencies. - public let dependencies: [Dependency] + public package(set) var dependencies: [Dependency] /// The custom public headers path. public let publicHeadersPath: String? diff --git a/Sources/PackageModelSyntax/AddTarget.swift b/Sources/PackageModelSyntax/AddTarget.swift index 74f7d068f89..1157775d985 100644 --- a/Sources/PackageModelSyntax/AddTarget.swift +++ b/Sources/PackageModelSyntax/AddTarget.swift @@ -43,6 +43,16 @@ public struct AddTarget { throw ManifestEditError.cannotFindPackage } + // Create a mutable version of target to which we can add more + // content when needed. + var target = target + + // Macro targets need to depend on a couple of libraries from + // SwiftSyntax. + if target.type == .macro { + target.dependencies.append(contentsOf: macroTargetDependencies) + } + let manifestEdits = try packageCall.appendingToArrayArgument( label: "targets", trailingLabels: Self.argumentLabelsAfterTargets, @@ -50,8 +60,8 @@ public struct AddTarget { ) let outerDirectory: String? = switch target.type { - case .binary, .macro, .plugin, .system: nil - case .executable, .regular: "Sources" + case .binary, .plugin, .system: nil + case .executable, .regular, .macro: "Sources" case .test: "Tests" } @@ -59,8 +69,58 @@ public struct AddTarget { return PackageEditResult(manifestEdits: manifestEdits) } - let sourceFilePath = try RelativePath(validating: outerDirectory) - .appending(components: [target.name, "\(target.name).swift"]) + let outerPath = try RelativePath(validating: outerDirectory) + + /// The set of auxiliary files this refactoring will create. + var auxiliaryFiles: AuxiliaryFiles = [] + + // Add the primary source file. Every target type has this. + addPrimarySourceFile( + outerPath: outerPath, + target: target, + to: &auxiliaryFiles + ) + + // Perform any other actions that are needed for this target type. + switch target.type { + case .macro: + // Macros need a file that introduces the main entrypoint + // describing all of the macros. + auxiliaryFiles.addSourceFile( + path: outerPath.appending( + components: [target.name, "ProvidedMacros.swift"] + ), + sourceCode: """ + import SwiftCompilerPlugin + + @main + struct \(raw: target.name)Macros: CompilerPlugin { + let providingMacros: [Macro.Type] = [ + \(raw: target.name).self, + ] + } + """ + ) + + default: break; + } + + return PackageEditResult( + manifestEdits: manifestEdits, + auxiliaryFiles: auxiliaryFiles + ) + } + + /// Add the primary source file for a target to the list of auxiliary + /// source files. + fileprivate static func addPrimarySourceFile( + outerPath: RelativePath, + target: TargetDescription, + to auxiliaryFiles: inout AuxiliaryFiles + ) { + let sourceFilePath = outerPath.appending( + components: [target.name, "\(target.name).swift"] + ) // Introduce imports for each of the dependencies that were specified. var importModuleNames = target.dependencies.map { @@ -83,9 +143,22 @@ public struct AddTarget { } let sourceFileText: SourceFileSyntax = switch target.type { - case .binary, .macro, .plugin, .system: + case .binary, .plugin, .system: fatalError("should have exited above") + case .macro: + """ + \(imports) + struct \(raw: target.name): Macro { + /// TODO: Implement one or more of the protocols that inherit + /// from Macro. The appropriate macro protocol is determined + /// by the "macro" declaration that \(raw: target.name) implements. + /// Examples include: + /// @freestanding(expression) macro --> ExpressionMacro + /// @attached(member) macro --> MemberMacro + } + """ + case .test: """ \(imports) @@ -113,9 +186,9 @@ public struct AddTarget { """ } - return PackageEditResult( - manifestEdits: manifestEdits, - auxiliaryFiles: [(sourceFilePath, sourceFileText)] + auxiliaryFiles.addSourceFile( + path: sourceFilePath, + sourceCode: sourceFileText ) } } @@ -131,3 +204,24 @@ fileprivate extension TargetDescription.Dependency { } } } + +/// The array of auxiliary files that can be added by a package editing +/// operation. +fileprivate typealias AuxiliaryFiles = [(RelativePath, SourceFileSyntax)] + +fileprivate extension AuxiliaryFiles { + /// Add a source file to the list of auxiliary files. + mutating func addSourceFile( + path: RelativePath, + sourceCode: SourceFileSyntax + ) { + self.append((path, sourceCode)) + } +} + +/// The set of dependencies we need to introduce to a newly-created macro +/// target. +fileprivate let macroTargetDependencies: [TargetDescription.Dependency] = [ + .product(name: "SwiftCompilerPlugin", package: "swift-syntax"), + .product(name: "SwiftSyntaxMacros", package: "swift-syntax"), +] diff --git a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift index 726fcf116ce..c61a6036007 100644 --- a/Tests/PackageModelSyntaxTests/ManifestEditTests.swift +++ b/Tests/PackageModelSyntaxTests/ManifestEditTests.swift @@ -461,6 +461,62 @@ class ManifestEditTests: XCTestCase { ) } } + + func testAddMacroTarget() throws { + try assertManifestRefactor(""" + // swift-tools-version: 5.5 + let package = Package( + name: "packages" + ) + """, + expectedManifest: """ + // swift-tools-version: 5.5 + let package = Package( + name: "packages", + targets: [ + .macro( + name: "MyMacro", + dependencies: [ + .product(name: "SwiftCompilerPlugin", package: "swift-syntax"), + .product(name: "SwiftSyntaxMacros", package: "swift-syntax") + ] + ), + ] + ) + """, + expectedAuxiliarySources: [ + RelativePath("Sources/MyMacro/MyMacro.swift") : """ + import SwiftCompilerPlugin + import SwiftSyntaxMacros + + struct MyMacro: Macro { + /// TODO: Implement one or more of the protocols that inherit + /// from Macro. The appropriate macro protocol is determined + /// by the "macro" declaration that MyMacro implements. + /// Examples include: + /// @freestanding(expression) macro --> ExpressionMacro + /// @attached(member) macro --> MemberMacro + } + """, + RelativePath("Sources/MyMacro/ProvidedMacros.swift") : """ + import SwiftCompilerPlugin + + @main + struct MyMacroMacros: CompilerPlugin { + let providingMacros: [Macro.Type] = [ + MyMacro.self, + ] + } + """ + ] + ) { manifest in + try AddTarget.addTarget( + TargetDescription(name: "MyMacro", type: .macro), + to: manifest + ) + } + } + } From 1e0a269b7a5ede77f92d6122f2028e065c14f208 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 21 Apr 2024 17:37:16 -0700 Subject: [PATCH 22/22] More CMake build fixes --- Sources/PackageModelSyntax/CMakeLists.txt | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/Sources/PackageModelSyntax/CMakeLists.txt b/Sources/PackageModelSyntax/CMakeLists.txt index a5d51c760e2..04f4b298bed 100644 --- a/Sources/PackageModelSyntax/CMakeLists.txt +++ b/Sources/PackageModelSyntax/CMakeLists.txt @@ -20,9 +20,7 @@ target_link_libraries(PackageModelSyntax PUBLIC Basics PackageLoading PackageModel -) -target_link_libraries(PackageModelSyntax PRIVATE SwiftBasicFormat SwiftDiagnostics SwiftIDEUtils @@ -40,3 +38,23 @@ install(TARGETS PackageModelSyntax LIBRARY DESTINATION lib RUNTIME DESTINATION bin) set_property(GLOBAL APPEND PROPERTY SwiftPM_EXPORTS PackageModelSyntax) + +set(SWIFT_SYNTAX_MODULES + SwiftBasicFormat + SwiftParser + SwiftParserDiagnostics + SwiftDiagnostics + SwiftSyntax + SwiftOperators + SwiftSyntaxBuilder + SwiftSyntaxMacros + SwiftSyntaxMacroExpansion + SwiftCompilerPluginMessageHandling + # Support for LSP + SwiftIDEUtils + SwiftRefactor +) +export(TARGETS ${SWIFT_SYNTAX_MODULES} + NAMESPACE SwiftSyntax:: + FILE ${CMAKE_BINARY_DIR}/cmake/modules/SwiftSyntaxConfig.cmake + EXPORT_LINK_INTERFACE_LIBRARIES) \ No newline at end of file