Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve heuristic for extending return value with "on failure" info #1096

Merged
16 changes: 14 additions & 2 deletions Sources/SwiftDocC/Model/ParametersAndReturnValidator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ struct ParametersAndReturnValidator {
}
}

if returns.contents.contains(where: { $0.format().lowercased().contains("error") }) {
// If the existing returns value documentation mentions "error" at all, don't add anything
if returns.possiblyDocumentsFailureBehavior() {
// If the existing returns value documentation appears to describe the failure / error behavior, don't add anything
return nil
}
if signatures[.objectiveC]?.returns == [.init(kind: .typeIdentifier, spelling: "BOOL", preciseIdentifier: "c:@T@BOOL")] {
Expand Down Expand Up @@ -664,6 +664,18 @@ struct ParametersAndReturnValidator {

// MARK: Helper extensions

private extension Return {
/// Applies a basic heuristic to give an indication if the return value documentation possibly documents what happens when an error occurs
func possiblyDocumentsFailureBehavior() -> Bool {
contents.contains(where: { markup in
let formatted = markup.format().lowercased()
// Check if the authored markup contains one of a handful of words as an indication that it possibly documents what happens when an error occurs.
return ["error", "`nil`", "fails", "failure"].contains(where: { word in
d-ronnqvist marked this conversation as resolved.
Show resolved Hide resolved
formatted.contains(word)
})
}
}

private extension SymbolGraph.Symbol.FunctionSignature {
mutating func merge(with signature: Self, selector: UnifiedSymbolGraph.Selector) {
// An internal helper function that compares parameter names
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ When a symbol has different parameters or return values in different source lang
```objc
/// - Parameters:
/// - someValue: Some description of this parameter.
/// - error: On output, a pointer to an error object that describes why "doing somehting" failed, or `nil` if no error occurred.
/// - error: On output, a pointer to an error object that describes why "doing something" failed, or `nil` if no error occurred.
/// - Returns: `YES` if "doing something" was successful, `NO` if an error occurred.
- (BOOL)doSomethingWith:(NSInteger)someValue
error:(NSError **)error;
Expand All @@ -52,12 +52,43 @@ func doSomething(with someValue: Int) throws

Because the Swift representation of this method only has the "someValue" parameter and no return value, DocC hides the "error" parameter documentation and the return value documentation from the Swift version of this symbol's page.

You don't need to document the Objective-C representation's "error" parameter or the Objective-C specific return value for methods that correspond to throwing functions in Swift.
DocC automatically adds a generic description for the "error" parameter for the Objective-C version of that symbol's page.

You don't need to document the Objective-C representation's "error" parameter or Objective-C specific return value for symbols defined in Swift.
DocC automatically adds a generic description for the "error" parameter and extends your return value documentation to describe the Objective-C specific return value behavior.
When the Swift representation returns `Void`---which corresponds to a `BOOL` return value in Objective-C (like in the example above)---DocC adds a generic description of the `BOOL` return value to the Objective-C version of that symbol's page.

If you want to customize this documentation you can manually document the "error" parameter and return value.
When the Swift representation returns a value---which corresponds to a `nullable` return value in Objective-C---DocC extends your return value documentation, for the Objective-C version of that symbol's page, to describe that the methods returns `nil` if an error occurs unless your documentation already covers this behavior.
For example, consider this throwing function in Swift and its corresponding Objective-C interface:

**Swift definition**

```swift
/// - Parameters:
/// - someValue: Some description of this parameter.
/// - Returns: Some description of this return value.
@objc func doSomething(with someValue: Int) throws -> String
```

**Generated Objective-C interface**

```objc
- (nullable NSString *)doSomethingWith:(NSInteger)someValue
error:(NSError **)error;
```

For the Swift representation, with one parameter and a non-optional return value,
DocC displays the "someValue" parameter documentation and return value documentation as-is.
For the Objective-C representation, with two parameters and a nullable return value,
DocC displays the "someValue" parameter documentation, generic "error" parameter documentation, and extends the return value documentation with a generic description about the Objective-C specific `nil` return value when the method encounters an error.

If you want to customize this documentation you can manually document the "error" parameter and return value.
Doing so won't change the Swift version of that symbol's page.
DocC will still hide the parameter and return value documentation that doesn't apply to each source language's version of that symbol's page.

If your return value documentation already describes the `nil` return value, DocC won't extent it with the generic `nil` value description.
d-ronnqvist marked this conversation as resolved.
Show resolved Hide resolved

> Note:
> If you document the Objective-C specific `nil` return value for a method that correspond to a throwing function in Swift,
> DocC will display that documentation as-is on the Swift version of that page where is has a non-optional return type.
d-ronnqvist marked this conversation as resolved.
Show resolved Hide resolved
d-ronnqvist marked this conversation as resolved.
Show resolved Hide resolved

<!-- Copyright (c) 2024 Apple Inc and the Swift Project authors. All Rights Reserved. -->
56 changes: 56 additions & 0 deletions Tests/SwiftDocCTests/Model/ParametersAndReturnValidatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,62 @@ class ParametersAndReturnValidatorTests: XCTestCase {
}
}

func testExtendsReturnValueDocumentation() throws {
for (returnValueDescription, expectsExtendedDocumentation) in [
("Returns some value.", true),
d-ronnqvist marked this conversation as resolved.
Show resolved Hide resolved
("Returns some value, except if the function fails.", false),
("Returns some value. If an error occurs, this function doesn't return a value.", false),
("Returns some value. On failure, this function doesn't return a value.", false),
("Returns some value. If something happens, this function returns `nil` instead.", false),
("Returns some value, or `nil` if something goes wrong.", false),
] {
let catalog = Folder(name: "unit-test.docc", content: [
Folder(name: "swift", content: [
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
docComment: nil,
sourceLanguage: .swift,
parameters: [],
returnValue: .init(kind: .typeIdentifier, spelling: "String", preciseIdentifier: "s:SS")
))
]),
Folder(name: "clang", content: [
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
docComment: """
Some function description

- Returns: \(returnValueDescription)
""",
sourceLanguage: .objectiveC,
parameters: [(name: "error", externalName: nil)],
returnValue: .init(kind: .typeIdentifier, spelling: "NSString", preciseIdentifier: "c:objc(cs)NSString")
))
])
])

let (bundle, context) = try loadBundle(catalog: catalog)

XCTAssert(context.problems.isEmpty, "Unexpected problems: \(context.problems.map(\.diagnostic.summary))")

let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/ModuleName/functionName(...)", sourceLanguage: .swift)
let node = try context.entity(with: reference)
let symbol = try XCTUnwrap(node.semantic as? Symbol)

let parameterSections = symbol.parametersSectionVariants
XCTAssertEqual(parameterSections[.swift]?.parameters.map(\.name), [], "The Swift variant has no error parameter")
XCTAssertEqual(parameterSections[.objectiveC]?.parameters.map(\.name), ["error"])
XCTAssertEqual(parameterSections[.objectiveC]?.parameters.last?.contents.map({ $0.format() }).joined(), "On output, a pointer to an error object that describes why the method failed, or `nil` if no error occurred. If you are not interested in the error information, pass `nil` for this parameter.")

let returnsSections = symbol.returnsSectionVariants
let expectedReturnValueDescription = returnValueDescription.replacingOccurrences(of: "\'", with: "’")
XCTAssertEqual(returnsSections[.swift]?.content.map({ $0.format() }).joined(), expectedReturnValueDescription)
if expectsExtendedDocumentation {
XCTAssertEqual(returnsSections[.objectiveC]?.content.map({ $0.format() }).joined(), "\(expectedReturnValueDescription) On failure, this method returns `nil`.")
} else {
XCTAssertEqual(returnsSections[.objectiveC]?.content.map({ $0.format() }).joined(), expectedReturnValueDescription)
}
}
}

func testParametersWithAlternateSignatures() throws {
let (_, _, context) = try testBundleAndContext(copying: "AlternateDeclarations") { url in
try """
Expand Down