Fix keyword group formatting inside parameter groups when enclosed by conditional loop directives

xterao · xterao · commit 0344060c4be0 · 2025-08-29T11:00:35.000+09:00
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/comma/SqlCommaBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/comma/SqlCommaBlock.kt
@@ -146,7 +146,14 @@ open class SqlCommaBlock(
                     }
                     else -> {
                         // No indent after ORDER BY within function parameters
-                        if (parent is SqlSecondKeywordBlock && parent.parentBlock is SqlFunctionParamBlock) {
+                        val grand = parent.parentBlock
+                        val conditionParent =
+                            if (grand is SqlElConditionLoopCommentBlock) {
+                                grand.parentBlock
+                            } else {
+                                grand
+                            }
+                        if (parent is SqlSecondKeywordBlock && conditionParent is SqlFunctionParamBlock) {
                             0
                         } else {
                             parent.indent.groupIndentLen.plus(1)
@@ -164,7 +171,14 @@ open class SqlCommaBlock(
         parentBlock?.let { parent ->
             if (parent is SqlConditionalExpressionGroupBlock) return false
             // Don't allow line breaks after ORDER BY within function parameters
-            if (parent.parentBlock is SqlFunctionParamBlock) {
+            val grand = parent.parentBlock
+            val conditionParent =
+                if (grand is SqlElConditionLoopCommentBlock) {
+                    grand.parentBlock
+                } else {
+                    grand
+                }
+            if (conditionParent is SqlFunctionParamBlock) {
                 return false
             }
             return TypeUtil.isExpectedClassType(EXPECTED_TYPES, parent)
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/comment/SqlElConditionLoopCommentBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/comment/SqlElConditionLoopCommentBlock.kt
@@ -74,7 +74,6 @@ class SqlElConditionLoopCommentBlock(
             listOf(
                 SqlSubGroupBlock::class,
                 SqlColumnRawGroupBlock::class,
-                SqlElConditionLoopCommentBlock::class,
             )
     }
 
@@ -173,10 +172,14 @@ class SqlElConditionLoopCommentBlock(
         if (conditionType.isEnd() || conditionType.isElse()) {
             return true
         }
+        if (lastGroup is SqlElConditionLoopCommentBlock) {
+            return true
+        }
+        val firstChild = lastGroup?.childBlocks?.firstOrNull()
         if (TypeUtil.isExpectedClassType(LINE_BREAK_PARENT_TYPES, lastGroup)) {
-            return lastGroup?.childBlocks?.dropLast(1)?.isNotEmpty() == true || lastGroup is SqlElConditionLoopCommentBlock
+            return firstChild != null && firstChild != this
         }
-        return lastGroup?.childBlocks?.firstOrNull() != this
+        return firstChild == null || firstChild != this
     }
 
     /**
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/keyword/SqlKeywordGroupBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/keyword/SqlKeywordGroupBlock.kt
@@ -179,8 +179,15 @@ open class SqlKeywordGroupBlock(
     override fun createGroupIndentLen(): Int = indent.indentLen.plus(topKeywordBlocks.sumOf { it.getNodeText().length.plus(1) }.minus(1))
 
     override fun isSaveSpace(lastGroup: SqlBlock?): Boolean {
+        val conditionLastGroup =
+            if (parentBlock is SqlElConditionLoopCommentBlock) {
+                parentBlock?.parentBlock
+            } else {
+                lastGroup
+            }
         val prevWord = prevBlocks.findLast { it is SqlKeywordBlock || it is SqlKeywordGroupBlock }
         return !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), prevWord?.getNodeText() ?: "") &&
+            !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), conditionLastGroup?.getNodeText() ?: "") &&
             !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), lastGroup?.getNodeText() ?: "") &&
             lastGroup !is SqlFunctionParamBlock
     }
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/keyword/second/SqlSecondKeywordBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/keyword/second/SqlSecondKeywordBlock.kt
@@ -63,9 +63,15 @@ open class SqlSecondKeywordBlock(
 
     override fun isSaveSpace(lastGroup: SqlBlock?): Boolean {
         parentBlock?.let { parent ->
-            if (parent is SqlFunctionParamBlock) {
+            val conditionParent =
+                if (parent is SqlElConditionLoopCommentBlock) {
+                    parent.parentBlock
+                } else {
+                    parent
+                }
+            if (conditionParent is SqlFunctionParamBlock) {
                 val firstKeywordParam =
-                    parent.childBlocks.firstOrNull { it is SqlNewGroupBlock }
+                    conditionParent.childBlocks.firstOrNull { it is SqlNewGroupBlock }
                 return firstKeywordParam != null && firstKeywordParam != this
             } else {
                 return super.isSaveSpace(lastGroup)
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubGroupBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubGroupBlock.kt
@@ -128,7 +128,14 @@ abstract class SqlSubGroupBlock(
                     return false
                 }
             }
-            return TypeUtil.isExpectedClassType(NEW_LINE_EXPECTED_TYPES, lastBlock.parentBlock)
+            val lastParent = lastBlock.parentBlock
+            val expectedParent =
+                if (lastParent is SqlElConditionLoopCommentBlock) {
+                    lastParent.parentBlock?.parentBlock
+                } else {
+                    lastBlock.parentBlock
+                }
+            return TypeUtil.isExpectedClassType(NEW_LINE_EXPECTED_TYPES, expectedParent)
         }
         return false
     }
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/word/SqlFunctionGroupBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/word/SqlFunctionGroupBlock.kt
@@ -39,7 +39,7 @@ class SqlFunctionGroupBlock(
         indent.groupIndentLen = createGroupIndentLen()
     }
 
-    override fun createBlockIndentLen(): Int = parentBlock?.indent?.groupIndentLen ?: 0
+    override fun createBlockIndentLen(): Int = parentBlock?.indent?.groupIndentLen?.plus(1) ?: 0
 
     override fun createGroupIndentLen(): Int {
         val baseIndent =
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt
@@ -529,9 +529,15 @@ class SqlBlockRelationBuilder(
             if (parentSubGroup is SqlFunctionParamBlock) {
                 // If the parent is a function parameter group, remove up to the parent function name and keyword group.
                 val parent = blockBuilder.getGroupTopNodeIndexHistory()[paramIndex].parentBlock
+                val searchFunctionName =
+                    if (parent is SqlElConditionLoopCommentBlock) {
+                        parent.parentBlock
+                    } else {
+                        parent
+                    }
                 val functionParent =
                     blockBuilder.getGroupTopNodeIndex {
-                        it == parent
+                        it == searchFunctionName
                     }
                 if (functionParent >= 0) {
                     blockBuilder.clearSubListGroupTopNodeIndexHistory(functionParent)
diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/processor/SqlFormatPreProcessor.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/processor/SqlFormatPreProcessor.kt
@@ -224,7 +224,9 @@ class SqlFormatPreProcessor : PreFormatProcessor {
     private fun getKeywordNewText(element: PsiElement): String {
         val keywordText = element.text.lowercase()
         val upperText = getUpperText(element)
-        return if (SqlKeywordUtil.getIndentType(keywordText).isNewLineGroup()) {
+        return if (SqlKeywordUtil.getIndentType(keywordText).isNewLineGroup() ||
+            element.prevSibling.elementType == SqlTypes.BLOCK_COMMENT
+        ) {
             val prevElement = element.prevSibling
             getNewLineString(prevElement, upperText)
         } else {
diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql
@@ -0,0 +1,28 @@
+SELECT e.id
+        -- with condition
+         , ROW_NUMBER() OVER(/*%if order */ORDER BY e.manager_id DESC/*%end*/) AS row_num
+       , RANK() OVER(PARTITION BY department_id
+/*%if order */ORDER BY e.manager_id DESC/*%end */
+ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS rank_num
+       , DENSE_RANK() OVER(PARTITION BY department_id
+ /*%if order */
+ORDER BY e.id ASC, e.manager_id ASC, created_at DESC
+    /*%else */
+ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING/*%end */) AS dense_rank_num
+       , SUM(amount)/*%if filter */ FILTER(WHERE status = 'active')/*%end*/ AS dept_salary_avg
+       , COUNT(*) OVER(ORDER BY e.id, e.manager_id, created_at
+    /*%if rows */
+ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING/*%end*/)
+       , FIRST_VALUE(salary) IGNORE NULLS OVER(PARTITION BY department_id
+ORDER BY e.id ASC, e.manager_id ASC, created_at DESC)
+       , LAST_VALUE() OVER(ORDER BY e.manager_id ASC
+    /*%if rows */
+ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+/*%end*/) AS bottom_salary
+       , LISTAGG(e.name
+                 , ', ') /*%if filter */WITHIN GROUP(ORDER BY name DESC)/*%end*/
+  FROM employees e
+ WHERE e.status = 1
+ ORDER BY e.id
+          , e
+          , manager_id
diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql
@@ -0,0 +1,41 @@
+SELECT e.id
+       -- with condition
+       , ROW_NUMBER() OVER(/*%if order */
+                           ORDER BY e.manager_id DESC
+                           /*%end*/) AS row_num
+       , RANK() OVER(PARTITION BY department_id
+                     /*%if order */
+                     ORDER BY e.manager_id DESC
+                     /*%end */
+                     ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS rank_num
+       , DENSE_RANK() OVER(PARTITION BY department_id
+                           /*%if order */
+                           ORDER BY e.id ASC, e.manager_id ASC, created_at DESC
+                           /*%else */
+                           ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+                           /*%end */) AS dense_rank_num
+       , SUM(amount)
+         /*%if filter */
+         FILTER(WHERE status = 'active')
+         /*%end*/
+         AS dept_salary_avg
+       , COUNT(*) OVER(ORDER BY e.id, e.manager_id, created_at
+                       /*%if rows */
+                       ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING
+                       /*%end*/)
+       , FIRST_VALUE(salary) IGNORE NULLS OVER(PARTITION BY department_id
+ORDER BY e.id ASC, e.manager_id ASC, created_at DESC)
+       , LAST_VALUE() OVER(ORDER BY e.manager_id ASC
+                           /*%if rows */
+                           ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+                           /*%end*/) AS bottom_salary
+       , LISTAGG(e.name
+                 , ', ') 
+         /*%if filter */         
+         WITHIN GROUP (ORDER BY name DESC)
+         /*%end*/
+  FROM employees e
+ WHERE e.status = 1
+ ORDER BY e.id
+          , e
+          , manager_id

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,6 @@ class SqlElConditionLoopCommentBlock(`
`74`	`74`	`listOf(`
`75`	`75`	`SqlSubGroupBlock::class,`
`76`	`76`	`SqlColumnRawGroupBlock::class,`
`77`		`- SqlElConditionLoopCommentBlock::class,`
`78`	`77`	`)`
`79`	`78`	`}`
`80`	`79`
`@@ -173,10 +172,14 @@ class SqlElConditionLoopCommentBlock(`
`173`	`172`	`if (conditionType.isEnd() \|\| conditionType.isElse()) {`
`174`	`173`	`return true`
`175`	`174`	`}`
	`175`	`+ if (lastGroup is SqlElConditionLoopCommentBlock) {`
	`176`	`+ return true`
	`177`	`+ }`
	`178`	`+ val firstChild = lastGroup?.childBlocks?.firstOrNull()`
`176`	`179`	`if (TypeUtil.isExpectedClassType(LINE_BREAK_PARENT_TYPES, lastGroup)) {`
`177`		`- return lastGroup?.childBlocks?.dropLast(1)?.isNotEmpty() == true \|\| lastGroup is SqlElConditionLoopCommentBlock`
	`180`	`+ return firstChild != null && firstChild != this`
`178`	`181`	`}`
`179`		`- return lastGroup?.childBlocks?.firstOrNull() != this`
	`182`	`+ return firstChild == null \|\| firstChild != this`
`180`	`183`	`}`
`181`	`184`
`182`	`185`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,14 @@ abstract class SqlSubGroupBlock(`
`128`	`128`	`return false`
`129`	`129`	`}`
`130`	`130`	`}`
`131`		`- return TypeUtil.isExpectedClassType(NEW_LINE_EXPECTED_TYPES, lastBlock.parentBlock)`
	`131`	`+ val lastParent = lastBlock.parentBlock`
	`132`	`+ val expectedParent =`
	`133`	`+ if (lastParent is SqlElConditionLoopCommentBlock) {`
	`134`	`+ lastParent.parentBlock?.parentBlock`
	`135`	`+ } else {`
	`136`	`+ lastBlock.parentBlock`
	`137`	`+ }`
	`138`	`+ return TypeUtil.isExpectedClassType(NEW_LINE_EXPECTED_TYPES, expectedParent)`
`132`	`139`	`}`
`133`	`140`	`return false`
`134`	`141`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ class SqlFunctionGroupBlock(`
`39`	`39`	`indent.groupIndentLen = createGroupIndentLen()`
`40`	`40`	`}`
`41`	`41`
`42`		`- override fun createBlockIndentLen(): Int = parentBlock?.indent?.groupIndentLen ?: 0`
	`42`	`+ override fun createBlockIndentLen(): Int = parentBlock?.indent?.groupIndentLen?.plus(1) ?: 0`
`43`	`43`
`44`	`44`	`override fun createGroupIndentLen(): Int {`
`45`	`45`	`val baseIndent =`