Repository: sqlfluff/sqlfluff
Pull request: Issue 1207: Fix looping interaction between L008 and L030
Resolves #1207 

Problem statement:
COUNT dup in sqlfluff fix in 0.6.0 (from 0.5.3)
## Expected Behaviour

```
SELECT FLOOR(dt) ,count(*) FROM test
```
## Observed Behaviour

```
~ # sqlfluff fix --rules L008,L030 -f test.sql
==== finding fixable violations ====
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    One fix for L008 not applied, it would re-cause the same error.
WARNING    Loop limit on fixes reached [10].
== [test.sql] FAIL
L:   1 | P:  19 | L008 | Commas should be followed by a single whitespace unless
                       | followed by a comment.
L:   1 | P:  19 | L008 | Commas should be followed by a single whitespace unless
                       | followed by a comment.
L:   1 | P:  19 | L030 | Inconsistent capitalisation of function names.
==== fixing violations ====
3 fixable linting violations found
FORCE MODE: Attempting fixes...
Persisting Changes...
== [test.sql] PASS
Done. Please check your files to confirm.
```
fixed(?) sql.
```
SELECT FLOOR(dt) , COUNTCOUNTCOUNTCOUNTCOUNTCOUNTCOUNTCOUNTCOUNTCOUNTcount(*) FROM test
```

## Steps to Reproduce

on minimal environment.

```
docker run --rm -it python:3.9-alpine
pip install sqlfluff
```

## Version

```
~ # sqlfluff --version
sqlfluff, version 0.6.0
```

version 0.5.2 (SQLFluff Online version) run correctly, and 0.5.3 generate COUNT dup.

```
~ # python --version
Python 3.9.6
```
## Configuration

w/o .sqlfluff

Hints from issue discussion:
I did some experimenting: it seems that rules L008 and L030 are the "minimum set" in order to reproduce the error, i.e.:

```
sqlfluff fix test.sql --rules L008,L030
```

Commit message: Remove now-unused code

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 06054d0d1e6..61fd77853cf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Added dbt 0.20.* to the default test suite.
+- Fix looping interaction between L008 and L030 [#1207](https://github.com/sqlfluff/sqlfluff/issues/1207)
 
 ## [0.6.1] - 2021-07-16
 ### Added
diff --git a/src/sqlfluff/rules/L008.py b/src/sqlfluff/rules/L008.py
index 5d7e11723ba..e6292228730 100644
--- a/src/sqlfluff/rules/L008.py
+++ b/src/sqlfluff/rules/L008.py
@@ -38,16 +38,24 @@ def _eval(self, segment, raw_stack, **kwargs):
         This is a slightly odd one, because we'll almost always evaluate from a point a few places
         after the problem site. NB: We need at least two segments behind us for this to work.
         """
+        if len(raw_stack) < 1:
+            return None
+
+        cm2 = raw_stack[-1]
+        if cm2.name == "comma":
+            # comma followed by something that isn't whitespace?
+            if segment.name not in ["whitespace", "newline"]:
+                ins = WhitespaceSegment(raw=" ")
+                return LintResult(
+                    anchor=cm2, fixes=[LintFix("edit", segment, [ins, segment])]
+                )
+
         if len(raw_stack) < 2:
             return None
 
         cm1 = raw_stack[-1]
         cm2 = raw_stack[-2]
         if cm2.name == "comma":
-            # comma followed by something that isn't whitespace?
-            if cm1.name not in ["whitespace", "newline"]:
-                ins = WhitespaceSegment(raw=" ")
-                return LintResult(anchor=cm1, fixes=[LintFix("create", cm1, ins)])
             # comma followed by too much whitespace?
             if (cm1.raw != " " and cm1.name != "newline") and not segment.is_comment:
                 repl = WhitespaceSegment(raw=" ")
diff --git a/src/sqlfluff/rules/L030.py b/src/sqlfluff/rules/L030.py
index 34e40cea569..66743c9e0c4 100644
--- a/src/sqlfluff/rules/L030.py
+++ b/src/sqlfluff/rules/L030.py
@@ -47,9 +47,13 @@ def _get_fix(self, segment, fixed_raw):
         We need to do this because function_name nodes have a child
         function_name_identifier that holds the actual name.
         """
-        child_segment = segment.segments[0]
+        child_segment = [
+            child
+            for child in segment.segments
+            if child.get_type() == "function_name_identifier"
+        ][0]
         return LintFix(
             "edit",
             child_segment,
-            child_segment.__class__(raw=fixed_raw, pos_marker=child_segment.pos_marker),
+            child_segment.edit(fixed_raw.strip()),
         )
diff --git a/test/fixtures/rules/std_rule_cases/L008.yml b/test/fixtures/rules/std_rule_cases/L008.yml
index 2ef4aba9c1d..daf87fba5a7 100644
--- a/test/fixtures/rules/std_rule_cases/L008.yml
+++ b/test/fixtures/rules/std_rule_cases/L008.yml
@@ -10,3 +10,7 @@ test_fail_multiple_whitespace_after_comma:
 test_fail_no_whitespace_after_comma:
   fail_str: SELECT 1,4
   fix_str: SELECT 1, 4
+
+test_fail_no_whitespace_after_comma_2:
+  fail_str: SELECT FLOOR(dt) ,count(*) FROM test
+  fix_str: SELECT FLOOR(dt) , count(*) FROM test
diff --git a/test/fixtures/rules/std_rule_cases/L030.yml b/test/fixtures/rules/std_rule_cases/L030.yml
index d3c4656ef17..62f4364c18d 100644
--- a/test/fixtures/rules/std_rule_cases/L030.yml
+++ b/test/fixtures/rules/std_rule_cases/L030.yml
@@ -15,3 +15,7 @@ test_2:
     rules:
       L030:
         capitalisation_policy: lower
+
+test_fail_capitalization_after_comma:
+  fail_str: SELECT FLOOR(dt) ,count(*) FROM test
+  fix_str: SELECT FLOOR(dt) ,COUNT(*) FROM test


Code review comments:
diff_hunk: @@ -38,16 +38,24 @@ def _eval(self, segment, raw_stack, **kwargs):
         This is a slightly odd one, because we'll almost always evaluate from a point a few places
         after the problem site. NB: We need at least two segments behind us for this to work.
         """
+        if len(raw_stack) < 1:
+            return None
+
+        cm2 = raw_stack[-1]
+        if cm2.name == "comma":
+            # comma followed by something that isn't whitespace?
+            if segment.name not in ["whitespace", "newline"]:
+                ins = WhitespaceSegment(raw=" ")
+                return LintResult(
+                    anchor=cm2, fixes=[LintFix("edit", segment, [ins, segment])]
+                )
path: src/sqlfluff/rules/L008.py
text: @user1:
Structure looks good. Nitpick: `cm2` refers to _something-minus-2_ and in this case we're `raw_stack[-1]`. We should either call it `cm1` or maybe even just refer to the element in the raw stack directly. The `cm2` here I think is confusing.

Arguably the `cm1` and `cm2`  in the part below aren't much clearer, but I don't think they _need_ to be refactored as part of this PR.

@author:
Oh yes, I meant to do that. Will clean this up. It may have been possible to fix this without splitting the two cases, but it seemed good to avoid going any farther up `raw_stack` than necessary