diff --git a/CHANGELOG.md b/CHANGELOG.md index 494631ee..dfa541a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,22 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixes + +- fixes issue causing unstable formatting of multiline jinja tags when black is unable to parse the tag ([#176](https://github.com/tconbeer/sqlfmt/issues/176)) +- fixes issue for developers where pre-commit hooks would not install + +### Features + +- sqlfmt_primer now runs against forked (formatted) repos to make changes easier to detect + ## [0.8.0] - 2022-05-04 ### Formatting Changes -- sqlfmt is now more conservative about preserving whitespace around jinja expressions when we remove newlines ([#162](https://github.com/tconbeer/sqlfmt/issues/162), [#165](https://github.com/tconbeer/sqlfmt/issues/165)) -- Jinja blocks are now dedented before line merging, instead of after. This results in small changes to formatted output in some cases where jinja blocks are used -- Fixes an issue where jinja else and elif statements could cause unstable formatting. May impact whitespace for the tokens following `{% else %}` and `{% elif %}` statements +- sqlfmt is now more conservative about preserving whitespace around jinja expressions when we remove newlines ([#162](https://github.com/tconbeer/sqlfmt/issues/162), [#165](https://github.com/tconbeer/sqlfmt/issues/165) - thank you [@rcaddell](https://github.com/rcaddell) and [@rjay98](https://github.com/rjay98)!) +- jinja blocks are now dedented before line merging, instead of after. This results in small changes to formatted output in some cases where jinja blocks are used +- fixes an issue where jinja else and elif statements could cause unstable formatting. May impact whitespace for the tokens following `{% else %}` and `{% elif %}` statements ## [0.7.0] - 2022-04-24 diff --git a/src/sqlfmt/jinjafmt.py b/src/sqlfmt/jinjafmt.py index e65639d1..7cbc2a29 100644 --- a/src/sqlfmt/jinjafmt.py +++ b/src/sqlfmt/jinjafmt.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field from importlib import import_module from types import ModuleType -from typing import Optional +from typing import Optional, Tuple from sqlfmt.line import Line from sqlfmt.mode import Mode @@ -21,15 +21,19 @@ def __init__(self) -> None: except ImportError: self.black = None - def format_string(self, source_string: str, max_length: int) -> str: + def format_string(self, source_string: str, max_length: int) -> Tuple[str, bool]: """ Attempt to use black to format source_string to a line_length of max_length. - Return source_string if black isn't installed or it can't parse source_string + Return source_string if black isn't installed or it can't parse source_string. + + Return a tuple of the formatted string and a boolean that indicates whether + black successfully ran on the string """ formatted_string = source_string + is_blackened = False if not self.black: - return formatted_string + return formatted_string, is_blackened black_mode = self.black.Mode(line_length=max_length) try: @@ -49,8 +53,12 @@ def format_string(self, source_string: str, max_length: int) -> str: # there is other jinja syntax that isn't valid python, # so if this still fails, just stop trying pass + else: + is_blackened = True + else: + is_blackened = True finally: - return formatted_string + return formatted_string, is_blackened @dataclass @@ -64,15 +72,19 @@ class JinjaTag: (opening_marker, verb, code, closing_marker) = ("{%-", "set", "my_var=4", "%}") """ + source_string: str opening_marker: str verb: str code: str closing_marker: str depth: int + is_blackened: bool = False def __str__(self) -> str: - if self.is_indented_multiline_tag: + if self.is_indented_multiline_tag and self.is_blackened: return self._multiline_str() + elif self.is_indented_multiline_tag: + return self.source_string else: s = f"{self.opening_marker} {self.verb}{self.code} {self.closing_marker}" return s @@ -123,7 +135,9 @@ def from_string(cls, source_string: str, depth: int) -> "JinjaTag": if verb and code: verb = f"{verb} " - return JinjaTag(opening_marker, verb, code, closing_marker, depth) + return JinjaTag( + source_string, opening_marker, verb, code, closing_marker, depth + ) def max_code_length(self, max_length: int) -> int: """ @@ -174,15 +188,9 @@ def _format_jinja_node(self, node: Node, max_length: int) -> None: tag = JinjaTag.from_string(node.value, node.depth[0]) if tag.code and self.use_black: - tag.code = self._format_python_string( + tag.code, tag.is_blackened = self.code_formatter.format_string( tag.code, max_length=tag.max_code_length(max_length), ) - if (not self.use_black) and tag.is_indented_multiline_tag: - return - else: - node.value = str(tag) - - def _format_python_string(self, source_string: str, max_length: int) -> str: - return self.code_formatter.format_string(source_string, max_length) + node.value = str(tag) diff --git a/src/sqlfmt_primer/primer.py b/src/sqlfmt_primer/primer.py index d6c43c3f..19882541 100644 --- a/src/sqlfmt_primer/primer.py +++ b/src/sqlfmt_primer/primer.py @@ -31,8 +31,8 @@ def get_projects() -> List[SQLProject]: name="gitlab", git_url="https://github.com/tconbeer/gitlab-analytics-sqlfmt.git", git_ref="f4168905006e604eaac284a9e08885f15e514fce", # sqlfmt 0.8.0 - expected_changed=19, - expected_unchanged=2187, + expected_changed=12, + expected_unchanged=2194, expected_errored=0, sub_directory=Path("transform/snowflake-dbt/models"), ), @@ -40,8 +40,8 @@ def get_projects() -> List[SQLProject]: name="rittman", git_url="https://github.com/tconbeer/rittman_ra_data_warehouse.git", git_ref="00c7e4b8d93f8ca0460ad3d9b3f9ea48c213627b", # sqlfmt 0.8.0 - expected_changed=3, - expected_unchanged=277, + expected_changed=1, + expected_unchanged=279, expected_errored=4, # true mismatching brackets sub_directory=Path("models"), ), @@ -76,8 +76,8 @@ def get_projects() -> List[SQLProject]: name="dbt_utils", git_url="https://github.com/tconbeer/dbt-utils.git", git_ref="7c4d7f2a8dca533207b4e8a63a919f73eb0bb136", # sqlfmt 0.8.0 - expected_changed=7, - expected_unchanged=124, + expected_changed=3, + expected_unchanged=128, expected_errored=0, sub_directory=Path(""), ), diff --git a/tests/data/unformatted/300_jinjafmt.sql b/tests/data/unformatted/300_jinjafmt.sql index 51c4ef7d..456a9af3 100644 --- a/tests/data/unformatted/300_jinjafmt.sql +++ b/tests/data/unformatted/300_jinjafmt.sql @@ -16,6 +16,16 @@ {% do long_list.append( "another_long_name" )%} + +-- ensure jinja with python errors are not indented further when formatting +{{ + dbt_utils.star( + from=ref(model_name), + except=long_list, + relation_alias=model_name, + suffix="something_else" + ) +}} )))))__SQLFMT_OUTPUT__((((( {{ config( @@ -40,3 +50,13 @@ ] %} {% do long_list.append("another_long_name") %} + +-- ensure jinja with python errors are not indented further when formatting +{{ + dbt_utils.star( + from=ref(model_name), + except=long_list, + relation_alias=model_name, + suffix="something_else" + ) +}} diff --git a/tests/unit_tests/test_jinjafmt.py b/tests/unit_tests/test_jinjafmt.py index 251dd936..d0591948 100644 --- a/tests/unit_tests/test_jinjafmt.py +++ b/tests/unit_tests/test_jinjafmt.py @@ -57,7 +57,7 @@ def uninstall_black(monkeypatch: pytest.MonkeyPatch) -> None: ], ) def test_jinja_tag_from_string(tag: str, result: Tuple[str, str, str, str]) -> None: - assert JinjaTag.from_string(tag, 0) == JinjaTag(*result, 0) + assert JinjaTag.from_string(tag, 0) == JinjaTag(tag, *result, 0) @pytest.mark.parametrize( @@ -180,14 +180,14 @@ def test_black_wrapper_format_string_no_black( result = jinja_formatter.code_formatter.format_string( source_string=source_string, max_length=88 ) - assert result == source_string + assert result == (source_string, False) @pytest.mark.parametrize( "tag,expected", [ - (JinjaTag("{{", "", "any code!", "}}", 0), 88 - 2 - 2 - 2), - (JinjaTag("{%-", "set", "_", "%}", 0), 88 - 3 - 3 - 2 - 2), + (JinjaTag("{{ any code! }}", "{{", "", "any code!", "}}", 0), 88 - 2 - 2 - 2), + (JinjaTag("{%- set _ %}", "{%-", "set", "_", "%}", 0), 88 - 3 - 3 - 2 - 2), ], ) def test_max_code_length(tag: JinjaTag, expected: int) -> None: