[PackageLoading] Diagnose duplicate dependency declaration

<rdar://problem/42625896> [SR-8379]: SwiftPM? Crash when compiling LinkerKitIRCBot

Author: aciidgh

Sources/Basic/CollectionAlgorithms.swift

@@ -48,3 +48,15 @@ public extension Sequence where Iterator.Element: Hashable {
         return duplicate
     }
 }
+
+extension Collection where Element: Hashable {
+
+    /// Finds duplicates in given collection of Hashables.
+    public func findDuplicateElements() -> [[Element]] {
+        var table: [Element: [Element]] = [:]
+        for element in self {
+            table[element, default: []].append(element)
+        }
+        return table.values.filter({ $0.count > 1 })
+    }
+}

Sources/PackageLoading/ManifestLoader.swift

@@ -21,6 +21,8 @@ public enum ManifestParseError: Swift.Error {
 
     /// The manifest was successfully loaded by swift interpreter but there were runtime issues.
     case runtimeManifestErrors([String])
+
+    case duplicateDependencyDecl([[PackageDependencyDescription]])
 }
 
 /// Resources required for manifest loading.
@@ -229,9 +231,19 @@ public final class ManifestLoader: ManifestLoaderProtocol {
             targets: manifestBuilder.targets
         )
 
+        try validate(manifest)
+
         return manifest
     }
 
+    /// Validate the provided manifest.
+    private func validate(_ manifest: Manifest) throws {
+        let duplicateDecls = manifest.dependencies.map({ KeyedPair($0, key: PackageReference.computeIdentity(packageURL: $0.url)) }).findDuplicateElements()
+        if !duplicateDecls.isEmpty {
+            throw ManifestParseError.duplicateDependencyDecl(duplicateDecls.map({ $0.map({ $0.item }) }))
+        }
+    }
+
     /// Load the JSON string for the given manifest.
     func loadJSONString(
         path inputPath: AbsolutePath,

Sources/PackageModel/Manifest.swift

@@ -283,7 +283,7 @@ public enum SystemPackageProviderDescription: Equatable {
 public struct PackageDependencyDescription: Equatable, Codable {
 
     /// The dependency requirement.
-    public enum Requirement: Equatable {
+    public enum Requirement: Equatable, CustomStringConvertible {
         case exact(Version)
         case range(Range<Version>)
         case revision(String)
@@ -297,6 +297,21 @@ public struct PackageDependencyDescription: Equatable, Codable {
         public static func upToNextMinor(from version: Utility.Version) -> Requirement {
             return .range(version..<Version(version.major, version.minor + 1, 0))
         }
+
+        public var description: String {
+            switch self {
+            case .exact(let version):
+                return version.description
+            case .range(let range):
+                return range.description
+            case .revision(let revision):
+                return "revision[\(revision)]"
+            case .branch(let branch):
+                return "branch[\(branch)]"
+            case .localPackage:
+                return "local"
+            }
+        }
     }
 
     /// The url of the dependency.

Sources/Workspace/Diagnostics.swift

@@ -30,13 +30,45 @@ public struct ManifestParseDiagnostic: DiagnosticData {
     }
 }
 
+public struct ManifestDuplicateDeclDiagnostic: DiagnosticData {
+    public static let id = DiagnosticID(
+        type: ManifestParseDiagnostic.self,
+        name: "org.swift.diags.manifest-dup-dep-decl",
+        description: {
+            $0 <<< .substitution({
+                let `self` = $0 as! ManifestDuplicateDeclDiagnostic
+                let stream = BufferedOutputByteStream()
+
+                stream <<< "manifest parse error(s): duplicate dependency declaration\n"
+                let indent = Format.asRepeating(string: " ", count: 4)
+
+                for duplicateDecls in self.duplicates {
+                    for duplicate in duplicateDecls {
+                        stream <<< indent <<< duplicate.url <<< " @ " <<< "\(duplicate.requirement)" <<< "\n"
+                    }
+                    stream <<< "\n"
+                }
+
+                return stream.bytes.asString!
+            }, preference: .default)
+        }
+    )
+
+    public let duplicates: [[PackageDependencyDescription]]
+    public init(_ duplicates: [[PackageDependencyDescription]]) {
+        self.duplicates = duplicates
+    }
+}
+
 extension ManifestParseError: DiagnosticDataConvertible {
     public var diagnosticData: DiagnosticData {
         switch self {
         case .invalidManifestFormat(let error):
             return ManifestParseDiagnostic([error])
         case .runtimeManifestErrors(let errors):
             return ManifestParseDiagnostic(errors)
+        case .duplicateDependencyDecl(let duplicates):
+            return ManifestDuplicateDeclDiagnostic(duplicates)
         }
     }
 }

Tests/PackageLoadingTests/PD4_2LoadingTests.swift

@@ -309,6 +309,38 @@ class PackageDescription4_2LoadingTests: XCTestCase {
         }
     }
 
+    func testDuplicateDependencyDecl() throws {
+        let stream = BufferedOutputByteStream()
+        stream <<< """
+            import PackageDescription
+            let package = Package(
+                name: "Trivial",
+                dependencies: [
+                    .package(path: "../foo1"),
+                    .package(url: "/foo1.git", from: "1.0.1"),
+                    .package(url: "path/to/foo1", from: "3.0.0"),
+                    .package(url: "/foo2.git", from: "1.0.1"),
+                    .package(url: "/foo2.git", from: "1.1.1"),
+                    .package(url: "/foo3.git", from: "1.0.1"),
+                ],
+                targets: [
+                    .target(
+                        name: "foo",
+                        dependencies: ["dep1", .target(name: "target")]),
+                ]
+            )
+            """
+
+        do {
+            try loadManifestThrowing(stream.bytes) { _ in }
+            XCTFail("Unexpected success")
+        } catch ManifestParseError.duplicateDependencyDecl(let duplicates) {
+            XCTAssertEqual(duplicates.count, 2)
+            let urls = duplicates.flatMap({$0}).map({ $0.url }).sorted()
+            XCTAssertEqual(urls, ["/foo1", "/foo1.git", "/foo2.git", "/foo2.git", "/path/to/foo1"])
+        }
+    }
+
     func testCaching() {
         mktmpdir { path in
             let fs = localFileSystem