fix/#365/multiline jinja merge (#374)

* fix #365: don't merge multiline jinja unless operator * chore: bump primer refs * chore: update dbt_utils primer stats
tconbeer · Jan 24, 2023 · dd485df · dd485df
1 parent 54b7a2c
commit dd485df
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,8 +4,15 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Formatting Changes + Bug Fixes
+
+-   sqlfmt no longer merges together lines containing multiline jinja blocks unless those lines start with an operator or comma ([#365](https://github.com/tconbeer/sqlfmt/issues/365) - thank you, [@gavlt](https://github.com/gavlt)!).
+-   fixed a bug where adding a jinja end tag (e.g., `{% endif %}`) to a line could cause bad formatting of everything on that line
+
 ## [0.15.2] - 2023-01-23
 
+### Features
+
 -   adds support for ARM-based platforms using Docker.
 
 ## [0.15.1] - 2023-01-20

diff --git a/src/sqlfmt/analyzer.py b/src/sqlfmt/analyzer.py
@@ -79,7 +79,10 @@ def write_buffers_to_query(self, query: Query) -> None:
         # indented too far -- they should be formatted as if they
         # have no open_brackets
         for line in reversed(self.line_buffer):
-            if line.closes_jinja_block_from_previous_line:
+            if (
+                line.is_standalone_jinja_statement
+                and line.closes_jinja_block_from_previous_line
+            ):
                 for node in line.nodes:
                     node.open_brackets = []
             else:

diff --git a/src/sqlfmt/merger.py b/src/sqlfmt/merger.py
@@ -58,12 +58,22 @@ def _extract_components(
         Given a list of lines, return 2 components:
         1. list of all nodes in those lines, with only a single trailing newline
         2. list of all comments in all of those lines
+
+        Raise CannotMergeException if lines contain nodes that cannot
+        be merged.
         """
         nodes: List[Node] = []
         comments: List[Comment] = []
         final_newline: Optional[Node] = None
         allow_multiline_jinja = True
+        has_multiline_jinja = False
         for line in lines:
+            if has_multiline_jinja and not (
+                line.starts_with_operator or line.starts_with_comma
+            ):
+                raise CannotMergeException(
+                    "Can't merge lines containing multiline nodes"
+                )
             # skip over newline nodes
             content_nodes = [
                 cls._raise_unmergeable(node, allow_multiline_jinja)
@@ -73,17 +83,22 @@ def _extract_components(
             if content_nodes:
                 final_newline = line.nodes[-1]
                 nodes.extend(content_nodes)
-                # we can merge lines containing multiline jinja nodes iff:
-                # 1. the multiline node is on the first line (allow_multiline
-                #    is initialized to True)
-                # 2. the multiline node is on the second line and follows a
-                #    standalone operator
+                # we can merge a line containing multiline jinja
+                # into a preceding line iff:
+                # the multiline node is on the second line and follows a
+                # standalone operator
                 if not (
                     allow_multiline_jinja
                     and len(content_nodes) == 1
                     and content_nodes[0].is_operator
                 ):
                     allow_multiline_jinja = False
+                # we can merge a line into a preceding line that
+                # contains multiline jinja iff:
+                # the line starts with an operator or a comma
+                has_multiline_jinja = any(
+                    [node.is_multiline_jinja for node in content_nodes]
+                )
             comments.extend(line.comments)
 
         if not nodes or not final_newline:

diff --git a/src/sqlfmt_primer/primer.py b/src/sqlfmt_primer/primer.py
@@ -30,7 +30,7 @@ def get_projects() -> List[SQLProject]:
         SQLProject(
             name="gitlab",
             git_url="https://github.com/tconbeer/gitlab-analytics-sqlfmt.git",
-            git_ref="aa609dc",  # sqlfmt e6f9f4c
+            git_ref="c786f5c",  # sqlfmt d97af4e
             expected_changed=3,
             expected_unchanged=2414,
             expected_errored=0,
@@ -39,7 +39,7 @@ def get_projects() -> List[SQLProject]:
         SQLProject(
             name="rittman",
             git_url="https://github.com/tconbeer/rittman_ra_data_warehouse.git",
-            git_ref="b1e45fa",  # sqlfmt e6f9f4c
+            git_ref="067c14b",  # sqlfmt d97af4e
             expected_changed=0,
             expected_unchanged=307,
             expected_errored=4,  # true mismatching brackets
@@ -75,9 +75,9 @@ def get_projects() -> List[SQLProject]:
         SQLProject(
             name="dbt_utils",
             git_url="https://github.com/tconbeer/dbt-utils.git",
-            git_ref="664fd68",  # sqlfmt e6f9f4c
-            expected_changed=2,
-            expected_unchanged=129,
+            git_ref="8d81899",  # sqlfmt d97af4e
+            expected_changed=0,
+            expected_unchanged=131,
             expected_errored=0,
             sub_directory=Path(""),
         ),

diff --git a/tests/data/unformatted/201_basic_snapshot.sql b/tests/data/unformatted/201_basic_snapshot.sql
@@ -2,8 +2,7 @@
 {{
     config(
           target_database='analytics',
-          target_schema=target.schema + '_snapshots',
-          unique_key='id',
+          target_schema=target.schema+'_snapshots', unique_key='id',
           strategy='timestamp',
           updated_at='updated_at',
         )
@@ -20,5 +19,8 @@ select * from {{ ref('stg_my_model') }}{% endsnapshot %}
         strategy="timestamp",
         updated_at="updated_at",
     )
-}} select * from {{ ref("stg_my_model") }}
+}}
+
+select *
+from {{ ref("stg_my_model") }}
 {% endsnapshot %}