Improve some diagnostics messages

<rdar://problem/50456674>

Author: aciidgh

Sources/PackageDescription4/Version+StringLiteralConvertible.swift

@@ -18,7 +18,7 @@ extension Version: ExpressibleByStringLiteral {
             // the error and initialize with a dummy value.  This is done to
             // report error to the invoking tool (like swift build) gracefully
             // rather than just crashing.
-            errors.append("Invalid version string: \(value)")
+            errors.append("Invalid semantic version string '\(value)'")
             self.init(0, 0, 0)
         }
     }

Sources/PackageGraph/DependencyResolver.swift

@@ -60,9 +60,9 @@ public enum DependencyResolverError: Error, Equatable, CustomStringConvertible {
     public var description: String {
         switch self {
         case .cancelled:
-            return "the dependency resolution was cancelled"
+            return "the package resolution operation was cancelled"
         case .unsatisfiable:
-            return "unable to resolve dependencies"
+            return "the package dependency graph could not be resolved due to an unknown conflict"
         case .cycle(let package):
             return "the package \(package) depends on itself"
         case let .incompatibleConstraints(dependency, revisions):
@@ -79,7 +79,7 @@ public enum DependencyResolverError: Error, Equatable, CustomStringConvertible {
 
         case let .missingVersions(constraints):
             let stream = BufferedOutputByteStream()
-            stream <<< "the package dependency graph is unresolvable; unable find any available tag for the following requirements:\n"
+            stream <<< "the package dependency graph could not be resolved; unable find any available tag for the following requirements:\n"
             for (i, constraint) in constraints.enumerated() {
                 stream <<< "    "
                 stream <<< "\(constraint.identifier.path)" <<< " @ "

Sources/PackageGraph/PackageGraphLoader.swift

@@ -54,7 +54,7 @@ enum PackageGraphError: Swift.Error {
     case cycleDetected((path: [Manifest], cycle: [Manifest]))
 
     /// The product dependency not found.
-    case productDependencyNotFound(name: String, package: String?)
+    case productDependencyNotFound(name: String, target: String)
 
     /// The product dependency was found but the package name did not match.
     case productDependencyIncorrectPackage(name: String, package: String)
@@ -74,8 +74,8 @@ extension PackageGraphError: CustomStringConvertible {
                 (cycle.path + cycle.cycle).map({ $0.name }).joined(separator: " -> ") +
                 " -> " + cycle.cycle[0].name
 
-        case .productDependencyNotFound(let name, _):
-            return "product dependency '\(name)' not found"
+        case .productDependencyNotFound(let name, let target):
+            return "Product '\(name)' not found. It is required by target '\(target)'."
 
         case .productDependencyIncorrectPackage(let name, let package):
             return "product dependency '\(name)' in package '\(package)' not found"
@@ -346,7 +346,7 @@ private func createResolvedPackages(
                     // found errors when there are more important errors to
                     // resolve (like authentication issues).
                     if !diagnostics.hasErrors {
-                        let error = PackageGraphError.productDependencyNotFound(name: productRef.name, package: productRef.package)
+                        let error = PackageGraphError.productDependencyNotFound(name: productRef.name, target: targetBuilder.target.name)
                         diagnostics.emit(error, location: diagnosticLocation())
                     }
                     continue

Sources/PackageLoading/Diagnostics.swift

@@ -21,14 +21,13 @@ public enum PackageBuilderDiagnostics {
             name: "org.swift.diags.pkg-builder.nosources",
             defaultBehavior: .warning,
             description: {
-                $0 <<< "target" <<< { "'\($0.target)'" }
-                $0 <<< "in package" <<< { "'\($0.package)'" }
-                $0 <<< "contains no valid source files"
+                $0 <<< "Source files for target" <<< { "\($0.target)" }
+                $0 <<< "should be located under" <<< { "\($0.targetPath)" }
             }
         )
 
-        /// The name of the package.
-        public let package: String
+        /// The path of the target.
+        public let targetPath: String
 
         /// The name of the target which has no sources.
         public let target: String

Sources/PackageLoading/PackageBuilder.swift

@@ -24,8 +24,8 @@ public enum ModuleError: Swift.Error {
     /// Indicates two targets with the same name and their corresponding packages.
     case duplicateModule(String, [String])
 
-    /// One or more referenced targets could not be found.
-    case modulesNotFound([String])
+    /// The eferenced target could not be found.
+    case moduleNotFound(String)
 
     /// Invalid custom path.
     case invalidCustomPath(target: String, path: String)
@@ -67,9 +67,8 @@ extension ModuleError: CustomStringConvertible {
         case .duplicateModule(let name, let packages):
             let packages = packages.joined(separator: ", ")
             return "multiple targets named '\(name)' in: \(packages)"
-        case .modulesNotFound(let targets):
-            let targets = targets.joined(separator: ", ")
-            return "could not find source files for target(s): \(targets); use the 'path' property in the Swift 4 manifest to set a custom target path"
+        case .moduleNotFound(let target):
+            return "Source files for target \(target) should be located under 'Sources/\(target)', or a custom sources path can be set with the 'path' property in Package.swift"
         case .invalidLayout(let type):
             return "package has unsupported layout; \(type)"
         case .invalidManifestConfig(let package, let message):
@@ -456,7 +455,7 @@ public final class PackageBuilder {
             if fileSystem.isDirectory(path) {
                 return path
             }
-            throw ModuleError.modulesNotFound([target.name])
+            throw ModuleError.moduleNotFound(target.name)
         }
 
         // Create potential targets.
@@ -474,8 +473,8 @@ public final class PackageBuilder {
         let allReferencedModules = manifest.allReferencedModules()
         let potentialModulesName = Set(potentialModules.map({ $0.name }))
         let missingModules = allReferencedModules.subtracting(potentialModulesName).intersection(allReferencedModules)
-        guard missingModules.isEmpty else {
-            throw ModuleError.modulesNotFound(missingModules.map({ $0 }))
+        if let missingModule = missingModules.first {
+            throw ModuleError.moduleNotFound(missingModule)
         }
 
         let targetItems = manifest.targets.map({ ($0.name, $0 as TargetDescription) })
@@ -562,7 +561,7 @@ public final class PackageBuilder {
                 targets[createdTarget.name] = createdTarget
             } else {
                 emptyModules.insert(potentialModule.name)
-                diagnostics.emit(data: PackageBuilderDiagnostics.NoSources(package: manifest.name, target: potentialModule.name))
+                diagnostics.emit(data: PackageBuilderDiagnostics.NoSources(targetPath: potentialModule.path.pathString, target: potentialModule.name))
             }
         }
         return targets.values.map({ $0 })

Sources/PackageModel/Package.swift

@@ -94,7 +94,7 @@ extension Package.Error: CustomStringConvertible {
    public var description: String {
         switch self {
         case .noManifest(let baseURL, let version):
-            var string = "\(baseURL) has no manifest"
+            var string = "\(baseURL) has no Package.swift manifest"
             if let version = version {
                 string += " for version \(version)"
             }

Sources/Workspace/Diagnostics.swift

@@ -82,26 +82,21 @@ public enum ResolverDiagnostics {
             type: Unsatisfiable.self,
             name: "org.swift.diags.resolver.unsatisfiable",
             description: {
-                $0 <<< "dependency graph is unresolvable;"
+                $0 <<< "the package dependency graph could not be resolved;"
                 $0 <<< .substitution({
                     let `self` = $0 as! Unsatisfiable
 
                     // If we don't have any additional data, return empty string.
                     if self.dependencies.isEmpty && self.pins.isEmpty {
                         return ""
                     }
-                    var diag = "found these conflicting requirements:"
+                    var diag = "possibly because of these requirements:"
                     let indent = "    "
 
                     if !self.dependencies.isEmpty {
-                        diag += "\n\nDependencies: \n"
                         diag += self.dependencies.map({ indent + Unsatisfiable.toString($0) }).joined(separator: "\n")
                     }
 
-                    if !self.pins.isEmpty {
-                        diag += "\n\nPins: \n"
-                        diag += self.pins.map({ indent + Unsatisfiable.toString($0) }).joined(separator: "\n")
-                    }
                     return diag
                 }, preference: .default)
             }

Tests/PackageGraphTests/PackageGraphTests.swift

@@ -497,7 +497,7 @@ class PackageGraphTests: XCTestCase {
         )
 
         DiagnosticsEngineTester(diagnostics) { result in
-            result.check(diagnostic: "target 'Bar' in package 'Bar' contains no valid source files", behavior: .warning)
+            result.check(diagnostic: "Source files for target Bar should be located under /Bar/Sources/Bar", behavior: .warning)
             result.check(diagnostic: "target 'Bar' referenced in product 'Bar' could not be found", behavior: .error, location: "'Bar' /Bar")
         }
     }
@@ -521,7 +521,7 @@ class PackageGraphTests: XCTestCase {
         )
 
         DiagnosticsEngineTester(diagnostics) { result in
-            result.check(diagnostic: "product dependency 'Barx' not found", behavior: .error, location: "'Foo' /Foo")
+            result.check(diagnostic: "Product 'Barx' not found. It is required by target 'Foo'.", behavior: .error, location: "'Foo' /Foo")
         }
     }
 

Tests/PackageLoadingTests/PD4_2LoadingTests.swift

@@ -420,7 +420,7 @@ class PackageDescription4_2LoadingTests: XCTestCase {
             try loadManifestThrowing(stream.bytes) { _ in }
             XCTFail("Unexpected success")
         } catch ManifestParseError.runtimeManifestErrors(let errors) {
-            XCTAssertEqual(errors, ["Invalid version string: 1.0,0"])
+            XCTAssertEqual(errors, ["Invalid semantic version string '1.0,0'"])
         }
     }
 

Tests/PackageLoadingTests/PackageBuilderTests.swift

@@ -734,7 +734,7 @@ class PackageBuilderTests: XCTestCase {
                 ]
             )
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("could not find source files for target(s): Random; use the 'path' property in the Swift 4 manifest to set a custom target path")
+                result.checkDiagnostic("Source files for target Random should be located under 'Sources/Random', or a custom sources path can be set with the 'path' property in Package.swift")
             }
         }
 
@@ -749,7 +749,7 @@ class PackageBuilderTests: XCTestCase {
                 ]
             )
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("could not find source files for target(s): Foo; use the 'path' property in the Swift 4 manifest to set a custom target path")
+                result.checkDiagnostic("Source files for target Foo should be located under 'Sources/Foo', or a custom sources path can be set with the 'path' property in Package.swift")
             }
         }
 
@@ -779,7 +779,7 @@ class PackageBuilderTests: XCTestCase {
                 ]
             )
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("could not find source files for target(s): foo; use the 'path' property in the Swift 4 manifest to set a custom target path")
+                result.checkDiagnostic("Source files for target foo should be located under 'Sources/foo', or a custom sources path can be set with the 'path' property in Package.swift")
             }
         }
 
@@ -829,7 +829,7 @@ class PackageBuilderTests: XCTestCase {
                 ]
             )
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("target 'pkg2' in package 'pkg' contains no valid source files")
+                result.checkDiagnostic("Source files for target pkg2 should be located under /Sources/pkg2")
                 result.checkModule("pkg1") { moduleResult in
                     moduleResult.check(c99name: "pkg1", type: .library)
                     moduleResult.checkSources(root: "/Sources/pkg1", paths: "Foo.swift")
@@ -1058,7 +1058,7 @@ class PackageBuilderTests: XCTestCase {
             )
 
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("could not find source files for target(s): Bar; use the 'path' property in the Swift 4 manifest to set a custom target path")
+                result.checkDiagnostic("Source files for target Bar should be located under 'Sources/Bar', or a custom sources path can be set with the 'path' property in Package.swift")
             }
         }
 
@@ -1077,7 +1077,7 @@ class PackageBuilderTests: XCTestCase {
                 ]
             )
             PackageBuilderTester(manifest, in: fs) { result in
-                result.checkDiagnostic("could not find source files for target(s): BarTests; use the 'path' property in the Swift 4 manifest to set a custom target path")
+                result.checkDiagnostic("Source files for target BarTests should be located under 'Sources/BarTests', or a custom sources path can be set with the 'path' property in Package.swift")
             }
 
             // We should be able to fix this by using custom paths.

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -743,7 +743,7 @@ final class WorkspaceTests: XCTestCase {
         ]
         workspace.checkPackageGraph(deps: deps) { (_, diagnostics) in
             DiagnosticsEngineTester(diagnostics) { result in
-                result.check(diagnostic: .contains("dependency graph is unresolvable;"), behavior: .error)
+                result.check(diagnostic: .contains("the package dependency graph could not be resolved; possibly because of these requirements"), behavior: .error)
             }
         }
         // There should be no extra fetches.