Improves formatting of long patterns (split before nodes, not relations) (#421)

Author: tomasnyberg

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

@@ -63,6 +63,7 @@ import {
   ParenthesizedPathContext,
   PathLengthContext,
   PatternContext,
+  PatternElementContext,
   PatternListContext,
   ProcedureNameContext,
   PropertyContext,
@@ -1239,6 +1240,69 @@ export class TreePrintVisitor extends CypherCmdParserVisitor<void> {
     this.endGroup(selectorAnonymousPatternGrp);
   };
 
+  visitPatternElement = (ctx: PatternElementContext) => {
+    const n = ctx.getChildCount();
+    let i = 0;
+
+    while (i < n) {
+      const child = ctx.getChild(i);
+
+      if (
+        child instanceof NodePatternContext &&
+        i + 1 < n &&
+        ctx.getChild(i + 1) instanceof RelationshipPatternContext
+      ) {
+        i = this._processNodeRelSequence(ctx, i, n);
+      } else if (child instanceof ParenthesizedPathContext) {
+        this.visitParenthesizedPath(child);
+        i++;
+      } else {
+        this._visit(child as ParserRuleContext);
+        i++;
+      }
+    }
+  };
+
+  // Very convoluted logic related to visitPatternElement, used becase we want to be able
+  // to put groups around node patterns and relationships like this: START (node)-[rel](qpp?) END
+  // and the grammar is very unhelpful...
+  _processNodeRelSequence = (
+    ctx: PatternElementContext,
+    startIndex: number,
+    n: number,
+  ): number => {
+    let i = startIndex;
+    let nodeRelPatternGrp = this.startGroup();
+
+    this.visitNodePattern(ctx.getChild(i) as NodePatternContext);
+    i++;
+    while (i < n && ctx.getChild(i) instanceof RelationshipPatternContext) {
+      this.visitRelationshipPattern(
+        ctx.getChild(i) as RelationshipPatternContext,
+      );
+      i++;
+
+      if (i < n && ctx.getChild(i) instanceof QuantifierContext) {
+        this.visitQuantifier(ctx.getChild(i) as QuantifierContext);
+        i++;
+      }
+      this.endGroup(nodeRelPatternGrp);
+      if (i < n && ctx.getChild(i) instanceof NodePatternContext) {
+        if (
+          i + 1 < n &&
+          ctx.getChild(i + 1) instanceof RelationshipPatternContext
+        ) {
+          nodeRelPatternGrp = this.startGroup();
+        }
+        this.visitNodePattern(ctx.getChild(i) as NodePatternContext);
+        i++;
+      } else {
+        break;
+      }
+    }
+    return i;
+  };
+
   visitPatternList = (ctx: PatternListContext) => {
     const n = ctx.pattern_list().length;
     if (n === 1) {

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

@@ -389,9 +389,10 @@ MATCH (dmk:Station {name: 'Denmark Hill'})<-[:CALLS_AT]-(l1a:CallingPoint)-[:NEX
         (l2b)-[:CALLS_AT]->(gtw:Station {name: 'Gatwick Airport'})
 RETURN dmk`;
     const expected = `
-MATCH (dmk:Station {name: 'Denmark Hill'})<-[:CALLS_AT]-(l1a:CallingPoint)-
-      [:NEXT]->+(l1b)-[:CALLS_AT]->(x:Station)<-[:CALLS_AT]-(l2a:CallingPoint)-
-      [:NEXT]->*(l2b)-[:CALLS_AT]->(gtw:Station {name: 'Gatwick Airport'})
+MATCH (dmk:Station {name: 'Denmark Hill'})<-[:CALLS_AT]-
+      (l1a:CallingPoint)-[:NEXT]->+(l1b)-[:CALLS_AT]->(x:Station)<-[:CALLS_AT]-
+      (l2a:CallingPoint)-[:NEXT]->*(l2b)-[:CALLS_AT]->
+      (gtw:Station {name: 'Gatwick Airport'})
 RETURN dmk`.trim();
     verifyFormatting(query, expected);
   });
@@ -544,6 +545,69 @@ RETURN person.name AS name, COUNT {
     const expected = query;
     verifyFormatting(query, expected);
   });
+
+  test('should prefer to not split before a relation 1', () => {
+    const query = `
+MATCH (a:person {name: 'alice', age: 30})-[r:friend_of]->
+      (b:person {name: 'bob'})-[s:colleague_of]->(c:person {name: 'carol'})-
+      [t:partner_of]->(d:person {name: 'david'})-[u:mentor_and_friend_of]->
+      (e:person {name: 'eve'})
+RETURN a`;
+    const expected = `
+MATCH (a:person {name: 'alice', age: 30})-[r:friend_of]->
+      (b:person {name: 'bob'})-[s:colleague_of]->
+      (c:person {name: 'carol'})-[t:partner_of]->
+      (d:person {name: 'david'})-[u:mentor_and_friend_of]->
+      (e:person {name: 'eve'})
+RETURN a`.trimStart();
+    verifyFormatting(query, expected);
+  });
+
+  test('should prefer to not split before a relation 2', () => {
+    const query = `
+MATCH (a:Person {name: 'Alice'})   -[r:KNOWS]->
+      (b:Person {name: 'Bob'}) -[s:FRIEND_OF]->
+      (c:Person {name: 'Charlie'})
+RETURN a`;
+    const expected = `
+MATCH (a:Person {name: 'Alice'})-[r:KNOWS]->
+      (b:Person {name: 'Bob'})-[s:FRIEND_OF]->(c:Person {name: 'Charlie'})
+RETURN a`.trimStart();
+    verifyFormatting(query, expected);
+  });
+
+  test('should prefer to not split before a relation 3', () => {
+    const query = `
+MATCH (NOD01)-[REL01]->
+      (NOD02)-[REL02]->
+      (NOD03)-[REL03]->
+      (NOD04)-[REL04]->
+      (N)-[REL05]->
+      (NOD06)-[REL06]->
+      (NOD07)
+RETURN NOD01`;
+    // The node (N) would fit on the previous line but we prefer to split before nodes
+    const expected = `
+MATCH (NOD01)-[REL01]->(NOD02)-[REL02]->(NOD03)-[REL03]->(NOD04)-[REL04]->
+      (N)-[REL05]->(NOD06)-[REL06]->(NOD07)
+RETURN NOD01`.trimStart();
+    verifyFormatting(query, expected);
+  });
+
+  test('should prefer to not split before a relation 4', () => {
+    const query = `
+MATCH (Alice123:Person)-[FRND_REL:friendship]->
+      (Bob:Indiv)-[COWORK_REL:colleagueRelationship]->
+      (Carla55:EmployeeType)-[PARTNR:partner_of]->
+      (Dave:Short)
+RETURN Alice123`;
+    const expected = `
+MATCH (Alice123:Person)-[FRND_REL:friendship]->
+      (Bob:Indiv)-[COWORK_REL:colleagueRelationship]->
+      (Carla55:EmployeeType)-[PARTNR:partner_of]->(Dave:Short)
+RETURN Alice123`.trimStart();
+    verifyFormatting(query, expected);
+  });
 });
 
 describe('tests for respcecting user line breaks', () => {