-
Notifications
You must be signed in to change notification settings - Fork 446
Swift package manifest refactoring actions #2904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
8e71dfd
Introduce package manifest refactoring action "Add Package Dependency"
DougGregor 4a6e7bd
FIXUP for adding package dependency
DougGregor 86581a0
Port "Add target dependency" package manifest editing action to Swift…
DougGregor 1b624fc
Port "Add target" from the Swift Package Manager code base to SwiftRe…
DougGregor d308aca
Port "Add Product" manifest edit refactor over from SwiftPM
DougGregor 11649fd
Add a package manifest edit refactor to introduce a new plugin usage …
DougGregor 1aa14e6
Implement a better "contains string literal" check for a source file
DougGregor 89c2854
Remove more uses of switch expressions
DougGregor ba0ac8a
Remove yet more uses of switch expressions
DougGregor 18c9f0e
Rename PackageEditResult -> PackageEdit
DougGregor 4b2073f
Rename TargetDescription -> Target
DougGregor 7293a79
Sink ProductType into ProductDescription
DougGregor e6bd577
Rename AddTarget -> AddPackageTarget
DougGregor a1ac7ca
[PackageManifest] Address NFC review feedback
xedin 9370e43
[SwiftRefactor] Replace custom `XCTAssertThrows` with `XCTAssertThrow…
xedin 902b53e
[SwiftRefactor] Fix handling of leading trivia for new elements/argum…
xedin 95190dc
[Tests] Add more information to unexpected aux files failure to diagn…
xedin de9afa3
[SwiftRefactor] PackageManifest: Rename `Target` to `PackageTarget`
xedin cd7758e
[SwiftRefactor] PackageManifest: Replace custom types with a simple `…
xedin b44a72f
[SwiftRefactor] PackageManifest: Replace `SemanticVersion` with a `St…
xedin 58a01d3
[SwiftRefactor] PackageManifest: Fix formatting for license headers
xedin 621ae13
[SwiftRefactor] PackageManifest: Add duplicate dependency checking to…
xedin fb6939c
[SwiftRefactor] PackageManifest: Add refactoring to add introduce swi…
xedin 68432ea
[SwiftRefactor] PackageManifest: Remove an outdated TODO that is no l…
xedin b896109
[SwiftRefactor] PackageManifest: Change source control dependency to …
xedin 966e0d4
[SwiftRefactor] PackageManifest: Remove identity from `FileSystem` an…
xedin db2b78f
[SwiftRefactor] PackageManifest: Remove unused property from `AddPlug…
xedin 6a98663
[SwiftRefactor] PackageManifest: Inline `Configuration` into `AddPack…
xedin 9e08670
[SwiftRefactor] PackageManifest: Address stylistic review feedback
xedin 2f6937d
[SwiftRefactor] PackageManifest: Mark all of the refactorings and sup…
xedin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
138 changes: 138 additions & 0 deletions
138
Sources/SwiftRefactor/PackageManifest/AddPackageDependency.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import SwiftParser | ||
import SwiftSyntax | ||
import SwiftSyntaxBuilder | ||
|
||
/// Add a package dependency to a package manifest's source code. | ||
public struct AddPackageDependency: ManifestEditRefactoringProvider { | ||
public struct Context { | ||
public var dependency: PackageDependency | ||
|
||
public init(dependency: PackageDependency) { | ||
self.dependency = dependency | ||
} | ||
} | ||
|
||
/// The set of argument labels that can occur after the "dependencies" | ||
/// argument in the Package initializers. | ||
private static let argumentLabelsAfterDependencies: Set<String> = [ | ||
"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 manifestRefactor( | ||
syntax manifest: SourceFileSyntax, | ||
in context: Context | ||
) throws -> PackageEdit { | ||
let dependency = context.dependency | ||
guard let packageCall = manifest.findCall(calleeName: "Package") else { | ||
throw ManifestEditError.cannotFindPackage | ||
} | ||
|
||
guard | ||
try !dependencyAlreadyAdded( | ||
dependency, | ||
in: packageCall | ||
) | ||
else { | ||
return PackageEdit(manifestEdits: []) | ||
} | ||
|
||
let newPackageCall = try addPackageDependencyLocal( | ||
dependency, | ||
to: packageCall | ||
) | ||
|
||
return PackageEdit( | ||
manifestEdits: [ | ||
.replace(packageCall, with: newPackageCall.description) | ||
] | ||
) | ||
} | ||
|
||
/// Return `true` if the dependency already exists in the manifest, otherwise return `false`. | ||
/// Throws an error if a dependency already exists with the same id or url, but different arguments. | ||
private static func dependencyAlreadyAdded( | ||
_ dependency: PackageDependency, | ||
in packageCall: FunctionCallExprSyntax | ||
) throws -> Bool { | ||
let dependencySyntax = dependency.asSyntax() | ||
guard let dependencyFnSyntax = dependencySyntax.as(FunctionCallExprSyntax.self) else { | ||
throw ManifestEditError.cannotFindPackage | ||
} | ||
|
||
guard | ||
let id = dependencyFnSyntax.arguments.first(where: { | ||
$0.label?.text == "url" || $0.label?.text == "id" || $0.label?.text == "path" | ||
}) | ||
else { | ||
throw ManifestEditError.malformedManifest(error: "missing id or url argument in dependency syntax") | ||
} | ||
|
||
if let existingDependencies = packageCall.findArgument(labeled: "dependencies") { | ||
// If we have an existing dependencies array, we need to check if | ||
xedin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if let expr = existingDependencies.expression.as(ArrayExprSyntax.self) { | ||
// Iterate through existing dependencies and look for an argument that matches | ||
// either the `id` or `url` argument of the new dependency. | ||
let existingArgument = expr.elements.first { elem in | ||
if let funcExpr = elem.expression.as(FunctionCallExprSyntax.self) { | ||
return funcExpr.arguments.contains { | ||
$0.trimmedDescription == id.trimmedDescription | ||
} | ||
} | ||
return true | ||
} | ||
|
||
if let existingArgument { | ||
let normalizedExistingArgument = existingArgument.detached.with(\.trailingComma, nil) | ||
// This exact dependency already exists, return false to indicate we should do nothing. | ||
if normalizedExistingArgument.trimmedDescription == dependencySyntax.trimmedDescription { | ||
return true | ||
} | ||
throw ManifestEditError.existingDependency(dependencyName: dependency.identifier) | ||
} | ||
} | ||
} | ||
return false | ||
} | ||
|
||
/// Implementation of adding a package dependency to an existing call. | ||
static func addPackageDependencyLocal( | ||
_ dependency: PackageDependency, | ||
to packageCall: FunctionCallExprSyntax | ||
) throws -> FunctionCallExprSyntax { | ||
try packageCall.appendingToArrayArgument( | ||
label: "dependencies", | ||
labelsAfter: Self.argumentLabelsAfterDependencies, | ||
newElement: dependency.asSyntax() | ||
) | ||
} | ||
} | ||
|
||
fileprivate extension PackageDependency { | ||
var identifier: String { | ||
switch self { | ||
case .sourceControl(let info): | ||
return info.identity | ||
case .fileSystem(let info): | ||
return info.identity | ||
case .registry(let info): | ||
return info.identity | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to say this should be optional and we could add placeholder if not given?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd think that the whole point of the refactoring is to actually add a dependency that's why it's non-optional at the moment, I'm not sure how we'd placeholder that efficiently because different dependency kinds have different arguments...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd just pick a reasonable default, ie.
SourceControl
andfrom
:(though those placeholders could also be typed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe that is the way to go here but instead of having it optional we can make it a defaulted parameter on initializer in both cases.