-
Notifications
You must be signed in to change notification settings - Fork 433
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
Swift package manifest refactoring actions #2904
base: main
Are you sure you want to change the base?
Changes from all commits
acf9708
4161443
fe50821
1abc48f
30ae9e6
4061163
189e29c
370eea6
31291af
ec5afbe
96f01d3
a3cc837
ede1bea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,21 @@ | ||||||
//===----------------------------------------------------------------------===// | ||||||
// | ||||||
// 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 | ||||||
// | ||||||
//===----------------------------------------------------------------------===// | ||||||
|
||||||
/// Syntactic wrapper type that describes an absolute path for refactoring | ||||||
/// purposes but does not interpret its contents. | ||||||
public struct AbsolutePath: CustomStringConvertible, Equatable, Hashable, Sendable { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer not introducing so many fairly generic public types here (AbsolutePath, PackageIdentity, RelativePath, SemanticVersion, SourceControlURL). How much do you prefer this over just strings for these cases? Another alternative would be to namespace them in an enum, but I believe @ahoppen is generally not a fan of that. Also, could we make |
||||||
public private(set) var description: String | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn’t look like you ever modify it so could be
Suggested change
Same for the other wrapper types. |
||||||
|
||||||
public init(_ description: String) { | ||||||
self.description = description | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// 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 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. | ||
/// | ||
/// 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<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 | ||
} | ||
|
||
let newPackageCall = try addPackageDependencyLocal( | ||
dependency, | ||
to: packageCall | ||
) | ||
|
||
return PackageEdit( | ||
manifestEdits: [ | ||
.replace(packageCall, with: newPackageCall.description) | ||
] | ||
) | ||
} | ||
|
||
/// 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", | ||
trailingLabels: Self.argumentLabelsAfterDependencies, | ||
newElement: dependency.asSyntax() | ||
) | ||
} | ||
} |
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.
Since you’re adding new public declarations, could you add a release notes entry?