Fix a bug where targets with multiple dependencies was only passed on…

…e dependency (#98)

* Refine command line argument types to define flag/option specific names

This means that flag/option specific API can leverage types instead of including "flag" or "option" in the API's name.
This also means that a named option requires a value to create an argument.

Co-authored-by: Sofía Rodríguez <[email protected]>

* Support defining an option that supports an array of values.

Also, define `--dependency` as an array-of-values option.

Co-authored-by: Sofía Rodríguez <[email protected]>

* Fix a logic bug where an alternate argument spellings with common prefixes would sometimes not extract a value.

* Move the inverse names configuration into CommandLineArgument.Flag

* Add integration test that verifies that targets are passed all their dependencies

* Use the CommandLineArguments type to construct the merge arguments

* Print the 'docc merge' call when the `--verbose` flag is passed

* Fix an unrelated code warning in Swift 5.7

* Fix an unrelated code warning and avoid undefined behavior in snippet-extract

* Correctly document updated parameters

Co-authored-by: Sofía Rodríguez <[email protected]>

* Fix other unrelated documentation warnings

---------

Co-authored-by: Sofía Rodríguez <[email protected]>

IntegrationTests/Package.swift

@@ -28,6 +28,7 @@ let package = Package(
                 .copy("Fixtures/SingleTestTarget"),
                 .copy("Fixtures/SingleExecutableTarget"),
                 .copy("Fixtures/MixedTargets"),
+                .copy("Fixtures/TargetsWithDependencies"),
                 .copy("Fixtures/TargetWithDocCCatalog"),
                 .copy("Fixtures/PackageWithSnippets"),
                 .copy("Fixtures/PackageWithConformanceSymbols"),

IntegrationTests/Tests/CombinedDocumentationTests.swift

@@ -0,0 +1,99 @@
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+import XCTest
+
+final class CombinedDocumentationTests: ConcurrencyRequiringTestCase {
+    func testCombinedDocumentation() throws {
+#if compiler(>=6.0)
+        let result = try swiftPackage(
+            "generate-documentation",
+            "--enable-experimental-combined-documentation",
+            "--verbose", // Necessary to see the 'docc convert' calls in the log and verify their parameters.
+            workingDirectory: try setupTemporaryDirectoryForFixture(named: "TargetsWithDependencies")
+        )
+
+        result.assertExitStatusEquals(0)
+        let outputArchives = result.referencedDocCArchives
+        XCTAssertEqual(outputArchives.count, 1)
+        XCTAssertEqual(outputArchives.map(\.lastPathComponent), [
+            "TargetsWithDependencies.doccarchive",
+        ])
+
+        // Verify that the combined archive contains all target's documentation
+
+        let combinedArchiveURL = try XCTUnwrap(outputArchives.first)
+        let combinedDataDirectoryContents = try filesIn(.dataSubdirectory, of: combinedArchiveURL)
+            .map(\.relativePath)
+            .sorted()
+
+        XCTAssertEqual(combinedDataDirectoryContents, [
+            "documentation.json",
+            "documentation/innerfirst.json",
+            "documentation/innerfirst/somethingpublic.json",
+            "documentation/innersecond.json",
+            "documentation/innersecond/somethingpublic.json",
+            "documentation/nestedinner.json",
+            "documentation/nestedinner/somethingpublic.json",
+            "documentation/outer.json",
+            "documentation/outer/somethingpublic.json",
+        ])
+
+        // Verify that each 'docc convert' call was passed the expected dependencies
+
+        let doccConvertCalls = result.standardOutput
+            .components(separatedBy: .newlines)
+            .filter { line in
+                line.hasPrefix("docc invocation: '") && line.utf8.contains("docc convert ".utf8)
+            }.map { line in
+                line.trimmingCharacters(in: CharacterSet(charactersIn: "'"))
+                    .components(separatedBy: .whitespaces)
+                    .drop(while: { $0 != "convert" })
+            }
+
+        XCTAssertEqual(doccConvertCalls.count, 4)
+
+        func extractDependencyArchives(targetName: String, file: StaticString = #filePath, line: UInt = #line) throws -> [String] {
+            let arguments = try XCTUnwrap(
+                doccConvertCalls.first(where: { $0.contains(["--fallback-display-name", targetName]) }),
+                file: file, line: line
+            )
+            var dependencyPaths: [URL] = []
+
+            var remaining = arguments[...]
+            while !remaining.isEmpty {
+                remaining = remaining.drop(while: { $0 != "--dependency" }).dropFirst(/* the '--dependency' element */)
+                if let path = remaining.popFirst() {
+                    dependencyPaths.append(URL(fileURLWithPath: path))
+                }
+            }
+
+            return dependencyPaths.map { $0.lastPathComponent }.sorted()
+        }
+        // Outer
+        // ├─ InnerFirst
+        // ╰─ InnerSecond
+        //    ╰─ NestedInner
+
+        XCTAssertEqual(try extractDependencyArchives(targetName: "Outer"), [
+            "InnerFirst.doccarchive",
+            "InnerSecond.doccarchive",
+        ], "The outer target has depends on both inner targets")
+
+        XCTAssertEqual(try extractDependencyArchives(targetName: "InnerFirst"), [], "The first inner target has no dependencies")
+
+        XCTAssertEqual(try extractDependencyArchives(targetName: "InnerSecond"), [
+            "NestedInner.doccarchive",
+        ], "The second inner target has depends on the nested inner target")
+
+        XCTAssertEqual(try extractDependencyArchives(targetName: "NestedInner"), [], "The nested inner target has no dependencies")
+#else
+        XCTSkip("This test requires a Swift-DocC version that support the link-dependencies feature")
+#endif
+    }
+}

IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Package.swift

@@ -0,0 +1,42 @@
+// swift-tools-version: 5.7
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+import Foundation
+import PackageDescription
+
+let package = Package(
+    name: "TargetsWithDependencies",
+    targets: [
+        // Outer
+        // ├─ InnerFirst
+        // ╰─ InnerSecond
+        //    ╰─ NestedInner
+        .target(name: "Outer", dependencies: [
+            "InnerFirst",
+            "InnerSecond",
+        ]),
+        .target(name: "InnerFirst"),
+        .target(name: "InnerSecond", dependencies: [
+            "NestedInner"
+        ]),
+        .target(name: "NestedInner"),
+    ]
+)
+
+// We only expect 'swift-docc-plugin' to be a sibling when this package
+// is running as part of a test.
+//
+// This allows the package to compile outside of tests for easier
+// test development.
+if FileManager.default.fileExists(atPath: "../swift-docc-plugin") {
+    package.dependencies += [
+        .package(path: "../swift-docc-plugin"),
+    ]
+}

IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Sources/InnerFirst/SomeFile.swift

@@ -0,0 +1,13 @@
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+/// This is a public struct and should be included in the documentation for this library.
+public struct SomethingPublic {}
+
+/// This is an internal struct and should not be included in the documentation for this library.
+struct SomethingInternal {}

IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Sources/InnerSecond/SomeFile.swift

@@ -0,0 +1,13 @@
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+/// This is a public struct and should be included in the documentation for this library.
+public struct SomethingPublic {}
+
+/// This is an internal struct and should not be included in the documentation for this library.
+struct SomethingInternal {}

IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Sources/NestedInner/SomeFile.swift

@@ -0,0 +1,13 @@
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+/// This is a public struct and should be included in the documentation for this library.
+public struct SomethingPublic {}
+
+/// This is an internal struct and should not be included in the documentation for this library.
+struct SomethingInternal {}

IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Sources/Outer/SomeFile.swift

@@ -0,0 +1,13 @@
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+
+/// This is a public struct and should be included in the documentation for this library.
+public struct SomethingPublic {}
+
+/// This is an internal struct and should not be included in the documentation for this library.
+struct SomethingInternal {}

Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

@@ -46,7 +46,7 @@ import PackagePlugin
         if isCombinedDocumentationEnabled, doccFeatures?.contains(.linkDependencies) == false {
             // The developer uses the combined documentation plugin flag with a DocC version that doesn't support combined documentation.
             Diagnostics.error("""
-            Unsupported use of '\(DocumentedFlag.enableCombinedDocumentation.names.preferred)'. \
+            Unsupported use of '\(DocumentedArgument.enableCombinedDocumentation.names.preferred)'. \
             DocC version at '\(doccExecutableURL.path)' doesn't support combined documentation.
             """)
             return
@@ -209,20 +209,26 @@ import PackagePlugin
             combinedArchiveOutput = URL(fileURLWithPath: context.pluginWorkDirectory.appending(combinedArchiveName).string)
         }
 
-        var mergeCommandArguments = ["merge"]
-        mergeCommandArguments.append(contentsOf: intermediateDocumentationArchives.map(\.standardizedFileURL.path))
-        mergeCommandArguments.append(contentsOf: [DocCArguments.outputPath.preferred, combinedArchiveOutput.path])
+        var mergeCommandArguments = CommandLineArguments(
+            ["merge"] + intermediateDocumentationArchives.map(\.standardizedFileURL.path)
+        )
+        mergeCommandArguments.insertIfMissing(DocCArguments.outputPath, value: combinedArchiveOutput.path)
 
         if let doccFeatures, doccFeatures.contains(.synthesizedLandingPageName) {
-            mergeCommandArguments.append(contentsOf: [DocCArguments.synthesizedLandingPageName.preferred, context.package.displayName])
-            mergeCommandArguments.append(contentsOf: [DocCArguments.synthesizedLandingPageKind.preferred, "Package"])
+            mergeCommandArguments.insertIfMissing(DocCArguments.synthesizedLandingPageName, value: context.package.displayName)
+            mergeCommandArguments.insertIfMissing(DocCArguments.synthesizedLandingPageKind, value: "Package")
         }
 
         // Remove the combined archive if it already exists
         try? FileManager.default.removeItem(at: combinedArchiveOutput)
 
+        if verbose {
+            let arguments = mergeCommandArguments.remainingArguments.joined(separator: " ")
+            print("docc invocation: '\(doccExecutableURL.path) \(arguments)'")
+        }
+
         // Create a new combined archive
-        let process = try Process.run(doccExecutableURL, arguments: mergeCommandArguments)
+        let process = try Process.run(doccExecutableURL, arguments: mergeCommandArguments.remainingArguments)
         process.waitUntilExit()
 
         print("""

Sources/Snippets/Model/Snippet.swift

@@ -1,6 +1,6 @@
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2022 Apple Inc. and the Swift project authors
+// Copyright (c) 2022-2024 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -12,7 +12,7 @@ import Foundation
 ///
 /// A *snippet* is a short, focused code example that can be shown with little to no context or prose.
 public struct Snippet {
-    /// The ``URL`` of the source file for this snippet.
+    /// The URL of the source file for this snippet.
     public var sourceFile: URL
 
     /// A short abstract explaining what the snippet does.
@@ -39,8 +39,7 @@ public struct Snippet {
 
     /// Create a Swift snippet by parsing a file.
     ///
-    /// - parameter sourceURL: The URL of the file to parse.
-    /// - parameter syntax: The name of the syntax of the source file if known.
+    /// - Parameter sourceFile: The URL of the file to parse.
     public init(parsing sourceFile: URL) throws {
         let source = try String(contentsOf: sourceFile)
         self.init(parsing: source, sourceFile: sourceFile)

Sources/SwiftDocCPluginUtilities/CommandLineArguments/CommandLineArgument.swift

@@ -40,21 +40,79 @@ public struct CommandLineArgument {
         /// An option argument with an associated value.
         ///
         /// For example: `"--some-option", "value"` or `"--some-option=value"`.
-        case option(value: String)
+        case option(value: String, kind: Option.Kind)
     }
 
-    /// Creates a new command line flag with the given names.
-    /// - Parameters:
-    ///   - names: The names for the new command line flag.
-    public static func flag(_ names: Names) -> Self {
-        .init(names: names, kind: .flag)
+    // Only create arguments from flags or options (with a value)
+
+    init(_ flag: Flag) {
+        names = flag.names
+        kind = .flag
+    }
+
+    init(_ option: Option, value: String) {
+        names = option.names
+        kind = .option(value: value, kind: option.kind)
+    }
+}
+
+extension CommandLineArgument {
+    /// A flag argument without an associated value.
+    ///
+    /// For example: `"--some-flag"`.
+    public struct Flag {
+        /// The names of this command line flag.
+        public var names: Names
+        /// The negative names for this flag, if any.
+        public var inverseNames: CommandLineArgument.Names?
+
+        /// Creates a new command line flag.
+        ///
+        /// - Parameters:
+        ///   - preferred: The preferred name for this flag.
+        ///   - alternatives: A collection of alternative names for this flag.
+        ///   - inverseNames: The negative names for this flag, if any.
+        public init(preferred: String, alternatives: Set<String> = [], inverseNames: CommandLineArgument.Names? = nil) {
+            // This is duplicating the `Names` parameters to offer a nicer initializer for the common case.
+            names = .init(preferred: preferred, alternatives: alternatives)
+            self.inverseNames = inverseNames
+        }
     }
 
-    /// Creates a new command option with the given names and associated value.
-    /// - Parameters:
-    ///   - names: The names for the new command line option.
-    ///   - value: The value that's associated with this command line option.
-    public static func option(_ names: Names, value: String) -> Self {
-        .init(names: names, kind: .option(value: value))
+    /// An option argument that will eventually associated with a value.
+    ///
+    /// For example: `"--some-option", "value"` or `"--some-option=value"`.
+    public struct Option {
+        /// The names of this command line option.
+        public var names: Names
+        /// The kind of value for this command line option.
+        public var kind: Kind
+
+        /// A kind of value(s) that a command line option supports.
+        public enum Kind {
+            /// An option that supports a single value.
+            case singleValue
+            /// An option that supports an array of different values.
+            case arrayOfValues
+        }
+
+        /// Creates a new command line option.
+        ///
+        /// - Parameters:
+        ///   - preferred: The preferred name for this option.
+        ///   - alternatives: A collection of alternative names for this option.
+        ///   - kind: The kind of value(s) that this option supports.
+        public init(preferred: String, alternatives: Set<String> = [], kind: Kind = .singleValue) {
+            // This is duplicating the `Names` parameters to offer a nicer initializer for the common case.
+            self.init(
+                Names(preferred: preferred, alternatives: alternatives),
+                kind: kind
+            )
+        }
+
+        init(_ names: Names, kind: Kind = .singleValue) {
+            self.names = names
+            self.kind  = kind
+        }
     }
 }