Skip to content

Commit 85e4bb4

Browse files
d-ronnqvistsofiaromorales
d-ronnqvist
and
sofiaromorales
authored
Fix a bug where targets with multiple dependencies was only passed one 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]>
1 parent 300ee6c commit 85e4bb4

File tree

18 files changed

+571
-150
lines changed

18 files changed

+571
-150
lines changed

‎IntegrationTests/Package.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ let package = Package(
2828
.copy("Fixtures/SingleTestTarget"),
2929
.copy("Fixtures/SingleExecutableTarget"),
3030
.copy("Fixtures/MixedTargets"),
31+
.copy("Fixtures/TargetsWithDependencies"),
3132
.copy("Fixtures/TargetWithDocCCatalog"),
3233
.copy("Fixtures/PackageWithSnippets"),
3334
.copy("Fixtures/PackageWithConformanceSymbols"),

‎IntegrationTests/Tests/CombinedDocumentationTests.swift

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// This source file is part of the Swift.org open source project
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
8+
9+
import XCTest
10+
11+
final class CombinedDocumentationTests: ConcurrencyRequiringTestCase {
12+
func testCombinedDocumentation() throws {
13+
#if compiler(>=6.0)
14+
let result = try swiftPackage(
15+
"generate-documentation",
16+
"--enable-experimental-combined-documentation",
17+
"--verbose", // Necessary to see the 'docc convert' calls in the log and verify their parameters.
18+
workingDirectory: try setupTemporaryDirectoryForFixture(named: "TargetsWithDependencies")
19+
)
20+
21+
result.assertExitStatusEquals(0)
22+
let outputArchives = result.referencedDocCArchives
23+
XCTAssertEqual(outputArchives.count, 1)
24+
XCTAssertEqual(outputArchives.map(\.lastPathComponent), [
25+
"TargetsWithDependencies.doccarchive",
26+
])
27+
28+
// Verify that the combined archive contains all target's documentation
29+
30+
let combinedArchiveURL = try XCTUnwrap(outputArchives.first)
31+
let combinedDataDirectoryContents = try filesIn(.dataSubdirectory, of: combinedArchiveURL)
32+
.map(\.relativePath)
33+
.sorted()
34+
35+
XCTAssertEqual(combinedDataDirectoryContents, [
36+
"documentation.json",
37+
"documentation/innerfirst.json",
38+
"documentation/innerfirst/somethingpublic.json",
39+
"documentation/innersecond.json",
40+
"documentation/innersecond/somethingpublic.json",
41+
"documentation/nestedinner.json",
42+
"documentation/nestedinner/somethingpublic.json",
43+
"documentation/outer.json",
44+
"documentation/outer/somethingpublic.json",
45+
])
46+
47+
// Verify that each 'docc convert' call was passed the expected dependencies
48+
49+
let doccConvertCalls = result.standardOutput
50+
.components(separatedBy: .newlines)
51+
.filter { line in
52+
line.hasPrefix("docc invocation: '") && line.utf8.contains("docc convert ".utf8)
53+
}.map { line in
54+
line.trimmingCharacters(in: CharacterSet(charactersIn: "'"))
55+
.components(separatedBy: .whitespaces)
56+
.drop(while: { $0 != "convert" })
57+
}
58+
59+
XCTAssertEqual(doccConvertCalls.count, 4)
60+
61+
func extractDependencyArchives(targetName: String, file: StaticString = #filePath, line: UInt = #line) throws -> [String] {
62+
let arguments = try XCTUnwrap(
63+
doccConvertCalls.first(where: { $0.contains(["--fallback-display-name", targetName]) }),
64+
file: file, line: line
65+
)
66+
var dependencyPaths: [URL] = []
67+
68+
var remaining = arguments[...]
69+
while !remaining.isEmpty {
70+
remaining = remaining.drop(while: { $0 != "--dependency" }).dropFirst(/* the '--dependency' element */)
71+
if let path = remaining.popFirst() {
72+
dependencyPaths.append(URL(fileURLWithPath: path))
73+
}
74+
}
75+
76+
return dependencyPaths.map { $0.lastPathComponent }.sorted()
77+
}
78+
// Outer
79+
// ├─ InnerFirst
80+
// ╰─ InnerSecond
81+
// ╰─ NestedInner
82+
83+
XCTAssertEqual(try extractDependencyArchives(targetName: "Outer"), [
84+
"InnerFirst.doccarchive",
85+
"InnerSecond.doccarchive",
86+
], "The outer target has depends on both inner targets")
87+
88+
XCTAssertEqual(try extractDependencyArchives(targetName: "InnerFirst"), [], "The first inner target has no dependencies")
89+
90+
XCTAssertEqual(try extractDependencyArchives(targetName: "InnerSecond"), [
91+
"NestedInner.doccarchive",
92+
], "The second inner target has depends on the nested inner target")
93+
94+
XCTAssertEqual(try extractDependencyArchives(targetName: "NestedInner"), [], "The nested inner target has no dependencies")
95+
#else
96+
XCTSkip("This test requires a Swift-DocC version that support the link-dependencies feature")
97+
#endif
98+
}
99+
}

‎IntegrationTests/Tests/Fixtures/TargetsWithDependencies/Package.swift

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// swift-tools-version: 5.7
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
10+
11+
import Foundation
12+
import PackageDescription
13+
14+
let package = Package(
15+
name: "TargetsWithDependencies",
16+
targets: [
17+
// Outer
18+
// ├─ InnerFirst
19+
// ╰─ InnerSecond
20+
// ╰─ NestedInner
21+
.target(name: "Outer", dependencies: [
22+
"InnerFirst",
23+
"InnerSecond",
24+
]),
25+
.target(name: "InnerFirst"),
26+
.target(name: "InnerSecond", dependencies: [
27+
"NestedInner"
28+
]),
29+
.target(name: "NestedInner"),
30+
]
31+
)
32+
33+
// We only expect 'swift-docc-plugin' to be a sibling when this package
34+
// is running as part of a test.
35+
//
36+
// This allows the package to compile outside of tests for easier
37+
// test development.
38+
if FileManager.default.fileExists(atPath: "../swift-docc-plugin") {
39+
package.dependencies += [
40+
.package(path: "../swift-docc-plugin"),
41+
]
42+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This source file is part of the Swift.org open source project
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
8+
9+
/// This is a public struct and should be included in the documentation for this library.
10+
public struct SomethingPublic {}
11+
12+
/// This is an internal struct and should not be included in the documentation for this library.
13+
struct SomethingInternal {}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This source file is part of the Swift.org open source project
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
8+
9+
/// This is a public struct and should be included in the documentation for this library.
10+
public struct SomethingPublic {}
11+
12+
/// This is an internal struct and should not be included in the documentation for this library.
13+
struct SomethingInternal {}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This source file is part of the Swift.org open source project
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
8+
9+
/// This is a public struct and should be included in the documentation for this library.
10+
public struct SomethingPublic {}
11+
12+
/// This is an internal struct and should not be included in the documentation for this library.
13+
struct SomethingInternal {}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This source file is part of the Swift.org open source project
2+
//
3+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
8+
9+
/// This is a public struct and should be included in the documentation for this library.
10+
public struct SomethingPublic {}
11+
12+
/// This is an internal struct and should not be included in the documentation for this library.
13+
struct SomethingInternal {}

‎Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import PackagePlugin
4646
if isCombinedDocumentationEnabled, doccFeatures?.contains(.linkDependencies) == false {
4747
// The developer uses the combined documentation plugin flag with a DocC version that doesn't support combined documentation.
4848
Diagnostics.error("""
49-
Unsupported use of '\(DocumentedFlag.enableCombinedDocumentation.names.preferred)'. \
49+
Unsupported use of '\(DocumentedArgument.enableCombinedDocumentation.names.preferred)'. \
5050
DocC version at '\(doccExecutableURL.path)' doesn't support combined documentation.
5151
""")
5252
return
@@ -209,20 +209,26 @@ import PackagePlugin
209209
combinedArchiveOutput = URL(fileURLWithPath: context.pluginWorkDirectory.appending(combinedArchiveName).string)
210210
}
211211

212-
var mergeCommandArguments = ["merge"]
213-
mergeCommandArguments.append(contentsOf: intermediateDocumentationArchives.map(\.standardizedFileURL.path))
214-
mergeCommandArguments.append(contentsOf: [DocCArguments.outputPath.preferred, combinedArchiveOutput.path])
212+
var mergeCommandArguments = CommandLineArguments(
213+
["merge"] + intermediateDocumentationArchives.map(\.standardizedFileURL.path)
214+
)
215+
mergeCommandArguments.insertIfMissing(DocCArguments.outputPath, value: combinedArchiveOutput.path)
215216

216217
if let doccFeatures, doccFeatures.contains(.synthesizedLandingPageName) {
217-
mergeCommandArguments.append(contentsOf: [DocCArguments.synthesizedLandingPageName.preferred, context.package.displayName])
218-
mergeCommandArguments.append(contentsOf: [DocCArguments.synthesizedLandingPageKind.preferred, "Package"])
218+
mergeCommandArguments.insertIfMissing(DocCArguments.synthesizedLandingPageName, value: context.package.displayName)
219+
mergeCommandArguments.insertIfMissing(DocCArguments.synthesizedLandingPageKind, value: "Package")
219220
}
220221

221222
// Remove the combined archive if it already exists
222223
try? FileManager.default.removeItem(at: combinedArchiveOutput)
223224

225+
if verbose {
226+
let arguments = mergeCommandArguments.remainingArguments.joined(separator: " ")
227+
print("docc invocation: '\(doccExecutableURL.path) \(arguments)'")
228+
}
229+
224230
// Create a new combined archive
225-
let process = try Process.run(doccExecutableURL, arguments: mergeCommandArguments)
231+
let process = try Process.run(doccExecutableURL, arguments: mergeCommandArguments.remainingArguments)
226232
process.waitUntilExit()
227233

228234
print("""

‎Sources/Snippets/Model/Snippet.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// This source file is part of the Swift.org open source project
22
//
3-
// Copyright (c) 2022 Apple Inc. and the Swift project authors
3+
// Copyright (c) 2022-2024 Apple Inc. and the Swift project authors
44
// Licensed under Apache License v2.0 with Runtime Library Exception
55
//
66
// See https://swift.org/LICENSE.txt for license information
@@ -12,7 +12,7 @@ import Foundation
1212
///
1313
/// A *snippet* is a short, focused code example that can be shown with little to no context or prose.
1414
public struct Snippet {
15-
/// The ``URL`` of the source file for this snippet.
15+
/// The URL of the source file for this snippet.
1616
public var sourceFile: URL
1717

1818
/// A short abstract explaining what the snippet does.
@@ -39,8 +39,7 @@ public struct Snippet {
3939

4040
/// Create a Swift snippet by parsing a file.
4141
///
42-
/// - parameter sourceURL: The URL of the file to parse.
43-
/// - parameter syntax: The name of the syntax of the source file if known.
42+
/// - Parameter sourceFile: The URL of the file to parse.
4443
public init(parsing sourceFile: URL) throws {
4544
let source = try String(contentsOf: sourceFile)
4645
self.init(parsing: source, sourceFile: sourceFile)

‎Sources/SwiftDocCPluginUtilities/CommandLineArguments/CommandLineArgument.swift

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,79 @@ public struct CommandLineArgument {
4040
/// An option argument with an associated value.
4141
///
4242
/// For example: `"--some-option", "value"` or `"--some-option=value"`.
43-
case option(value: String)
43+
case option(value: String, kind: Option.Kind)
4444
}
4545

46-
/// Creates a new command line flag with the given names.
47-
/// - Parameters:
48-
/// - names: The names for the new command line flag.
49-
public static func flag(_ names: Names) -> Self {
50-
.init(names: names, kind: .flag)
46+
// Only create arguments from flags or options (with a value)
47+
48+
init(_ flag: Flag) {
49+
names = flag.names
50+
kind = .flag
51+
}
52+
53+
init(_ option: Option, value: String) {
54+
names = option.names
55+
kind = .option(value: value, kind: option.kind)
56+
}
57+
}
58+
59+
extension CommandLineArgument {
60+
/// A flag argument without an associated value.
61+
///
62+
/// For example: `"--some-flag"`.
63+
public struct Flag {
64+
/// The names of this command line flag.
65+
public var names: Names
66+
/// The negative names for this flag, if any.
67+
public var inverseNames: CommandLineArgument.Names?
68+
69+
/// Creates a new command line flag.
70+
///
71+
/// - Parameters:
72+
/// - preferred: The preferred name for this flag.
73+
/// - alternatives: A collection of alternative names for this flag.
74+
/// - inverseNames: The negative names for this flag, if any.
75+
public init(preferred: String, alternatives: Set<String> = [], inverseNames: CommandLineArgument.Names? = nil) {
76+
// This is duplicating the `Names` parameters to offer a nicer initializer for the common case.
77+
names = .init(preferred: preferred, alternatives: alternatives)
78+
self.inverseNames = inverseNames
79+
}
5180
}
5281

53-
/// Creates a new command option with the given names and associated value.
54-
/// - Parameters:
55-
/// - names: The names for the new command line option.
56-
/// - value: The value that's associated with this command line option.
57-
public static func option(_ names: Names, value: String) -> Self {
58-
.init(names: names, kind: .option(value: value))
82+
/// An option argument that will eventually associated with a value.
83+
///
84+
/// For example: `"--some-option", "value"` or `"--some-option=value"`.
85+
public struct Option {
86+
/// The names of this command line option.
87+
public var names: Names
88+
/// The kind of value for this command line option.
89+
public var kind: Kind
90+
91+
/// A kind of value(s) that a command line option supports.
92+
public enum Kind {
93+
/// An option that supports a single value.
94+
case singleValue
95+
/// An option that supports an array of different values.
96+
case arrayOfValues
97+
}
98+
99+
/// Creates a new command line option.
100+
///
101+
/// - Parameters:
102+
/// - preferred: The preferred name for this option.
103+
/// - alternatives: A collection of alternative names for this option.
104+
/// - kind: The kind of value(s) that this option supports.
105+
public init(preferred: String, alternatives: Set<String> = [], kind: Kind = .singleValue) {
106+
// This is duplicating the `Names` parameters to offer a nicer initializer for the common case.
107+
self.init(
108+
Names(preferred: preferred, alternatives: alternatives),
109+
kind: kind
110+
)
111+
}
112+
113+
init(_ names: Names, kind: Kind = .singleValue) {
114+
self.names = names
115+
self.kind = kind
116+
}
59117
}
60118
}

0 commit comments

Comments
 (0)