Merge pull request swiftlang#258 from google/format-doc-comments

Fix parameter validation for functions with labeled arguments; add su…

Author: dabelknap

Sources/SwiftFormat/Pipelines+Generated.swift

@@ -158,6 +158,7 @@ extension LintPipeline {
     _ = AllPublicDeclarationsHaveDocumentation(context: context).visit(node)
     _ = BeginDocumentationCommentWithOneLineSummary(context: context).visit(node)
     _ = UseTripleSlashForDocumentationComments(context: context).visit(node)
+    _ = ValidateDocumentationComments(context: context).visit(node)
     return .visitChildren
   }
 

Sources/SwiftFormatRules/ValidateDocumentationComments.swift

@@ -29,7 +29,23 @@ public struct ValidateDocumentationComments: SyntaxLintRule {
     self.context = context
   }
 
+  public func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+    return checkFunctionLikeDocumentation(
+      node, name: "init", parameters: node.parameters.parameterList)
+  }
+
   public func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
+    return checkFunctionLikeDocumentation(
+      node, name: node.identifier.text, parameters: node.signature.input.parameterList,
+      returnClause: node.signature.output)
+  }
+
+  private func checkFunctionLikeDocumentation(
+    _ node: DeclSyntax,
+    name: String,
+    parameters: FunctionParameterListSyntax,
+    returnClause: ReturnClauseSyntax? = nil
+  ) -> SyntaxVisitorContinueKind {
     guard let declComment = node.docComment else { return .skipChildren }
     guard let commentInfo = node.docCommentInfo else { return .skipChildren }
     guard let params = commentInfo.parameters else { return .skipChildren }
@@ -48,8 +64,8 @@ public struct ValidateDocumentationComments: SyntaxLintRule {
     let hasPluralDesc = declComment.components(separatedBy: .newlines)
       .contains { $0.trimmingCharacters(in: .whitespaces).starts(with: "- Parameters") }
 
-    validateReturn(node, returnDesc: commentInfo.returnsDescription)
-    let funcParameters = funcParametersIdentifiers(in: node.signature.input.parameterList)
+    validateReturn(returnClause, name: name, returnDesc: commentInfo.returnsDescription)
+    let funcParameters = funcParametersIdentifiers(in: parameters)
 
     // If the documentation of the parameters is wrong 'docCommentInfo' won't
     // parse the parameters correctly. First the documentation has to be fix
@@ -67,20 +83,20 @@ public struct ValidateDocumentationComments: SyntaxLintRule {
     // are the same.
     if (params.count != funcParameters.count) ||
       !parametersAreEqual(params: params, funcParam: funcParameters) {
-      diagnose(.parametersDontMatch(funcName: node.identifier.text), on: node)
+      diagnose(.parametersDontMatch(funcName: name), on: node)
     }
 
     return .skipChildren
   }
 
   /// Ensures the function has a return documentation if it actually returns
   /// a value.
-  func validateReturn(_ node: FunctionDeclSyntax, returnDesc: String?) {
-    if node.signature.output == nil && returnDesc != nil {
-      diagnose(.removeReturnComment(funcName: node.identifier.text), on: node)
+  func validateReturn(_ returnClause: ReturnClauseSyntax?, name: String, returnDesc: String?) {
+    if returnClause == nil && returnDesc != nil {
+      diagnose(.removeReturnComment(funcName: name), on: returnClause)
     }
-    else if node.signature.output != nil && returnDesc == nil {
-      diagnose(.documentReturnValue(funcName: node.identifier.text), on: node)
+    else if returnClause != nil && returnDesc == nil {
+      diagnose(.documentReturnValue(funcName: name), on: returnClause)
     }
   }
 }
@@ -89,9 +105,13 @@ public struct ValidateDocumentationComments: SyntaxLintRule {
 /// paramters identifiers.
 func funcParametersIdentifiers(in paramList: FunctionParameterListSyntax) -> [String] {
   var funcParameters = [String]()
-  for parameter in paramList
-  {
-    guard let parameterIdentifier = parameter.firstName else { continue }
+  for parameter in paramList {
+    // If there is a label and an identifier, then the identifier (`secondName`) is the name that
+    // should be documented. Otherwise, the label and identifier are the same, occupying
+    // `firstName`.
+    guard let parameterIdentifier = parameter.secondName ?? parameter.firstName else {
+      continue
+    }
     funcParameters.append(parameterIdentifier.text)
   }
   return funcParameters

Tests/SwiftFormatRulesTests/ValidateDocumentationCommentsTests.swift

@@ -135,4 +135,95 @@ public class ValidateDocumentationCommentsTests: DiagnosingTestCase {
     XCTAssertNotDiagnosed(.removeReturnComment(funcName: "ommitedFunc"))
     XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "ommitedFunc"))
   }
+
+  public func testSeparateLabelAndIdentifier() {
+    let input =
+    """
+    /// Returns the output generated by executing a command.
+    ///
+    /// - Parameter command: The command to execute in the shell environment.
+    /// - Returns: A string containing the contents of the invoked process's
+    ///   standard output.
+    func incorrectParam(label commando: String) -> String {
+    // ...
+    }
+
+    /// Returns the output generated by executing a command.
+    ///
+    /// - Parameter command: The command to execute in the shell environment.
+    /// - Returns: A string containing the contents of the invoked process's
+    ///   standard output.
+    func singularParam(label command: String) -> String {
+    // ...
+    }
+
+    /// Returns the output generated by executing a command with the given string
+    /// used as standard input.
+    ///
+    /// - Parameters:
+    ///   - command: The command to execute in the shell environment.
+    ///   - stdin: The string to use as standard input.
+    /// - Returns: A string containing the contents of the invoked process's
+    ///   standard output.
+    func pluralParam(label command: String, label2 stdin: String) -> String {
+    // ...
+    }
+    """
+    performLint(ValidateDocumentationComments.self, input: input)
+    XCTAssertNotDiagnosed(.useSingularParameter)
+    XCTAssertNotDiagnosed(.usePluralParameters)
+
+    XCTAssertDiagnosed(.parametersDontMatch(funcName: "incorrectParam"))
+
+    XCTAssertNotDiagnosed(.documentReturnValue(funcName: "singularParam"))
+    XCTAssertNotDiagnosed(.removeReturnComment(funcName: "singularParam"))
+    XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "singularParam"))
+
+    XCTAssertNotDiagnosed(.documentReturnValue(funcName: "pluralParam"))
+    XCTAssertNotDiagnosed(.removeReturnComment(funcName: "pluralParam"))
+    XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "pluralParam"))
+  }
+
+  public func testInitializer() {
+    let input =
+    """
+    /// Brief summary.
+    ///
+    /// - Parameter command: The command to execute in the shell environment.
+    /// - Returns: Shouldn't be here.
+    init(label commando: String) {
+    // ...
+    }
+
+    /// Brief summary.
+    ///
+    /// - Parameter command: The command to execute in the shell environment.
+    init(label command: String) {
+    // ...
+    }
+
+    /// Brief summary.
+    ///
+    /// - Parameters:
+    ///   - command: The command to execute in the shell environment.
+    ///   - stdin: The string to use as standard input.
+    init(label command: String, label2 stdin: String) {
+    // ...
+    }
+    """
+    performLint(ValidateDocumentationComments.self, input: input)
+    XCTAssertNotDiagnosed(.useSingularParameter)
+    XCTAssertNotDiagnosed(.usePluralParameters)
+
+    XCTAssertDiagnosed(.parametersDontMatch(funcName: "init"))
+    XCTAssertDiagnosed(.removeReturnComment(funcName: "init"))
+
+    XCTAssertNotDiagnosed(.documentReturnValue(funcName: "init"))
+    XCTAssertNotDiagnosed(.removeReturnComment(funcName: "init"))
+    XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "init"))
+
+    XCTAssertNotDiagnosed(.documentReturnValue(funcName: "init"))
+    XCTAssertNotDiagnosed(.removeReturnComment(funcName: "init"))
+    XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "init"))
+  }
 }

Tests/SwiftFormatRulesTests/XCTestManifests.swift

@@ -53,6 +53,7 @@ extension BlankLineBetweenMembersTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__BlankLineBetweenMembersTests = [
+        ("testBlankLineBeforeFirstChildOrNot", testBlankLineBeforeFirstChildOrNot),
         ("testInvalidBlankLineBetweenMembers", testInvalidBlankLineBetweenMembers),
         ("testNestedMembers", testNestedMembers),
         ("testTwoMembers", testTwoMembers),
@@ -381,9 +382,11 @@ extension ValidateDocumentationCommentsTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__ValidateDocumentationCommentsTests = [
+        ("testInitializer", testInitializer),
         ("testParameterDocumentation", testParameterDocumentation),
         ("testParametersName", testParametersName),
         ("testReturnDocumentation", testReturnDocumentation),
+        ("testSeparateLabelAndIdentifier", testSeparateLabelAndIdentifier),
         ("testValidDocumentation", testValidDocumentation),
     ]
 }