Remove implicit availability of Foundation from future manifest versions

neonichu · neonichu · commit 202c4db51bb9 · 2022-11-16T08:43:59.000-08:00
This changes all imports of `Foundation` in the `PackageDescription` module to be implementation-only, making it so that it needs to be explicitly imported if being used. To avoid breaking existing packages, we inject an explicit import into manifests with tools-version 5.7 or earlier. In the next tools-version, `Foundation` will no longer be implicitly available.

rdar://92879696
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@ Note: This is in reverse chronological order, so newer entries are added to the
 Swift 5.8
 -----------
 
+* [#5874]
+
+  In packages using tools version 5.8 or later, Foundation is no longer implicitly imported into package manifests. If Foundation APIs are used, the module needs to be imported explicitly.
+
 * [#5728]
 
   In packages that specify resources using tools version 5.8 or later, the generated resource bundle accessor will import `Foundation.Bundle` for its own implementation only. _Clients_ of such packages therefore no longer silently import `Foundation`, preventing inadvertent use of Foundation extensions to standard library APIs, which helps to avoid unexpected code size increases.
diff --git a/Sources/PackageDescription/PackageDependency.swift b/Sources/PackageDescription/PackageDependency.swift
@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
+@_implementationOnly import Foundation
 
 // MARK: - file system
 
diff --git a/Sources/PackageDescription/PackageDescription.swift b/Sources/PackageDescription/PackageDescription.swift
@@ -18,7 +18,7 @@
 @_implementationOnly import ucrt
 @_implementationOnly import struct WinSDK.HANDLE
 #endif
-import Foundation
+@_implementationOnly import Foundation
 
 /// The configuration of a Swift package.
 ///
diff --git a/Sources/PackageDescription/PackageDescriptionSerialization.swift b/Sources/PackageDescription/PackageDescriptionSerialization.swift
@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
+@_implementationOnly import Foundation
 #if canImport(Glibc)
 @_implementationOnly import Glibc
 #elseif canImport(Darwin)
diff --git a/Sources/PackageDescription/Target.swift b/Sources/PackageDescription/Target.swift
@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
+@_implementationOnly import Foundation
 
 /// The basic building block of a Swift package.
 ///
diff --git a/Sources/PackageLoading/CMakeLists.txt b/Sources/PackageLoading/CMakeLists.txt
@@ -9,7 +9,6 @@
 add_library(PackageLoading
   ContextModel.swift
   Diagnostics.swift
-  IdentityResolver.swift
   ManifestLoader.swift
   ManifestLoader+Validation.swift
   ModuleMapGenerator.swift
diff --git a/Sources/PackageLoading/ContextModel.swift b/Sources/PackageLoading/ContextModel.swift
@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
+@_implementationOnly import Foundation
 
 struct ContextModel {
     let packageDirectory : String
diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 
diff --git a/Sources/PackageLoading/ManifestLoader+Validation.swift b/Sources/PackageLoading/ManifestLoader+Validation.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 
diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift
@@ -11,7 +11,8 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+import Dispatch
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 import enum TSCUtility.Diagnostics
@@ -431,10 +432,18 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         callbackQueue: DispatchQueue,
         completion: @escaping (Result<EvaluationResult, Error>) -> Void
     ) throws {
+        let manifestPreamble: ByteString
+        switch toolsVersion {
+        case .vNext:
+            manifestPreamble = ByteString()
+        default:
+            manifestPreamble = ByteString("import Foundation\n")
+        }
+
         do {
             try withTemporaryDirectory { tempDir, cleanupTempDir in
                 let manifestTempFilePath = tempDir.appending(component: "manifest.swift")
-                try localFileSystem.writeFileContents(manifestTempFilePath, bytes: ByteString(manifestContents))
+                try localFileSystem.writeFileContents(manifestTempFilePath, bytes: ByteString(manifestPreamble.contents + manifestContents))
 
                 let vfsOverlayTempFilePath = tempDir.appending(component: "vfs.yaml")
                 try VFSOverlay(roots: [
diff --git a/Sources/PackageLoading/ModuleMapGenerator.swift b/Sources/PackageLoading/ModuleMapGenerator.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 
diff --git a/Sources/PackageLoading/PkgConfig.swift b/Sources/PackageLoading/PkgConfig.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import TSCBasic
 
 /// Information on an individual `pkg-config` supported package.
diff --git a/Sources/PackageLoading/TargetSourcesBuilder.swift b/Sources/PackageLoading/TargetSourcesBuilder.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 
diff --git a/Sources/PackageLoading/ToolsVersionParser.swift b/Sources/PackageLoading/ToolsVersionParser.swift
@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
-import Foundation
+@_implementationOnly import Foundation
 import PackageModel
 import TSCBasic
 
diff --git a/Sources/PackageModel/CMakeLists.txt b/Sources/PackageModel/CMakeLists.txt
@@ -13,6 +13,7 @@ add_library(PackageModel
   BuildSettings.swift
   Destination.swift
   Diagnostics.swift
+  IdentityResolver.swift
   Manifest.swift
   Manifest/PackageConditionDescription.swift
   Manifest/PackageDependencyDescription.swift
diff --git a/Sources/PackageModel/IdentityResolver.swift b/Sources/PackageModel/IdentityResolver.swift
@@ -11,7 +11,6 @@
 //===----------------------------------------------------------------------===//
 
 import Foundation
-import PackageModel
 import TSCBasic
 
 // TODO: refactor this when adding registry support
diff --git a/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift b/Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift
@@ -22,6 +22,22 @@ class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests {
         .v5_7
     }
 
+    func testImplicitFoundationImportWorks() throws {
+        let content = """
+            import PackageDescription
+
+            _ = FileManager.default
+
+            let package = Package(name: "MyPackage")
+            """
+
+        let observability = ObservabilitySystem.makeForTesting()
+        let (manifest, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope)
+        XCTAssertNoDiagnostics(observability.diagnostics)
+        XCTAssertNoDiagnostics(validationDiagnostics)
+        XCTAssertEqual(manifest.displayName, "MyPackage")
+    }
+
     func testRegistryDependencies() throws {
         let content = """
             import PackageDescription
diff --git a/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift b/Tests/PackageLoadingTests/PD_Next_LoadingTests.swift
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Basics
+import PackageLoading
 import PackageModel
 import SPMTestSupport
 import XCTest
@@ -19,4 +20,23 @@ class PackageDescriptionNextLoadingTests: PackageDescriptionLoadingTests {
     override var toolsVersion: ToolsVersion {
         .vNext
     }
+
+    func testImplicitFoundationImportFails() throws {
+        let content = """
+            import PackageDescription
+
+            _ = FileManager.default
+
+            let package = Package(name: "MyPackage")
+            """
+
+        let observability = ObservabilitySystem.makeForTesting()
+        XCTAssertThrowsError(try loadAndValidateManifest(content, observabilityScope: observability.topScope), "expected error") {
+            if case ManifestParseError.invalidManifestFormat(let error, _) = $0 {
+                XCTAssertMatch(error, .contains("cannot find 'FileManager' in scope"))
+            } else {
+                XCTFail("unexpected error: \($0)")
+            }
+        }
+    }
 }
diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift
@@ -215,7 +215,7 @@ final class WorkspaceTests: XCTestCase {
             XCTAssert(rootManifests.count == 0, "\(rootManifests)")
 
             testDiagnostics(observability.diagnostics) { result in
-                let diagnostic = result.check(diagnostic: .prefix("Invalid manifest\n\(path.appending(components: "MyPkg", "Package.swift")):3:8: error: An error in MyPkg"), severity: .error)
+                let diagnostic = result.check(diagnostic: .prefix("Invalid manifest\n\(path.appending(components: "MyPkg", "Package.swift")):4:8: error: An error in MyPkg"), severity: .error)
                 XCTAssertEqual(diagnostic?.metadata?.packageIdentity, .init(path: pkgDir))
                 XCTAssertEqual(diagnostic?.metadata?.packageKind, .root(pkgDir))
             }
