feat: remove extra blank lines, close #249

CHANGELOG.md

@@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
 
 ### Formatting Changes + Bug Fixes
 
+-   sqlfmt now removes extra blank lines ([#249](https://github.com/tconbeer/sqlfmt/issues/313) - thank you, [@nfcampos](https://github.com/nfcampos)!). Basically, no more than 1 blank line inside queries or blocks; no more than 2 between queries or blocks.
 -   sqlfmt now supports `create <object> ... clone` statements ([#313](https://github.com/tconbeer/sqlfmt/issues/313)).
 -   sqlfmt will now format all files that end with `*.sql` and `*.sql.jinja`, even those with other dots in their filenames ([#354](https://github.com/tconbeer/sqlfmt/issues/354) - thank you [@ysmilda](https://github.com/ysmilda)!).
 -   fixed a bug where `{% call %}` blocks with arguments like `{% call(foo) bar(baz) %}` would cause a parsing error ([#353](https://github.com/tconbeer/sqlfmt/issues/353) - thank you [@IgnorantWalking](https://github.com/IgnorantWalking)!).

src/sqlfmt/formatter.py

@@ -71,6 +71,25 @@ def _dedent_jinja_blocks(self, lines: List[Line]) -> List[Line]:
 
         return lines
 
+    def _remove_extra_blank_lines(self, lines: List[Line]) -> List[Line]:
+        """
+        A query can have at most 2 consecutive blank lines at depth (0,0)
+        and 1 consecutive blank line at any other depth. See issue #249
+        for motivation and details.
+        """
+        new_lines: List[Line] = []
+        cnt = 0
+        for line in lines:
+            if line.is_blank_line:
+                max_cnt = 2 if line.depth == (0, 0) else 1
+                if cnt < max_cnt:
+                    new_lines.append(line)
+                cnt += 1
+            else:
+                new_lines.append(line)
+                cnt = 0
+        return new_lines
+
     def format(self, raw_query: Query) -> Query:
         """
         Applies 4 transformations to a Query:
@@ -86,6 +105,7 @@ def format(self, raw_query: Query) -> Query:
             self._format_jinja,
             self._dedent_jinja_blocks,
             self._merge_lines,
+            self._remove_extra_blank_lines,
         ]
 
         for transform in pipeline:

tests/data/unformatted/126_blank_lines.sql

@@ -0,0 +1,79 @@
+{{ config(foo='bar') }}
+
+
+
+select
+
+
+foooooooooooo,
+barrrrrrrrrrr,
+
+
+bazzzzzzzzzzz,
+
+quxxxxxxxxxxx,
+
+
+foooooooooooo + barrrrrrrrrr + bazzzzzzzzzzz + quxxxxxxxxxxx as foooooooooooo_bar_baz_qux
+from
+fooooooooooooo.barrrrrrrrrr.bazzzzzzzzzzzzzz
+
+where
+
+
+quxxxxxxxxxxx=1
+{% if is_incremental() %}
+
+
+and barrrrrrrrrr = 1
+
+
+
+
+
+and bazzzzzzzzzzz = 2
+and quxxxxxxxxxxx = 3
+
+
+
+
+{% endif %}
+;
+
+
+
+
+
+
+select 1
+)))))__SQLFMT_OUTPUT__(((((
+{{ config(foo="bar") }}
+
+
+select
+
+    foooooooooooo,
+    barrrrrrrrrrr,
+
+    bazzzzzzzzzzz,
+
+    quxxxxxxxxxxx,
+
+    foooooooooooo
+    + barrrrrrrrrr
+    + bazzzzzzzzzzz
+    + quxxxxxxxxxxx as foooooooooooo_bar_baz_qux
+from fooooooooooooo.barrrrrrrrrr.bazzzzzzzzzzzzzz
+
+where
+
+    quxxxxxxxxxxx = 1
+    {% if is_incremental() %}
+
+    and barrrrrrrrrr = 1 and bazzzzzzzzzzz = 2 and quxxxxxxxxxxx = 3
+
+    {% endif %}
+;
+
+
+select 1

tests/data/unformatted/202_unpivot_macro.sql

@@ -77,7 +77,6 @@
 {%- endif %}
 {%- endfor %}
 
-
 {%- for col in include_cols -%}
 select
     {%- for exclude_col in exclude %} {{ exclude_col }}, {%- endfor %}

tests/data/unformatted/203_gitlab_email_domain_type.sql

@@ -47,7 +47,6 @@ END
     "full_match"
 ) -%}
 
-
 case
     when
         {{ lead_source }} in ('DiscoverOrg', 'Zoominfo', 'Purchased List', 'GitLab.com')

tests/data/unformatted/205_rittman_hubspot_deals.sql

@@ -105,7 +105,6 @@ select * from joined
 {% if "hubspot_crm" in var("marketing_warehouse_deal_sources") %}
 {% if var("stg_hubspot_crm_etl") == "fivetran" %}
 
-
 with
     source as (select * from {{ source("fivetran_hubspot_crm", "deals") }}),
     hubspot_deal_company as (

tests/data/unformatted/207_rittman_int_journals.sql

@@ -32,7 +32,6 @@ select * from journal_merge_list
 # https://github.com/rittmananalytics/ra_data_warehouse/blob/d8dc7bd1c008ca79f9d09c909734e28a66ef6366/LICENSE.txt
 {% if var("finance_warehouse_journal_sources") %}
 
-
 with
     journal_merge_list as (
         {% for source in var("finance_warehouse_journal_sources") %}

tests/data/unformatted/208_rittman_int_plan_breakout_metrics.sql

@@ -28,7 +28,6 @@ select * from plans_breakout_merge_list
 # https://github.com/rittmananalytics/ra_data_warehouse/blob/d8dc7bd1c008ca79f9d09c909734e28a66ef6366/LICENSE.txt
 {% if var("subscriptions_warehouse_sources") %}
 
-
 with
     plans_breakout_merge_list as (
         select * from {{ ref("stg_baremetrics_plan_breakout") }}
@@ -38,5 +37,4 @@ from plans_breakout_merge_list
 
 {% else %} {{ config(enabled=false) }}
 
-
 {% endif %}

tests/data/unformatted/209_rittman_int_web_events_sessionized.sql

@@ -243,7 +243,6 @@ with
 
     new_sessions as (
 
-
         select
             *,
             case
@@ -257,7 +256,6 @@ with
 
     session_numbers as (
 
-
         select
 
             *,
@@ -409,7 +407,6 @@ with
 select *
 from ordered_conversion_tagged
 
-
 {% else %} {{ config(enabled=false) }}
 
 {% endif %}

tests/functional_tests/test_general_formatting.py

@@ -39,6 +39,7 @@
         "unformatted/123_spark_keywords.sql",
         "unformatted/124_bq_compound_types.sql",
         "unformatted/125_numeric_constants.sql",
+        "unformatted/126_blank_lines.sql",
         "unformatted/200_base_model.sql",
         "unformatted/201_basic_snapshot.sql",
         "unformatted/202_unpivot_macro.sql",

tests/unit_tests/test_formatter.py

@@ -46,3 +46,22 @@ def test_dedent_jinja_blocks(default_mode: Mode) -> None:
         line.depth[0] for line in new_lines if line.is_standalone_jinja_statement
     ]
     assert all([line_depth == 0 for line_depth in jinja_depths])
+
+
+def test_remove_extra_blank_lines(default_mode: Mode) -> None:
+    formatter = QueryFormatter(default_mode)
+    source_string = (
+        "select 1\n;\n\n\n\n"
+        "select\n    1,\n\n\n    2\n;\n"
+        "{% macro foo() %}\n\n\n\n\nfoo\n{% endmacro %}\n\n\n\n\n\n\n"
+    )
+    expected_string = (
+        "select 1\n;\n\n\n"
+        "select\n    1,\n\n    2\n;\n"
+        "{% macro foo() %}\n\nfoo\n{% endmacro %}\n"
+    )
+    raw_query = formatter.mode.dialect.initialize_analyzer(
+        formatter.mode.line_length
+    ).parse_query(source_string)
+    new_lines = formatter._remove_extra_blank_lines(raw_query.lines)
+    assert "".join([str(line) for line in new_lines]) == expected_string