From a42bc7e264358bc86937fb87d38117bdef7ae3f7 Mon Sep 17 00:00:00 2001 From: xterao Date: Wed, 27 Aug 2025 18:02:50 +0900 Subject: [PATCH 01/13] Improve SQL formatting for ORDER BY and GROUP BY clauses, including adjustments for function parameter blocks --- .../intellij/formatter/block/SqlFileBlock.kt | 3 ++- .../formatter/block/comma/SqlCommaBlock.kt | 21 +++++++++++++--- .../group/keyword/SqlKeywordGroupBlock.kt | 4 ++- .../keyword/second/SqlSecondKeywordBlock.kt | 16 +++++++----- .../builder/SqlBlockRelationBuilder.kt | 5 +++- .../intellij/formatter/util/SqlKeywordUtil.kt | 2 +- .../intellij/formatter/SqlFormatterTest.kt | 4 +++ .../sql/formatter/DeleteWithSubQuery.sql | 5 +++- .../formatter/DeleteWithSubQuery_format.sql | 3 +++ .../testData/sql/formatter/OrderByGroup.sql | 25 +++++++++++++++++++ .../sql/formatter/OrderByGroup_format.sql | 16 ++++++++++++ 11 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 src/test/testData/sql/formatter/OrderByGroup.sql create mode 100644 src/test/testData/sql/formatter/OrderByGroup_format.sql diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt index 2ba0ad40..8afcd20b 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt @@ -495,9 +495,10 @@ open class SqlFileBlock( } if (childBlock1 is SqlSubGroupBlock) { - if (childBlock2 is SqlSubGroupBlock) { + if (childBlock2 is SqlSubGroupBlock || childBlock1 is SqlFunctionParamBlock) { return SqlCustomSpacingBuilder.nonSpacing } + if (childBlock1 is SqlInsertValueGroupBlock || childBlock1 is SqlUpdateValueGroupBlock ) { 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 index 79129f91..4b6c6a1c 100644 --- 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 @@ -26,6 +26,7 @@ import org.domaframework.doma.intellij.formatter.block.group.keyword.condition.S import org.domaframework.doma.intellij.formatter.block.group.keyword.insert.SqlInsertColumnGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.insert.SqlInsertValueGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.second.SqlFromGroupBlock +import org.domaframework.doma.intellij.formatter.block.group.keyword.second.SqlSecondKeywordBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.second.SqlValuesGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.update.SqlUpdateColumnGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.update.SqlUpdateSetGroupBlock @@ -143,7 +144,14 @@ open class SqlCommaBlock( val parentIndent = firstChild?.indent ?: parent.indent parentIndent.groupIndentLen.plus(1) } - else -> parent.indent.groupIndentLen.plus(1) + else -> { + // No indent after ORDER BY within function parameters + if (parent is SqlSecondKeywordBlock && parent.parentBlock is SqlFunctionParamBlock) { + 0 + } else { + parent.indent.groupIndentLen.plus(1) + } + } } } } @@ -153,7 +161,14 @@ open class SqlCommaBlock( override fun createGroupIndentLen(): Int = indent.indentLen.plus(1) override fun isSaveSpace(lastGroup: SqlBlock?): Boolean { - if (parentBlock is SqlConditionalExpressionGroupBlock) return false - return TypeUtil.isExpectedClassType(EXPECTED_TYPES, parentBlock) + 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) { + return false + } + return TypeUtil.isExpectedClassType(EXPECTED_TYPES, parent) + } + return false } } 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 index efc413c4..983a03e0 100644 --- 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 @@ -24,6 +24,7 @@ import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoo import org.domaframework.doma.intellij.formatter.block.group.SqlNewGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.top.SqlSelectQueryGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.with.SqlWithCommonTableGroupBlock +import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlFunctionParamBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock import org.domaframework.doma.intellij.formatter.util.IndentType import org.domaframework.doma.intellij.formatter.util.SqlBlockFormattingContext @@ -180,6 +181,7 @@ open class SqlKeywordGroupBlock( override fun isSaveSpace(lastGroup: SqlBlock?): Boolean { val prevWord = prevBlocks.lastOrNull() return !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), prevWord?.getNodeText() ?: "") && - !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), lastGroup?.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 index 018f1308..0e7f617c 100644 --- 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 @@ -22,9 +22,9 @@ import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoo import org.domaframework.doma.intellij.formatter.block.group.keyword.SqlKeywordGroupBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlFunctionParamBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock +import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubQueryGroupBlock import org.domaframework.doma.intellij.formatter.util.IndentType import org.domaframework.doma.intellij.formatter.util.SqlBlockFormattingContext -import org.domaframework.doma.intellij.formatter.util.SqlKeywordUtil open class SqlSecondKeywordBlock( node: ASTNode, @@ -38,6 +38,7 @@ open class SqlSecondKeywordBlock( indent.groupIndentLen = createGroupIndentLen() } + // TODO Calculate indent for ORDER BY within function parameters override fun createBlockIndentLen(): Int { parentBlock?.let { parent -> val groupLen = parent.indent.groupIndentLen @@ -56,11 +57,14 @@ open class SqlSecondKeywordBlock( override fun isSaveSpace(lastGroup: SqlBlock?): Boolean { lastGroup?.let { last -> - val prevKeyword = last.childBlocks.findLast { it is SqlKeywordBlock } - prevKeyword?.let { prev -> - return !SqlKeywordUtil.isSetLineKeyword(getNodeText(), prev.getNodeText()) && last !is SqlFunctionParamBlock - } - return !SqlKeywordUtil.isSetLineKeyword(getNodeText(), last.getNodeText()) + val isFirstGroup = + if (lastGroup is SqlFunctionParamBlock || lastGroup is SqlSubQueryGroupBlock) { + val firstKeywordParam = lastGroup.childBlocks.firstOrNull { it is SqlKeywordGroupBlock || it is SqlKeywordBlock } + firstKeywordParam == this + } else { + false + } + return super.isSaveSpace(parentBlock) && !isFirstGroup } return true } 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 index 8d4591c0..c45427f1 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt @@ -234,7 +234,10 @@ class SqlBlockRelationBuilder( } else -> { setParentGroups(context) { history -> - history.lastOrNull { it.indent.indentLevel < childBlock.indent.indentLevel } + history.lastOrNull { + it.indent.indentLevel < childBlock.indent.indentLevel || + TypeUtil.isTopLevelExpectedType(it) + } } } } diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt index a6814e14..0c327573 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt @@ -314,7 +314,7 @@ class SqlKeywordUtil { "outer" to setOf("left", "right"), "inner" to setOf("left", "right"), "by" to setOf("group", "order", "first"), - "and" to setOf("between"), + "and" to setOf("between", "preceding"), "if" to setOf("table", "create"), "exists" to setOf("if", "where"), "conflict" to setOf("on"), diff --git a/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt b/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt index ce035893..0cc2ca3b 100644 --- a/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt +++ b/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt @@ -270,6 +270,10 @@ class SqlFormatterTest : BasePlatformTestCase() { formatSqlFile("FunctionNameColumn.sql", "FunctionNameColumn$formatDataPrefix.sql") } + fun testOrderByGroupFormatter() { + formatSqlFile("OrderByGroup.sql", "OrderByGroup$formatDataPrefix.sql") + } + private fun formatSqlFile( beforeFile: String, afterFile: String, diff --git a/src/test/testData/sql/formatter/DeleteWithSubQuery.sql b/src/test/testData/sql/formatter/DeleteWithSubQuery.sql index b79297f0..ea30fa3f 100644 --- a/src/test/testData/sql/formatter/DeleteWithSubQuery.sql +++ b/src/test/testData/sql/formatter/DeleteWithSubQuery.sql @@ -8,4 +8,7 @@ DELETE FROM user_session s FROM user u WHERE u.id = /* id */1 AND u.session_id = u.id -AND u.time_stamp < /* current */'2099-12-31 00:00:00') \ No newline at end of file +AND u.time_stamp < /* current */'2099-12-31 00:00:00') + AND s.user_number IN (SELECT u.number + FROM user u + WHERE u.status = /* status */'active' ) \ No newline at end of file diff --git a/src/test/testData/sql/formatter/DeleteWithSubQuery_format.sql b/src/test/testData/sql/formatter/DeleteWithSubQuery_format.sql index 9786eb04..b1c14a62 100644 --- a/src/test/testData/sql/formatter/DeleteWithSubQuery_format.sql +++ b/src/test/testData/sql/formatter/DeleteWithSubQuery_format.sql @@ -9,3 +9,6 @@ DELETE FROM user_session s WHERE u.id = /* id */1 AND u.session_id = u.id AND u.time_stamp < /* current */'2099-12-31 00:00:00' ) + AND s.user_number IN ( SELECT u.number + FROM user u + WHERE u.status = /* status */'active' ) diff --git a/src/test/testData/sql/formatter/OrderByGroup.sql b/src/test/testData/sql/formatter/OrderByGroup.sql new file mode 100644 index 00000000..4c1ba21b --- /dev/null +++ b/src/test/testData/sql/formatter/OrderByGroup.sql @@ -0,0 +1,25 @@ +SELECT e.id + , e.name + , ROW_NUMBER() OVER( + ORDER BY e.manager_id DESC) AS row_num + , RANK() OVER( + ORDER BY e.manager_id DESC) AS rank_num + , DENSE_RANK() OVER( + ORDER BY e.manager_id DESC) AS dense_rank_num + , SUM(e.manager_id) OVER(PARTITION BY e.id +ORDER BY e.manager_id DESC) AS dept_salary_sum + , AVG(e.manager_id) OVER( +PARTITION BY e.id) AS dept_salary_avg + , COUNT(*) OVER( +PARTITION BY e.id) AS dept_count + , FIRST_VALUE(e.manager_id) OVER( + ORDER BY e.manager_id DESC) AS top_salary + , LAST_VALUE(e.manager_id) OVER( + ORDER BY e.manager_id ASC +ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary + , LEAD(e.manager_id) OVER( + ORDER BY e.manager_id DESC) AS next_salary + , LAG(e.manager_id) OVER( + ORDER BY e.manager_id DESC) AS prev_salary + FROM employees e + order by e.id desc,e.name asc \ No newline at end of file diff --git a/src/test/testData/sql/formatter/OrderByGroup_format.sql b/src/test/testData/sql/formatter/OrderByGroup_format.sql new file mode 100644 index 00000000..08f88c1f --- /dev/null +++ b/src/test/testData/sql/formatter/OrderByGroup_format.sql @@ -0,0 +1,16 @@ +SELECT e.id + , e.name + , ROW_NUMBER() OVER(ORDER BY e.manager_id DESC) AS row_num + , RANK() OVER(ORDER BY e.manager_id DESC) AS rank_num + , DENSE_RANK() OVER(ORDER BY e.manager_id DESC) AS dense_rank_num + , SUM(e.manager_id) OVER(PARTITION BY e.id ORDER BY e.manager_id DESC) AS dept_salary_sum + , AVG(e.manager_id) OVER(PARTITION BY e.id) AS dept_salary_avg + , COUNT(*) OVER(PARTITION BY e.id) AS dept_count + , FIRST_VALUE(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS top_salary + , LAST_VALUE(e.manager_id) OVER(ORDER BY e.manager_id ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary + , LEAD(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS next_salary + , LAG(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS prev_salary + FROM employees e + WHERE e.status = 1 + ORDER BY e.id DESC + , e.name ASC From ea32943559686fba6d259dc65d685815b9a7f0c7 Mon Sep 17 00:00:00 2001 From: xterao Date: Thu, 28 Aug 2025 11:21:48 +0900 Subject: [PATCH 02/13] Fixed the line break logic so that WITH within a GROUP BY clause is formatted correctly. --- .../intellij/tokens/OracleFunctionToken.java | 1 + .../intellij/tokens/SqlFunctionToken.java | 39 +++++++++--------- .../intellij/tokens/SqlKeywordTokenUtil.java | 4 +- .../doma/intellij/common/util/TypeUtil.kt | 2 + .../formatter/block/SqlRightPatternBlock.kt | 2 + .../group/keyword/SqlKeywordGroupBlock.kt | 2 +- .../keyword/second/SqlSecondKeywordBlock.kt | 30 ++++++++------ .../group/subgroup/SqlSubQueryGroupBlock.kt | 6 +-- .../builder/SqlBlockRelationBuilder.kt | 40 +++++++++++++++++-- .../formatter/handler/NotQueryGroupHandler.kt | 4 ++ .../intellij/formatter/util/SqlKeywordUtil.kt | 3 +- 11 files changed, 91 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/domaframework/doma/intellij/tokens/OracleFunctionToken.java b/src/main/java/org/domaframework/doma/intellij/tokens/OracleFunctionToken.java index f81408fd..aa1cbff3 100644 --- a/src/main/java/org/domaframework/doma/intellij/tokens/OracleFunctionToken.java +++ b/src/main/java/org/domaframework/doma/intellij/tokens/OracleFunctionToken.java @@ -38,6 +38,7 @@ public class OracleFunctionToken { "length", "lengthb", "length4", + "listagg", "cast", "numtodsinterval", "numtoyminterval", diff --git a/src/main/java/org/domaframework/doma/intellij/tokens/SqlFunctionToken.java b/src/main/java/org/domaframework/doma/intellij/tokens/SqlFunctionToken.java index ed434b02..9f96872d 100644 --- a/src/main/java/org/domaframework/doma/intellij/tokens/SqlFunctionToken.java +++ b/src/main/java/org/domaframework/doma/intellij/tokens/SqlFunctionToken.java @@ -25,31 +25,32 @@ public class SqlFunctionToken { static { TOKENS.addAll( Set.of( - "coalesce", - "row_number", - "row_count", - "sum", "avg", + "btrim", + "coalesce", "count", - "max", - "min", + "current_date", + "current_timestamp", + "dense_rank", + "filter", "log", - "substring", - "trim", "ltrim", - "rtlim", - "trim_array", - "btrim", - "replace", - "regexp_replace", + "max", + "min", + "mod", + "now", "over", - "rank", "percent_rank", - "dense_rank", - "current_date", - "current_timestamp", - "now", - "mod")); + "rank", + "regexp_replace", + "replace", + "row_count", + "row_number", + "rtlim", + "substring", + "sum", + "trim", + "trim_array")); } public static Set getTokens() { diff --git a/src/main/java/org/domaframework/doma/intellij/tokens/SqlKeywordTokenUtil.java b/src/main/java/org/domaframework/doma/intellij/tokens/SqlKeywordTokenUtil.java index a005c18e..fe592e00 100644 --- a/src/main/java/org/domaframework/doma/intellij/tokens/SqlKeywordTokenUtil.java +++ b/src/main/java/org/domaframework/doma/intellij/tokens/SqlKeywordTokenUtil.java @@ -107,6 +107,7 @@ public class SqlKeywordTokenUtil { "then", "to", "truncate", + "unbounded", "union", "unique", "update", @@ -115,7 +116,8 @@ public class SqlKeywordTokenUtil { "view", "when", "where", - "with")); + "with", + "within")); } public static Set getTokens() { diff --git a/src/main/kotlin/org/domaframework/doma/intellij/common/util/TypeUtil.kt b/src/main/kotlin/org/domaframework/doma/intellij/common/util/TypeUtil.kt index 691df5b8..7a82a7a6 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/common/util/TypeUtil.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/common/util/TypeUtil.kt @@ -28,6 +28,7 @@ import org.domaframework.doma.intellij.extension.psi.isDomain import org.domaframework.doma.intellij.extension.psi.isEntity import org.domaframework.doma.intellij.formatter.block.SqlBlock import org.domaframework.doma.intellij.formatter.block.comma.SqlCommaBlock +import org.domaframework.doma.intellij.formatter.block.group.column.SqlColumnRawGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.create.SqlCreateViewGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.with.SqlWithQuerySubGroupBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock @@ -38,6 +39,7 @@ object TypeUtil { listOf( SqlSubGroupBlock::class, SqlCommaBlock::class, + SqlColumnRawGroupBlock::class, SqlWithQuerySubGroupBlock::class, SqlCreateViewGroupBlock::class, ) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlRightPatternBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlRightPatternBlock.kt index 2b2a3b65..79eee5cd 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlRightPatternBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlRightPatternBlock.kt @@ -210,6 +210,8 @@ open class SqlRightPatternBlock( val firstChild = parent.getChildBlocksDropLast().firstOrNull() if (firstChild is SqlKeywordGroupBlock) { + // For subgroups other than function parameters, if the first element is a keyword group, add a line break before the closing parenthesis except at the top level. + // For subgroups created by WITHIN GROUP (), do not add a line break. val lineBreak = firstChild.indent.indentLevel != IndentType.TOP && !isExpectedClassType(NOT_NEW_LINE_EXPECTED_TYPES, parent) 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 index 983a03e0..4c927a80 100644 --- 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,7 +179,7 @@ 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 prevWord = prevBlocks.lastOrNull() + val prevWord = prevBlocks.findLast { it is SqlKeywordBlock || it is SqlKeywordGroupBlock } return !SqlKeywordUtil.isSetLineKeyword(this.getNodeText(), prevWord?.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 index 0e7f617c..57875110 100644 --- 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 @@ -16,13 +16,14 @@ package org.domaframework.doma.intellij.formatter.block.group.keyword.second import com.intellij.lang.ASTNode +import org.domaframework.doma.intellij.common.util.TypeUtil import org.domaframework.doma.intellij.formatter.block.SqlBlock -import org.domaframework.doma.intellij.formatter.block.SqlKeywordBlock +import org.domaframework.doma.intellij.formatter.block.SqlRightPatternBlock import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoopCommentBlock +import org.domaframework.doma.intellij.formatter.block.group.SqlNewGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.SqlKeywordGroupBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlFunctionParamBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock -import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubQueryGroupBlock import org.domaframework.doma.intellij.formatter.util.IndentType import org.domaframework.doma.intellij.formatter.util.SqlBlockFormattingContext @@ -38,14 +39,19 @@ open class SqlSecondKeywordBlock( indent.groupIndentLen = createGroupIndentLen() } - // TODO Calculate indent for ORDER BY within function parameters override fun createBlockIndentLen(): Int { parentBlock?.let { parent -> val groupLen = parent.indent.groupIndentLen return if (parent.indent.indentLevel == IndentType.FILE) { offset } else if (parent is SqlSubGroupBlock) { - groupLen.plus(1) + val space = + if (TypeUtil.isExpectedClassType(SqlRightPatternBlock.NOT_INDENT_EXPECTED_TYPES, parent)) { + 0 + } else { + 1 + } + groupLen.plus(space) } else if (parent is SqlElConditionLoopCommentBlock) { groupLen } else { @@ -56,15 +62,13 @@ open class SqlSecondKeywordBlock( } override fun isSaveSpace(lastGroup: SqlBlock?): Boolean { - lastGroup?.let { last -> - val isFirstGroup = - if (lastGroup is SqlFunctionParamBlock || lastGroup is SqlSubQueryGroupBlock) { - val firstKeywordParam = lastGroup.childBlocks.firstOrNull { it is SqlKeywordGroupBlock || it is SqlKeywordBlock } - firstKeywordParam == this - } else { - false - } - return super.isSaveSpace(parentBlock) && !isFirstGroup + parentBlock?.let { parent -> + if (parent is SqlFunctionParamBlock) { + val firstKeywordParam = + parent.childBlocks.firstOrNull { it is SqlNewGroupBlock } + return firstKeywordParam != null && firstKeywordParam != this + } + return super.isSaveSpace(parent) } return true } diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubQueryGroupBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubQueryGroupBlock.kt index 19ec3387..7cc61e03 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubQueryGroupBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/group/subgroup/SqlSubQueryGroupBlock.kt @@ -66,14 +66,14 @@ open class SqlSubQueryGroupBlock( is SqlJoinQueriesGroupBlock -> return parent.indent.indentLen is SqlJoinGroupBlock -> return parent.indent.groupIndentLen.plus(1) else -> { - val children = prevChildren?.filter { shouldIncludeChildBlock(it, parent) } + val children = prevChildren?.filter { shouldIncludeChildBlock(it, parent) }?.dropLast(1) // Retrieve the list of child blocks excluding the conditional directive that appears immediately before this block, // as it is already included as a child block. val sumChildren = if (children?.firstOrNull() is SqlElConditionLoopCommentBlock) { - children.drop(1).dropLast(1) + children.drop(1) } else { - children?.dropLast(1) ?: emptyList() + children ?: emptyList() } return sumChildren .sumOf { prev -> 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 index c45427f1..28ab7ada 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt @@ -19,6 +19,7 @@ import org.domaframework.doma.intellij.common.util.TypeUtil import org.domaframework.doma.intellij.formatter.block.SqlBlock import org.domaframework.doma.intellij.formatter.block.SqlKeywordBlock import org.domaframework.doma.intellij.formatter.block.SqlRightPatternBlock +import org.domaframework.doma.intellij.formatter.block.comma.SqlCommaBlock import org.domaframework.doma.intellij.formatter.block.comment.SqlDefaultCommentBlock import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoopCommentBlock import org.domaframework.doma.intellij.formatter.block.conflict.SqlDoGroupBlock @@ -234,15 +235,46 @@ class SqlBlockRelationBuilder( } else -> { setParentGroups(context) { history -> - history.lastOrNull { - it.indent.indentLevel < childBlock.indent.indentLevel || - TypeUtil.isTopLevelExpectedType(it) - } + getLastGroupKeywordText(history, childBlock) ?: history.lastOrNull() } } } } + /** + * Searches for the most recent group block with a lower indent level than the current block + * and returns it as a candidate for the parent block. + * + * @note + * If the most recent group block is a comma, it checks its parent and grandparent blocks + * to determine the appropriate parent block. + * + * @example + * OVER(ORDER BY e.id, e.manager_id, created_at + * ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING) + */ + private fun getLastGroupKeywordText( + history: MutableList, + childBlock: SqlBlock, + ): SqlBlock? { + val lastGroupBlock = + history.lastOrNull { + it.indent.indentLevel < childBlock.indent.indentLevel || + // Add [SqlColumnRawGroupBlock] as a parent block candidate + // to support cases like WITHIN GROUP used in column lines. + TypeUtil.isTopLevelExpectedType(it) + } + + if (lastGroupBlock is SqlCommaBlock) { + val lastGroupParentLevel = lastGroupBlock.parentBlock?.indent?.indentLevel ?: IndentType.NONE + if (lastGroupParentLevel < childBlock.indent.indentLevel) { + return lastGroupBlock.parentBlock + } + return lastGroupBlock.parentBlock?.parentBlock + } + return lastGroupBlock + } + private fun handleSameLevelKeyword( lastGroupBlock: SqlBlock, childBlock: SqlKeywordGroupBlock, diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/handler/NotQueryGroupHandler.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/handler/NotQueryGroupHandler.kt index 199fd1bc..9188d0ee 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/handler/NotQueryGroupHandler.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/handler/NotQueryGroupHandler.kt @@ -34,6 +34,7 @@ import org.domaframework.doma.intellij.formatter.block.word.SqlAliasBlock import org.domaframework.doma.intellij.formatter.block.word.SqlTableBlock import org.domaframework.doma.intellij.formatter.block.word.SqlWordBlock import org.domaframework.doma.intellij.formatter.util.SqlBlockFormattingContext +import org.domaframework.doma.intellij.formatter.util.SqlKeywordUtil object NotQueryGroupHandler { private const val RETURNING_KEYWORD = "returning" @@ -53,6 +54,7 @@ object NotQueryGroupHandler { hasFunctionOrAliasContext(lastGroup) -> createFunctionOrValueBlock(lastGroup, child, sqlBlockFormattingCtx) lastGroup is SqlConflictClauseBlock -> SqlConflictExpressionSubGroupBlock(child, sqlBlockFormattingCtx) hasValuesContext(lastGroup) -> SqlValuesParamGroupBlock(child, sqlBlockFormattingCtx) + hasParameterKeyword(lastGroup) -> SqlFunctionParamBlock(child, sqlBlockFormattingCtx) else -> null } @@ -106,6 +108,8 @@ object NotQueryGroupHandler { return false } + private fun hasParameterKeyword(lastGroup: SqlBlock?): Boolean = SqlKeywordUtil.hasFilterParam(lastGroup?.getNodeText() ?: "") + /** * Creates either a function parameter block or values parameter block based on the previous child type. */ diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt index 0c327573..f5fc7691 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt @@ -313,7 +313,8 @@ class SqlKeywordUtil { "join" to setOf("outer", "inner", "left", "right"), "outer" to setOf("left", "right"), "inner" to setOf("left", "right"), - "by" to setOf("group", "order", "first"), + "group" to setOf("within"), + "by" to setOf("group", "order", "first", "partition"), "and" to setOf("between", "preceding"), "if" to setOf("table", "create"), "exists" to setOf("if", "where"), From 1af85967153b06e01146c29450f7cab8844d774b Mon Sep 17 00:00:00 2001 From: xterao Date: Thu, 28 Aug 2025 15:12:49 +0900 Subject: [PATCH 03/13] Adjust text length to include closing parenthesis '(' when the preceding function block has no argument parameters --- .../org/domaframework/doma/intellij/formatter/block/SqlBlock.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt index 8baa2450..13c8f05c 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt @@ -83,7 +83,7 @@ open class SqlBlock( return when { nonCommentChildren.isNotEmpty() -> child.getChildrenTextLen() + child.getNodeText().length - isExcludedFromTextLength(child) -> 0 + isExcludedFromTextLength(child) -> if (childBlocks.firstOrNull() == child) child.getNodeText().length else 0 else -> child.getNodeText().length + DEFAULT_TEXT_LENGTH_INCREMENT } } From 4a6547173ad0563203d460b6cf740edbb8557ca3 Mon Sep 17 00:00:00 2001 From: xterao Date: Thu, 28 Aug 2025 15:12:55 +0900 Subject: [PATCH 04/13] Add 'rows' to SQL keywords and introduce filter parameter check --- .../doma/intellij/formatter/util/SqlKeywordUtil.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt index f5fc7691..229cdcb6 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlKeywordUtil.kt @@ -131,6 +131,7 @@ class SqlKeywordUtil { "group", "having", "order", + "rows", ) fun isSelectSecondOptionKeyword(keyword: String): Boolean = SELECT_SECOND_OPTION_KEYWORD.contains(keyword.lowercase()) @@ -301,6 +302,11 @@ class SqlKeywordUtil { fun isExistsKeyword(keyword: String): Boolean = EXISTS_KEYWORDS.contains(keyword.lowercase()) + private val HAS_FILTER_PARAM = + setOf("group", "in", "over", "into", "values", "filter", "references", "using") + + fun hasFilterParam(keyword: String): Boolean = HAS_FILTER_PARAM.contains(keyword.lowercase()) + private val SET_LINE_KEYWORDS = mapOf( "into" to setOf("insert"), @@ -323,7 +329,7 @@ class SqlKeywordUtil { "constraint" to setOf("on"), "update" to setOf("do"), "set" to setOf("by", "cycle"), - "order" to setOf("partition", "by"), + "order" to setOf("partition"), "select" to setOf("if", "exists"), ) From cb4758556a387b6fc423a92306b0889c95791998 Mon Sep 17 00:00:00 2001 From: xterao Date: Thu, 28 Aug 2025 18:20:03 +0900 Subject: [PATCH 05/13] Set parent block based on whether adjacent keyword groups at the same level should be combined into a single line --- .../keyword/second/SqlSecondKeywordBlock.kt | 3 +- .../block/word/SqlFunctionGroupBlock.kt | 3 +- .../builder/SqlBlockRelationBuilder.kt | 55 +++++++++++++++---- .../formatter/util/SqlBlockGenerator.kt | 7 +++ .../testData/sql/formatter/OrderByGroup.sql | 41 +++++++------- .../sql/formatter/OrderByGroup_format.sql | 30 ++++++---- 6 files changed, 93 insertions(+), 46 deletions(-) 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 index 57875110..5a1f81a2 100644 --- 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 @@ -67,8 +67,9 @@ open class SqlSecondKeywordBlock( val firstKeywordParam = parent.childBlocks.firstOrNull { it is SqlNewGroupBlock } return firstKeywordParam != null && firstKeywordParam != this + } else { + return super.isSaveSpace(lastGroup) } - return super.isSaveSpace(parent) } return true } 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 index 036b309f..16f7cf7c 100644 --- 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 @@ -22,7 +22,8 @@ import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoo import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlFunctionParamBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock import org.domaframework.doma.intellij.formatter.util.SqlBlockFormattingContext -import org.domaframework.doma.intellij.psi.SqlTypes +import kotlin.collections.emptyList +import kotlin.collections.toList class SqlFunctionGroupBlock( node: ASTNode, 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 index 28ab7ada..51f4c836 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt @@ -19,7 +19,6 @@ import org.domaframework.doma.intellij.common.util.TypeUtil import org.domaframework.doma.intellij.formatter.block.SqlBlock import org.domaframework.doma.intellij.formatter.block.SqlKeywordBlock import org.domaframework.doma.intellij.formatter.block.SqlRightPatternBlock -import org.domaframework.doma.intellij.formatter.block.comma.SqlCommaBlock import org.domaframework.doma.intellij.formatter.block.comment.SqlDefaultCommentBlock import org.domaframework.doma.intellij.formatter.block.comment.SqlElConditionLoopCommentBlock import org.domaframework.doma.intellij.formatter.block.conflict.SqlDoGroupBlock @@ -37,7 +36,9 @@ import org.domaframework.doma.intellij.formatter.block.group.keyword.top.SqlTopQ import org.domaframework.doma.intellij.formatter.block.group.keyword.update.SqlUpdateQueryGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.with.SqlWithCommonTableGroupBlock import org.domaframework.doma.intellij.formatter.block.group.keyword.with.SqlWithQuerySubGroupBlock +import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlFunctionParamBlock import org.domaframework.doma.intellij.formatter.block.group.subgroup.SqlSubGroupBlock +import org.domaframework.doma.intellij.formatter.block.word.SqlAliasBlock import org.domaframework.doma.intellij.formatter.block.word.SqlFunctionGroupBlock import org.domaframework.doma.intellij.formatter.handler.UpdateClauseHandler import org.domaframework.doma.intellij.formatter.util.IndentType @@ -264,13 +265,25 @@ class SqlBlockRelationBuilder( // to support cases like WITHIN GROUP used in column lines. TypeUtil.isTopLevelExpectedType(it) } - - if (lastGroupBlock is SqlCommaBlock) { - val lastGroupParentLevel = lastGroupBlock.parentBlock?.indent?.indentLevel ?: IndentType.NONE + if (lastGroupBlock == null) return lastGroupBlock + + if (lastGroupBlock.indent.indentLevel > childBlock.indent.indentLevel && lastGroupBlock !is SqlSubGroupBlock) { + val lastParent = lastGroupBlock.parentBlock + val lastGroupParentLevel = lastParent?.indent?.indentLevel ?: IndentType.NONE + val lastKeyword = + lastGroupBlock.childBlocks + .lastOrNull { + it is SqlKeywordBlock || it is SqlKeywordGroupBlock + }?.getNodeText() ?: "" + val setKeyword = SqlKeywordUtil.isSetLineKeyword(childBlock.getNodeText(), lastKeyword) if (lastGroupParentLevel < childBlock.indent.indentLevel) { - return lastGroupBlock.parentBlock + return if (setKeyword) { + lastGroupBlock + } else { + lastParent + } } - return lastGroupBlock.parentBlock?.parentBlock + return lastParent?.parentBlock } return lastGroupBlock } @@ -281,8 +294,13 @@ class SqlBlockRelationBuilder( context: SetParentContext, ) { val prevKeyword = lastGroupBlock.childBlocks.findLast { it is SqlKeywordBlock } - if (prevKeyword != null && SqlKeywordUtil.Companion.isSetLineKeyword(childBlock.getNodeText(), prevKeyword.getNodeText())) { - updateGroupBlockLastGroupParentAddGroup(lastGroupBlock, childBlock) + if (prevKeyword != null && + SqlKeywordUtil.isSetLineKeyword( + childBlock.getNodeText(), + prevKeyword.getNodeText(), + ) + ) { + updateGroupBlockLastGroupParentAddGroup(prevKeyword, childBlock) return } @@ -497,7 +515,8 @@ class SqlBlockRelationBuilder( return@setParentGroups history[paramIndex] } - if (blockBuilder.getGroupTopNodeIndexHistory()[paramIndex] is SqlWithQuerySubGroupBlock) { + val parentSubGroup = blockBuilder.getGroupTopNodeIndexHistory()[paramIndex] + if (parentSubGroup is SqlWithQuerySubGroupBlock) { val withCommonBlockIndex = blockBuilder.getGroupTopNodeIndex { block -> block is SqlWithCommonTableGroupBlock @@ -505,9 +524,21 @@ class SqlBlockRelationBuilder( if (withCommonBlockIndex >= 0) { blockBuilder.clearSubListGroupTopNodeIndexHistory(withCommonBlockIndex) } - } else { - blockBuilder.clearSubListGroupTopNodeIndexHistory(paramIndex) + return + } + 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 functionParent = + blockBuilder.getGroupTopNodeIndex { + it == parent + } + if (functionParent >= 0) { + blockBuilder.clearSubListGroupTopNodeIndexHistory(functionParent) + return + } } + blockBuilder.clearSubListGroupTopNodeIndexHistory(paramIndex) } } @@ -516,7 +547,7 @@ class SqlBlockRelationBuilder( childBlock: SqlSubGroupBlock, ) { val prevBlock = lastGroupBlock.childBlocks.lastOrNull() - if (prevBlock is SqlFunctionGroupBlock) { + if (prevBlock is SqlFunctionGroupBlock || prevBlock is SqlAliasBlock) { setParentGroups( SetParentContext( childBlock, diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlBlockGenerator.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlBlockGenerator.kt index 4b002d4b..b03736f8 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlBlockGenerator.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/util/SqlBlockGenerator.kt @@ -455,6 +455,11 @@ class SqlBlockGenerator( child, sqlBlockFormattingCtx, ) + + lastGroup is SqlInsertQueryGroupBlock -> return SqlTableBlock( + child, + sqlBlockFormattingCtx, + ) } } @@ -478,6 +483,8 @@ class SqlBlockGenerator( if (lastGroup is SqlFromGroupBlock || lastGroup?.parentBlock is SqlFromGroupBlock) { return SqlAliasBlock(child, sqlBlockFormattingCtx) } + val functionNameBlock = getFunctionName(child, sqlBlockFormattingCtx) + if (functionNameBlock != null) return functionNameBlock return SqlWordBlock(child, sqlBlockFormattingCtx) } diff --git a/src/test/testData/sql/formatter/OrderByGroup.sql b/src/test/testData/sql/formatter/OrderByGroup.sql index 4c1ba21b..8c8394ed 100644 --- a/src/test/testData/sql/formatter/OrderByGroup.sql +++ b/src/test/testData/sql/formatter/OrderByGroup.sql @@ -1,25 +1,24 @@ SELECT e.id , e.name - , ROW_NUMBER() OVER( - ORDER BY e.manager_id DESC) AS row_num - , RANK() OVER( - ORDER BY e.manager_id DESC) AS rank_num - , DENSE_RANK() OVER( - ORDER BY e.manager_id DESC) AS dense_rank_num - , SUM(e.manager_id) OVER(PARTITION BY e.id -ORDER BY e.manager_id DESC) AS dept_salary_sum - , AVG(e.manager_id) OVER( -PARTITION BY e.id) AS dept_salary_avg - , COUNT(*) OVER( -PARTITION BY e.id) AS dept_count - , FIRST_VALUE(e.manager_id) OVER( - ORDER BY e.manager_id DESC) AS top_salary - , LAST_VALUE(e.manager_id) OVER( - ORDER BY e.manager_id ASC + , ROW_NUMBER() OVER(ORDER BY e.manager_id DESC) AS row_num + , RANK() OVER(PARTITION BY department_id +ORDER BY e.manager_id DESC +ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS rank_num + , DENSE_RANK() OVER(PARTITION BY department_id +ORDER BY e.id ASC, e.manager_id ASC, created_at DESC +ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS dense_rank_num + , SUM(amount) FILTER(WHERE status = 'active') AS dept_salary_avg + , COUNT(*) OVER(ORDER BY e.id, e.manager_id, created_at +ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING) + , 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 ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary - , LEAD(e.manager_id) OVER( - ORDER BY e.manager_id DESC) AS next_salary - , LAG(e.manager_id) OVER( - ORDER BY e.manager_id DESC) AS prev_salary + , COUNT(*) FILTER(WHERE gender = 'F') AS female_count + , LISTAGG(e.name + , ', ') WITHIN GROUP(ORDER BY name DESC) FROM employees e - order by e.id desc,e.name asc \ No newline at end of file + WHERE e.status = 1 + ORDER BY e.id + , e + , manager_id diff --git a/src/test/testData/sql/formatter/OrderByGroup_format.sql b/src/test/testData/sql/formatter/OrderByGroup_format.sql index 08f88c1f..d70b46d9 100644 --- a/src/test/testData/sql/formatter/OrderByGroup_format.sql +++ b/src/test/testData/sql/formatter/OrderByGroup_format.sql @@ -1,16 +1,24 @@ SELECT e.id , e.name , ROW_NUMBER() OVER(ORDER BY e.manager_id DESC) AS row_num - , RANK() OVER(ORDER BY e.manager_id DESC) AS rank_num - , DENSE_RANK() OVER(ORDER BY e.manager_id DESC) AS dense_rank_num - , SUM(e.manager_id) OVER(PARTITION BY e.id ORDER BY e.manager_id DESC) AS dept_salary_sum - , AVG(e.manager_id) OVER(PARTITION BY e.id) AS dept_salary_avg - , COUNT(*) OVER(PARTITION BY e.id) AS dept_count - , FIRST_VALUE(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS top_salary - , LAST_VALUE(e.manager_id) OVER(ORDER BY e.manager_id ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary - , LEAD(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS next_salary - , LAG(e.manager_id) OVER(ORDER BY e.manager_id DESC) AS prev_salary + , RANK() OVER(PARTITION BY department_id + ORDER BY e.manager_id DESC + ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS rank_num + , DENSE_RANK() OVER(PARTITION BY department_id + ORDER BY e.id ASC, e.manager_id ASC, created_at DESC + ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS dense_rank_num + , SUM(amount) FILTER(WHERE status = 'active') AS dept_salary_avg + , COUNT(*) OVER(ORDER BY e.id, e.manager_id, created_at + ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING) + , 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 + ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary + , COUNT(*) FILTER(WHERE gender = 'F') AS female_count + , LISTAGG(e.name + , ', ') WITHIN GROUP(ORDER BY name DESC) FROM employees e WHERE e.status = 1 - ORDER BY e.id DESC - , e.name ASC + ORDER BY e.id + , e + , manager_id From 0344060c4be00262f89b21b65f4745bae890a989 Mon Sep 17 00:00:00 2001 From: xterao Date: Thu, 28 Aug 2025 19:11:48 +0900 Subject: [PATCH 06/13] Fix keyword group formatting inside parameter groups when enclosed by conditional loop directives --- .../formatter/block/comma/SqlCommaBlock.kt | 18 +++++++- .../comment/SqlElConditionLoopCommentBlock.kt | 9 ++-- .../group/keyword/SqlKeywordGroupBlock.kt | 7 ++++ .../keyword/second/SqlSecondKeywordBlock.kt | 10 ++++- .../block/group/subgroup/SqlSubGroupBlock.kt | 9 +++- .../block/word/SqlFunctionGroupBlock.kt | 2 +- .../builder/SqlBlockRelationBuilder.kt | 8 +++- .../processor/SqlFormatPreProcessor.kt | 4 +- .../OrderByGroupWithConditionDirective.sql | 28 +++++++++++++ ...erByGroupWithConditionDirective_format.sql | 41 +++++++++++++++++++ 10 files changed, 125 insertions(+), 11 deletions(-) create mode 100644 src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql create mode 100644 src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql 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 index 4b6c6a1c..0357f9fd 100644 --- 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 index 17c206b7..4da449eb 100644 --- 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 index 4c927a80..375a3caa 100644 --- 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 index 5a1f81a2..1780ed64 100644 --- 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 index 69497384..87698059 100644 --- 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 index 16f7cf7c..a73fae12 100644 --- 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 index 51f4c836..cf33c1c8 100644 --- 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 index 73a97043..bc1fcf65 100644 --- 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 new file mode 100644 index 00000000..038c316b --- /dev/null +++ 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 new file mode 100644 index 00000000..95f8a4a6 --- /dev/null +++ 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 From 2325ddf9dd616aae9c3d352262ea212d61b8526b Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:01:35 +0900 Subject: [PATCH 07/13] Fix indentation calculation for consecutive operators --- .../doma/intellij/formatter/block/SqlBlock.kt | 49 ++++++++++++++----- .../intellij/formatter/block/SqlFileBlock.kt | 3 +- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt index 13c8f05c..c8b15e30 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt @@ -371,24 +371,51 @@ open class SqlBlock( 0 } + var prevBlock: SqlBlock? = null return children .filter { it !is SqlDefaultCommentBlock && it !is SqlElConditionLoopCommentBlock } .sumOf { prev -> - prev - .getChildrenTextLen() - .plus( - if (prev.node.elementType == SqlTypes.DOT || - prev.node.elementType == SqlTypes.RIGHT_PAREN - ) { - 0 - } else { - prev.getNodeText().length.plus(1) - }, - ) + val sum = + prev + .getChildrenTextLen() + .plus( + if (prev.node.elementType == SqlTypes.DOT || + prev.node.elementType == SqlTypes.RIGHT_PAREN + ) { + 0 + } else if (prev.isOperationSymbol() && prevBlock?.isOperationSymbol() == true) { + // When operators appear consecutively, the first symbol includes the text length for the last space. + // Subsequent symbols add only their own symbol length. + prev.getNodeText().length + } else { + prev.getNodeText().length.plus(1) + }, + ) + prevBlock = prev + return@sumOf sum }.plus(parent.indent.groupIndentLen) .plus(directiveParentIndent) } + fun isOperationSymbol(): Boolean = + node.elementType in + listOf( + SqlTypes.PLUS, + SqlTypes.MINUS, + SqlTypes.ASTERISK, + SqlTypes.AT_SIGN, + SqlTypes.SLASH, + SqlTypes.HASH, + SqlTypes.LE, + SqlTypes.LT, + SqlTypes.EL_EQ, + SqlTypes.EL_NE, + SqlTypes.GE, + SqlTypes.GT, + SqlTypes.TILDE, + SqlTypes.OTHER, + ) + /** * Returns the child indentation for the block. * diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt index 8afcd20b..e056d4c9 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt @@ -648,7 +648,8 @@ open class SqlFileBlock( childBlock1 is SqlOtherBlock && childBlock2 is SqlElSymbolBlock || childBlock1 is SqlElSymbolBlock && childBlock2 is SqlElAtSignBlock || childBlock1 is SqlOtherBlock && childBlock2 is SqlOtherBlock || - childBlock1 is SqlElSymbolBlock && childBlock2 is SqlOtherBlock + childBlock1 is SqlElSymbolBlock && childBlock2 is SqlOtherBlock || + childBlock1?.isOperationSymbol() == true && childBlock2.isOperationSymbol() override fun isLeaf(): Boolean = false From 70067e9e592ec479ef9fe1eacb752e92fa633d82 Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:02:11 +0900 Subject: [PATCH 08/13] Remove space between keyword group used as function name and parameter group --- .../doma/intellij/formatter/block/SqlFileBlock.kt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt index e056d4c9..385b9590 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlFileBlock.kt @@ -587,7 +587,15 @@ open class SqlFileBlock( } } - is SqlDataTypeParamBlock, is SqlFunctionParamBlock -> return SqlCustomSpacingBuilder.nonSpacing + is SqlFunctionParamBlock -> { + return if (childBlock1?.node?.elementType in listOf(SqlTypes.FUNCTION_NAME, SqlTypes.WORD)) { + SqlCustomSpacingBuilder.nonSpacing + } else { + SqlCustomSpacingBuilder.normalSpacing + } + } + + is SqlDataTypeParamBlock -> return SqlCustomSpacingBuilder.nonSpacing } } From 18ba6cd4517c3cfa42c8cfccf7ca80c2cb9ad8b9 Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:02:24 +0900 Subject: [PATCH 09/13] Add tests and improve formatting for ORDER BY with condition directives --- .../doma/intellij/formatter/SqlFormatterTest.kt | 4 ++++ .../OrderByGroupWithConditionDirective.sql | 2 +- .../OrderByGroupWithConditionDirective_format.sql | 14 ++++++++------ .../testData/sql/formatter/OrderByGroup_format.sql | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt b/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt index 0cc2ca3b..c57447e1 100644 --- a/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt +++ b/src/test/kotlin/org/domaframework/doma/intellij/formatter/SqlFormatterTest.kt @@ -274,6 +274,10 @@ class SqlFormatterTest : BasePlatformTestCase() { formatSqlFile("OrderByGroup.sql", "OrderByGroup$formatDataPrefix.sql") } + fun testOrderByGroupWithConditionDirectiveFormatter() { + formatSqlFile("OrderByGroupWithConditionDirective.sql", "OrderByGroupWithConditionDirective$formatDataPrefix.sql") + } + private fun formatSqlFile( beforeFile: String, afterFile: String, diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql index 038c316b..aaae8498 100644 --- a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql +++ b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql @@ -1,6 +1,6 @@ SELECT e.id -- with condition - , ROW_NUMBER() OVER(/*%if order */ORDER BY e.manager_id DESC/*%end*/) AS row_num + , ROW_NUMBER() /*%if order */OVER(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 diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql index 95f8a4a6..88243d97 100644 --- a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql +++ b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql @@ -1,8 +1,10 @@ SELECT e.id -- with condition - , ROW_NUMBER() OVER(/*%if order */ - ORDER BY e.manager_id DESC - /*%end*/) AS row_num + , ROW_NUMBER() + /*%if order */ + OVER(ORDER BY e.manager_id DESC) + /*%end*/ + AS row_num , RANK() OVER(PARTITION BY department_id /*%if order */ ORDER BY e.manager_id DESC @@ -24,14 +26,14 @@ SELECT e.id 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) + 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 */ + , ', ') + /*%if filter */ WITHIN GROUP (ORDER BY name DESC) /*%end*/ FROM employees e diff --git a/src/test/testData/sql/formatter/OrderByGroup_format.sql b/src/test/testData/sql/formatter/OrderByGroup_format.sql index d70b46d9..a0cc2724 100644 --- a/src/test/testData/sql/formatter/OrderByGroup_format.sql +++ b/src/test/testData/sql/formatter/OrderByGroup_format.sql @@ -16,7 +16,7 @@ SELECT e.id ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary , COUNT(*) FILTER(WHERE gender = 'F') AS female_count , LISTAGG(e.name - , ', ') WITHIN GROUP(ORDER BY name DESC) + , ', ') WITHIN GROUP (ORDER BY name DESC) FROM employees e WHERE e.status = 1 ORDER BY e.id From 3a07e63afd58cff7c41e682088ba88e7ebb91ea4 Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:13:16 +0900 Subject: [PATCH 10/13] Refactor parent group assignment logic and improve block indentation handling --- .../builder/SqlBlockRelationBuilder.kt | 69 ++++++++++++++----- 1 file changed, 51 insertions(+), 18 deletions(-) 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 index cf33c1c8..6f3e9bb9 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/builder/SqlBlockRelationBuilder.kt @@ -575,31 +575,64 @@ class SqlBlockRelationBuilder( getParentGroup: (MutableList) -> SqlBlock?, ) { val parentGroup = - getParentGroup(context.blockBuilder.getGroupTopNodeIndexHistory() as MutableList) - + getParentGroup( + context.blockBuilder.getGroupTopNodeIndexHistory() as MutableList, + ) val targetChildBlock = context.childBlock - if (targetChildBlock is SqlDefaultCommentBlock) return + if (shouldSkipParentSetting(targetChildBlock)) return + + assignParentGroup(targetChildBlock, parentGroup) + registerAsNewGroupIfNeeded(targetChildBlock, context.blockBuilder) + updateBlockIndents(targetChildBlock, context.blockBuilder) + } + + private fun shouldSkipParentSetting(block: SqlBlock) = block is SqlDefaultCommentBlock - // The parent block for SqlElConditionLoopCommentBlock will be set later - if (targetChildBlock is SqlElConditionLoopCommentBlock && targetChildBlock.conditionType.isStartDirective()) { - targetChildBlock.tempParentBlock = parentGroup - if ((parentGroup is SqlElConditionLoopCommentBlock || parentGroup is SqlSubGroupBlock) && parentGroup.parentBlock != null) { - targetChildBlock.setParentGroupBlock(parentGroup) + private fun assignParentGroup( + targetChildBlock: SqlBlock, + parentGroup: SqlBlock?, + ) { + when { + isStartDirectiveConditionLoop(targetChildBlock) -> { + val conditionLoop = targetChildBlock as SqlElConditionLoopCommentBlock + conditionLoop.tempParentBlock = parentGroup + if (shouldSetParentImmediately(parentGroup)) { + conditionLoop.setParentGroupBlock(parentGroup) + } } - } else { - targetChildBlock.setParentGroupBlock(parentGroup) + else -> targetChildBlock.setParentGroupBlock(parentGroup) } + } - if (isNewGroup(targetChildBlock, context.blockBuilder) || - TypeUtil.isExpectedClassType(NEW_GROUP_EXPECTED_TYPES, targetChildBlock) - ) { - context.blockBuilder.addGroupTopNodeIndexHistory(targetChildBlock) + private fun isStartDirectiveConditionLoop(block: SqlBlock) = + block is SqlElConditionLoopCommentBlock && block.conditionType.isStartDirective() + + private fun shouldSetParentImmediately(parentGroup: SqlBlock?) = + (parentGroup is SqlElConditionLoopCommentBlock || parentGroup is SqlSubGroupBlock) && + parentGroup.parentBlock != null + + private fun registerAsNewGroupIfNeeded( + targetChildBlock: SqlBlock, + blockBuilder: SqlBlockBuilder, + ) { + if (shouldRegisterAsNewGroup(targetChildBlock, blockBuilder)) { + blockBuilder.addGroupTopNodeIndexHistory(targetChildBlock) } + } - context.blockBuilder.updateCommentBlockIndent(targetChildBlock) - // Set parent-child relationship and indent for preceding comment at beginning of block group - context.blockBuilder.updateConditionLoopBlockIndent(targetChildBlock) + private fun shouldRegisterAsNewGroup( + block: SqlBlock, + blockBuilder: SqlBlockBuilder, + ) = isNewGroup(block, blockBuilder) || + TypeUtil.isExpectedClassType(NEW_GROUP_EXPECTED_TYPES, block) + + private fun updateBlockIndents( + targetChildBlock: SqlBlock, + blockBuilder: SqlBlockBuilder, + ) { + blockBuilder.updateCommentBlockIndent(targetChildBlock) + blockBuilder.updateConditionLoopBlockIndent(targetChildBlock) } /** @@ -634,7 +667,7 @@ class SqlBlockRelationBuilder( } val isSetLineGroup = - SqlKeywordUtil.Companion.isSetLineKeyword( + SqlKeywordUtil.isSetLineKeyword( childBlock.getNodeText(), lastKeywordText, ) From 49d2967944c509a0186e831c2acb36162218d359 Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:31:05 +0900 Subject: [PATCH 11/13] Remove unused SqlTypes.TILDE from SqlBlock.kt --- .../org/domaframework/doma/intellij/formatter/block/SqlBlock.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt index c8b15e30..55342206 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt @@ -412,7 +412,6 @@ open class SqlBlock( SqlTypes.EL_NE, SqlTypes.GE, SqlTypes.GT, - SqlTypes.TILDE, SqlTypes.OTHER, ) From 554ab4e75f3bc93507bbe875b25597feec25a2ad Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:36:20 +0900 Subject: [PATCH 12/13] Add comments to clarify logic in calculateChildTextLength function --- .../domaframework/doma/intellij/formatter/block/SqlBlock.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt index 55342206..20cbfca0 100644 --- a/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt +++ b/src/main/kotlin/org/domaframework/doma/intellij/formatter/block/SqlBlock.kt @@ -81,6 +81,9 @@ open class SqlBlock( private fun calculateChildTextLength(child: SqlBlock): Int { val nonCommentChildren = child.childBlocks.filterNot { it is SqlDefaultCommentBlock } + // True only on the first loop iteration when the current element is the first child. + // If the subgroup is empty, return the length of “)”; + // otherwise DEFAULT_TEXT_LENGTH_INCREMENT already adds a space, so “)” needs no extra length. return when { nonCommentChildren.isNotEmpty() -> child.getChildrenTextLen() + child.getNodeText().length isExcludedFromTextLength(child) -> if (childBlocks.firstOrNull() == child) child.getNodeText().length else 0 From c37d5093c00f8306936fd0cceac2bd397b87bdda Mon Sep 17 00:00:00 2001 From: xterao Date: Fri, 29 Aug 2025 11:54:49 +0900 Subject: [PATCH 13/13] Add additional female_count filter for IDs greater than 10 in SQL queries --- src/test/testData/sql/formatter/OrderByGroup.sql | 1 + .../sql/formatter/OrderByGroupWithConditionDirective.sql | 1 + .../sql/formatter/OrderByGroupWithConditionDirective_format.sql | 2 ++ src/test/testData/sql/formatter/OrderByGroup_format.sql | 2 ++ 4 files changed, 6 insertions(+) diff --git a/src/test/testData/sql/formatter/OrderByGroup.sql b/src/test/testData/sql/formatter/OrderByGroup.sql index 8c8394ed..5a1f2357 100644 --- a/src/test/testData/sql/formatter/OrderByGroup.sql +++ b/src/test/testData/sql/formatter/OrderByGroup.sql @@ -15,6 +15,7 @@ ORDER BY e.id ASC, e.manager_id ASC, created_at DESC) , LAST_VALUE() OVER(ORDER BY e.manager_id ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary , COUNT(*) FILTER(WHERE gender = 'F') AS female_count + , COUNT(*) FILTER(WHERE gender = 'F' AND id > 10) AS female_count , LISTAGG(e.name , ', ') WITHIN GROUP(ORDER BY name DESC) FROM employees e diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql index aaae8498..69945a1f 100644 --- a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql +++ b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective.sql @@ -13,6 +13,7 @@ ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING/*%end */) AS dense_rank , COUNT(*) OVER(ORDER BY e.id, e.manager_id, created_at /*%if rows */ ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING/*%end*/) + , COUNT(*) FILTER(WHERE gender = 'F' AND id > 10) AS female_count , 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 diff --git a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql index 88243d97..1f4fe159 100644 --- a/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql +++ b/src/test/testData/sql/formatter/OrderByGroupWithConditionDirective_format.sql @@ -25,6 +25,8 @@ SELECT e.id /*%if rows */ ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING /*%end*/) + , COUNT(*) FILTER(WHERE gender = 'F' + AND id > 10) AS female_count , 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 diff --git a/src/test/testData/sql/formatter/OrderByGroup_format.sql b/src/test/testData/sql/formatter/OrderByGroup_format.sql index a0cc2724..1b46f275 100644 --- a/src/test/testData/sql/formatter/OrderByGroup_format.sql +++ b/src/test/testData/sql/formatter/OrderByGroup_format.sql @@ -15,6 +15,8 @@ SELECT e.id , LAST_VALUE() OVER(ORDER BY e.manager_id ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS bottom_salary , COUNT(*) FILTER(WHERE gender = 'F') AS female_count + , COUNT(*) FILTER(WHERE gender = 'F' + AND id > 10) AS female_count , LISTAGG(e.name , ', ') WITHIN GROUP (ORDER BY name DESC) FROM employees e