Move the instantiation of PackageBuilder into the diagnostics.with(location:) closure that sets the package location.

Without this, errors thrown for a package would be associated with a location, but regularly emitted diagnostics would not.  The reason is that when the PackageBuilder is instantiated outside of the `diagnostics.with(location:)`, it keeps the unlocationed diagnostics engine as a property, and then uses that for all the diagnostics even though the `construct()` call happens inside of a with-location closure.

This change allows all the diagnostics emitted by the PackageBuilder to be properly associated with the package.

Author: abertelrud

Sources/PackageGraph/PackageGraphLoader.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Copyright (c) 2014 - 2020 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
@@ -189,23 +189,22 @@ public struct PackageGraphLoader {
             // FIXME: Lift this out of the manifest.
             let packagePath = manifest.path.parentDirectory
 
-            // Create a package from the manifest and sources.
-            let builder = PackageBuilder(
-                manifest: manifest,
-                productFilter: node.productFilter,
-                path: packagePath,
-                additionalFileRules: additionalFileRules,
-                remoteArtifacts: remoteArtifacts,
-                xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets,
-                fileSystem: fileSystem,
-                diagnostics: diagnostics,
-                shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
-                createREPLProduct: manifest.packageKind == .root ? createREPLProduct : false
-            )
-
             let packageLocation = PackageLocation.Local(name: manifest.name, packagePath: packagePath)
             diagnostics.with(location: packageLocation) { diagnostics in
                 diagnostics.wrap {
+                    // Create a package from the manifest and sources.
+                    let builder = PackageBuilder(
+                        manifest: manifest,
+                        productFilter: node.productFilter,
+                        path: packagePath,
+                        additionalFileRules: additionalFileRules,
+                        remoteArtifacts: remoteArtifacts,
+                        xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets,
+                        fileSystem: fileSystem,
+                        diagnostics: diagnostics,
+                        shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
+                        createREPLProduct: manifest.packageKind == .root ? createREPLProduct : false
+                    )
                     let package = try builder.construct()
                     manifestToPackage[manifest] = package
 

Tests/PackageGraphTests/PackageGraphTests.swift

@@ -529,7 +529,7 @@ class PackageGraphTests: XCTestCase {
         )
 
         DiagnosticsEngineTester(diagnostics) { result in
-            result.check(diagnostic: "Source files for target Bar should be located under /Bar/Sources/Bar", behavior: .warning)
+            result.check(diagnostic: "Source files for target Bar should be located under /Bar/Sources/Bar", behavior: .warning, location: "'Bar' /Bar")
             result.check(diagnostic: "target 'Bar' referenced in product 'Bar' is empty", behavior: .error, location: "'Bar' /Bar")
         }
     }