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

Reposition before recursion in fixes to avoid internal error #3658

Merged
merged 8 commits into from Jul 25, 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
15 changes: 10 additions & 5 deletions plugins/sqlfluff-templater-dbt/test/templater_test.py
Expand Up @@ -295,15 +295,21 @@ def test__dbt_templated_models_do_not_raise_lint_error(
assert len(violations) == 0


def _clean_path(glob_expression):
"""Clear out files matching the provided glob expression."""
for fsp in glob.glob(glob_expression):
os.remove(fsp)


@pytest.mark.parametrize(
"path", ["models/my_new_project/issue_1608.sql", "snapshots/issue_1771.sql"]
)
def test__dbt_templated_models_fix_does_not_corrupt_file(
project_dir, path # noqa: F811
):
"""Test fix for issue 1608. Previously "sqlfluff fix" corrupted the file."""
for fsp in glob.glob(os.path.join(project_dir, "snapshots", "*FIXED.sql")):
os.remove(fsp)
"""Test issues where previously "sqlfluff fix" corrupted the file."""
test_glob = os.path.join(project_dir, os.path.dirname(path), "*FIXED.sql")
_clean_path(test_glob)
lntr = Linter(config=FluffConfig(configs=DBT_FLUFF_CONFIG))
lnt = lntr.lint_path(os.path.join(project_dir, path), fix=True)
try:
Expand All @@ -314,8 +320,7 @@ def test__dbt_templated_models_fix_does_not_corrupt_file(
fixed_buff = f.read()
assert fixed_buff == comp_buff
finally:
for fsp in glob.glob(os.path.join(project_dir, "snapshots", "*FIXED.sql")):
os.remove(fsp)
_clean_path(test_glob)


def test__templater_dbt_templating_absolute_path(
Expand Down
10 changes: 10 additions & 0 deletions src/sqlfluff/core/parser/segments/base.py
Expand Up @@ -1275,6 +1275,16 @@ def apply_fixes(
# Invalidate any caches
self.invalidate_caches()

# If any fixes applied, do an intermediate reposition. When applying
# fixes to children and then trying to reposition them, that recursion
# may rely on the parent having already populated positions for any
# of the fixes applied there first. This ensures those segments have
# working positions to work with.
if fixes_applied:
seg_buffer = list(
self._position_segments(tuple(seg_buffer), parent_pos=r.pos_marker)
)

# Then recurse (i.e. deal with the children) (Requeueing)
seg_queue = seg_buffer
seg_buffer = []
Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/rules/std_rule_cases/L032.yml
Expand Up @@ -47,3 +47,12 @@ select_using_fail:
SELECT margin
FROM B_TABLE
) USING (SOME_COLUMN)

test_fail_parent_child_positioning:
# Check for issue from https://github.com/sqlfluff/sqlfluff/issues/3656
fail_str: |
select * from c1 join c2 using (ID)
join (select * from c3 join c4 using (ID)) as c5 on c1.ID = c5.ID
fix_str: |
select * from c1 join c2 ON c1.ID = c2.ID
join (select * from c3 join c4 ON c3.ID = c4.ID) as c5 on c1.ID = c5.ID