Skip to content

Conversation

@xterao
Copy link
Collaborator

@xterao xterao commented Sep 8, 2025

Summary

This PR refactors the SQL formatter's handling of condition/loop directives (/*%if*/, /*%for*/, /*%else*/, /*%end*/) by introducing a dedicated management system separate from regular group blocks. This change simplifies indent calculation logic and makes the formatter more maintainable.

Problem

Previously, condition/loop directives were managed as part of the group block hierarchy, which caused several issues:

  • Complex parent-child relationships between directives and regular blocks
  • Difficult-to-maintain indent calculation logic
  • Inconsistent formatting when directives were nested or interleaved with SQL statements

Solution

1. Separate Directive Management

  • New Management System: Introduced conditionOrLoopBlocks list in SqlBlockBuilder to manage directives independently from group blocks
  • Dedicated Methods: Added specialized methods for directive handling:
    • getLastConditionOrLoopBlock(): Get the most recent directive
    • getLastNotDependOnConditionOrLoopBlock(): Get directives without dependencies
    • getNotClosedConditionOrLoopBlock(): Get unclosed directives for proper nesting

2. Simplified Parent-Child Relationships

  • Removed Complex Hierarchies: Condition/loop directives no longer establish parent-child relationships with regular SQL blocks
  • Traditional Indent Calculation: Each block class can now calculate indentation based on its conventional parent block without special directive handling
  • Cleaner Separation: Directives maintain their own dependency relationships separate from the main block tree

Key Changes

SqlBlockRelationBuilder.kt

  • Refactored setParentLoopConditionDirective() to handle directive relationships independently
  • Simplified setParentNonLoopConditionDirective() to work with traditional block parents
  • Removed complex nested directive parent calculations

SqlElConditionLoopCommentBlock.kt

  • Added dependsOnBlock property to track directive dependencies separately from parent blocks
  • Simplified indent calculation to follow standard block indentation rules
  • Maintained directive-specific nesting through dedicated dependency tracking

Implementation Notes

Package Reorganization

  • Moved SqlConditionalExpressionGroupBlock: Relocated from formatter/block/group/keyword/condition/ to formatter/block/group/subgroup/
    • This better reflects its role as a subgroup rather than a keyword-specific condition group
    • Improves package structure consistency

API Changes

  • SqlKeywordBlockFactory Methods: Added openConditionLoopDirective parameter to methods that need to check for unclosed condition/loop directive blocks
    • Affected methods:
      • createOptionsBlock() - Now accepts openConditionLoopDirective: SqlElConditionLoopCommentBlock?
      • createExistsBlock() - Internal method updated to handle directive context
    • This allows proper handling of keyword blocks within directive contexts

Backward Compatibility

  • All changes are internal to the formatter implementation
  • No breaking changes to public APIs
  • Existing import statements for SqlConditionalExpressionGroupBlock will need to be updated

Benefits

  1. Improved Maintainability: Clear separation between directive management and regular block hierarchy
  2. Simplified Logic: Indent calculations now follow predictable patterns without special cases
  3. Better Debugging: Easier to trace directive relationships and block indentation
  4. Reduced Complexity: Removed convoluted parent-child relationship management between directives and blocks

Testing

All existing tests pass with the refactored implementation, confirming that formatting behavior remains consistent while the internal structure is improved.

Test Coverage

  • Nested directives: NestedDirectives_format.sql
  • Conditional subqueries: ConditionalSubquery_format.sql
  • Loop directives: LoopDirective_format.sql
  • Mixed directive and SQL blocks: OrderByGroupWithConditionDirective_format.sql

Migration Notes

This is an internal refactoring with no changes to the public API or formatting output. The changes are:

  • Transparent to end users
  • Backward compatible with existing SQL files
  • Performance neutral (same complexity, better organization)

Future Improvements

This refactoring lays the groundwork for:

  • Easier implementation of new directive types
  • More flexible formatting options for directives
  • Potential performance optimizations in directive processing

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the SQL formatter's block relationship handling logic and translates all Japanese TODO comments to English for improved international collaboration. The changes streamline the conditional directive processing while maintaining formatting functionality.

  • Simplifies SQL formatter's conditional block relationship logic and removes complex parent tracking mechanisms
  • Translates Japanese TODO comments to English across multiple formatter classes
  • Adds new test case for UPDATE statements with POPULATE clause

Reviewed Changes

Copilot reviewed 65 out of 70 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
SqlConditionalExpressionGroupBlock.kt Moved from condition package to subgroup package for better organization
SqlBlockRelationBuilder.kt Major refactoring to simplify conditional directive parent-child relationship handling
SqlBlockBuilder.kt Removed unused properties and translated Japanese comments to English
Various formatter block classes Simplified indent calculation logic by removing conditional directive special cases
Test files Updated expected formatting outputs and added new UPDATE POPULATE test case

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@xterao xterao merged commit c70f555 into main Sep 8, 2025
5 checks passed
@xterao xterao deleted the chore/sql-format-refactoring-blocks-relation branch September 8, 2025 08:43
@xterao xterao added this to the 2.3.0 Release milestone Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Change Parent-Child Relationship Logic for Conditional/Loop Directives in SQL Formatting Blocks

2 participants