Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JinjaTracer fix for endif/endfor inside "set" or "macro" blocks #2868

Merged
merged 1 commit into from Mar 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/sqlfluff/core/templaters/slicers/tracer.py
Expand Up @@ -431,9 +431,14 @@ def _slice_template(self) -> List[RawFileSlice]:
)
stack.pop()
stack.append(block_idx)
elif block_type == "block_end" and trimmed_parts[0] in (
"endfor",
"endif",
elif (
block_type == "block_end"
and set_idx is None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the code I added. The rest is just Black reformatting the existing code.

and trimmed_parts[0]
in (
"endfor",
"endif",
)
):
# Replace RawSliceInfo for this slice with one that has
# alternate ID and code for tracking. This ensures, for
Expand Down
59 changes: 51 additions & 8 deletions test/core/templaters/jinja_test.py
Expand Up @@ -6,7 +6,7 @@
import pytest

from sqlfluff.core.templaters import JinjaTemplater
from sqlfluff.core.templaters.base import RawFileSlice
from sqlfluff.core.templaters.base import RawFileSlice, TemplatedFile
from sqlfluff.core.templaters.jinja import JinjaTracer
from sqlfluff.core import Linter, FluffConfig

Expand Down Expand Up @@ -910,12 +910,52 @@ def test__templater_jinja_slice_template(test, result):
("block_start", slice(0, 32, None), slice(0, 0, None)),
("literal", slice(32, 37, None), slice(0, 0, None)),
("block_start", slice(37, 62, None), slice(0, 0, None)),
("literal", slice(62, 67, None), slice(0, 0, None)),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the expected test results for the previously added test. I manually reviewed these and confirmed that the old results were incorrect.

("templated", slice(67, 74, None), slice(0, 0, None)),
("literal", slice(74, 79, None), slice(0, 0, None)),
("block_end", slice(79, 91, None), slice(0, 0, None)),
("literal", slice(91, 92, None), slice(0, 0, None)),
("block_end", slice(92, 104, None), slice(0, 0, None)),
("literal", slice(104, 113, None), slice(0, 9, None)),
("templated", slice(113, 139, None), slice(9, 28, None)),
("literal", slice(139, 156, None), slice(28, 28, None)),
("templated", slice(113, 139, None), slice(9, 29, None)),
("literal", slice(139, 156, None), slice(29, 46, None)),
],
),
(
# Third test for issue 2835. This was the original SQL provided in
# the issue report.
"""{% set whitelisted= [
{'name': 'COL_1'},
{'name': 'COL_2'},
{'name': 'COL_3'}
] %}

{% set some_part_of_the_query %}
{% for col in whitelisted %}
{{col.name}}{{ ", " if not loop.last }}
{% endfor %}
{% endset %}

SELECT {{some_part_of_the_query}}
FROM SOME_TABLE
""",
None,
[
("block_start", slice(0, 94, None), slice(0, 0, None)),
("literal", slice(94, 96, None), slice(0, 2, None)),
("block_start", slice(96, 128, None), slice(2, 2, None)),
("literal", slice(128, 133, None), slice(2, 2, None)),
("block_start", slice(133, 161, None), slice(2, 2, None)),
("literal", slice(161, 166, None), slice(2, 2, None)),
("templated", slice(166, 178, None), slice(2, 2, None)),
("templated", slice(178, 205, None), slice(2, 2, None)),
("literal", slice(205, 210, None), slice(2, 2, None)),
("block_end", slice(210, 222, None), slice(2, 2, None)),
("literal", slice(222, 223, None), slice(2, 2, None)),
("block_end", slice(223, 235, None), slice(2, 2, None)),
("literal", slice(235, 244, None), slice(2, 11, None)),
("templated", slice(244, 270, None), slice(11, 66, None)),
("literal", slice(270, 287, None), slice(66, 83, None)),
],
),
(
Expand Down Expand Up @@ -943,19 +983,22 @@ def test__templater_jinja_slice_file(raw_file, override_context, result, caplog)

templated_file = make_template(raw_file).render()
with caplog.at_level(logging.DEBUG, logger="sqlfluff.templater"):
_, resp, _ = templater.slice_file(
raw_sliced, sliced_file, templated_str = templater.slice_file(
raw_file, templated_file, make_template=make_template
)
# Create a TemplatedFile from the results. This runs some useful sanity
# checks.
_ = TemplatedFile(raw_file, "<<DUMMY>>", templated_str, sliced_file, raw_sliced)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line runs the same sanity check we use in production and will help avoid introducing any future "bad" test results.

# Check contiguous on the TEMPLATED VERSION
print(resp)
print(sliced_file)
prev_slice = None
for elem in resp:
for elem in sliced_file:
print(elem)
if prev_slice:
assert elem[2].start == prev_slice.stop
prev_slice = elem[2]
# Check that all literal segments have a raw slice
for elem in resp:
for elem in sliced_file:
if elem[0] == "literal":
assert elem[1] is not None
# check result
Expand All @@ -965,6 +1008,6 @@ def test__templater_jinja_slice_file(raw_file, override_context, result, caplog)
templated_file_slice.source_slice,
templated_file_slice.templated_slice,
)
for templated_file_slice in resp
for templated_file_slice in sliced_file
]
assert actual == result