Skip to content

Commit

Permalink
Don't reorder @sample tag (#406)
Browse files Browse the repository at this point in the history
Summary:
Some users place `sample` tags at the beginning (before any other tag). This conflicts with our fixed ordering of kdoc tags.

Since there's good reason for this ordering, don't reorder `sample` tags (but only if they're at the beginning already). This way we'll avoid messing with code that's already been formatted with `ktfmt` while supporting this use case.

See [#406](#406) for more info.

Closes #406

Reviewed By: cortinico, hick209

Differential Revision: D58198948

fbshipit-source-id: 00335bdb04ccc668419e1443196fdce4103a763b
  • Loading branch information
David Torosyan authored and facebook-github-bot committed Jun 6, 2024
1 parent dafd1cb commit 2ba3b09
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 3 deletions.
25 changes: 22 additions & 3 deletions core/src/main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,11 @@ class ParagraphListBuilder(
return false
}

private fun docTagRank(tag: String): Int {
private fun docTagRank(tag: String, isPriority: Boolean): Int {
// Canonical kdoc order -- https://kotlinlang.org/docs/kotlin-doc.html#block-tags
// Full list in Dokka's sources: plugins/base/src/main/kotlin/parsers/Parser.kt
return when {
isPriority -> -1
tag.startsWith("@param") -> 0
tag.startsWith("@return") -> 1
tag.startsWith("@constructor") -> 2
Expand All @@ -586,6 +587,21 @@ class ParagraphListBuilder(
}
}

/**
* Tags that are "priority" are placed before other tags, with their order unchanged.
*
* Note that if a priority tag comes after a regular tag (before formatting), it doesn't get
* treated as priority.
*
* See: https://github.com/facebook/ktfmt/issues/406
*/
private fun docTagIsPriority(tag: String): Boolean {
return when {
tag.startsWith("@sample") -> true
else -> false
}
}

/**
* Make a pass over the paragraphs and make sure that we (for example) place blank lines around
* preformatted text.
Expand All @@ -605,13 +621,16 @@ class ParagraphListBuilder(
private fun sortDocTags() {
if (options.orderDocTags && paragraphs.any { it.doc }) {
val order = paragraphs.mapIndexed { index, paragraph -> paragraph to index }.toMap()
val firstNonPriorityDocTag = paragraphs.indexOfFirst { it.doc && !docTagIsPriority(it.text) }
val comparator =
object : Comparator<List<Paragraph>> {
override fun compare(l1: List<Paragraph>, l2: List<Paragraph>): Int {
val p1 = l1.first()
val p2 = l2.first()
val o1 = order[p1]!!
val o2 = order[p2]!!
val isPriority1 = p1.doc && docTagIsPriority(p1.text) && o1 < firstNonPriorityDocTag
val isPriority2 = p2.doc && docTagIsPriority(p2.text) && o2 < firstNonPriorityDocTag

// Sort TODOs to the end
if (p1.text.isTodo() != p2.text.isTodo()) {
Expand All @@ -621,8 +640,8 @@ class ParagraphListBuilder(
if (p1.doc == p2.doc) {
if (p1.doc) {
// Sort @return after @param etc
val r1 = docTagRank(p1.text)
val r2 = docTagRank(p2.text)
val r1 = docTagRank(p1.text, isPriority1)
val r2 = docTagRank(p2.text, isPriority2)
if (r1 != r2) {
return r1 - r2
}
Expand Down
54 changes: 54 additions & 0 deletions core/src/test/java/com/facebook/ktfmt/kdoc/KDocFormatterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2031,6 +2031,60 @@ class KDocFormatterTest {
)
}

@Test
fun testNoReorderSample() {
val source =
"""
/**
* Constructs a new location range for the given file, from start to
* end. If the length of the range is not known, end may be null.
*
* @sample abc
*
* You might want to see another sample.
*
* @sample xyz
*
* Makes sense?
* @return Something
* @see more
* @sample foo
*
* Note that samples after another tag don't get special treatment.
*/
"""
.trimIndent()
checkFormatter(
FormattingTask(
KDocFormattingOptions(72),
source,
" ",
orderedParameterNames = listOf("file", "start", "end")),
"""
/**
* Constructs a new location range for the given file, from start to
* end. If the length of the range is not known, end may be null.
*
* @sample abc
*
* You might want to see another sample.
*
* @sample xyz
*
* Makes sense?
*
* @return Something
* @sample foo
*
* Note that samples after another tag don't get special treatment.
*
* @see more
*/
"""
.trimIndent(),
)
}

@Test
fun testKDocOrdering() {
// From AndroidX'
Expand Down

0 comments on commit 2ba3b09

Please sign in to comment.