Validate and filter parameter and return value documentation

[d-ronnqvist]

Bug/issue #, if applicable: rdar://118739612

Summary

This adds validation and filtering of parameter and return value documentation based on a symbol's function signature in each language representation.

Note
This PR isn't as big as it looks. ~2k added lines are test symbol graph files.

This results in 3 new behaviors:

• A parameter or return value that's not applicable to a specific language representation of a symbol isn't displayed in that language's variant of the page.
• Objective-C error parameters or return values that are not documented for symbols defined in other languages are synthesized if any other parameters are documented.
• User facing diagnostics with fixits are emitted for parameters that are misspelled, not found, or repeated and for return values for symbols that return void in all language representations.

Filtering parameters

If a symbol has different parameters and return values in different languages, then each language's version of that symbol's page will only display the parameters that's applicable to that language.

This is for example relevant for API with errors in Objective-C that bridge to API that throws in Swift;

/// Does something in Objective-C
/// - Parameters:
///   - someValue: Some value.
///   - error: On input, a pointer to an error object. If an error occurs, this pointer is set to an actual error object containing the error information.
/// - Returns: `YES` if doing something was successful, or `NO` if an error occurred.
- (BOOL)doSomethingWith:(NSInteger)someValue
                  error:(NSError **)error;

This method become func doSomething(with someValue: Int) throws in Swift which returns Void and doesn't have an error parameter, so displaying documentation for the error parameter and the return value wouldn't be helpful for someone reading the Swift version of that method's documentation.

Before this PR we displayed all documented parameters in all language variants of a symbol's page. With these changes, the Swift page won't display the "error" parameter documentation or the return value documentation.

Adding Objective-C error documentation

The above works well for functions defined in Objective-C that document parameters and return values that aren't available in Swift, but it doesn't work in the other direction;

/// Does something in Swift.
/// - Parameter someValue: Some value.
/// - Throws: Some error if something does wrong
@objc public func doSomething(with someValue: Int) throws { }

This becomes - (BOOL)doSomethingWith:(NSInteger)someValue error:(NSError **)error; in Objective-C but the Swift documentation comment doesn't have any documentation for the Objective-C "error" parameter or the Objective-C specific return value.

Before this PR we only displayed the developer documented parameters. With these changes, the Objective-C page will display a synthesized "error" parameter documentation with the generic description:

error
On input, a pointer to an error object. If an error occurs, this pointer is set to an actual error object containing the error information.

and a synthesized return value documentation with the generic description:

Return Value
YES if successful, or NO if an error occurred.

If the throwing Swift function was defined with a non-null return value that bridges to a nullable return value in Objective-C;

/// Returns something from Swift.
/// - Returns: Some string.
/// - Throws: Some error if something does wrong
@objc public func returnSomething() throws -> String { "" }

Then the Objective-C return value documentation is appended with a generic description of this automatic bridging behavior:

Return Value
Some string. If an error occurs, this method returns nil and assigns an appropriate error object to the error parameter.

Parameter and return value diagnostics

If the symbol documents at least one parameter or its return value, then DocC will validate that symbol's parameters section and return value section. Specifically, DocC will warn in 5 cases:

1. A parameter is documented more than once
2. A parameter that doesn't exist in the function signature is documented
3. A parameter from the function signature isn't documented
4. A parameter is documented using its argument label instead of its parameter name
5. A symbol that returns Void has documented a return value

The Swift function below shows minimal examples of each of these cases:

/// - Parameters:
///   - first: Something
///   - first: Something else (1)
///   - with: Something (4)
///   - third: Something (2)
///  (3)
/// - Returns: Something (5)
func doSomething(with first: Int, and second: Int) { }

Dependencies

swiftlang/swift-docc-symbolkit#64

Testing

• In a multi-language project, add:

  • Objective-C API with errors with documentation for all parameters and return values
  • Swift @objc API that throws with documentation for the Swift parameters.

• Build documentation for the project and pass --enable-experimental-parameters-and-returns-validation.

• View both the Swift and Objective-C versions of the documentation pages for both the API defined in Objective-C and the API defined in Swift.

  • Each language variant of the documentation pages should only display the parameters and return values applicable to that language.

• Add parameter documentation for parameters that doesn't exist to the Swift and Objective-C definitions and build documentation again.

• View both the Swift and Objective-C versions of the documentation pages for both the API defined in Objective-C and the API defined in Swift.

  • The documentation for the parameters that doesn't exist shouldn't be displayed on either languages's variant of the documentation page.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

• Added tests
• Ran the ./bin/test script and it succeeded
• Updated documentation if necessary

[d-ronnqvist]

@swift-ci please test

[d-ronnqvist]

@swift-ci please test

[patshaughnessy]

Amazing work, and a great improvement for DocC!

I'm about halfway through reviewing this. I've read the code and have a number of questions for you. Most of my concerns are about understand your new algorithms... in some places it's a bit hard to follow. Some more code comments and different variable names might help a lot.

Next week I'll find some time to test this more carefully and review the test code.

[patshaughnessy]

I feel like you might be able to refactor this function and avoid these separate sorting loops for each trait. Above you iterate over the signature parameter also... If you could find a way to add each language's parameter in the loop above then the result array would already be sorted.

[d-ronnqvist]

I could avoid one loop of the parameters but there is a sequence to these steps; the sorted parameters for the trait needs to know the parameters that's applicable to that language which in turn needs to know the parameter names.

[d-ronnqvist]

@swift-ci please test

[d-ronnqvist]

@patshaughnessy I addressed all the feedback. Would you mind taking another look?

[patshaughnessy]

Fantastic. Thank you for adding all those new comments and for renaming the local variables. Helps a lot to understand the new code.

Maybe fix a typo and merge it 👍

[patshaughnessy]

Thanks for this explanation! Love the ascii-art :)

[patshaughnessy]

Can we be sure first will return the proper parameter? Does it matter? Is the call to (grouping: by:) above guaranteed to retain the original order?

[d-ronnqvist]

Yes, the order is documented to be the same as the original order. It mostly doesn't matter since documenting the same parameter more than once results in a warning but it's nice that the behavior is stable across documentation builds even when there are warnings.

The arrays in the “values” position of the new dictionary each contain at least one element, with the elements in the same order as the source sequence.

[patshaughnessy]

Typo/spelling - and throughout this appears in various places.

[patshaughnessy]

I love these examples of the error messages and how they will appear - thank you!

[patshaughnessy]

Why include /// here? Does that make the replacement ("Fix It" in the Xcode UI?) look better?

[d-ronnqvist]

Because the fix it inserts the placeholder text for the new parameter on its own line we need to include "///" after the newline so that the new line is part of the documentation comment.

For example, consider this declaration and documentation comment where the "first" parameter is undocumented and should be inserted before the "second".

/// - Paramaters:
///   - second: Some description of the first parameter
func someFunction(first: Int, second: Int) {}

Inserting only - first: placeholder without a newline would result in

/// - Paramaters:
///   - first: placeholder- second: Some description of the first parameter
func someFunction(first: Int, second: Int) {}

Inserting only - parameterName: placeholder\n with a newline would result in

/// - Paramaters:
///   - first: placeholder
- second: Some description of the first parameter
func someFunction(first: Int, second: Int) {}

But inserting - parameterName: placeholder\n/// with a newline, a triple slash, and the same leading whitespace as the previous parameter had would result in

/// - Paramaters:
///   - first: placeholder
///   - second: Some description of the first parameter
func someFunction(first: Int, second: Int) {}

[d-ronnqvist]

@swift-ci please test