Repository: sqlfluff/sqlfluff

Issue:
CV03 corrupts SQL when applied to looped sql.
### Search before asking

- [X] I searched the [issues](https://github.com/sqlfluff/sqlfluff/issues) and found no similar issues.


### What Happened

User @iKwesi found that running `fix` on a statement where the final select target in a loop and contains a trailing comma results in a fix which corrupts the SQL.

This minimal query demonstrates:
```sql
SELECT
    {% for col in ['a', 'b', 'c'] %}
        {{col}},
    {% endfor %}
FROM tbl
```

Results in:
```
> sqlfluff fix test.sql -d bigquery -r CV03 -x f
==== finding fixable violations ====
== [test.sql] FAIL
L:   3 | P:  16 | CV03 | Trailing comma in select statement forbidden
                       | [convention.select_trailing_comma]
==== fixing violations ====
1 fixable linting violations found
Are you sure you wish to attempt to fix these? [Y/n] ...
```

After applying the fix we get:

```sql
SELECT
    {% for col in ['a', 'b', 'c'] %}
        {{col}}{% endfor %}
FROM tbl
```

...which is no longer valid SQL.

### Expected Behaviour

In this case, the rule should still flag the issue - but _it should not attempt the fix_, because it cannot uniquely identify the final comma.

### Observed Behaviour

Fix corrupts SQL.

### How to reproduce

See above.

### Dialect

all - but only really relevant for dialects which allow this (e.g. bigquery).

### Version

Confirmed on 2.0.3

### Configuration

occurs on default config

### Are you willing to work on and submit a PR to address the issue?

- [X] Yes I am willing to submit a PR!

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/sqlfluff/sqlfluff/blob/main/CODE_OF_CONDUCT.md)


Discussion:
greg-finley: Check CI
coveralls: ## Pull Request Test Coverage Report for [Build 4633356109](https://coveralls.io/builds/58653195)

* **8** of **13**   **(61.54%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.03%**) to **99.972%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [src/sqlfluff/rules/convention/CV03.py](https://coveralls.io/builds/58653195/source?filename=src%2Fsqlfluff%2Frules%2Fconvention%2FCV03.py#L70) | 8 | 13 | 61.54%
<!-- | **Total:** | **8** | **13** | **61.54%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/58653195/badge)](https://coveralls.io/builds/58653195) |
| :-- | --: |
| Change from base [Build 4627564153](https://coveralls.io/builds/58629055): |  -0.03% |
| Covered Lines: | 17716 |
| Relevant Lines: | 17721 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/src/sqlfluff/cli/commands.py b/src/sqlfluff/cli/commands.py
index f870f16f9..e874ae844 100644
--- a/src/sqlfluff/cli/commands.py
+++ b/src/sqlfluff/cli/commands.py
@@ -335,6 +335,16 @@ def lint_options(f: Callable) -> Callable:
         cls=DeprecatedOption,
         deprecated=["--disable_progress_bar"],
     )(f)
+    f = click.option(
+        "--persist-timing",
+        default=None,
+        help=(
+            "A filename to persist the timing information for a linting run to "
+            "in csv format for external analysis. NOTE: This feature should be "
+            "treated as beta, and the format of the csv file may change in "
+            "future releases without warning."
+        ),
+    )(f)
     return f
 
 
@@ -526,16 +536,6 @@ def dump_file_payload(filename: Optional[str], payload: str):
     is_flag=True,
     help="Perform the operation regardless of .sqlfluffignore configurations",
 )
-@click.option(
-    "--persist-timing",
-    default=None,
-    help=(
-        "A filename to persist the timing information for a linting run to "
-        "in csv format for external analysis. NOTE: This feature should be "
-        "treated as beta, and the format of the csv file may change in "
-        "future releases without warning."
-    ),
-)
 @click.argument("paths", nargs=-1, type=click.Path(allow_dash=True))
 def lint(
     paths: Tuple[str],
@@ -548,9 +548,9 @@ def lint(
     bench: bool = False,
     processes: Optional[int] = None,
     disable_progress_bar: Optional[bool] = False,
+    persist_timing: Optional[str] = None,
     extra_config_path: Optional[str] = None,
     ignore_local_config: bool = False,
-    persist_timing: Optional[str] = None,
     **kwargs,
 ) -> None:
     """Lint SQL files via passing a list of files or using stdin.
@@ -759,6 +759,7 @@ def _paths_fix(
     bench,
     show_lint_violations,
     warn_force: bool = True,
+    persist_timing: Optional[str] = None,
 ):
     """Handle fixing from paths."""
     # Lint the paths (not with the fix argument at this stage), outputting as we go.
@@ -862,6 +863,9 @@ def _paths_fix(
             for violation in violations:
                 click.echo(formatter.format_violation(violation))
 
+    if persist_timing:
+        result.persist_timing_records(persist_timing)
+
     sys.exit(exit_code)
 
 
@@ -911,6 +915,7 @@ def fix(
     logger: Optional[logging.Logger] = None,
     processes: Optional[int] = None,
     disable_progress_bar: Optional[bool] = False,
+    persist_timing: Optional[str] = None,
     extra_config_path: Optional[str] = None,
     ignore_local_config: bool = False,
     show_lint_violations: bool = False,
@@ -962,6 +967,7 @@ def fix(
             fixed_suffix,
             bench,
             show_lint_violations,
+            persist_timing=persist_timing,
         )
 
 
@@ -983,6 +989,7 @@ def cli_format(
     logger: Optional[logging.Logger] = None,
     processes: Optional[int] = None,
     disable_progress_bar: Optional[bool] = False,
+    persist_timing: Optional[str] = None,
     extra_config_path: Optional[str] = None,
     ignore_local_config: bool = False,
     **kwargs,
@@ -1059,6 +1066,7 @@ def cli_format(
             bench=bench,
             show_lint_violations=False,
             warn_force=False,  # don't warn about being in force mode.
+            persist_timing=persist_timing,
         )
 
 
diff --git a/src/sqlfluff/rules/convention/CV03.py b/src/sqlfluff/rules/convention/CV03.py
index 937afed92..b066c2ec8 100644
--- a/src/sqlfluff/rules/convention/CV03.py
+++ b/src/sqlfluff/rules/convention/CV03.py
@@ -58,9 +58,43 @@ class Rule_CV03(BaseRule):
         if self.select_clause_trailing_comma == "forbid":
             # Is it a comma?
             if last_content.is_type("comma"):
+                # The last content is a comma. Before we try and remove it, we
+                # should check that it's safe. One edge case is that it's a trailing
+                # comma in a loop, but that if we try and remove it, we also break
+                # the previous examples. We should check that this comma doesn't
+                # share a source position with any other commas in the same select.
+
+                # If there isn't a source position, then it's safe to remove, it's
+                # a recent addition.
+                if not last_content.pos_marker:  # pragma: no cover
+                    fixes = [LintFix.delete(last_content)]
+                else:
+                    comma_pos = last_content.pos_marker.source_position()
+                    for seg in context.segment.segments:
+                        if seg.is_type("comma"):
+                            if not seg.pos_marker:  # pragma: no cover
+                                continue
+                            elif seg.pos_marker.source_position() == comma_pos:
+                                if seg is not last_content:
+                                    # Not safe to fix
+                                    self.logger.info(
+                                        "Preventing deletion of %s, because source "
+                                        "position is the same as %s. Templated "
+                                        "positions are %s and %s.",
+                                        last_content,
+                                        seg,
+                                        last_content.pos_marker.templated_position(),
+                                        seg.pos_marker.templated_position(),
+                                    )
+                                    fixes = []
+                                    break
+                    else:
+                        # No matching commas found. It's safe.
+                        fixes = [LintFix.delete(last_content)]
+
                 return LintResult(
                     anchor=last_content,
-                    fixes=[LintFix.delete(last_content)],
+                    fixes=fixes,
                     description="Trailing comma in select statement forbidden",
                 )
         elif self.select_clause_trailing_comma == "require":


Test patch:
diff --git a/test/cli/commands_test.py b/test/cli/commands_test.py
index 467db3f00..a3d7e9570 100644
--- a/test/cli/commands_test.py
+++ b/test/cli/commands_test.py
@@ -499,6 +499,20 @@ def test__cli__command_lint_parse(command):
             ),
             0,
         ),
+        # Format with --persist-timing
+        (
+            (
+                cli_format,
+                [
+                    "--fixed-suffix",
+                    "_fix",
+                    "test/fixtures/linter/whitespace_errors.sql",
+                    "--persist-timing",
+                    "test.csv",
+                ],
+            ),
+            0,
+        ),
         # Format (specifying rules)
         (
             (
diff --git a/test/fixtures/rules/std_rule_cases/CV03.yml b/test/fixtures/rules/std_rule_cases/CV03.yml
index b1098569c..dec12d689 100644
--- a/test/fixtures/rules/std_rule_cases/CV03.yml
+++ b/test/fixtures/rules/std_rule_cases/CV03.yml
@@ -30,3 +30,28 @@ test_forbid_fail:
     rules:
       convention.select_trailing_comma:
         select_clause_trailing_comma: forbid
+
+test_fail_templated:
+  # NOTE: Check no fix, because it's not safe.
+  fail_str: |
+    SELECT
+        {% for col in ['a', 'b', 'c'] %}
+            {{col}},
+        {% endfor %}
+    FROM tbl
+  fix_str: |
+    SELECT
+        {% for col in ['a', 'b', 'c'] %}
+            {{col}},
+        {% endfor %}
+    FROM tbl
+  violations_after_fix:
+  - code: CV03
+    description: Trailing comma in select statement forbidden
+    line_no: 3
+    line_pos: 16
+    name: "convention.select_trailing_comma"
+  configs:
+    rules:
+      convention.select_trailing_comma:
+        select_clause_trailing_comma: forbid