[PackageModel] Add modules and products to Package.

 - Now that all of the individual package loading is centralized, coalesce it
   into a single method (not yet an initializer) within PackageLoading. This
   should be the sole place responsible for parsing all of the conventions and
   creating the model for an individual package.

Author: ddunbar

Sources/PackageGraph/PackageGraphLoader.swift

@@ -67,21 +67,25 @@ public struct PackageGraphLoader {
         let allManifests = externalManifests + [rootManifest]
 
         // Create the packages and convert to modules.
-        //
-        // FIXME: This needs to be torn about, the module conversion should be
-        // done on an individual package basis.
         var packages: [Package] = []
         var products: [Product] = []
         var map: [Package: [Module]] = [:]
         for (i, manifest) in allManifests.enumerated() {
-            let package = Package(manifest: manifest)
             let isRootPackage = (i + 1) == allManifests.count
-            packages.append(package)
 
-            let modules: [Module] = try package.modules()
+            // Create a package from the manifest and sources.
+            //
+            // FIXME: We should always load the tests, but just change which
+            // tests we build based on higher-level logic. This would make it
+            // easier to allow testing of external package tests.
+            let package = try Package.createUsingConventions(manifest: manifest, includingTestModules: isRootPackage)
+            packages.append(package)
+            
+            products += package.products
+            map[package] = package.modules + package.testModules
 
             // Diagnose empty non-root packages, which are something we allow as a special case.
-            if modules.isEmpty {
+            if package.modules.isEmpty {
                 if isRootPackage {
                     // Ignore and print warning if root package doesn't contain any sources.
                     print("warning: root package '\(package)' does not contain any sources")
@@ -94,17 +98,6 @@ public struct PackageGraphLoader {
                     throw PackageGraphError.noModules(package)
                 }
             }
-    
-            // TODO: Allow testing of external package tests.
-            var testModules: [Module]
-            if isRootPackage {
-                testModules = try package.testModules(modules: modules)
-            } else {
-                testModules = []
-            }
-    
-            products += try package.products(modules, testModules: testModules)
-            map[package] = modules + testModules
         }
 
         // Load all of the package dependencies.

Sources/PackageLoading/PackageExtensions.swift

@@ -148,9 +148,7 @@ extension Package {
     }
 
     /// Collects the modules which are defined by a package.
-    //
-    // FIXME: This should not be public.
-    public func modules() throws -> [Module] {
+    fileprivate func modules() throws -> [Module] {
         let moduleMapPath = path.appending("module.modulemap")
         if moduleMapPath.asString.isFile {
             let sources = Sources(paths: [moduleMapPath], root: path)
@@ -273,9 +271,7 @@ extension Package {
 
 extension Package {
     /// Collects the products defined by a package.
-    //
-    // FIXME: This should not be public.
-    public func products(_ modules: [Module], testModules: [Module]) throws -> [Product] {
+    fileprivate func products(_ modules: [Module], testModules: [Module]) throws -> [Product] {
         var products = [Product]()
 
     ////// first auto-determine executables
@@ -374,8 +370,7 @@ extension Package {
 }
 
 extension Package {
-    // FIXME: This should not be public.
-    public func testModules(modules: [Module]) throws -> [Module] {
+    fileprivate func testModules(modules: [Module]) throws -> [Module] {
         let testsPath = self.path.appending("Tests")
 
         // Don't try to walk Tests if it is in excludes.
@@ -436,3 +431,24 @@ extension Package {
         return testModules
     }
 }
+
+extension Package {
+    /// Load the package for the given manifest.
+    ///
+    /// - Parameters:
+    ///   - includingTestModules: Whether the package's test modules should be loaded.
+    //
+    // FIXME: Rearrange this to be immutable and move to using an initializer if
+    // it makes sense.
+    public static func createUsingConventions(manifest: Manifest, includingTestModules: Bool) throws -> Package {
+        let package = Package(manifest: manifest)
+        package.modules = try package.modules()
+        if includingTestModules {
+            package.testModules = try package.testModules(modules: package.modules)
+        } else {
+            package.testModules = []
+        }
+        package.products = try package.products(package.modules, testModules: package.testModules)
+        return package
+    }
+}

Sources/PackageModel/Package.swift

@@ -45,7 +45,9 @@ public final class Package {
     public let manifest: Manifest
 
     /// The name of the package.
-    public let name: String
+    public var name: String {
+        return manifest.package.name
+    }        
 
     /// The URL the package was loaded from.
     //
@@ -68,14 +70,30 @@ public final class Package {
         return manifest.version
     }
 
+    /// The modules contained in the package.
+    //
+    // FIXME: Move to an immutable model.
+    public var modules: [Module] = []
+
+    /// The test modules contained in the package.
+    //
+    // FIXME: Move to an immutable model. Also, these should potentially just be
+    // merged with the regular modules.
+    public var testModules: [Module] = []
+
+    /// The products produced by the package.
+    //
+    // FIXME: Move to an immutable model. Also, these should potentially just be
+    // merged with the regular modules.
+    public var products: [Product] = []
+
     /// The resolved dependencies of the package.
     ///
     /// This value is only available once package loading is complete.
     public var dependencies: [Package] = []
 
     public init(manifest: Manifest) {
         self.manifest = manifest
-        self.name = manifest.package.name
     }
 
     public enum Error: Swift.Error {

Tests/PackageLoading/ConventionTests.swift

@@ -90,8 +90,7 @@ private func fixture(files: [RelativePath], body: @noescape (AbsolutePath) throw
 private func fixture(files: [RelativePath], file: StaticString = #file, line: UInt = #line, body: @noescape (PackageModel.Package, [Module]) throws -> ()) throws {
     fixture(files: files) { (prefix: AbsolutePath) in
         let manifest = Manifest(path: prefix.appending("Package.swift"), url: prefix.asString, package: Package(name: "name"), products: [], version: nil)
-        let package = Package(manifest: manifest)
-        let modules = try package.modules()
-        try body(package, modules)
+        let package = try Package.createUsingConventions(manifest: manifest, includingTestModules: false)
+        try body(package, package.modules)
     }
 }

Tests/PackageLoading/ManifestTests.swift

@@ -102,10 +102,7 @@ class ManifestTests: XCTestCase {
         fixture(name: "Miscellaneous/PackageWithInvalidTargets") { (prefix: AbsolutePath) in
             do {
                 let manifest = try ManifestLoader(resources: Resources()).load(path: prefix.appending("Package.swift"), baseURL: prefix.asString, version: nil)
-                let package = Package(manifest: manifest)
-
-                let _ = try package.modules()
-
+                _ = try Package.createUsingConventions(manifest: manifest, includingTestModules: false)
             } catch ModuleError.modulesNotFound(let moduleNames) {
                 XCTAssertEqual(Set(moduleNames), Set(["Bake", "Fake"]))
             } catch {