[PackageLoading] Diagnose broken symlink sources

<rdar://problem/34346476> swiftpm ignores missing source symlinks

Author: aciidgh

Sources/PackageLoading/Diagnostics.swift

@@ -125,4 +125,17 @@ public enum PackageBuilderDiagnostics {
 
         let product: String
     }
+
+    struct BorkenSymlinkDiagnostic: DiagnosticData {
+        static let id = DiagnosticID(
+            type: BorkenSymlinkDiagnostic.self,
+            name: "org.swift.diags.pkg-builder.borken-symlink",
+            defaultBehavior: .warning,
+            description: {
+                $0 <<< "ignoring broken symlink" <<< { $0.path }
+            }
+        )
+
+        let path: String
+    }
 }

Sources/PackageLoading/PackageBuilder.swift

@@ -238,6 +238,10 @@ public final class PackageBuilder {
         )
     }
 
+    private func diagnosticLocation() -> DiagnosticLocation {
+        return PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+    }
+
     /// Computes the special directory where targets are present or should be placed in future.
     private func findTargetSpecialDirs(_ targets: [Target]) -> (targetDir: String, testTargetDir: String) {
         let predefinedDirs = findPredefinedTargetDirectory()
@@ -277,8 +281,19 @@ public final class PackageBuilder {
         // Ignore linux main.
         if basename == SwiftTarget.linuxMainBasename { return false }
 
-        // Ignore symlinks to non-files.
-        if !fileSystem.isFile(path) { return false }
+        // Ignore paths which are not valid files.
+        if !fileSystem.isFile(path) {
+
+            // Diagnose broken symlinks.
+            if fileSystem.isSymlink(path) {
+                diagnostics.emit(
+                    data: PackageBuilderDiagnostics.BorkenSymlinkDiagnostic(path: path.asString),
+                    location: diagnosticLocation()
+                )
+            }
+
+            return false
+        }
 
         // Ignore manifest files.
         if path.parentDirectory == packagePath {
@@ -336,7 +351,7 @@ public final class PackageBuilder {
             if !targets.isEmpty {
                 diagnostics.emit(
                     data: PackageBuilderDiagnostics.SystemPackageDeclaresTargetsDiagnostic(targets: targets.map({ $0.name })),
-                    location: PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+                    location: diagnosticLocation()
                 )
             }
 
@@ -346,7 +361,7 @@ public final class PackageBuilder {
             case .v4_2:
                 diagnostics.emit(
                     data: PackageBuilderDiagnostics.SystemPackageDeprecatedDiagnostic(),
-                    location: PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+                    location: diagnosticLocation()
                 )
             }
 
@@ -786,7 +801,7 @@ public final class PackageBuilder {
             if !inserted {
                 diagnostics.emit(
                     data: PackageBuilderDiagnostics.DuplicateProduct(product: product),
-                    location: PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+                    location: diagnosticLocation()
                 )
             }
         }
@@ -877,7 +892,7 @@ public final class PackageBuilder {
                 if product.type != .library(.automatic) || targets.count != 1 {
                     diagnostics.emit(
                         data: PackageBuilderDiagnostics.SystemPackageProductValidationDiagnostic(product: product.name),
-                        location: PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+                        location: diagnosticLocation()
                     )
                     continue
                 }
@@ -892,7 +907,7 @@ public final class PackageBuilder {
                 if executableTargets.count != 1 {
                     diagnostics.emit(
                         data: PackageBuilderDiagnostics.InvalidExecutableProductDecl(product: product.name),
-                        location: PackageLocation.Local(name: manifest.name, packagePath: packagePath)
+                        location: diagnosticLocation()
                     )
                     continue
                 }

Tests/PackageLoadingTests/PackageBuilderTests.swift

@@ -9,6 +9,7 @@
 */
 
 import XCTest
+import TestSupport
 
 import Basic
 import PackageModel
@@ -54,6 +55,35 @@ class PackageBuilderTests: XCTestCase {
         }
     }
 
+    func testBrokenSymlink() throws {
+        mktmpdir { path in
+            let fs = localFileSystem
+
+            let sources = path.appending(components: "Sources", "foo")
+            try fs.createDirectory(sources, recursive: true)
+            try fs.writeFileContents(sources.appending(components: "foo.swift"), bytes: "")
+
+            // Create a stray symlink in sources.
+            let linkDestPath = path.appending(components: "link.swift")
+            let linkPath = sources.appending(components: "link.swift")
+            try fs.writeFileContents(linkDestPath, bytes: "")
+            try createSymlink(linkPath, pointingAt: linkDestPath)
+            try fs.removeFileTree(linkDestPath)
+
+            let manifest = Manifest.createV4Manifest(
+                name: "pkg",
+                targets: [
+                    TargetDescription(name: "foo"),
+                ]
+            )
+
+            PackageBuilderTester(manifest, path: path, in: fs) { result in
+                result.checkDiagnostic("ignoring broken symlink \(linkPath.asString)")
+                result.checkModule("foo")
+            }
+        }
+    }
+
     func testCInTests() throws {
         let fs = InMemoryFileSystem(emptyFiles:
             "/Sources/MyPackage/main.swift",

Tests/PackageLoadingTests/XCTestManifests.swift

@@ -14,6 +14,7 @@ extension ModuleMapGeneration {
 extension PackageBuilderTests {
     static let __allTests = [
         ("testBadExecutableProductDecl", testBadExecutableProductDecl),
+        ("testBrokenSymlink", testBrokenSymlink),
         ("testCInTests", testCInTests),
         ("testCompatibleSwiftVersions", testCompatibleSwiftVersions),
         ("testCustomTargetDependencies", testCustomTargetDependencies),