From 6be3f4c8f3268ac8080ba4a3f768f6ccd84ba98f Mon Sep 17 00:00:00 2001 From: Tor Norbye Date: Mon, 20 Jan 2025 09:57:22 -0800 Subject: [PATCH] Version 1.6.7: Fix bugs #105, #106 and fix IDE plugin deprecated usages This CL: - Updates plugin code to replace deprecated API usages - Fixes issue #104: Include type parameters in parameter list reordering - Fixes issue #105: Make add punctuation apply to all paragraphs, not just last A few other misc improvements (e.g. make punctuation smarter around non-letter paragraph endings, such as `, and avoids adding punctuation on paths and URLs, and also gracefully handle <>'s around type variable names in @param documentation. --- CHANGELOG.md | 7 +- README.md | 2 +- .../kotlin/kdocformatter/cli/KotlinLexer.kt | 26 ++++- .../cli/KDocFileFormatterTest.kt | 94 +++++++++++++++++++ ide-plugin/CHANGELOG.md | 7 +- ide-plugin/gradle.properties | 4 +- .../plugin/ReformatKDocAction.kt | 16 +++- .../ktfmt/kdoc/ParagraphListBuilder.kt | 50 +++++++--- .../com/facebook/ktfmt/kdoc/Utilities.kt | 2 +- library/src/main/resources/version.properties | 2 +- .../facebook/ktfmt/kdoc/KDocFormatterTest.kt | 76 +++++++++++++++ 11 files changed, 255 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d15d623..5d94eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ # KDoc Formatter Changelog +## [1.6.7] +- Fix issue #104: Include type parameters in parameter list reordering +- Fix issue #105: Make add punctuation apply to all paragraphs, not just last +- IDE plugin fixes (replaced deprecated API calls) + ## [1.6.6] - IDE-only update: Marked compatible with IntelliJ IDEA 2025.1. - Updated dependencies @@ -249,4 +254,4 @@ - Adds hanging indents for ordered and unordered indents. - Cleans up the double spaces left by the IntelliJ "Convert to Kotlin" action right before the closing comment token. -- Removes trailing spaces. \ No newline at end of file +- Removes trailing spaces. diff --git a/README.md b/README.md index ce8f6e2..39342b3 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ Options: @ Read filenames from file. -kdoc-formatter: Version 1.6.6 +kdoc-formatter: Version 1.6.7 https://github.com/tnorbye/kdoc-formatter ``` diff --git a/cli/src/main/kotlin/kdocformatter/cli/KotlinLexer.kt b/cli/src/main/kotlin/kdocformatter/cli/KotlinLexer.kt index c74917a..f63529c 100644 --- a/cli/src/main/kotlin/kdocformatter/cli/KotlinLexer.kt +++ b/cli/src/main/kotlin/kdocformatter/cli/KotlinLexer.kt @@ -346,14 +346,28 @@ class KotlinLexer(private val source: String) { skipSpace() val token = nextToken() ?: return null if (token == "fun") { - // Find beginning of parameter list + val parameters = mutableListOf() + + // Find beginning of parameter list, and pick up + // any type parameters in the signature while (i < length) { - val name = nextToken(matchParens = false) ?: return null - if (name == "(") { + val token = nextToken(matchParens = false) ?: return null + if (token == "<") { + var nextIsVariable = true + while (i < length) { + val varName = nextToken(matchParens = false) ?: return null + if (nextIsVariable && varName.isIdentifier()) { + parameters.add(varName) + } + if (varName == ">") { + break + } + nextIsVariable = varName == "," + } + } else if (token == "(") { break } } - val parameters = mutableListOf() while (i < length) { // Name is last symbol before ":" @@ -400,6 +414,10 @@ class KotlinLexer(private val source: String) { return null } + private fun String.isIdentifier(): Boolean { + return this[0].isJavaIdentifierStart() && this.all { it.isJavaIdentifierPart() } + } + companion object { private const val PLAIN_TEXT = 1 private const val LINE_COMMENT = 2 diff --git a/cli/src/test/kotlin/kdocformatter/cli/KDocFileFormatterTest.kt b/cli/src/test/kotlin/kdocformatter/cli/KDocFileFormatterTest.kt index e304bf8..07dc4ed 100644 --- a/cli/src/test/kotlin/kdocformatter/cli/KDocFileFormatterTest.kt +++ b/cli/src/test/kotlin/kdocformatter/cli/KDocFileFormatterTest.kt @@ -370,6 +370,100 @@ class KDocFileFormatterTest { reformatted.trim()) } + @Test + fun testKDocOrderingForGenerics() { + // Regression test for https://github.com/tnorbye/kdoc-formatter/issues/104 + val source = + """ + class KType + class Test { + /** + * @param type The [KType] representing the type to be instantiated. + * @param T The type of the instance to be created. + * @return An instance of the specified type, or `null` if instantiation fails. + */ + public fun createInstance(type: KType): T? { + return null + } + } + + /** + * @param clazz the class + * @param constructor to invoke + * @param T the view + * @param S the stage + */ + fun ?, T : StageView<*>?> bind(clazz: Class, constructor: BiFunction) { + } + + object Kt { + /** + * Run the loop and wait until condition is true or the retry limit is reached. Returns the + * result afterwards. + * + * @param supplier a function that returns the desired result. + * @param condition tests whether the result is desired. + * @param retryLimit Limit to retry before return the result. If not specified, try forever. + * @param type of the desired result. + * @return the result from the last run (condition met or timeout). + */ + fun waitForAndReturn( + supplier: () -> T, + condition: (T) -> Boolean, + retryLimit: Int = NO_LIMIT, + ): T = TODO() + } + """ + .trimIndent() + + val reformatted = reformatFile(source, KDocFormattingOptions(72).apply { orderDocTags = true }) + assertEquals( + """ + class KType + class Test { + /** + * @param T The type of the instance to be created. + * @param type The [KType] representing the type to be instantiated. + * @return An instance of the specified type, or `null` if + * instantiation fails. + */ + public fun createInstance(type: KType): T? { + return null + } + } + + /** + * @param S the stage + * @param T the view + * @param clazz the class + * @param constructor to invoke + */ + fun ?, T : StageView<*>?> bind(clazz: Class, constructor: BiFunction) { + } + + object Kt { + /** + * Run the loop and wait until condition is true or the retry limit is + * reached. Returns the result afterwards. + * + * @param type of the desired result. + * @param supplier a function that returns the desired result. + * @param condition tests whether the result is desired. + * @param retryLimit Limit to retry before return the result. If not + * specified, try forever. + * @return the result from the last run (condition met or timeout). + */ + fun waitForAndReturn( + supplier: () -> T, + condition: (T) -> Boolean, + retryLimit: Int = NO_LIMIT, + ): T = TODO() + } + """ + .trimIndent(), + reformatted.trim()) + } + @Test fun testLineWidth() { // Perform in KDocFileFormatter test too to make sure we properly account diff --git a/ide-plugin/CHANGELOG.md b/ide-plugin/CHANGELOG.md index 09ef49c..aeb6d36 100644 --- a/ide-plugin/CHANGELOG.md +++ b/ide-plugin/CHANGELOG.md @@ -2,6 +2,11 @@ # KDoc Formatter Plugin Changelog +## [1.6.7] +- Updated code to replace deprecated API usages +- Fix issue #104: Include type parameters in parameter list reordering +- Fix issue #105: Make add punctuation apply to all paragraphs, not just last + ## [1.6.6] - Add support for 2025.1 EAP, and migrate to 2.x version of IntelliJ gradle plugin. - Fix https://github.com/tnorbye/kdoc-formatter/issues/106 @@ -159,4 +164,4 @@ width). ## [1.0.0] -- Initial version \ No newline at end of file +- Initial version diff --git a/ide-plugin/gradle.properties b/ide-plugin/gradle.properties index 20a5880..1d220e7 100644 --- a/ide-plugin/gradle.properties +++ b/ide-plugin/gradle.properties @@ -7,12 +7,12 @@ pluginRepositoryUrl = https://github.com/tnorbye/kdoc-formatter # See https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html # for insight into build numbers and IntelliJ Platform versions. -pluginSinceBuild = 232 +pluginSinceBuild = 243 pluginUntilBuild = 251.* # IntelliJ Platform Properties -> https://github.com/JetBrains/gradle-intellij-plugin#intellij-platform-properties platformType = IC -platformVersion = 2024.2 +platformVersion = 2024.3 #platformVersion = 251.14649-EAP-CANDIDATE-SNAPSHOT # Plugin Dependencies -> https://plugins.jetbrains.com/docs/intellij/plugin-dependencies.html diff --git a/ide-plugin/src/main/kotlin/kdocformatter/plugin/ReformatKDocAction.kt b/ide-plugin/src/main/kotlin/kdocformatter/plugin/ReformatKDocAction.kt index 0dec7e3..e422ce6 100644 --- a/ide-plugin/src/main/kotlin/kdocformatter/plugin/ReformatKDocAction.kt +++ b/ide-plugin/src/main/kotlin/kdocformatter/plugin/ReformatKDocAction.kt @@ -25,7 +25,11 @@ import com.facebook.ktfmt.kdoc.findSamePosition import com.facebook.ktfmt.kdoc.isLineComment import com.intellij.application.options.CodeStyle import com.intellij.lang.Language -import com.intellij.openapi.actionSystem.* +import com.intellij.openapi.actionSystem.ActionUiKind +import com.intellij.openapi.actionSystem.ActionUpdateThread +import com.intellij.openapi.actionSystem.AnAction +import com.intellij.openapi.actionSystem.AnActionEvent +import com.intellij.openapi.actionSystem.CommonDataKeys import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.editor.Document import com.intellij.openapi.project.DumbAware @@ -39,8 +43,8 @@ import com.intellij.psi.PsiFile import com.intellij.psi.PsiManager import com.intellij.psi.PsiWhiteSpace import com.intellij.psi.util.PsiTreeUtil -import com.intellij.refactoring.suggested.endOffset -import com.intellij.refactoring.suggested.startOffset +import com.intellij.psi.util.endOffset +import com.intellij.psi.util.startOffset import com.intellij.util.ThrowableRunnable import kotlin.math.min import org.jetbrains.kotlin.idea.KotlinLanguage @@ -263,7 +267,7 @@ class ReformatKDocAction : AnAction(), DumbAware { val presentation = event.presentation val type = getApplicableCommentType(event) val available = type != CommentType.NONE - if (ActionPlaces.isPopupPlace(event.place)) { + if (event.uiKind == ActionUiKind.POPUP) { presentation.isEnabledAndVisible = available } else { presentation.isEnabled = available @@ -349,7 +353,9 @@ fun createFormattingTask( if (task.type == CommentType.KDOC && options.orderDocTags) { val parent = kdoc.parent if (parent is KtCallableDeclaration) { - task.orderedParameterNames = parent.valueParameters.mapNotNull { it.name }.toList() + task.orderedParameterNames = + parent.typeParameters.mapNotNull { it.name } + + parent.valueParameters.mapNotNull { it.name } } } diff --git a/library/src/main/kotlin/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt b/library/src/main/kotlin/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt index 592e207..2ec4a14 100644 --- a/library/src/main/kotlin/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt +++ b/library/src/main/kotlin/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt @@ -826,28 +826,48 @@ class ParagraphListBuilder( if (!options.addPunctuation || paragraphs.isEmpty()) { return } - val last = paragraphs.last() - if (last.preformatted || last.doc || last.hanging && !last.continuation || last.isEmpty()) { - return - } - - val text = last.content - if (!text.startsWithUpperCaseLetter()) { - return - } + for (last in paragraphs) { + if (last.preformatted || last.doc || last.hanging && !last.continuation || last.isEmpty()) { + continue + } - for (i in text.length - 1 downTo 0) { - val c = text[i] - if (c.isWhitespace()) { + val text = last.content + if (!text.startsWithUpperCaseLetter() || text.contentEquals("TODO")) { continue } - if (c.isLetterOrDigit() || c.isCloseSquareBracket()) { - text.setLength(i + 1) - text.append('.') + + for (i in text.length - 1 downTo 0) { + val c = text[i] + if (c.isWhitespace()) { + continue + } + val isQuoteEnd = c.isCloseSquareBracket() || c == '`' + if (c.isLetterOrDigit() || isQuoteEnd) { + if (!isQuoteEnd && isUrlOrPathEnd(text, i)) { + break + } + text.setLength(i + 1) + text.append('.') + } + break } + } + } +} + +/** Returns true if the given string appears to be the tail end of a URL or path reference. */ +private fun isUrlOrPathEnd(s: CharSequence, offset: Int): Boolean { + var i = offset - 1 + while (i >= 0) { + val c = s[i] + if (c == '/' || c == '.') { + return true + } else if (c.isWhitespace()) { break } + i-- } + return false } fun String.containsOnly(vararg s: Char): Boolean { diff --git a/library/src/main/kotlin/com/facebook/ktfmt/kdoc/Utilities.kt b/library/src/main/kotlin/com/facebook/ktfmt/kdoc/Utilities.kt index 597a285..21fe326 100644 --- a/library/src/main/kotlin/com/facebook/ktfmt/kdoc/Utilities.kt +++ b/library/src/main/kotlin/com/facebook/ktfmt/kdoc/Utilities.kt @@ -161,7 +161,7 @@ fun String.getParamName(): String? { } } - if (start < length && this[start] == '[') { + if (start < length && (this[start] == '[' || this[start] == '<')) { start++ while (start < length) { if (this[start].isWhitespace()) { diff --git a/library/src/main/resources/version.properties b/library/src/main/resources/version.properties index 016a66f..8919221 100644 --- a/library/src/main/resources/version.properties +++ b/library/src/main/resources/version.properties @@ -15,4 +15,4 @@ # # Release version definition -buildVersion = 1.6.6 +buildVersion = 1.6.7 diff --git a/library/src/test/kotlin/com/facebook/ktfmt/kdoc/KDocFormatterTest.kt b/library/src/test/kotlin/com/facebook/ktfmt/kdoc/KDocFormatterTest.kt index 523deeb..8cf868a 100644 --- a/library/src/test/kotlin/com/facebook/ktfmt/kdoc/KDocFormatterTest.kt +++ b/library/src/test/kotlin/com/facebook/ktfmt/kdoc/KDocFormatterTest.kt @@ -736,6 +736,82 @@ class KDocFormatterTest { .trimIndent()) } + @Test + fun testAddPunctuationWithBlocks() { + // Regression test for https://github.com/tnorbye/kdoc-formatter/issues/105 + val source = + """ + /** + * This is a test + * ```kotlin + * println("true") + * ``` + * This is a test + */ + """ + .trimIndent() + + val options = KDocFormattingOptions(72) + options.addPunctuation = true + checkFormatter( + source, + options, + """ + /** + * This is a test. + * + * ```kotlin + * println("true") + * ``` + * + * This is a test. + */ + """ + .trimIndent()) + } + + @Test + fun testDoNotPunctuateUrls() { + // * Make sure we don't punctuate file paths and URLs + // * Do punctuate sentences that end with ` + val source = + """ + /** + * Calling `setCommunicationDevice()` without `clearCommunicationDevice()` + * + * Creates a [Pattern] corresponding to a [ReplaceString.oldPattern] + * + * There is more documentation for this tool in lint/docs/internal/document-checks.md.html + * + * Initial focus is on the subset used and supported by GitHub: + * https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/sarif-support-for-code-scanning#supported-sarif-output-file-properties + */ + """ + .trimIndent() + + val options = KDocFormattingOptions(72) + options.addPunctuation = true + checkFormatter( + source, + options, + """ + /** + * Calling `setCommunicationDevice()` without + * `clearCommunicationDevice()`. + * + * Creates a [Pattern] corresponding to a + * [ReplaceString.oldPattern]. + * + * There is more documentation for this tool in + * lint/docs/internal/document-checks.md.html + * + * Initial focus is on the subset used and supported by GitHub: + * https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/sarif-support-for-code-scanning#supported-sarif-output-file-properties + */ + """ + .trimIndent()) + } + @Test fun testWrapingOfLinkText() { val source =