Skip to content

Commit adc2535

Browse files
Move files when using package add-target on single target package (#8413)
### Motivation: Currently running `swift package init --type executable` in a folder called `Example` creates a project with the following structure: ``` ./Example/Sources ./Example/Sources/main.swift ./Example/Package.swift ``` Following this up with a `swift package add-target Foo` produces a package that no longer builds. It now has the structure: ``` ./Example/Sources ./Example/Sources/main.swift ./Example/Sources/Foo/Foo.swift ./Example/Package.swift ``` Packages with multiple targets cannot have code directly in `./Sources`, and the user gets the error: `Source files for target Example should be located under 'Sources/Example'` ### Modifications: To work around this in the AddTarget command we can check if a package is structured as a single target package with sources directly in `./Sources` and move these files into a new folder located at `./Sources/Example`. ### Result: This allows the project to build after the new target has been added. Issue: #8410 --------- Co-authored-by: Max Desiatov <[email protected]>
1 parent 8e3e11c commit adc2535

File tree

4 files changed

+261
-1
lines changed

4 files changed

+261
-1
lines changed

Sources/Commands/PackageCommands/AddTarget.swift

+8
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ extension SwiftPackageCommand {
8989
}
9090
}
9191

92+
// Move sources into their own folder if they're directly in `./Sources`.
93+
try PackageModelSyntax.AddTarget.moveSingleTargetSources(
94+
packagePath: packagePath,
95+
manifest: manifestSyntax,
96+
fileSystem: fileSystem,
97+
verbose: !globalOptions.logging.quiet
98+
)
99+
92100
// Map the target type.
93101
let type: TargetDescription.TargetKind = switch self.type {
94102
case .library: .regular

Sources/PackageModelSyntax/AddTarget.swift

+65
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,71 @@ public enum AddTarget {
5757
}
5858
}
5959

60+
// Check if the package has a single target with that target's sources located
61+
// directly in `./Sources`. If so, move the sources into a folder named after
62+
// the target before adding a new target.
63+
package static func moveSingleTargetSources(
64+
packagePath: AbsolutePath,
65+
manifest: SourceFileSyntax,
66+
fileSystem: any FileSystem,
67+
verbose: Bool = false
68+
) throws {
69+
// Make sure we have a suitable tools version in the manifest.
70+
try manifest.checkEditManifestToolsVersion()
71+
72+
guard let packageCall = manifest.findCall(calleeName: "Package") else {
73+
throw ManifestEditError.cannotFindPackage
74+
}
75+
76+
if let arg = packageCall.findArgument(labeled: "targets") {
77+
guard let argArray = arg.expression.findArrayArgument() else {
78+
throw ManifestEditError.cannotFindArrayLiteralArgument(
79+
argumentName: "targets",
80+
node: Syntax(arg.expression)
81+
)
82+
}
83+
84+
// Check the contents of the `targets` array to see if there is only one target defined.
85+
guard argArray.elements.count == 1,
86+
let firstTarget = argArray.elements.first?.expression.as(FunctionCallExprSyntax.self),
87+
let targetStringLiteral = firstTarget.arguments.first?.expression.as(StringLiteralExprSyntax.self) else {
88+
return
89+
}
90+
91+
let targetName = targetStringLiteral.segments.description
92+
let sourcesFolder = packagePath.appending("Sources")
93+
let expectedTargetFolder = sourcesFolder.appending(targetName)
94+
95+
// If there is one target then pull its name out and use that to look for a folder in `Sources/TargetName`.
96+
// If the folder doesn't exist then we know we have a single target package and we need to migrate files
97+
// into this folder before we can add a new target.
98+
if !fileSystem.isDirectory(expectedTargetFolder) {
99+
if verbose {
100+
print(
101+
"""
102+
Moving existing files from \(
103+
sourcesFolder.relative(to: packagePath)
104+
) to \(
105+
expectedTargetFolder.relative(to: packagePath)
106+
)...
107+
""",
108+
terminator: ""
109+
)
110+
}
111+
let contentsToMove = try fileSystem.getDirectoryContents(sourcesFolder)
112+
try fileSystem.createDirectory(expectedTargetFolder)
113+
for file in contentsToMove {
114+
let source = sourcesFolder.appending(file)
115+
let destination = expectedTargetFolder.appending(file)
116+
try fileSystem.move(from: source, to: destination)
117+
}
118+
if verbose {
119+
print(" done.")
120+
}
121+
}
122+
}
123+
}
124+
60125
/// Add the given target to the manifest, producing a set of edit results
61126
/// that updates the manifest and adds some source files to stub out the
62127
/// new target.

Tests/CommandsTests/PackageCommandTests.swift

+52-1
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,6 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
11201120
}
11211121
}
11221122

1123-
11241123
func testPackageAddTarget() async throws {
11251124
try await testWithTemporaryDirectory { tmpPath in
11261125
let fs = localFileSystem
@@ -1152,6 +1151,58 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
11521151
}
11531152
}
11541153

1154+
func testPackageAddTargetWithoutModuleSourcesFolder() async throws {
1155+
try await testWithTemporaryDirectory { tmpPath in
1156+
let fs = localFileSystem
1157+
let manifest = tmpPath.appending("Package.swift")
1158+
try fs.writeFileContents(manifest, string:
1159+
"""
1160+
// swift-tools-version: 5.9
1161+
import PackageDescription
1162+
let package = Package(
1163+
name: "SimpleExecutable",
1164+
targets: [
1165+
.executableTarget(name: "SimpleExecutable"),
1166+
]
1167+
)
1168+
"""
1169+
)
1170+
1171+
let sourcesFolder = tmpPath.appending("Sources")
1172+
try fs.createDirectory(sourcesFolder)
1173+
1174+
try fs.writeFileContents(sourcesFolder.appending("main.swift"), string:
1175+
"""
1176+
print("Hello World")
1177+
"""
1178+
)
1179+
1180+
_ = try await execute(["add-target", "client"], packagePath: tmpPath)
1181+
1182+
XCTAssertFileExists(manifest)
1183+
let contents: String = try fs.readFileContents(manifest)
1184+
1185+
XCTAssertMatch(contents, .contains(#"targets:"#))
1186+
XCTAssertMatch(contents, .contains(#".executableTarget"#))
1187+
XCTAssertMatch(contents, .contains(#"name: "client""#))
1188+
1189+
let fileStructure = try fs.getDirectoryContents(sourcesFolder)
1190+
XCTAssertEqual(fileStructure, ["SimpleExecutable", "client"])
1191+
XCTAssertTrue(fs.isDirectory(sourcesFolder.appending("SimpleExecutable")))
1192+
XCTAssertTrue(fs.isDirectory(sourcesFolder.appending("client")))
1193+
XCTAssertEqual(try fs.getDirectoryContents(sourcesFolder.appending("SimpleExecutable")), ["main.swift"])
1194+
XCTAssertEqual(try fs.getDirectoryContents(sourcesFolder.appending("client")), ["client.swift"])
1195+
}
1196+
}
1197+
1198+
func testAddTargetWithoutManifestThrows() async throws {
1199+
try await testWithTemporaryDirectory { tmpPath in
1200+
await XCTAssertThrowsCommandExecutionError(try await execute(["add-target", "client"], packagePath: tmpPath)) { error in
1201+
XCTAssertMatch(error.stderr, .contains("error: Could not find Package.swift in this directory or any of its parent directories."))
1202+
}
1203+
}
1204+
}
1205+
11551206
func testPackageAddTargetDependency() async throws {
11561207
try await testWithTemporaryDirectory { tmpPath in
11571208
let fs = localFileSystem

Tests/PackageModelSyntaxTests/ManifestEditTests.swift

+136
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,142 @@ class ManifestEditTests: XCTestCase {
615615
}
616616
}
617617
}
618+
/// Assert that applying the moveSingleTargetSources operation to the manifest
619+
/// produces the expected file system.
620+
func assertFileSystemRefactor(
621+
_ manifest: String,
622+
inputFileLayout: [AbsolutePath] = [],
623+
expectedFileLayout: [AbsolutePath] = [],
624+
file: StaticString = #filePath,
625+
line: UInt = #line
626+
) throws {
627+
let mockFileSystem = InMemoryFileSystem();
628+
for path in inputFileLayout {
629+
try mockFileSystem.writeFileContents(path, string: "print(\"Hello, world!\")")
630+
}
631+
632+
try AddTarget.moveSingleTargetSources(
633+
packagePath: AbsolutePath("/"),
634+
manifest: Parser.parse(source: manifest),
635+
fileSystem: mockFileSystem
636+
)
637+
638+
for path in expectedFileLayout {
639+
XCTAssertTrue(mockFileSystem.exists(path))
640+
}
641+
642+
let unexpectedFiles = inputFileLayout.filter { !expectedFileLayout.contains($0) }
643+
for path in unexpectedFiles {
644+
XCTAssertFalse(mockFileSystem.exists(path))
645+
}
646+
}
647+
648+
class SingleTargetSourceTests: XCTestCase {
649+
func testMoveSingleTargetSources() throws {
650+
try assertFileSystemRefactor(
651+
"""
652+
// swift-tools-version: 5.5
653+
let package = Package(
654+
name: "packages",
655+
targets: [
656+
.executableTarget(name: "Foo"),
657+
]
658+
)
659+
""",
660+
inputFileLayout: [AbsolutePath("/Sources/Foo.swift")],
661+
expectedFileLayout: [AbsolutePath("/Sources/Foo/Foo.swift")]
662+
)
663+
}
664+
665+
func testMoveSingleTargetSourcesNoTargets() throws {
666+
try assertFileSystemRefactor(
667+
"""
668+
// swift-tools-version: 5.5
669+
let package = Package(
670+
name: "packages"
671+
)
672+
""",
673+
inputFileLayout: [AbsolutePath("/Sources/Foo.swift")],
674+
expectedFileLayout: [AbsolutePath("/Sources/Foo.swift")]
675+
)
676+
}
677+
678+
func testMoveSingleTargetSourcesAlreadyOrganized() throws {
679+
try assertFileSystemRefactor(
680+
"""
681+
// swift-tools-version: 5.5
682+
let package = Package(
683+
name: "packages",
684+
targets: [
685+
.executableTarget(name: "Foo"),
686+
]
687+
)
688+
""",
689+
inputFileLayout: [AbsolutePath("/Sources/Foo/Foo.swift")],
690+
expectedFileLayout: [AbsolutePath("/Sources/Foo/Foo.swift")]
691+
)
692+
}
693+
694+
func testMoveSingleTargetSourcesInvalidManifestTargets() throws {
695+
XCTAssertThrowsError(
696+
try assertFileSystemRefactor(
697+
"""
698+
// swift-tools-version: 5.5
699+
let package = Package(
700+
name: "packages",
701+
targets: "invalid"
702+
)
703+
"""
704+
)
705+
) { error in
706+
XCTAssertTrue(error is ManifestEditError)
707+
}
708+
}
709+
710+
func testMoveSingleTargetSourcesInvalidManifestToolsVersion() throws {
711+
XCTAssertThrowsError(
712+
try assertFileSystemRefactor(
713+
"""
714+
// swift-tools-version: 5.4
715+
let package = Package(
716+
name: "packages",
717+
targets: []
718+
)
719+
"""
720+
)
721+
) { error in
722+
XCTAssertTrue(error is ManifestEditError)
723+
}
724+
}
725+
726+
func testMoveSingleTargetSourcesInvalidManifestTarget() throws {
727+
XCTAssertThrowsError(
728+
try assertFileSystemRefactor(
729+
"""
730+
// swift-tools-version: 5.4
731+
let package = Package(
732+
name: "packages",
733+
targets: [.executableTarget(123)]
734+
)
735+
"""
736+
)
737+
) { error in
738+
XCTAssertTrue(error is ManifestEditError)
739+
}
740+
}
741+
742+
func testMoveSingleTargetSourcesInvalidManifestPackage() throws {
743+
XCTAssertThrowsError(
744+
try assertFileSystemRefactor(
745+
"""
746+
// swift-tools-version: 5.5
747+
"""
748+
)
749+
) { error in
750+
XCTAssertTrue(error is ManifestEditError)
751+
}
752+
}
753+
}
618754

619755

620756
// FIXME: Copy-paste from _SwiftSyntaxTestSupport

0 commit comments

Comments
 (0)