From 2336b82999bb26886c3ba598923ea459a4667468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Fri, 20 Dec 2024 13:29:31 +0100 Subject: [PATCH] Improve heuristic for extending return value with "on failure" info (#1096) --- .../Model/ParametersAndReturnValidator.swift | 22 +++++- ...with-different-language-representations.md | 73 +++++++++++++++++-- .../params-1-objc.svg | 28 +++++++ .../params-1-objc~dark.svg | 27 +++++++ .../params-1-swift.svg | 24 ++++++ .../params-1-swift~dark.svg | 22 ++++++ .../params-2-objc.svg | 44 +++++++++++ .../params-2-objc~dark.svg | 40 ++++++++++ .../params-2-swift.svg | 17 +++++ .../params-2-swift~dark.svg | 15 ++++ .../params-3-objc.svg | 41 +++++++++++ .../params-3-objc~dark.svg | 40 ++++++++++ .../params-3-swift.svg | 21 ++++++ .../params-3-swift~dark.svg | 19 +++++ .../ParametersAndReturnValidatorTests.swift | 64 ++++++++++++++++ 15 files changed, 489 insertions(+), 8 deletions(-) rename Sources/docc/DocCDocumentation.docc/{ => Documenting API with different language representations}/documenting-api-with-different-language-representations.md (50%) create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc~dark.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift~dark.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc~dark.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift~dark.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc~dark.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift.svg create mode 100644 Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift~dark.svg diff --git a/Sources/SwiftDocC/Model/ParametersAndReturnValidator.swift b/Sources/SwiftDocC/Model/ParametersAndReturnValidator.swift index 3b6dfe2ad4..dc620649a3 100644 --- a/Sources/SwiftDocC/Model/ParametersAndReturnValidator.swift +++ b/Sources/SwiftDocC/Model/ParametersAndReturnValidator.swift @@ -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")] { @@ -664,6 +664,24 @@ 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 returnValueDescribesErrorRegex.firstMatch(in: formatted, range: NSRange(formatted.startIndex ..< formatted.endIndex, in: formatted)) != nil + }) + } +} + +/// A regular expression that finds the words; "error", "fail", "fails", "failure", "failures", "nil", and "null". +/// These words only match at word boundaries or when surrounded by single backticks ("`"). +/// +/// This is used as a heuristic to give an indication if the return value documentation possibly documents what happens when an error occurs. +private let returnValueDescribesErrorRegex = try! NSRegularExpression(pattern: "(\\b|`)(error|fail(ure)?s?|nil|null)(\\b|`)", options: .caseInsensitive) + private extension SymbolGraph.Symbol.FunctionSignature { mutating func merge(with signature: Self, selector: UnifiedSymbolGraph.Selector) { // An internal helper function that compares parameter names diff --git a/Sources/docc/DocCDocumentation.docc/documenting-api-with-different-language-representations.md b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/documenting-api-with-different-language-representations.md similarity index 50% rename from Sources/docc/DocCDocumentation.docc/documenting-api-with-different-language-representations.md rename to Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/documenting-api-with-different-language-representations.md index 3f9db27dfd..5865ce3c83 100644 --- a/Sources/docc/DocCDocumentation.docc/documenting-api-with-different-language-representations.md +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/documenting-api-with-different-language-representations.md @@ -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; @@ -52,12 +52,73 @@ 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. +@Row { + @Column { + ![](params-1-swift) + } + @Column { + ![](params-1-objc) + } +} -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. - -If you want to customize this documentation you can manually document the "error" parameter and return value. +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. +If you want to customize this documentation you can manually document the "error" parameter. 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. + +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. + +@Row { + @Column { + ![](params-2-swift) + } + @Column { + ![](params-2-objc) + } +} + +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 general 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. + +@Row { + @Column { + ![](params-3-swift) + } + @Column { + ![](params-3-objc) + } +} + +> Tip: +> If you document the Objective-C specific `nil` return value for a method that corresponds to a throwing function in Swift, +> but don't provide more information to the reader than "On failure, this method returns `nil`", +> consider removing that written documentation to let DocC display its generic description about the Objective-C specific `nil` return value, +> only on the Objective-C version of that symbol's page. + +The return value documentation you write displays on both the Swift and Objective-C versions of that symbol's page. +DocC won't add its generic `nil` return value description to the Objective-C page, +if your return value documentation already describes that the method returns `nil` when it fails or encounters an error, +but that Objective-C specific documentation you've written will display on the Swift version of that page where the symbol has a non-optional return type. diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc.svg new file mode 100644 index 0000000000..42fd029a3d --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc.svg @@ -0,0 +1,28 @@ + + + Objective-C + + Parameters + someValue + + + error + + + + + + Return Value + + + + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc~dark.svg new file mode 100644 index 0000000000..8015a5489b --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-objc~dark.svg @@ -0,0 +1,27 @@ + + + Objective-C + + Parameters + someValue + + + error + + + + + + Return Value + + + + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift.svg new file mode 100644 index 0000000000..44ea8c24a0 --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift.svg @@ -0,0 +1,24 @@ + + + + Swift + + Parameters + someValue + + + + + + + + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift~dark.svg new file mode 100644 index 0000000000..f7e1d94d3d --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-1-swift~dark.svg @@ -0,0 +1,22 @@ + + + Swift + + Parameters + someValue + + + + + + + + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc.svg new file mode 100644 index 0000000000..e298319d2e --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc.svg @@ -0,0 +1,44 @@ + + + + Objective-C + + Parameters + someValue + + + + + + + + + error + 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. + + + + + Return Value + YES + if the method succeeded, otherwise + NO + . + + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc~dark.svg new file mode 100644 index 0000000000..5d8e612aae --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-objc~dark.svg @@ -0,0 +1,40 @@ + + + Objective-C + + Parameters + someValue + + + + + + + + error + 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. + + + + + Return Value + YES + if the method succeeded, otherwise + NO + . + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift.svg new file mode 100644 index 0000000000..bc155d504a --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift.svg @@ -0,0 +1,17 @@ + + + + Swift + + Parameters + someValue + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift~dark.svg new file mode 100644 index 0000000000..26ba158a62 --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-2-swift~dark.svg @@ -0,0 +1,15 @@ + + + Swift + + Parameters + someValue + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc.svg new file mode 100644 index 0000000000..ce5f0fa24c --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc.svg @@ -0,0 +1,41 @@ + + + Objective-C + + Parameters + someValue + + + + + + + + error + 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. + + + + + Return Value + + On failure, this method + returns + nil + . + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc~dark.svg new file mode 100644 index 0000000000..aaa2c1f09c --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-objc~dark.svg @@ -0,0 +1,40 @@ + + + Objective-C + + Parameters + someValue + + + + + + + + error + 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. + + + + + Return Value + + On failure, this method + returns + nil + . + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift.svg new file mode 100644 index 0000000000..994f41a56a --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift.svg @@ -0,0 +1,21 @@ + + + + Swift + + Parameters + someValue + + + Return Value + + diff --git a/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift~dark.svg b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift~dark.svg new file mode 100644 index 0000000000..cef742e73e --- /dev/null +++ b/Sources/docc/DocCDocumentation.docc/Documenting API with different language representations/params-3-swift~dark.svg @@ -0,0 +1,19 @@ + + + Swift + + Parameters + someValue + + + Return Value + + diff --git a/Tests/SwiftDocCTests/Model/ParametersAndReturnValidatorTests.swift b/Tests/SwiftDocCTests/Model/ParametersAndReturnValidatorTests.swift index 97a6b6bfad..0f88fc5af3 100644 --- a/Tests/SwiftDocCTests/Model/ParametersAndReturnValidatorTests.swift +++ b/Tests/SwiftDocCTests/Model/ParametersAndReturnValidatorTests.swift @@ -111,6 +111,70 @@ class ParametersAndReturnValidatorTests: XCTestCase { } } + func testExtendsReturnValueDocumentation() throws { + for (returnValueDescription, expectsExtendedDocumentation) in [ + // Expects to extend the documentation + ("Returns some value.", true), + ("Returns some failsafe value.", true), + ("Returns some errorless value.", true), + ("Returns a NSNull value.", true), + + // Expects to not extend the documentation + ("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), + ("Returns some value, or `NULL` if something goes wrong.", false), + ("Returns some value or a null-pointer.", 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 """