TrailingCommaInspection: should report Missing line break

#KT-34744

idea/src/org/jetbrains/kotlin/idea/formatter/TrailingCommaPostFormatProcessor.kt

@@ -6,20 +6,17 @@
 package org.jetbrains.kotlin.idea.formatter
 
 import com.intellij.application.options.CodeStyle
-import com.intellij.lang.ASTNode
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.util.TextRange
 import com.intellij.openapi.util.registry.Registry
-import com.intellij.psi.PsiComment
+import com.intellij.psi.*
-import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiFile
-import com.intellij.psi.PsiWhiteSpace
 import com.intellij.psi.codeStyle.CodeStyleManager
 import com.intellij.psi.codeStyle.CodeStyleSettings
 import com.intellij.psi.impl.source.PostprocessReformattingAspect
 import com.intellij.psi.impl.source.codeStyle.PostFormatProcessor
 import com.intellij.psi.impl.source.codeStyle.PostFormatProcessorHelper
 import com.intellij.psi.tree.TokenSet
+import com.intellij.psi.util.elementType
 import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor.Companion.findInvalidCommas
 import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor.Companion.needComma
@@ -33,7 +30,6 @@ import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.utils.addToStdlib.cast
-import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 
 class TrailingCommaPostFormatProcessor : PostFormatProcessor {
     override fun processElement(source: PsiElement, settings: CodeStyleSettings): PsiElement =
@@ -53,7 +49,7 @@ class TrailingCommaPostFormatProcessor : PostFormatProcessor {
         fun needComma(
             commaOwner: KtElement,
             settings: CodeStyleSettings = CodeStyle.getSettings(commaOwner.project),
-            checkExistingTrailingComma: Boolean = true
+            checkExistingTrailingComma: Boolean = true,
         ): Boolean = when {
             commaOwner is KtWhenEntry ->
                 commaOwner.needTrailingComma(settings, checkExistingTrailingComma)
@@ -71,23 +67,45 @@ class TrailingCommaPostFormatProcessor : PostFormatProcessor {
 
         fun trailingCommaOrLastElement(commaOwner: KtElement): PsiElement? {
             val lastChild = commaOwner.lastSignificantChild ?: return null
-            val withSelf = when (lastChild.safeAs<ASTNode>()?.elementType) {
+            val withSelf = when (lastChild.elementType) {
                 KtTokens.COMMA -> return lastChild
                 in RIGHT_BARRIERS -> false
                 else -> true
             }
 
             return lastChild.getPrevSiblingIgnoringWhitespaceAndComments(withSelf)?.takeIf {
-                it.safeAs<ASTNode>()?.elementType !in LEFT_BARRIERS
+                it.elementType !in LEFT_BARRIERS
-            }
+            }?.takeIfIsNotError()
         }
 
         fun trailingCommaAllowedInModule(source: PsiElement): Boolean =
             Registry.`is`("kotlin.formatter.allowTrailingCommaInAnyProject", false) ||
                     source.module?.languageVersionSettings?.supportsFeature(LanguageFeature.TrailingCommas) == true
+
+        fun elementBeforeFirstElement(commaOwner: KtElement): PsiElement? = when (commaOwner) {
+            is KtParameterList -> {
+                val parent = commaOwner.parent
+                if (parent is KtFunctionLiteral) parent.lBrace else commaOwner.leftParenthesis
+            }
+            is KtWhenEntry -> commaOwner.parent.cast<KtWhenExpression>().openBrace
+            is KtDestructuringDeclaration -> commaOwner.lPar
+            else -> commaOwner.firstChild?.takeIfIsNotError()
+        }
+
+        fun elementAfterLastElement(commaOwner: KtElement): PsiElement? = when (commaOwner) {
+            is KtParameterList -> {
+                val parent = commaOwner.parent
+                if (parent is KtFunctionLiteral) parent.arrow else commaOwner.rightParenthesis
+            }
+            is KtWhenEntry -> commaOwner.arrow
+            is KtDestructuringDeclaration -> commaOwner.rPar
+            else -> commaOwner.lastChild?.takeIfIsNotError()
+        }
     }
 }
 
+private fun PsiElement.takeIfIsNotError(): PsiElement? = takeIf { it !is PsiErrorElement }
+
 private class TrailingCommaPostFormatVisitor(val settings: CodeStyleSettings) : TrailingCommaVisitor() {
     private val myPostProcessor = PostFormatProcessorHelper(settings.kotlinCommonSettings)
 
@@ -103,7 +121,7 @@ private class TrailingCommaPostFormatVisitor(val settings: CodeStyleSettings) :
 
     private fun processCommaOwner(parent: KtElement) {
         val lastElement = trailingCommaOrLastElement(parent) ?: return
-        val elementType = lastElement.safeAs<ASTNode>()?.elementType
+        val elementType = lastElement.elementType
         when {
             needComma(parent, settings, false) -> {
                 // add a missing comma
@@ -152,7 +170,7 @@ private class TrailingCommaPostFormatVisitor(val settings: CodeStyleSettings) :
 
     fun processText(
         source: PsiFile,
-        rangeToReformat: TextRange
+        rangeToReformat: TextRange,
     ): TextRange {
         myPostProcessor.resultTextRange = rangeToReformat
         source.accept(this)
@@ -203,4 +221,4 @@ fun PsiElement.leaf(forward: Boolean = true, filter: (PsiElement) -> Boolean): P
     if (forward) nextLeaf(filter)
     else prevLeaf(filter)
 
-val PsiElement.isComma: Boolean get() = safeAs<ASTNode>()?.elementType === KtTokens.COMMA
+val PsiElement.isComma: Boolean get() = elementType === KtTokens.COMMA

idea/src/org/jetbrains/kotlin/idea/inspections/TrailingCommaInspection.kt

@@ -13,6 +13,7 @@ import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor
 import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor.Companion.findInvalidCommas
 import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor.Companion.needComma
 import org.jetbrains.kotlin.idea.formatter.TrailingCommaPostFormatProcessor.Companion.trailingCommaAllowedInModule
@@ -21,9 +22,11 @@ import org.jetbrains.kotlin.idea.formatter.TrailingCommaVisitor
 import org.jetbrains.kotlin.idea.formatter.isComma
 import org.jetbrains.kotlin.idea.formatter.leafIgnoringWhitespaceAndComments
 import org.jetbrains.kotlin.idea.quickfix.ReformatQuickFix
+import org.jetbrains.kotlin.idea.util.isLineBreak
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.psiUtil.endOffset
 import org.jetbrains.kotlin.psi.psiUtil.nextLeaf
+import org.jetbrains.kotlin.psi.psiUtil.prevLeaf
 import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import javax.swing.JComponent
 
@@ -38,10 +41,37 @@ class TrailingCommaInspection(
             val action = TrailingCommaAction.create(commaOwner)
             if (action != TrailingCommaAction.REMOVE) {
                 checkCommaPosition(commaOwner)
+                checkLineBreaks(commaOwner)
             }
             checkTrailingComma(commaOwner, action)
         }
 
+        private fun checkLineBreaks(commaOwner: KtElement) {
+            val first = TrailingCommaPostFormatProcessor.elementBeforeFirstElement(commaOwner)
+            if (first?.nextLeaf(true)?.isLineBreak() == false) {
+                first.nextSibling?.let {
+                    registerProblemForLineBreak(
+                        commaOwner,
+                        it,
+                        if (ApplicationManager.getApplication().isUnitTestMode)
+                            ProblemHighlightType.GENERIC_ERROR_OR_WARNING
+                        else
+                            ProblemHighlightType.INFORMATION,
+                    )
+                }
+
+            }
+
+            val last = TrailingCommaPostFormatProcessor.elementAfterLastElement(commaOwner)
+            if (last?.prevLeaf(true)?.isLineBreak() == false) {
+                registerProblemForLineBreak(
+                    commaOwner,
+                    last,
+                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                )
+            }
+        }
+
         private fun checkCommaPosition(commaOwner: KtElement) {
             for (invalidComma in findInvalidCommas(commaOwner)) {
                 reportProblem(invalidComma, "Comma loses the advantages in this position", "Fix comma position")
@@ -85,10 +115,25 @@ class TrailingCommaInspection(
             )
         }
 
+        private fun registerProblemForLineBreak(
+            commaOwner: KtElement,
+            elementForTextRange: PsiElement,
+            highlightType: ProblemHighlightType,
+        ) {
+            val problemElement = commaOwner.parent
+            holder.registerProblem(
+                problemElement,
+                "Missing line break",
+                highlightType,
+                TextRange.from(elementForTextRange.startOffset, 1).shiftLeft(problemElement.startOffset),
+                ReformatQuickFix("Add line break", commaOwner),
+            )
+        }
+
         private val PsiElement.textRangeOfCommaOrSymbolAfter: TextRange
             get() {
                 val textRange = textRange
-                if (textRange.length <= 1) return textRange
+                if (isComma) return textRange
 
                 val resultRange = nextLeaf()?.leafIgnoringWhitespaceAndComments(false)?.endOffset?.takeIf { it > 0 }?.let {
                     TextRange.create(it - 1, it).intersection(parent.textRange)

idea/testData/inspectionsLocal/trailingComma/missingLineBreak.kt

@@ -0,0 +1,5 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(i: Int,
+    b: Boolean<caret>) = Unit

idea/testData/inspectionsLocal/trailingComma/missingLineBreak.kt.after

@@ -0,0 +1,7 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(
+        i: Int,
+        b: Boolean,
+) = Unit

idea/testData/inspectionsLocal/trailingComma/missingLineBreak2.kt

@@ -0,0 +1,5 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(<caret>i: Int,
+    b: Boolean) = Unit

idea/testData/inspectionsLocal/trailingComma/missingLineBreak2.kt.after

@@ -0,0 +1,7 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(
+        i: Int,
+        b: Boolean,
+) = Unit

idea/testData/inspectionsLocal/trailingComma/missingLineBreak3.kt

@@ -0,0 +1,6 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(i: Int,
+    b: Boolean
+      , <caret>) = Unit

idea/testData/inspectionsLocal/trailingComma/missingLineBreak3.kt.after

@@ -0,0 +1,7 @@
+// COMPILER_ARGUMENTS: -XXLanguage:+TrailingCommas
+// FIX: Add line break
+
+fun a(
+        i: Int,
+        b: Boolean,
+) = Unit

idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java

@@ -12320,6 +12320,36 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
             runTest("idea/testData/inspectionsLocal/trailingComma/changeCommaPosition4.kt");
         }
 
+        @TestMetadata("missingLineBreak.kt")
+        public void testMissingLineBreak() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak.kt");
+        }
+
+        @TestMetadata("missingLineBreak2.kt")
+        public void testMissingLineBreak2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak2.kt");
+        }
+
+        @TestMetadata("missingLineBreak3.kt")
+        public void testMissingLineBreak3() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak3.kt");
+        }
+
+        @TestMetadata("missingLineBreak.kt")
+        public void testMissingLineBreak() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak.kt");
+        }
+
+        @TestMetadata("missingLineBreak2.kt")
+        public void testMissingLineBreak2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak2.kt");
+        }
+
+        @TestMetadata("missingLineBreak3.kt")
+        public void testMissingLineBreak3() throws Exception {
+            runTest("idea/testData/inspectionsLocal/trailingComma/missingLineBreak3.kt");
+        }
+
         @TestMetadata("removeComma.kt")
         public void testRemoveComma() throws Exception {
             runTest("idea/testData/inspectionsLocal/trailingComma/removeComma.kt");