diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Package.swift b/Fixtures/SwiftMigrate/UpdateManifest/Package.swift new file mode 100644 index 00000000000..fedb00e411e --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Package.swift @@ -0,0 +1,24 @@ +// swift-tools-version:5.8 + +import PackageDescription + +var swiftSettings: [SwiftSetting] = [] + +let package = Package( + name: "WithErrors", + targets: [ + .target( + name: "CannotFindSettings", + swiftSettings: swiftSettings + ), + .target(name: "A"), + .target(name: "B"), + ] +) + +package.targets.append( + .target( + name: "CannotFindTarget", + swiftSettings: swiftSettings + ), +) diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A-B.swift b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A-B.swift new file mode 100644 index 00000000000..420622ed6c6 --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A-B.swift @@ -0,0 +1,28 @@ +// swift-tools-version:5.8 + +import PackageDescription + +var swiftSettings: [SwiftSetting] = [] + +let package = Package( + name: "WithErrors", + targets: [ + .target( + name: "CannotFindSettings", + swiftSettings: swiftSettings + ), + .target(name: "A",swiftSettings: [ + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InferIsolatedConformances"),]), + .target(name: "B",swiftSettings: [ + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InferIsolatedConformances"),]), + ] +) + +package.targets.append( + .target( + name: "CannotFindTarget", + swiftSettings: swiftSettings + ), +) diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A.swift b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A.swift new file mode 100644 index 00000000000..6502c0cbe66 --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A.swift @@ -0,0 +1,26 @@ +// swift-tools-version:5.8 + +import PackageDescription + +var swiftSettings: [SwiftSetting] = [] + +let package = Package( + name: "WithErrors", + targets: [ + .target( + name: "CannotFindSettings", + swiftSettings: swiftSettings + ), + .target(name: "A",swiftSettings: [ + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InferIsolatedConformances"),]), + .target(name: "B"), + ] +) + +package.targets.append( + .target( + name: "CannotFindTarget", + swiftSettings: swiftSettings + ), +) diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-all.swift b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-all.swift new file mode 100644 index 00000000000..420622ed6c6 --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-all.swift @@ -0,0 +1,28 @@ +// swift-tools-version:5.8 + +import PackageDescription + +var swiftSettings: [SwiftSetting] = [] + +let package = Package( + name: "WithErrors", + targets: [ + .target( + name: "CannotFindSettings", + swiftSettings: swiftSettings + ), + .target(name: "A",swiftSettings: [ + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InferIsolatedConformances"),]), + .target(name: "B",swiftSettings: [ + .enableUpcomingFeature("ExistentialAny"), + .enableUpcomingFeature("InferIsolatedConformances"),]), + ] +) + +package.targets.append( + .target( + name: "CannotFindTarget", + swiftSettings: swiftSettings + ), +) diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Sources/A/File.swift b/Fixtures/SwiftMigrate/UpdateManifest/Sources/A/File.swift new file mode 100644 index 00000000000..6d71fe5d9ce --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Sources/A/File.swift @@ -0,0 +1 @@ +public func foo() {} diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Sources/B/File.swift b/Fixtures/SwiftMigrate/UpdateManifest/Sources/B/File.swift new file mode 100644 index 00000000000..6d71fe5d9ce --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Sources/B/File.swift @@ -0,0 +1 @@ +public func foo() {} diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindSettings/File.swift b/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindSettings/File.swift new file mode 100644 index 00000000000..6d71fe5d9ce --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindSettings/File.swift @@ -0,0 +1 @@ +public func foo() {} diff --git a/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindTarget/File.swift b/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindTarget/File.swift new file mode 100644 index 00000000000..6d71fe5d9ce --- /dev/null +++ b/Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindTarget/File.swift @@ -0,0 +1 @@ +public func foo() {} diff --git a/Sources/Commands/PackageCommands/Migrate.swift b/Sources/Commands/PackageCommands/Migrate.swift index eea927051cb..dfa1d3ddd44 100644 --- a/Sources/Commands/PackageCommands/Migrate.swift +++ b/Sources/Commands/PackageCommands/Migrate.swift @@ -23,6 +23,7 @@ import OrderedCollections import PackageGraph import PackageModel +import enum PackageModelSyntax.ManifestEditError import SPMBuildCore import SwiftFixIt @@ -32,7 +33,7 @@ import var TSCBasic.stdoutStream struct MigrateOptions: ParsableArguments { @Option( name: .customLong("target"), - help: "The targets to migrate to specified set of features." + help: "A comma-separated list of targets to migrate. (default: all Swift targets)" ) var _targets: String? @@ -42,7 +43,7 @@ struct MigrateOptions: ParsableArguments { @Option( name: .customLong("to-feature"), - help: "The Swift language upcoming/experimental feature to migrate to." + help: "A comma-separated list of Swift language features to migrate to." ) var _features: String @@ -64,31 +65,8 @@ extension SwiftPackageCommand { var options: MigrateOptions public func run(_ swiftCommandState: SwiftCommandState) async throws { - let toolchain = try swiftCommandState.productsBuildParameters.toolchain - - let supportedFeatures = try Dictionary( - uniqueKeysWithValues: toolchain.swiftCompilerSupportedFeatures - .map { ($0.name, $0) } - ) - - // First, let's validate that all of the features are supported - // by the compiler and are migratable. - - var features: [SwiftCompilerFeature] = [] - for name in self.options.features { - guard let feature = supportedFeatures[name] else { - let migratableFeatures = supportedFeatures.map(\.value).filter(\.migratable).map(\.name) - throw ValidationError( - "Unsupported feature: \(name). Available features: \(migratableFeatures.joined(separator: ", "))" - ) - } - - guard feature.migratable else { - throw ValidationError("Feature '\(name)' is not migratable") - } - - features.append(feature) - } + // First, validate and resolve the requested feature names. + let features = try self.resolveRequestedFeatures(swiftCommandState) let targets = self.options.targets @@ -176,9 +154,12 @@ extension SwiftPackageCommand { // Once the fix-its were applied, it's time to update the // manifest with newly adopted feature settings. + // + // Loop over a sorted array to produce deterministic results and + // order of diagnostics. print("> Updating manifest") - for (name, _) in modules { + for name in modules.keys.sorted() { swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(name)'") try self.updateManifest( for: name, @@ -188,6 +169,48 @@ extension SwiftPackageCommand { } } + /// Resolves the requested feature names. + /// + /// - Returns: An array of resolved features, sorted by name. + private func resolveRequestedFeatures( + _ swiftCommandState: SwiftCommandState + ) throws -> [SwiftCompilerFeature] { + let toolchain = try swiftCommandState.productsBuildParameters.toolchain + + // Query the compiler for supported features. + let supportedFeatures = try toolchain.swiftCompilerSupportedFeatures + + var resolvedFeatures: [SwiftCompilerFeature] = [] + + // Resolve the requested feature names, validating that they are + // supported by the compiler and migratable. + for name in self.options.features { + let feature = supportedFeatures.first { $0.name == name } + + guard let feature else { + let migratableCommaSeparatedFeatures = supportedFeatures + .filter(\.migratable) + .map(\.name) + .sorted() + .joined(separator: ", ") + + throw ValidationError( + "Unsupported feature '\(name)'. Available features: \(migratableCommaSeparatedFeatures)" + ) + } + + guard feature.migratable else { + throw ValidationError("Feature '\(name)' is not migratable") + } + + resolvedFeatures.append(feature) + } + + return resolvedFeatures.sorted { lhs, rhs in + lhs.name < rhs.name + } + } + private func createBuildSystem( _ swiftCommandState: SwiftCommandState, targets: OrderedSet, @@ -251,15 +274,37 @@ extension SwiftPackageCommand { verbose: !self.globalOptions.logging.quiet ) } catch { - try swiftCommandState.observabilityScope.emit( - error: """ - Could not update manifest for '\(target)' (\(error)). \ - Please enable '\( - features.map { try $0.swiftSettingDescription } - .joined(separator: ", ") - )' features manually - """ - ) + var message = + "Could not update manifest to enable requested features for target '\(target)' (\(error))" + + // Do not suggest manual addition if something else is wrong or + // if the error implies that it cannot be done. + if let error = error as? ManifestEditError { + switch error { + case .cannotFindPackage, + .cannotAddSettingsToPluginTarget, + .existingDependency: + break + case .cannotFindArrayLiteralArgument, + // This means the target could not be found + // syntactically, not that it does not exist. + .cannotFindTargets, + .cannotFindTarget, + // This means the swift-tools-version is lower than + // the version where one of the setting was introduced. + .oldManifest: + let settings = try features.map { + try $0.swiftSettingDescription + }.joined(separator: ", ") + + message += """ + . Please enable them manually by adding the following Swift settings to the target: \ + '\(settings)' + """ + } + } + + swiftCommandState.observabilityScope.emit(error: message) } } diff --git a/Tests/CommandsTests/PackageCommandTests.swift b/Tests/CommandsTests/PackageCommandTests.swift index ad46b119344..ef5ec4223a6 100644 --- a/Tests/CommandsTests/PackageCommandTests.swift +++ b/Tests/CommandsTests/PackageCommandTests.swift @@ -2115,6 +2115,49 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase { XCTAssertNoMatch(stdout, .contains("--package-path")) } + func testMigrateCommandNoFeatures() async throws { + try await XCTAssertThrowsCommandExecutionError( + await self.execute(["migrate"]) + ) { error in + XCTAssertMatch( + error.stderr, + .contains("error: Missing expected argument '--to-feature '") + ) + } + } + + func testMigrateCommandUnknownFeature() async throws { + try XCTSkipIf( + !UserToolchain.default.supportesSupportedFeatures, + "skipping because test environment compiler doesn't support `-print-supported-features`" + ) + + try await XCTAssertThrowsCommandExecutionError( + await self.execute(["migrate", "--to-feature", "X"]) + ) { error in + XCTAssertMatch( + error.stderr, + .contains("error: Unsupported feature 'X'. Available features:") + ) + } + } + + func testMigrateCommandNonMigratableFeature() async throws { + try XCTSkipIf( + !UserToolchain.default.supportesSupportedFeatures, + "skipping because test environment compiler doesn't support `-print-supported-features`" + ) + + try await XCTAssertThrowsCommandExecutionError( + await self.execute(["migrate", "--to-feature", "StrictConcurrency"]) + ) { error in + XCTAssertMatch( + error.stderr, + .contains("error: Feature 'StrictConcurrency' is not migratable") + ) + } + } + func testMigrateCommand() async throws { try XCTSkipIf( !UserToolchain.default.supportesSupportedFeatures, @@ -2205,6 +2248,100 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase { } } + func testMigrateCommandUpdateManifestSingleTarget() async throws { + try XCTSkipIf( + !UserToolchain.default.supportesSupportedFeatures, + "skipping because test environment compiler doesn't support `-print-supported-features`" + ) + + try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in + _ = try await self.execute( + [ + "migrate", + "--to-feature", + "ExistentialAny,InferIsolatedConformances", + "--target", + "A", + ], + packagePath: fixturePath + ) + + let updatedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.swift") + ) + let expectedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.updated.targets-A.swift") + ) + XCTAssertEqual(updatedManifest, expectedManifest) + } + + } + + func testMigrateCommandUpdateManifest2Targets() async throws { + try XCTSkipIf( + !UserToolchain.default.supportesSupportedFeatures, + "skipping because test environment compiler doesn't support `-print-supported-features`" + ) + + try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in + _ = try await self.execute( + [ + "migrate", + "--to-feature", + "ExistentialAny,InferIsolatedConformances", + "--target", + "A,B", + ], + packagePath: fixturePath + ) + + let updatedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.swift") + ) + let expectedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.updated.targets-A-B.swift") + ) + XCTAssertEqual(updatedManifest, expectedManifest) + } + } + + func testMigrateCommandUpdateManifestWithErrors() async throws { + try XCTSkipIf( + !UserToolchain.default.supportesSupportedFeatures, + "skipping because test environment compiler doesn't support `-print-supported-features`" + ) + + try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in + try await XCTAssertThrowsCommandExecutionError( + await self.execute( + ["migrate", "--to-feature", "ExistentialAny,InferIsolatedConformances,StrictMemorySafety"], + packagePath: fixturePath + ) + ) { error in + // 'SwiftMemorySafety.strictMemorySafety' was introduced in 6.2. + XCTAssertMatch( + error.stderr, + .contains( + """ + error: Could not update manifest to enable requested features for target 'A' (package manifest version 5.8.0 is too old: please update to manifest version 6.2.0 or newer). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()' + error: Could not update manifest to enable requested features for target 'B' (package manifest version 5.8.0 is too old: please update to manifest version 6.2.0 or newer). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()' + error: Could not update manifest to enable requested features for target 'CannotFindSettings' (unable to find array literal for 'swiftSettings' argument). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()' + error: Could not update manifest to enable requested features for target 'CannotFindTarget' (unable to find target named 'CannotFindTarget' in package). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()' + """ + ) + ) + } + + let updatedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.swift") + ) + let expectedManifest = try localFileSystem.readFileContents( + fixturePath.appending(components: "Package.updated.targets-all.swift") + ) + XCTAssertEqual(updatedManifest, expectedManifest) + } + } + func testBuildToolPlugin() async throws { try await testBuildToolPlugin(staticStdlib: false) } @@ -4163,6 +4300,18 @@ class PackageCommandSwiftBuildTests: PackageCommandTestCase { throw XCTSkip("SWBINTTODO: Build plan is not currently supported") } + override func testMigrateCommandUpdateManifestSingleTarget() async throws { + throw XCTSkip("SWBINTTODO: Build plan is not currently supported") + } + + override func testMigrateCommandUpdateManifest2Targets() async throws { + throw XCTSkip("SWBINTTODO: Build plan is not currently supported") + } + + override func testMigrateCommandUpdateManifestWithErrors() async throws { + throw XCTSkip("SWBINTTODO: Build plan is not currently supported") + } + override func testCommandPluginTestingCallbacks() async throws { throw XCTSkip("SWBINTTODO: Requires PIF generation to adopt new test runner product type") try XCTSkipOnWindows(because: "TSCBasic/Path.swift:969: Assertion failed, https://github.com/swiftlang/swift-package-manager/issues/8602")