Nesting parameter/returns/throws doc comments for closure parameters

Under parameter doc comment list items, allow function doc comment
syntax to nest so you can document the meaning of closure parameters'
signatures.

rdar://problem/24794725

bindings/xml/comment-xml-schema.rng

@@ -650,9 +650,28 @@
           <!-- In general, template parameters with whitespace discussion
                should not be emitted, unless direction is explicitly specified.
                Schema might be more strict here. -->
-          <element name="Discussion">
-            <ref name="BlockContent" />
-          </element>
+          <choice>
+            <element name="ClosureParameter">
+              <optional>
+                <ref name="Abstract" />
+              </optional>
+              <optional>
+                <ref name="Parameters" />
+              </optional>
+              <optional>
+                <ref name="ResultDiscussion" />
+              </optional>
+              <optional>
+                <ref name="ThrowsDiscussion" />
+              </optional>
+              <optional>
+                <ref name="Discussion" />
+              </optional>
+            </element>
+            <element name="Discussion">
+              <ref name="BlockContent" />
+            </element>
+          </choice>
         </element>
       </oneOrMore>
     </element>

include/swift/AST/Comment.h

@@ -22,34 +22,20 @@ class DocComment;
 struct RawComment;
 
 class DocComment {
-public:
-  struct CommentParts {
-    Optional<const swift::markup::Paragraph *>Brief;
-    ArrayRef<const swift::markup::MarkupASTNode *> BodyNodes;
-    ArrayRef<const swift::markup::ParamField *> ParamFields;
-    Optional<const swift::markup::ReturnsField *> ReturnsField;
-    Optional<const swift::markup::ThrowsField *> ThrowsField;
-
-    bool isEmpty() const {
-      return !Brief.hasValue() && !ReturnsField.hasValue() && !ThrowsField.hasValue() && BodyNodes.empty() && ParamFields.empty();
-    }
-  };
-
-private:
   const Decl *D;
   const swift::markup::Document *Doc = nullptr;
-  const CommentParts Parts;
+  const swift::markup::CommentParts Parts;
 
 public:
   DocComment(const Decl *D, swift::markup::Document *Doc,
-             CommentParts Parts)
+             swift::markup::CommentParts Parts)
       : D(D), Doc(Doc), Parts(Parts) {}
 
   const Decl *getDecl() const { return D; }
 
   const swift::markup::Document *getDocument() const { return Doc; }
 
-  CommentParts getParts() const {
+  swift::markup::CommentParts getParts() const {
     return Parts;
   }
 

include/swift/Markup/AST.h

@@ -22,6 +22,35 @@ namespace swift {
 namespace markup {
 
 class MarkupContext;
+class MarkupASTNode;
+class Paragraph;
+class ParamField;
+class ReturnsField;
+class ThrowsField;
+
+/// The basic structure of a doc comment attached to a Swift
+/// declaration.
+struct CommentParts {
+  Optional<const Paragraph *> Brief;
+  ArrayRef<const MarkupASTNode *> BodyNodes;
+  ArrayRef<ParamField *> ParamFields;
+  Optional<const ReturnsField *> ReturnsField;
+  Optional<const ThrowsField *> ThrowsField;
+
+  bool isEmpty() const {
+    return !Brief.hasValue() &&
+           !ReturnsField.hasValue() &&
+           !ThrowsField.hasValue() &&
+           BodyNodes.empty() &&
+           ParamFields.empty();
+  }
+
+  bool hasFunctionDocumentation() const {
+    return !ParamFields.empty() ||
+             ReturnsField.hasValue() ||
+             ThrowsField.hasValue();
+  }
+};
 
 #define MARKUP_AST_NODE(Id, Parent) class Id;
 #define ABSTRACT_MARKUP_AST_NODE(Id, Parent) class Id;
@@ -585,6 +614,10 @@ class ParamField final : public PrivateExtension,
 
   StringRef Name;
 
+  // Parameter fields can contain a substructure describing a
+  // function or closure parameter.
+  llvm::Optional<CommentParts> Parts;
+
   ParamField(StringRef Name, ArrayRef<MarkupASTNode *> Children);
 
 public:
@@ -596,6 +629,21 @@ public:
     return Name;
   }
 
+  llvm::Optional<CommentParts> getParts() const {
+    return Parts;
+  }
+
+  void setParts(CommentParts P) {
+    Parts = P;
+  }
+
+  bool isClosureParameter() const {
+    if (!Parts.hasValue())
+      return false;
+
+    return Parts.getValue().hasFunctionDocumentation();
+  }
+
   ArrayRef<MarkupASTNode *> getChildren() {
     return {getTrailingObjects<MarkupASTNode *>(), NumChildren};
   }

lib/AST/DocComment.cpp

@@ -70,7 +70,7 @@ Optional<swift::markup::ParamField *> extractParamOutlineItem(
 
 bool extractParameterOutline(
     swift::markup::MarkupContext &MC, swift::markup::List *L,
-    SmallVectorImpl<const swift::markup::ParamField *> &ParamFields) {
+    SmallVectorImpl<swift::markup::ParamField *> &ParamFields) {
   SmallVector<swift::markup::MarkupASTNode *, 8> NormalItems;
   auto Children = L->getChildren();
   if (Children.empty())
@@ -145,7 +145,7 @@ bool extractParameterOutline(
 
 bool extractSeparatedParams(
     swift::markup::MarkupContext &MC, swift::markup::List *L,
-    SmallVectorImpl<const swift::markup::ParamField *> &ParamFields) {
+    SmallVectorImpl<swift::markup::ParamField *> &ParamFields) {
   SmallVector<swift::markup::MarkupASTNode *, 8> NormalItems;
   auto Children = L->getChildren();
 
@@ -209,7 +209,7 @@ bool extractSeparatedParams(
 
 bool extractSimpleField(
     swift::markup::MarkupContext &MC, swift::markup::List *L,
-    DocComment::CommentParts &Parts,
+    swift::markup::CommentParts &Parts,
     SmallVectorImpl<const swift::markup::MarkupASTNode *> &BodyNodes) {
   auto Children = L->getChildren();
   SmallVector<swift::markup::MarkupASTNode *, 8> NormalItems;
@@ -274,11 +274,11 @@ bool extractSimpleField(
   return NormalItems.size() == 0;
 }
 
-static DocComment::CommentParts
+static swift::markup::CommentParts
 extractCommentParts(swift::markup::MarkupContext &MC,
                     swift::markup::MarkupASTNode *Node) {
 
-  DocComment::CommentParts Parts;
+  swift::markup::CommentParts Parts;
   auto Children = Node->getChildren();
   if (Children.empty())
     return Parts;
@@ -289,7 +289,7 @@ extractCommentParts(swift::markup::MarkupContext &MC,
     Parts.Brief = FirstParagraph;
 
   SmallVector<const swift::markup::MarkupASTNode *, 4> BodyNodes;
-  SmallVector<const swift::markup::ParamField *, 8> ParamFields;
+  SmallVector<swift::markup::ParamField *, 8> ParamFields;
 
   // Look for special top-level lists
   size_t StartOffset = FirstParagraph == nullptr ? 0 : 1;
@@ -317,6 +317,11 @@ extractCommentParts(swift::markup::MarkupContext &MC,
   Parts.BodyNodes = MC.allocateCopy(llvm::makeArrayRef(BodyNodes));
   Parts.ParamFields = MC.allocateCopy(llvm::makeArrayRef(ParamFields));
 
+  for (auto Param : Parts.ParamFields) {
+    auto ParamParts = extractCommentParts(MC, Param);
+    Param->setParts(ParamParts);
+  }
+
   return Parts;
 }
 

lib/IDE/CommentConversion.cpp

@@ -211,13 +211,24 @@ struct CommentToXMLConverter {
   }
 
   void printParamField(const ParamField *PF) {
-    OS << "<Parameter><Name>";
+    OS << "<Parameter>";
+    OS << "<Name>";
     OS << PF->getName();
-    OS << "</Name><Direction isExplicit=\"0\">in</Direction><Discussion>";
-    for (auto Child : PF->getChildren())
-      printASTNode(Child);
+    OS << "</Name>";
+    OS << "<Direction isExplicit=\"0\">in</Direction>";
 
-    OS << "</Discussion></Parameter>";
+    if (PF->isClosureParameter()) {
+      OS << "<ClosureParameter>";
+      visitCommentParts(PF->getParts().getValue());
+      OS << "</ClosureParameter>";
+    } else {
+      OS << "<Discussion>";
+      for (auto Child : PF->getChildren()) {
+        printASTNode(Child);
+      }
+      OS << "</Discussion>";
+    }
+    OS << "</Parameter>";
   }
 
   void printResultDiscussion(const ReturnsField *RF) {
@@ -235,9 +246,40 @@ struct CommentToXMLConverter {
   }
 
   void visitDocComment(const DocComment *DC);
+  void visitCommentParts(const swift::markup::CommentParts &Parts);
 };
 } // unnamed namespace
 
+void CommentToXMLConverter::visitCommentParts(const swift::markup::CommentParts &Parts) {
+  if (Parts.Brief.hasValue()) {
+    OS << "<Abstract>";
+    printASTNode(Parts.Brief.getValue());
+    OS << "</Abstract>";
+  }
+
+  if (!Parts.ParamFields.empty()) {
+    OS << "<Parameters>";
+    for (const auto *PF : Parts.ParamFields)
+      printParamField(PF);
+
+    OS << "</Parameters>";
+  }
+
+  if (Parts.ReturnsField.hasValue())
+    printResultDiscussion(Parts.ReturnsField.getValue());
+
+  if (Parts.ThrowsField.hasValue())
+    printThrowsDiscussion(Parts.ThrowsField.getValue());
+
+  if (!Parts.BodyNodes.empty()) {
+    OS << "<Discussion>";
+    for (const auto *N : Parts.BodyNodes)
+      printASTNode(N);
+
+    OS << "</Discussion>";
+  }
+}
+
 void CommentToXMLConverter::visitDocComment(const DocComment *DC) {
   const Decl *D = DC->getDecl();
 
@@ -313,36 +355,7 @@ void CommentToXMLConverter::visitDocComment(const DocComment *DC) {
     OS << "</Declaration>";
   }
 
-  auto Brief = DC->getBrief();
-  if (Brief.hasValue()) {
-    OS << "<Abstract>";
-    printASTNode(Brief.getValue());
-    OS << "</Abstract>";
-  }
-
-  if (!DC->getParamFields().empty()) {
-    OS << "<Parameters>";
-    for (const auto *PF : DC->getParamFields())
-      printParamField(PF);
-
-    OS << "</Parameters>";
-  }
-
-  auto RF = DC->getReturnsField();
-  if (RF.hasValue())
-    printResultDiscussion(RF.getValue());
-
-  auto TF = DC->getThrowsField();
-  if (TF.hasValue())
-    printThrowsDiscussion(TF.getValue());
-
-  if (!DC->getBodyNodes().empty()) {
-    OS << "<Discussion>";
-    for (const auto *N : DC->getBodyNodes())
-      printASTNode(N);
-
-    OS << "</Discussion>";
-  }
+  visitCommentParts(DC->getParts());
 
   OS << RootEndTag;
 }

lib/Markup/AST.cpp

@@ -205,7 +205,8 @@ Strong *Strong::create(MarkupContext &MC,
 
 ParamField::ParamField(StringRef Name, ArrayRef<MarkupASTNode *> Children)
     : PrivateExtension(ASTNodeKind::ParamField), NumChildren(Children.size()),
-      Name(Name) {
+      Name(Name),
+      Parts(None) {
   std::uninitialized_copy(Children.begin(), Children.end(),
                           getTrailingObjects<MarkupASTNode *>());
 }

test/Inputs/comment_to_something_conversion.swift

@@ -422,3 +422,39 @@ public func codeListingWithDefaultLanguage() {}
 /// ```
 public func codeListingWithOtherLanguage() {}
 // CHECK: DocCommentAsXML=[<Function file="{{.*}}" line="{{.*}}" column="{{.*}}"><Name>codeListingWithOtherLanguage()</Name><USR>s:F14swift_ide_test28codeListingWithOtherLanguageFT_T_</USR><Declaration>public func codeListingWithOtherLanguage()</Declaration><Abstract><Para>Brief.</Para></Abstract><Discussion><CodeListing language="c++"><zCodeLineNumbered><![CDATA[Something::Something::create();]]></zCodeLineNumbered><zCodeLineNumbered></zCodeLineNumbered></CodeListing></Discussion></Function>]
+
+/// Partially applies a binary operator.
+///
+/// - Parameter a: The left-hand side to partially apply.
+/// - Parameter combine: A binary operator.
+///   - Parameter lhs: The left-hand side of the operator
+///   - Parameter rhs: The right-hand side of the operator
+///   - Returns: A result.
+///   - Throws: Nothing.
+public func closureParameterExplodedExploded<T>(a: T, combine: (lhs: T, rhs: T) -> T) {}
+// CHECK: DocCommentAsXML=[<Function file="{{.*}} line="{{.*}}" column="{{.*}}"><Name>closureParameterExplodedExploded(a:combine:)</Name><USR>s:F14swift_ide_test32closureParameterExplodedExplodedurFT1ax7combineFT3lhsx3rhsx_x_T_</USR><Declaration>public func closureParameterExplodedExploded&lt;T&gt;(a: T, combine: (lhs: T, rhs: T) -&gt; T)</Declaration><Abstract><Para>Partially applies a binary operator.</Para></Abstract><Parameters><Parameter><Name>a</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side to partially apply.</Para></Discussion></Parameter><Parameter><Name>combine</Name><Direction isExplicit="0">in</Direction><ClosureParameter><Abstract><Para>A binary operator.</Para></Abstract><Parameters><Parameter><Name>lhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side of the operator</Para></Discussion></Parameter><Parameter><Name>rhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The right-hand side of the operator</Para></Discussion></Parameter></Parameters><ResultDiscussion><Para>A result.</Para></ResultDiscussion><ThrowsDiscussion><Para>Nothing.</Para></ThrowsDiscussion></ClosureParameter></Parameter></Parameters></Function>] CommentXMLValid
+
+/// Partially applies a binary operator.
+///
+/// - Parameters:
+///   - a: The left-hand side to partially apply.
+///   - combine: A binary operator.
+///     - Parameter lhs: The left-hand side of the operator
+///     - Parameter rhs: The right-hand side of the operator
+///     - Returns: A result.
+///     - Throws: Nothing.
+public func closureParameterOutlineExploded<T>(a: T, combine: (lhs: T, rhs: T) -> T) {}
+// CHECK: DocCommentAsXML=[<Function file="{{.*}} line="{{.*}}" column="{{.*}}"><Name>closureParameterOutlineExploded(a:combine:)</Name><USR>s:F14swift_ide_test31closureParameterOutlineExplodedurFT1ax7combineFT3lhsx3rhsx_x_T_</USR><Declaration>public func closureParameterOutlineExploded&lt;T&gt;(a: T, combine: (lhs: T, rhs: T) -&gt; T)</Declaration><Abstract><Para>Partially applies a binary operator.</Para></Abstract><Parameters><Parameter><Name>a</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side to partially apply.</Para></Discussion></Parameter><Parameter><Name>combine</Name><Direction isExplicit="0">in</Direction><ClosureParameter><Abstract><Para>A binary operator.</Para></Abstract><Parameters><Parameter><Name>lhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side of the operator</Para></Discussion></Parameter><Parameter><Name>rhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The right-hand side of the operator</Para></Discussion></Parameter></Parameters><ResultDiscussion><Para>A result.</Para></ResultDiscussion><ThrowsDiscussion><Para>Nothing.</Para></ThrowsDiscussion></ClosureParameter></Parameter></Parameters></Function>] CommentXMLValid
+
+/// Partially applies a binary operator.
+///
+/// - Parameters:
+///   - a: The left-hand side to partially apply.
+///   - combine: A binary operator.
+///     - Parameters:
+///       - lhs: The left-hand side of the operator
+///       - rhs: The right-hand side of the operator
+///     - Returns: A result.
+///     - Throws: Nothing.
+public func closureParameterOutlineOutline<T>(a: T, combine: (lhs: T, rhs: T) -> T) {}
+// CHECK: {{.*}}DocCommentAsXML=[<Function file="{{.*}}" line="{{.*}}" column="{{.*}}"><Name>closureParameterOutlineOutline(a:combine:)</Name><USR>s:F14swift_ide_test30closureParameterOutlineOutlineurFT1ax7combineFT3lhsx3rhsx_x_T_</USR><Declaration>public func closureParameterOutlineOutline&lt;T&gt;(a: T, combine: (lhs: T, rhs: T) -&gt; T)</Declaration><Abstract><Para>Partially applies a binary operator.</Para></Abstract><Parameters><Parameter><Name>a</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side to partially apply.</Para></Discussion></Parameter><Parameter><Name>combine</Name><Direction isExplicit="0">in</Direction><ClosureParameter><Abstract><Para>A binary operator.</Para></Abstract><Parameters><Parameter><Name>lhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The left-hand side of the operator</Para></Discussion></Parameter><Parameter><Name>rhs</Name><Direction isExplicit="0">in</Direction><Discussion><Para>The right-hand side of the operator</Para></Discussion></Parameter></Parameters><ResultDiscussion><Para>A result.</Para></ResultDiscussion><ThrowsDiscussion><Para>Nothing.</Para></ThrowsDiscussion></ClosureParameter></Parameter></Parameters></Function>]