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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlfluff fix keeps adding then removing whitespace #1438

Closed
CyberShadow opened this issue Sep 23, 2021 · 4 comments 路 Fixed by #1443
Closed

sqlfluff fix keeps adding then removing whitespace #1438

CyberShadow opened this issue Sep 23, 2021 · 4 comments 路 Fixed by #1443
Assignees
Labels
bug Something isn't working rule bug A rule is not working as intended, either missing errors or incorrectly highlighting non-errors

Comments

@CyberShadow
Copy link
Contributor

Expected Behaviour

After a finite number of iterations (ideally 1 馃檪), sqlfluff fix should eventually converge on an output that it is happy with.

Observed Behaviour

sqlfluff fix keeps adding whitespace before some for / endfor Jinja tags, and then removing it again upon a successive invocation (which makes it challenging to use as, e.g., a pre-commit hook).

Steps to Reproduce

$ echo '{% set elements = "foo" %}

SELECT
    {% for elem in elements %}
    {{ elem }},
    {% endfor %}

    {% for elem in elements %}
    {{ elem }}+
    {% endfor %}
    0
' > test_0.sql

$ for n in $(seq 5) ; do cp test_$((n-1)).sql test_$n.sql ; sqlfluff fix --force test_$n.sql ; done
(sqlfluff output omitted)
$ diff -u test_4.sql test_5.sql
--- test_4.sql	2021-09-23 16:36:30.696291287 +0000
+++ test_5.sql	2021-09-23 16:36:31.232985009 +0000
@@ -3,10 +3,10 @@
 SELECT
     {% for elem in elements %}
     {{ elem }},
-    {% endfor %}
+{% endfor %}
 
-        {% for elem in elements %}
+    {% for elem in elements %}
     {{ elem }} +
-    {% endfor %}
+{% endfor %}
     0

Dialect

None specified

Version

SQLFluff a50454c, Python 3.9.7

Configuration

None

@CyberShadow CyberShadow added the bug Something isn't working label Sep 23, 2021
@tunetheweb tunetheweb added the rule bug A rule is not working as intended, either missing errors or incorrectly highlighting non-errors label Sep 23, 2021
@barrywhart
Copy link
Member

I have a PR which addresses #1425. This seems like a different issue. Most likely, there are two different lint rules with contradictory "opinions" about good linting.

Until this is fixed, a possible workaround (and helpful info for fixing!) would be to run the fixer in verbose mode (-vvvv) and see which rules are firing each time. I think you'll see an alternating pair of rules from one run to the next. If you disable one of these rules, you should get stable lint output.

@CyberShadow
Copy link
Contributor Author

Yep, the minimal set of rules needed to reproduce this seem to be --rules L001,L003,L039.

The full verbose output is quite a lot (doesn't fit in a comment):
https://dump.cy.md/a8156741fe241e88d5d67cb32d172923/06%3A49%3A42-upload.txt

@barrywhart
Copy link
Member

I have a fix that makes L001 smarter -- it no longer removes "trailing whitespace" if there's template code following the whitespace. Does this fix the original issue, @CyberShadow?

@CyberShadow
Copy link
Contributor Author

Does this fix the original issue, @CyberShadow?

Sorry for the late reply - yes, as far as I can tell, it does, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule bug A rule is not working as intended, either missing errors or incorrectly highlighting non-errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants