implement review comments

Author: theMomax

Plugins/SharedPackagePluginExtensions/PackageManager+getSymbolGraphsForDocC.swift

@@ -39,14 +39,10 @@ extension PackageManager {
         context: PluginContext,
         verbose: Bool,
         snippetExtractor: SnippetExtractor?,
-        arguments: Arguments
+        symbolGraphOptions: SymbolGraphOptions
     ) throws -> DocCSymbolGraphResult {
         // First generate the primary symbol graphs containing information about the
-        // symbols defined in the target itself.
-        
-        var symbolGraphOptions = target.defaultSymbolGraphOptions(in: context.package)
-        symbolGraphOptions.override(with: arguments)
-        
+        // symbols defined in the target itself.        
         if verbose {
             print("symbol graph options: '\(symbolGraphOptions)'")
         }

Plugins/SharedPackagePluginExtensions/SymbolGraphOptions+override.swift

@@ -9,20 +9,15 @@
 import PackagePlugin
 
 extension PackageManager.SymbolGraphOptions {
-    /// Overrides individual values in the `SymbolGraphOptions`
-    /// according to the given `arguments`.
-    mutating func override(with arguments: Arguments) {
-        for argument in arguments {
-            switch argument {
-            case "--emit-extension-block-symbols":
+    /// Overrides individual values in the `PackageManager.SymbolGraphOptions`
+    /// according to the given configurable subset of `SymbolGraphOptions`.
+    mutating func override(with configuration: SymbolGraphOptions) {
+        if configuration.emitExtensionBlockSymbols {
 #if swift(>=5.8)
-                self.emitExtensionBlocks = true
+            self.emitExtensionBlocks = true
 #else
-                print("warning: detected '--emit-extension-block-symbols' option, which is incompatible with your swift version (required: 5.8)")
+            print("warning: detected '--include-extended-types' option, which is incompatible with your swift version (required: 5.8)")
 #endif
-            default:
-                print("warning: detected unknown dump-symbol-graph option '\(argument)'")
-            }
         }
     }
 }

Plugins/SharedPackagePluginExtensions/Target+defaultSymbolGraphOptions.swift

@@ -36,7 +36,7 @@ extension SwiftSourceModuleTarget {
 
 #if swift(<5.8)
 private extension PackageManager.SymbolGraphOptions {
-    /// A copatibility layer for lower Swift versions which discards unknown parameters.
+    /// A compatibility layer for lower Swift versions which discards unknown parameters.
     init(minimumAccessLevel: PackagePlugin.PackageManager.SymbolGraphOptions.AccessLevel = .public,
          includeSynthesized: Bool = false,
          includeSPI: Bool = false,

Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

@@ -65,13 +65,16 @@ import PackagePlugin
             }
 
             print("Generating documentation for '\(target.name)'...")
+        
+            var options = target.defaultSymbolGraphOptions(in: context.package)
+            options.override(with: parsedArguments.dumpSymbolGraphOptions())
 
             let symbolGraphs = try packageManager.doccSymbolGraphs(
                 for: target,
                 context: context,
                 verbose: verbose,
                 snippetExtractor: snippetExtractor,
-                arguments: parsedArguments.dumpSymbolGraphArguments()
+                symbolGraphOptions: options
             )
 
             if try FileManager.default.contentsOfDirectory(atPath: symbolGraphs.targetSymbolGraphsDirectory.path).isEmpty {

Plugins/Swift-DocC Preview/SwiftDocCPreview.swift

@@ -79,12 +79,15 @@ import PackagePlugin
         let snippetExtractor: SnippetExtractor? = nil
 #endif
 
+        var options = target.defaultSymbolGraphOptions(in: context.package)
+        options.override(with: parsedArguments.dumpSymbolGraphOptions())
+        
         let symbolGraphs = try packageManager.doccSymbolGraphs(
             for: target,
             context: context,
             verbose: verbose,
             snippetExtractor: snippetExtractor,
-            arguments: parsedArguments.dumpSymbolGraphArguments()
+            symbolGraphOptions: options
         )
 
         if try FileManager.default.contentsOfDirectory(atPath: symbolGraphs.targetSymbolGraphsDirectory.path).isEmpty {

Sources/SwiftDocCPluginUtilities/HelpInformation.swift

@@ -43,21 +43,19 @@ public enum HelpInformation {
             helpText = previewPluginHelpOverview
         }
 
-        var supportedPluginFlags = [
+        var supportedPluginFlags: [HelpTextProvider] = [
             PluginFlag.disableIndex,
         ]
 
+        // stops 'not mutated' warning for Swift 5.7 and lower
+        supportedPluginFlags += []
+        
 #if swift(>=5.8)
         supportedPluginFlags += [PluginFlag.extendedTypes]
 #endif
 
         for flag in supportedPluginFlags {
-            helpText += """
-                  \(flag.parsedValues.sorted().joined(separator: ", "))
-                                          \(flag.abstract)
-                        \(flag.description)
-                
-                """
+            helpText += flag.helpText
         }
 
         let doccHelp = try _doccHelp(pluginAction, doccExecutableURL)
@@ -141,3 +139,18 @@ private extension Process {
     }
 }
 
+private protocol HelpTextProvider {
+    var helpText: String { get }
+}
+
+extension PluginFlag: HelpTextProvider {
+    var helpText: String {
+        """
+          \(self.parsedValues.sorted().joined(separator: ", "))
+                                  \(self.abstract)
+                \(self.description)
+        
+        """
+    }
+}
+

Sources/SwiftDocCPluginUtilities/ParsedArguments.swift

@@ -24,14 +24,15 @@ public struct ParsedArguments {
     /// In practice, however we won't invoke the `dump-symbol-graph` command via the command line,
     /// but via the `PackagePlugin` API. Thus, these arguments have to be parsed later on and converted to
     /// `PackageManager.SymbolGraphOptions`.
-    public func dumpSymbolGraphArguments() -> Arguments {
+    public func dumpSymbolGraphOptions() -> SymbolGraphOptions {
         var dumpSymbolGraphArguments = arguments.filter(for: .dumpSymbolGraph)
+        var dumpSymbolGraphConfiguration = SymbolGraphOptions()
 
-        for argumentsTransformer in Self.argumentsTransformers {
-            dumpSymbolGraphArguments = argumentsTransformer.transform(dumpSymbolGraphArguments)
+        for transformer in Self.argumentsTransformers {
+            transformer.transform(arguments: &dumpSymbolGraphArguments, configuration: &dumpSymbolGraphConfiguration)
         }
 
-        return dumpSymbolGraphArguments
+        return dumpSymbolGraphConfiguration
     }
 
     /// Returns the arguments that should be passed to `docc` to invoke the given plugin action.
@@ -151,10 +152,12 @@ public struct ParsedArguments {
         // Add any required options that are specific to the kind of target being built
         doccArguments = targetKind.addRequiredOptions(to: doccArguments)
 
+        var configuration: () = Void()
+        
         // Apply any argument transformations. This allows for custom
         // flags that are specific to the plugin and not built-in to `docc`.
-        for argumentsTransformer in Self.argumentsTransformers {
-            doccArguments = argumentsTransformer.transform(doccArguments)
+        for transformer in Self.argumentsTransformers {
+            transformer.transform(arguments: &doccArguments, configuration: &configuration)
         }
 
         // If we were given a catalog path, prepend it to the set of arguments.
@@ -201,7 +204,7 @@ private extension ParsedArguments {
         ///
         /// If `flags` is `nil`, this `ArgumentConsumer` is assumed to
         /// consume all flags not consumed by any of the other `ArgumentConsumer`s.
-        var flags: [PluginFlag]? {
+        var flags: [FlagParser]? {
             switch self {
             case .dumpSymbolGraph:
                 return [
@@ -214,16 +217,29 @@ private extension ParsedArguments {
     }
 }
 
+private protocol FlagParser {
+    var parsedValues: Set<String> { get }
+}
+
+extension PluginFlag: FlagParser { }
+
 private extension Arguments {
     /// Returns the subset of arguments which are applicable to the given `consumer`.
     func filter(for consumer: ParsedArguments.ArgumentConsumer) -> Arguments {
         if let flagsToInclude = consumer.flags {
+            // If the consumer can provide a complete list of valid flags,
+            // we only include elements that are included in one of these flags'
+            // `parsedValues`, i.e. if one of these flags can be applied to the
+            // element.
             return self.filter { argument in
                 flagsToInclude.contains(where: { flag in
                     flag.parsedValues.contains(argument)
                 })
             }
         } else {
+            // If the consumer cannot provide a complete list of valid flags, (which
+            // should only happen for the `.docc` case) we return all elements
+            // that are not applicable to any of the other `ArgumentConsumer`s.
             let flagsToExclude = ParsedArguments.ArgumentConsumer.allCases.compactMap(\.flags).flatMap { $0 }
             return self.filter { argument in
                 !flagsToExclude.contains(where: { flag in

Sources/SwiftDocCPluginUtilities/PluginFlags/ArgumentsTransforming.swift

@@ -6,8 +6,30 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 
-/// Transforms a set of arguments.
+/// A type-erased ``ArgumentsTransformer``.
 protocol ArgumentsTransforming {
-    /// Apply the transformation to the given arguments.
-    func transform(_ arguments: Arguments) -> Arguments
+    /// Apply the transformation to the given arguments and configuration if the `Configuration` is compatible
+    /// with the implementation.
+    func transform<Configuration>(arguments: inout Arguments, configuration: inout Configuration)
+}
+
+/// Transforms a set of arguments, transferring information to a typed configuration.
+protocol ArgumentsTransformer: ArgumentsTransforming {
+    associatedtype Configuration
+    /// Apply the transformation to the given arguments and configuration.
+    func transform(arguments: inout Arguments, configuration: inout Configuration)
+}
+
+extension ArgumentsTransformer {
+    func transform<C>(arguments: inout Arguments, configuration: inout C) {
+        guard var typedConfig = configuration as? Configuration,
+              // we need strict type-equality as otherwise the transformed
+              // `typedConfig` might not conform to `C`
+              C.self == Configuration.self else {
+            return
+        }
+        
+        self.transform(arguments: &arguments, configuration: &typedConfig)
+        configuration = typedConfig as! C
+    }
 }

Sources/SwiftDocCPluginUtilities/PluginFlags/DisableIndexFlag.swift

@@ -6,7 +6,7 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 
-extension PluginFlag {
+extension PluginFlag where Configuration == Void {
     /// Disables indexing for the produced DocC archive.
     ///
     /// Removes the `--index` flag from a given set of arguments if the `--disable-index` flag
@@ -20,11 +20,9 @@ extension PluginFlag {
         description: """
             Produces a DocC archive that is best-suited for hosting online but incompatible with Xcode.
             """,
-        argumentTransformation: { arguments in
+        transformation: { arguments, _ in
             // Filter out any --index flags from the parsed arguments.
-            return arguments.filter { argument in
-                argument != "--index"
-            }
+            arguments.removeAll(where: { $0 == "--index" })
         }
     )
 }

Sources/SwiftDocCPluginUtilities/PluginFlags/ExtendedTypesFlag.swift

@@ -6,7 +6,7 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 
-extension PluginFlag {
+extension PluginFlag where Configuration == SymbolGraphOptions {
     /// Include extended types in documentation archives.
     ///
     /// Enables the extension block symbol format when calling the
@@ -16,14 +16,22 @@ extension PluginFlag {
     /// be hidden from the `--help` command for lower toolchain versions.
     /// However, we do not hide the flag entirely, because this enables us to give
     /// a more precise warning when accidentally used with Swift 5.7 or lower.
-    static let extendedTypes = PluginFlag(
+    static let extendedTypes: Self = PluginFlag(
         parsedValues: [
             "--include-extended-types",
         ],
         abstract: "Include extended types from other modules in the produced DocC archive.",
         description: """
             Allows documenting symbols that a target adds to its dependencies.
             """,
-        argumentTransformation: { $0 + ["--emit-extension-block-symbols"] }
+        transformation: { _, options in
+            options.emitExtensionBlockSymbols = true
+        }
     )
 }
+
+/// The subset of `PackagePlugin.SymbolGraphOptions` that can be configured in
+/// this plugin.
+public struct SymbolGraphOptions {
+    var emitExtensionBlockSymbols = false
+}

Sources/SwiftDocCPluginUtilities/PluginFlags/PluginFlag.swift

@@ -9,7 +9,7 @@
 /// A command-line flag for the docc plugin.
 ///
 /// Plugin flags are distinct from options that are for the docc command-line tool itself.
-struct PluginFlag: ArgumentsTransforming {
+struct PluginFlag<Configuration>: ArgumentsTransformer {
     /// The string values that will be parsed when detecting this flag.
     ///
     /// For example, this might be `["--disable-index"]`.
@@ -21,7 +21,7 @@ struct PluginFlag: ArgumentsTransforming {
     /// An expanded, user-facing description of this flag.
     let description: String
 
-    let argumentTransformation: (Arguments) -> Arguments
+    let transformation: (inout Arguments, inout Configuration) -> Void
 
     /// Transforms the given set of arguments if they include any of this flag's
     /// parsed values.
@@ -30,20 +30,20 @@ struct PluginFlag: ArgumentsTransforming {
     /// and the given parsed arguments contain `["--disable-index", "--index"]`,
     /// then the transformation would both consume the `"--disable-index"` flag
     /// and remove the `"--index"` flag since indexing should be disabled.
-    func transform(_ arguments: Arguments) -> Arguments {
+    func transform(arguments: inout Arguments, configuration: inout Configuration) {
         guard !parsedValues.isDisjoint(with: arguments) else {
             // The given parsed arguments do not contain any of this flags
             // parsed values so just return.
-            return arguments
+            return
         }
 
         // Consume the current flag
-        let arguments = arguments.filter { argument in
-            !parsedValues.contains(argument)
+        arguments.removeAll { argument in
+            parsedValues.contains(argument)
         }
 
         // Apply the flag to the set of arguments
-        return argumentTransformation(arguments)
+        transformation(&arguments, &configuration)
     }
 
     /// Create a new command-line flag.
@@ -58,11 +58,11 @@ struct PluginFlag: ArgumentsTransforming {
         parsedValues: Set<String>,
         abstract: String,
         description: String,
-        argumentTransformation: @escaping (Arguments) -> Arguments
+        transformation: @escaping (inout Arguments, inout Configuration) -> Void
     ) {
         self.parsedValues = parsedValues
         self.abstract = abstract
         self.description = description
-        self.argumentTransformation = argumentTransformation
+        self.transformation = transformation
     }
 }

Tests/SwiftDocCPluginUtilitiesTests/ParsedArgumentsTests.swift

@@ -462,13 +462,13 @@ final class ParsedArgumentsTests: XCTestCase {
     func testDumpSymbolGraphArguments() {
         let dumpSymbolGraphArguments = ParsedArguments(["--include-extended-types"])
 
-        XCTAssertEqual(dumpSymbolGraphArguments.dumpSymbolGraphArguments(), ["--emit-extension-block-symbols"])
+        XCTAssertEqual(dumpSymbolGraphArguments.dumpSymbolGraphOptions().emitExtensionBlockSymbols, true)
     }
 
     func testDumpSymbolGraphArgumentsWithDocCArguments() {
         let dumpSymbolGraphArguments = ParsedArguments(["--fallback-default-module-kind", "Executable"])
 
 
-        XCTAssertTrue(dumpSymbolGraphArguments.dumpSymbolGraphArguments().isEmpty)
+        XCTAssertEqual(dumpSymbolGraphArguments.dumpSymbolGraphOptions().emitExtensionBlockSymbols, false)
     }
 }

Tests/SwiftDocCPluginUtilitiesTests/PluginFlags/DisableIndexFlagTests.swift

@@ -66,3 +66,12 @@ final class DisableIndexFlagTests: XCTestCase {
         )
     }
 }
+
+private extension PluginFlag where Configuration == Void {
+    func transform(_ arguments: Arguments) -> Arguments {
+        var arguments = arguments
+        var configuration: () = Void()
+        self.transform(arguments: &arguments, configuration: &configuration)
+        return arguments
+    }
+}