Improve executable product diagnostics

Improve the diagnostics emitted when an executable product doesn’t have exactly one executable target by being more precise depending on if the number of executable targets is 0, and the user probably needs help on how to make a target executable, or if its more than 1, in that case we don’t need to provide that help.

Author: hartbit

Sources/PackageLoading/Diagnostics.swift

@@ -58,8 +58,22 @@ extension Diagnostic.Message {
         .error("system library product \(product) shouldn't have a type and contain only one target")
     }
 
-    static func invalidExecutableProductDecl(_ product: String) -> Diagnostic.Message {
-        .error("executable product '\(product)' should have exactly one executable target")
+    static func executableProductTargetNotExecutable(product: String, target: String) -> Diagnostic.Message {
+        .error("""
+            executable product '\(product)' expects target '\(target)' to be executable; an executable target requires \
+            a 'main.swift' file
+            """)
+    }
+
+    static func executableProductWithoutExecutableTarget(product: String) -> Diagnostic.Message {
+        .error("""
+            executable product '\(product)' should have one executable target; an executable target requires a \
+            'main.swift' file
+            """)
+    }
+
+    static func executableProductWithMoreThanOneExecutableTarget(product: String) -> Diagnostic.Message {
+        .error("executable product '\(product)' should not have more than one executable target")
     }
 
     static var noLibraryTargetsForREPL: Diagnostic.Message {

Sources/PackageLoading/PackageBuilder.swift

@@ -1075,12 +1075,7 @@ public final class PackageBuilder {
             case .library, .test:
                 break
             case .executable:
-                let executableTargets = targets.filter({ $0.type == .executable })
-                if executableTargets.count != 1 {
-                    diagnostics.emit(
-                        .invalidExecutableProductDecl(product.name),
-                        location: diagnosticLocation()
-                    )
+                guard validateExecutableProduct(product, with: targets) else {
                     continue
                 }
             }
@@ -1109,6 +1104,33 @@ public final class PackageBuilder {
         return products.map({ $0.item })
     }
 
+    private func validateExecutableProduct(_ product: ProductDescription, with targets: [Target]) -> Bool {
+        let executableTargetCount = targets.filter { $0.type == .executable }.count
+        guard executableTargetCount == 1 else {
+            if executableTargetCount == 0 {
+                if let target = targets.spm_only {
+                    diagnostics.emit(
+                        .executableProductTargetNotExecutable(product: product.name, target: target.name),
+                        location: diagnosticLocation()
+                    )
+                } else {
+                    diagnostics.emit(
+                        .executableProductWithoutExecutableTarget(product: product.name),
+                        location: diagnosticLocation()
+                    )
+                }
+            } else {
+                diagnostics.emit(
+                    .executableProductWithMoreThanOneExecutableTarget(product: product.name),
+                    location: diagnosticLocation()
+                )
+            }
+
+            return false
+        }
+
+        return true
+    }
 }
 
 /// We create this structure after scanning the filesystem for potential targets.

Tests/PackageLoadingTests/PackageBuilderTests.swift

@@ -1452,22 +1452,47 @@ class PackageBuilderTests: XCTestCase {
 
     func testBadExecutableProductDecl() {
         let fs = InMemoryFileSystem(emptyFiles:
-            "/Sources/foo/foo.swift"
+            "/Sources/foo1/main.swift",
+            "/Sources/foo2/main.swift",
+            "/Sources/FooLib1/lib.swift",
+            "/Sources/FooLib2/lib.swift"
         )
 
         let manifest = Manifest.createV4Manifest(
             name: "MyPackage",
             products: [
-                ProductDescription(name: "foo", type: .executable, targets: ["foo"]),
+                ProductDescription(name: "foo1", type: .executable, targets: ["FooLib1"]),
+                ProductDescription(name: "foo2", type: .executable, targets: ["FooLib1", "FooLib2"]),
+                ProductDescription(name: "foo3", type: .executable, targets: ["foo1", "foo2"]),
             ],
             targets: [
-                TargetDescription(name: "foo"),
+                TargetDescription(name: "foo1"),
+                TargetDescription(name: "foo2"),
+                TargetDescription(name: "FooLib1"),
+                TargetDescription(name: "FooLib2"),
             ]
         )
         PackageBuilderTester(manifest, in: fs) { package, diagnostics in
-            package.checkModule("foo") { _ in }
+            package.checkModule("foo1") { _ in }
+            package.checkModule("foo2") { _ in }
+            package.checkModule("FooLib1") { _ in }
+            package.checkModule("FooLib2") { _ in }
+            diagnostics.check(
+                diagnostic: """
+                    executable product 'foo1' expects target 'FooLib1' to be executable; an executable target requires \
+                    a 'main.swift' file
+                    """,
+                behavior: .error,
+                location: "'MyPackage' /")
+            diagnostics.check(
+                diagnostic: """
+                    executable product 'foo2' should have one executable target; an executable target requires a \
+                    'main.swift' file
+                    """,
+                behavior: .error,
+                location: "'MyPackage' /")
             diagnostics.check(
-                diagnostic: "executable product \'foo\' should have exactly one executable target",
+                diagnostic: "executable product 'foo3' should not have more than one executable target",
                 behavior: .error,
                 location: "'MyPackage' /")
         }