Removes reordering of ON CREATE/MATCH clauses in formatter (neo4j#399)

Author: tomasnyberg

packages/language-support/src/formatting/formatting.ts

@@ -65,7 +65,6 @@ import {
   CommentChunk,
   findTargetToken,
   getParseTreeAndTokens,
-  handleMergeClause,
   isComment,
   RegularChunk,
   wantsToBeConcatenated,
@@ -1167,15 +1166,16 @@ export class TreePrintVisitor extends CypherCmdParserVisitor<void> {
     this.endGroup(invocationGrp);
   };
 
-  // Handled separately because we want ON CREATE before ON MATCH
   visitMergeClause = (ctx: MergeClauseContext) => {
-    handleMergeClause(
-      ctx,
-      (node) => this.visit(node),
-      () => this.startGroupAlsoOnComment(),
-      (id) => this.endGroup(id),
-      () => this.avoidBreakBetween(),
-    );
+    this.visit(ctx.MERGE());
+    this.avoidBreakBetween();
+    const patternGrp = this.startGroupAlsoOnComment();
+    this.visit(ctx.pattern());
+    this.endGroup(patternGrp);
+    const n = ctx.mergeAction_list().length;
+    for (let i = 0; i < n; i++) {
+      this.visit(ctx.mergeAction(i));
+    }
   };
 
   // Handled separately because it wants indentation

packages/language-support/src/formatting/formattingHelpers.ts

@@ -3,15 +3,13 @@ import {
   CommonToken,
   CommonTokenStream,
   ErrorListener as ANTLRErrorListener,
-  ParseTree,
   Recognizer,
   TerminalNode,
   Token,
 } from 'antlr4';
 import { default as CypherCmdLexer } from '../generated-parser/CypherCmdLexer';
 import CypherCmdParser, {
   EscapedSymbolicNameStringContext,
-  MergeClauseContext,
   UnescapedSymbolicNameString_Context,
 } from '../generated-parser/CypherCmdParser';
 import { lexerKeywords } from '../lexerSymbols';
@@ -76,41 +74,6 @@ const traillingCharacters = [
   CypherCmdLexer.RBRACKET,
 ];
 
-// TODO: This function should probably not exist; we're not really fans of
-// shuffling around the AST like we're doing right now...
-export function handleMergeClause(
-  ctx: MergeClauseContext,
-  visit: (node: ParseTree) => void,
-  startGroup?: () => number,
-  endGroup?: (id: number) => void,
-  avoidBreakBetween?: () => void,
-) {
-  visit(ctx.MERGE());
-  avoidBreakBetween?.();
-  let patternGrp: number;
-  if (startGroup) {
-    patternGrp = startGroup();
-  }
-  visit(ctx.pattern());
-  if (endGroup) {
-    endGroup(patternGrp);
-  }
-  const mergeActions = ctx
-    .mergeAction_list()
-    .map((action, index) => ({ action, index }));
-  mergeActions.sort((a, b) => {
-    if (a.action.CREATE() && b.action.MATCH()) {
-      return -1;
-    } else if (a.action.MATCH() && b.action.CREATE()) {
-      return 1;
-    }
-    return a.index - b.index;
-  });
-  mergeActions.forEach(({ action }) => {
-    visit(action);
-  });
-}
-
 export function wantsToBeUpperCase(node: TerminalNode): boolean {
   return isKeywordTerminal(node);
 }

packages/language-support/src/formatting/standardizer.ts

@@ -1,10 +1,7 @@
 import { TerminalNode } from 'antlr4';
-import {
-  MergeClauseContext,
-  StatementsOrCommandsContext,
-} from '../generated-parser/CypherCmdParser';
+import { StatementsOrCommandsContext } from '../generated-parser/CypherCmdParser';
 import CypherCmdParserVisitor from '../generated-parser/CypherCmdParserVisitor';
-import { getParseTreeAndTokens, handleMergeClause } from './formattingHelpers';
+import { getParseTreeAndTokens } from './formattingHelpers';
 
 class StandardizingVisitor extends CypherCmdParserVisitor<void> {
   buffer = [];
@@ -14,10 +11,6 @@ class StandardizingVisitor extends CypherCmdParserVisitor<void> {
     return this.buffer.join('');
   };
 
-  visitMergeClause = (ctx: MergeClauseContext) => {
-    handleMergeClause(ctx, (node) => this.visit(node));
-  };
-
   visitTerminal = (node: TerminalNode) => {
     this.buffer.push(node.getText().toLowerCase());
     this.buffer.push(' ');

packages/language-support/src/tests/formatting/comments.test.ts

@@ -23,8 +23,8 @@ RETURN a.prop                     // Return the 'prop' of 'a'
 MERGE (n)
   ON CREATE SET n.prop = 0 // Ensure 'n' exists and initialize 'prop' to 0 if created
 MERGE (a:A)-[:T]->(b:B) // Create or match a relationship from 'a:A' to 'b:B'
-  ON CREATE SET a.name = 'me' // If 'a' is created, set its 'name' to 'me'
   ON MATCH SET b.name = 'you' // If 'b' already exists, set its 'name' to 'you'
+  ON CREATE SET a.name = 'me' // If 'a' is created, set its 'name' to 'me'
 RETURN a.prop // Return the 'prop' of 'a'`.trim();
     verifyFormatting(inlinecomments, expected);
   });

packages/language-support/src/tests/formatting/styleguide.test.ts

@@ -1,6 +1,8 @@
 import { verifyFormatting } from './testutil';
 
 describe('styleguide examples', () => {
+  // NOTE: We do not swap the order of ON MATCH and ON CREATE since
+  // we feel that it falls outside the responsbilities of a formatter.
   test('on match indentation example', () => {
     const query = `MERGE (n) ON CREATE SET n.prop = 0
 MERGE (a:A)-[:T]->(b:B)
@@ -10,8 +12,8 @@ RETURN a.prop`;
     const expected = `MERGE (n)
   ON CREATE SET n.prop = 0
 MERGE (a:A)-[:T]->(b:B)
-  ON CREATE SET a.name = 'me'
   ON MATCH SET b.name = 'you'
+  ON CREATE SET a.name = 'me'
 RETURN a.prop`;
     verifyFormatting(query, expected);
   });