more sane handling of overloads: don't duplicate signatures, show all documentation of a group of overloads with exactly the same documentation together

10 files changed, 149 insertions, 48 deletions

diff --git a/src/Formats/StructuredFormatService.kt b/src/Formats/StructuredFormatService.kt
index ec4cff89..0a146d84 100644
--- a/src/Formats/StructuredFormatService.kt
+++ b/src/Formats/StructuredFormatService.kt
@@ -80,29 +80,37 @@ public abstract class StructuredFormatService(val locationService: LocationServi
         return FormatLink(to.name, locationService.relativeLocation(from, to, extension))
     }
 
-    fun appendDescription(location: Location, to: StringBuilder, nodes: Iterable<DocumentationNode>) {
-        val described = nodes.filter { it.hasDescriptionOrTags() }
-        if (described.any()) {
-            val single = described.size() == 1
-            if (described.any { it.content.description != ContentEmpty }) {
-                appendHeader(to, "Description", 3)
+    fun appendDocumentation(location: Location, to: StringBuilder, overloads: Iterable<DocumentationNode>) {
+        val breakdownBySummary = overloads.groupByTo(LinkedHashMap()) { node -> node.content }
+
+        for ((summary, items) in breakdownBySummary) {
+            items.forEach {
+                appendBlockCode(to, formatText(location, languageService.render(it)))
+                it.appendOverrides(to)
+                it.appendDeprecation(to)
+                it.appendSourceLink(to)
             }
-            for (node in described) {
-                if (!single) {
-                    appendBlockCode(to, formatText(location, languageService.render(node)))
-                }
-                appendLine(to, formatText(location, node.content.description))
-                appendLine(to)
+            // All items have exactly the same documentation, so we can use any item to render it
+            val item = items.first()
+            appendLine(to, formatText(location, item.content.summary))
+            appendLine(to)
+            appendDescription(location, to, item)
+        }
+    }
 
-                node.content.getSectionsWithSubjects().forEach {
-                    appendSectionWithSubject(it.getKey(), location, it.getValue(), to)
-                }
+    fun appendDescription(location: Location, to: StringBuilder, node: DocumentationNode) {
+        if (node.content.description != ContentEmpty) {
+            appendHeader(to, "Description", 3)
+            appendLine(to, formatText(location, node.content.description))
+            appendLine(to)
+        }
+        node.content.getSectionsWithSubjects().forEach {
+            appendSectionWithSubject(it.getKey(), location, it.getValue(), to)
+        }
 
-                for (section in node.content.sections.filter { it.subjectName == null }) {
-                    appendLine(to, formatStrong(formatText(section.tag)))
-                    appendLine(to, formatText(location, section))
-                }
-            }
+        for (section in node.content.sections.filter { it.subjectName == null }) {
+            appendLine(to, formatStrong(formatText(section.tag)))
+            appendLine(to, formatText(location, section))
         }
     }
 
@@ -124,23 +132,6 @@ public abstract class StructuredFormatService(val locationService: LocationServi
     private fun DocumentationNode.hasDescriptionOrTags() =
             content.description != ContentEmpty || !content.sections.isEmpty()
 
-    fun appendSummary(location: Location, to: StringBuilder, nodes: Iterable<DocumentationNode>) {
-        val breakdownBySummary = nodes.groupByTo(LinkedHashMap()) { node ->
-            formatText(location, node.summary)
-        }
-
-        for ((summary, items) in breakdownBySummary) {
-            items.forEach {
-                appendBlockCode(to, formatText(location, languageService.render(it)))
-                it.appendOverrides(to)
-                it.appendDeprecation(to)
-                it.appendSourceLink(to)
-            }
-            appendLine(to, summary)
-            appendLine(to)
-        }
-    }
-
     private fun DocumentationNode.appendOverrides(to: StringBuilder) {
         overrides.forEach {
             to.append("Overrides ")
@@ -173,8 +164,7 @@ public abstract class StructuredFormatService(val locationService: LocationServi
         val breakdownByName = nodes.groupBy { node -> node.name }
         for ((name, items) in breakdownByName) {
             appendHeader(to, formatText(name))
-            appendSummary(location, to, items)
-            appendDescription(location, to, items)
+            appendDocumentation(location, to, items)
         }
     }
 
diff --git a/src/Model/Content.kt b/src/Model/Content.kt
index 165865a0..f264d623 100644
--- a/src/Model/Content.kt
+++ b/src/Model/Content.kt
@@ -18,6 +18,12 @@ public open class ContentBlock() : ContentNode() {
     }
 
     fun isEmpty() = children.isEmpty()
+
+    override fun equals(other: Any?): Boolean =
+        other is ContentBlock && javaClass == other.javaClass && children == other.children
+
+    override fun hashCode(): Int =
+        children.hashCode()
 }
 
 public data class ContentText(val text: String) : ContentNode()
@@ -31,11 +37,33 @@ public class ContentStrong() : ContentBlock()
 public class ContentStrikethrough() : ContentBlock()
 public class ContentCode() : ContentBlock()
 public class ContentBlockCode() : ContentBlock()
-public class ContentNodeLink(val node : DocumentationNode) : ContentBlock()
-public class ContentExternalLink(val href : String) : ContentBlock()
+
+public class ContentNodeLink(val node : DocumentationNode) : ContentBlock() {
+    override fun equals(other: Any?): Boolean =
+        super.equals(other) && other is ContentNodeLink && node == other.node
+
+    override fun hashCode(): Int =
+        children.hashCode() * 31 + node.hashCode()
+}
+
+public class ContentExternalLink(val href : String) : ContentBlock() {
+    override fun equals(other: Any?): Boolean =
+        super.equals(other) && other is ContentExternalLink && href == other.href
+
+    override fun hashCode(): Int =
+        children.hashCode() * 31 + href.hashCode()
+}
+
 public class ContentList() : ContentBlock()
 public class ContentListItem() : ContentBlock()
-public class ContentSection(public val tag: String, public val subjectName: String?) : ContentBlock()
+
+public class ContentSection(public val tag: String, public val subjectName: String?) : ContentBlock() {
+    override fun equals(other: Any?): Boolean =
+        super.equals(other) && other is ContentSection && tag == other.tag && subjectName == other.subjectName
+
+    override fun hashCode(): Int =
+        children.hashCode() * 31 * 31 + tag.hashCode() * 31 + (subjectName?.hashCode() ?: 0)
+}
 
 fun content(body: ContentBlock.() -> Unit): ContentBlock {
     val block = ContentBlock()
diff --git a/test/data/format/overloadsWithDescription.html b/test/data/format/overloadsWithDescription.html
new file mode 100644
index 00000000..6b593fcd
--- /dev/null
+++ b/test/data/format/overloadsWithDescription.html
@@ -0,0 +1,19 @@
+<HTML>
+<HEAD>
+<title>test / f</title>
+</HEAD>
+<BODY>
+<a href="out.html">test</a>&nbsp;/&nbsp;<a href="out.html"></a>&nbsp;/&nbsp;<a href="out.html">f</a><br/>
+<br/>
+<h1>f</h1>
+<pre><code><span class="keyword">fun </span><span class="identifier">f</span><span class="symbol">(</span><span class="identifier">x</span><span class="symbol">: </span><span class="identifier">Int</span><span class="symbol">)</span><span class="symbol">: </span><span class="identifier">Unit</span></code></pre><pre><code><span class="keyword">fun </span><span class="identifier">f</span><span class="symbol">(</span><span class="identifier">x</span><span class="symbol">: </span><span class="identifier">String</span><span class="symbol">)</span><span class="symbol">: </span><span class="identifier">Unit</span></code></pre><p>Performs an action on x.</p>
+<br/>
+<br/>
+<h3>Description</h3>
+<p>This is a long description.</p>
+<br/>
+<br/>
+<h3>Parameters</h3>
+<code>x</code> - the value to perform the action on.<br/>
+</BODY>
+</HTML>
diff --git a/test/data/format/overloadsWithDescription.kt b/test/data/format/overloadsWithDescription.kt
new file mode 100644
index 00000000..13c219a2
--- /dev/null
+++ b/test/data/format/overloadsWithDescription.kt
@@ -0,0 +1,15 @@
+/**
+ * Performs an action on x.
+ *
+ * This is a long description.
+ * @param x the value to perform the action on.
+ */
+fun f(x: Int) { }
+
+/**
+ * Performs an action on x.
+ *
+ * This is a long description.
+ * @param x the value to perform the action on.
+ */
+fun f(x: String) { }
diff --git a/test/data/format/overloadsWithDifferentDescriptions.html b/test/data/format/overloadsWithDifferentDescriptions.html
new file mode 100644
index 00000000..3ce974af
--- /dev/null
+++ b/test/data/format/overloadsWithDifferentDescriptions.html
@@ -0,0 +1,28 @@
+<HTML>
+<HEAD>
+<title>test / f</title>
+</HEAD>
+<BODY>
+<a href="out.html">test</a>&nbsp;/&nbsp;<a href="out.html"></a>&nbsp;/&nbsp;<a href="out.html">f</a><br/>
+<br/>
+<h1>f</h1>
+<pre><code><span class="keyword">fun </span><span class="identifier">f</span><span class="symbol">(</span><span class="identifier">x</span><span class="symbol">: </span><span class="identifier">Int</span><span class="symbol">)</span><span class="symbol">: </span><span class="identifier">Unit</span></code></pre><p>Performs an action on x.</p>
+<br/>
+<br/>
+<h3>Description</h3>
+<p>This is a long description.</p>
+<br/>
+<br/>
+<h3>Parameters</h3>
+<code>x</code> - the int value to perform the action on.<br/>
+<pre><code><span class="keyword">fun </span><span class="identifier">f</span><span class="symbol">(</span><span class="identifier">x</span><span class="symbol">: </span><span class="identifier">String</span><span class="symbol">)</span><span class="symbol">: </span><span class="identifier">Unit</span></code></pre><p>Performs an action on x.</p>
+<br/>
+<br/>
+<h3>Description</h3>
+<p>This is a long description.</p>
+<br/>
+<br/>
+<h3>Parameters</h3>
+<code>x</code> - the string value to perform the action on.<br/>
+</BODY>
+</HTML>
diff --git a/test/data/format/overloadsWithDifferentDescriptions.kt b/test/data/format/overloadsWithDifferentDescriptions.kt
new file mode 100644
index 00000000..ad3169b0
--- /dev/null
+++ b/test/data/format/overloadsWithDifferentDescriptions.kt
@@ -0,0 +1,15 @@
+/**
+ * Performs an action on x.
+ *
+ * This is a long description.
+ * @param x the int value to perform the action on.
+ */
+fun f(x: Int) { }
+
+/**
+ * Performs an action on x.
+ *
+ * This is a long description.
+ * @param x the string value to perform the action on.
+ */
+fun f(x: String) { }
diff --git a/test/data/format/paramTag.md b/test/data/format/paramTag.md
index 9534deb3..02927668 100644
--- a/test/data/format/paramTag.md
+++ b/test/data/format/paramTag.md
@@ -11,8 +11,6 @@ fun f(x: String, y: Int): Unit
 
 
 
-
-
 ### Parameters
 
 `x` - A string
diff --git a/test/data/format/see.html b/test/data/format/see.html
index fa283363..30409eb7 100644
--- a/test/data/format/see.html
+++ b/test/data/format/see.html
@@ -7,8 +7,6 @@
 <h1>quux</h1>
 <pre><code><span class="keyword">fun </span><span class="identifier">quux</span><span class="symbol">(</span><span class="symbol">)</span><span class="symbol">: </span><span class="identifier">Unit</span></code></pre><br/>
 <br/>
-<br/>
-<br/>
 <strong>See Also</strong><br/>
 <p><a href="out.html">foo</a></p>
 <p><a href="out.html">bar</a></p>
diff --git a/test/data/format/throwsTag.md b/test/data/format/throwsTag.md
index d968483b..e6d0c76e 100644
--- a/test/data/format/throwsTag.md
+++ b/test/data/format/throwsTag.md
@@ -11,8 +11,6 @@ fun f(): Unit
 
 
 
-
-
 ### Exceptions
 
 `IllegalArgumentException` - on Mondays
diff --git a/test/src/format/HtmlFormatTest.kt b/test/src/format/HtmlFormatTest.kt
index 7dfa80be..53c1a39d 100644
--- a/test/src/format/HtmlFormatTest.kt
+++ b/test/src/format/HtmlFormatTest.kt
@@ -25,6 +25,18 @@ public class HtmlFormatTest {
         }
     }
 
+    Test fun overloadsWithDescription() {
+        verifyOutput("test/data/format/overloadsWithDescription.kt", ".html") { model, output ->
+            htmlService.appendNodes(tempLocation, output, model.members.single().members)
+        }
+    }
+
+    Test fun overloadsWithDifferentDescriptions() {
+        verifyOutput("test/data/format/overloadsWithDifferentDescriptions.kt", ".html") { model, output ->
+            htmlService.appendNodes(tempLocation, output, model.members.single().members)
+        }
+    }
+
     Test fun deprecated() {
         verifyOutput("test/data/format/deprecated.kt", ".package.html") { model, output ->
             htmlService.appendNodes(tempLocation, output, model.members)