Repository: sql-formatter-org/sql-formatter
Pull request #397 (2022-08-17 09:50:35)

Change BetweenPredicate node field types
To be consistent with other AST nodes.

Discovered while working with Nearley that BetweenPredicate type expects single tokens as its sub-expressions:

```ts
type BetweenPredicate = {
  type: NodeType.between_predicate;
  betweenToken: Token;
  expr1: Token;
  andToken: Token;
  expr2: Token;
};
```

But we really want this:

```ts
type BetweenPredicate = {
  type: NodeType.between_predicate;
  betweenToken: Token;
  expr1: AstNode[];
  andToken: Token;
  expr2: AstNode[];
};
```

Already implemented it in Nearley branch the latter way, as that was a natural way to express it.


Patch:
diff --git a/src/formatter/ExpressionFormatter.ts b/src/formatter/ExpressionFormatter.ts
index 5ddcc61b..f3566c36 100644
--- a/src/formatter/ExpressionFormatter.ts
+++ b/src/formatter/ExpressionFormatter.ts
@@ -120,16 +120,11 @@ export default class ExpressionFormatter {
   }
 
   private formatBetweenPredicate(node: BetweenPredicate) {
-    this.layout.add(
-      this.show(node.betweenToken),
-      WS.SPACE,
-      this.show(node.expr1),
-      WS.SPACE,
-      this.showNonTabular(node.andToken),
-      WS.SPACE,
-      this.show(node.expr2),
-      WS.SPACE
-    );
+    this.layout.add(this.show(node.betweenToken), WS.SPACE);
+    this.layout = this.formatSubExpression(node.expr1);
+    this.layout.add(WS.NO_SPACE, WS.SPACE, this.showNonTabular(node.andToken), WS.SPACE);
+    this.layout = this.formatSubExpression(node.expr2);
+    this.layout.add(WS.SPACE);
   }
 
   private formatClause(node: Clause) {
diff --git a/src/formatter/InlineBlock.ts b/src/formatter/InlineBlock.ts
index d6a959e3..a760f4c5 100644
--- a/src/formatter/InlineBlock.ts
+++ b/src/formatter/InlineBlock.ts
@@ -1,6 +1,5 @@
-import { sum } from 'src/utils';
 import { type Token, TokenType, isLogicalOperator } from 'src/lexer/token';
-import { BetweenPredicate, NodeType, Parenthesis } from 'src/parser/ast';
+import { AstNode, BetweenPredicate, NodeType, Parenthesis } from 'src/parser/ast';
 
 /**
  * Bookkeeper for inline blocks.
@@ -17,22 +16,27 @@ export default class InlineBlock {
    * Examples are "NOW()", "COUNT(*)", "int(10)", key(`somecolumn`), DECIMAL(7,2)
    */
   public isInlineBlock(parenthesis: Parenthesis): boolean {
-    return this.inlineWidth(parenthesis) <= this.expressionWidth;
+    return this.inlineParenthesisWidth(parenthesis) <= this.expressionWidth;
   }
 
-  private inlineWidth(parenthesis: Parenthesis): number {
-    let length = 2; // two parenthesis
+  private inlineParenthesisWidth(parenthesis: Parenthesis): number {
+    // +2 for the two parenthesis
+    return this.inlineWidth(parenthesis.children) + 2;
+  }
+
+  private inlineWidth(nodes: AstNode[]): number {
+    let length = 0;
 
-    for (const node of parenthesis.children) {
+    for (const node of nodes) {
       switch (node.type) {
         case NodeType.function_call:
-          length += node.nameToken.text.length + this.inlineWidth(node.parenthesis);
+          length += node.nameToken.text.length + this.inlineParenthesisWidth(node.parenthesis);
           break;
         case NodeType.array_subscript:
-          length += node.arrayToken.text.length + this.inlineWidth(node.parenthesis);
+          length += node.arrayToken.text.length + this.inlineParenthesisWidth(node.parenthesis);
           break;
         case NodeType.parenthesis:
-          length += this.inlineWidth(node);
+          length += this.inlineParenthesisWidth(node);
           break;
         case NodeType.between_predicate:
           length += this.betweenWidth(node);
@@ -61,8 +65,11 @@ export default class InlineBlock {
   }
 
   private betweenWidth(node: BetweenPredicate): number {
-    return sum(
-      [node.betweenToken, node.expr1, node.andToken, node.expr2].map(token => token.text.length)
+    return (
+      node.betweenToken.text.length +
+      this.inlineWidth(node.expr1) +
+      node.andToken.text.length +
+      this.inlineWidth(node.expr2)
     );
   }
 
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index dbafbc27..77c13075 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -130,9 +130,9 @@ export default class Parser {
       return {
         type: NodeType.between_predicate,
         betweenToken: this.next(),
-        expr1: this.next(),
+        expr1: [this.nextTokenNode()],
         andToken: this.next(),
-        expr2: this.next(),
+        expr2: [this.nextTokenNode()],
       };
     }
     return undefined;
diff --git a/src/parser/ast.ts b/src/parser/ast.ts
index 6858d4b2..96531873 100644
--- a/src/parser/ast.ts
+++ b/src/parser/ast.ts
@@ -61,9 +61,9 @@ export type Parenthesis = {
 export type BetweenPredicate = {
   type: NodeType.between_predicate;
   betweenToken: Token;
-  expr1: Token;
+  expr1: AstNode[];
   andToken: Token;
-  expr2: Token;
+  expr2: AstNode[];
 };
 
 // LIMIT <count>


Test patch:
diff --git a/test/unit/Parser.test.ts b/test/unit/Parser.test.ts
index 1318144b..f6cdf421 100644
--- a/test/unit/Parser.test.ts
+++ b/test/unit/Parser.test.ts
@@ -321,22 +321,32 @@ describe('Parser', () => {
                     "text": "BETWEEN",
                     "type": "BETWEEN",
                   },
-                  "expr1": Object {
-                    "end": 20,
-                    "precedingWhitespace": " ",
-                    "raw": "10",
-                    "start": 18,
-                    "text": "10",
-                    "type": "NUMBER",
-                  },
-                  "expr2": Object {
-                    "end": 27,
-                    "precedingWhitespace": " ",
-                    "raw": "15",
-                    "start": 25,
-                    "text": "15",
-                    "type": "NUMBER",
-                  },
+                  "expr1": Array [
+                    Object {
+                      "token": Object {
+                        "end": 20,
+                        "precedingWhitespace": " ",
+                        "raw": "10",
+                        "start": 18,
+                        "text": "10",
+                        "type": "NUMBER",
+                      },
+                      "type": "token",
+                    },
+                  ],
+                  "expr2": Array [
+                    Object {
+                      "token": Object {
+                        "end": 27,
+                        "precedingWhitespace": " ",
+                        "raw": "15",
+                        "start": 25,
+                        "text": "15",
+                        "type": "NUMBER",
+                      },
+                      "type": "token",
+                    },
+                  ],
                   "type": "between_predicate",
                 },
               ],