Repository: sqlfluff/sqlfluff
Pull request: Fix L009 FileSegment child + new create_before/create_after edit types
<!--Firstly, thanks for adding this feature! Secondly, please check the key steps against the checklist below to make your contribution easy to merge.-->

<!--Please give the Pull Request a meaningful title (including the dialect this PR is for if it is dialect specific), as this will automatically be added to the release notes, and then the Change Log.-->

### Brief summary of the change made
<!--If there is an open issue for this, then please include `fixes #XXXX` or `closes #XXXX` replacing `XXXX` with the issue number and it will automatically close the issue when the pull request is merged. Alternatively if not fully closed you can say `makes progress on #XXXX` to create a link on that issue without closing it.-->
This PR fixes #1978. L009 and L052 were both being applied as edits on the final segment of the file, causing L052 to not be applied. This is similar to the behaviour described by @barrywhart in #1808.

The original reasoning for this approach, even though it leads to an incorrect parse tree, was because there wasn't a better way to discover where these segments should be placed , however this can actually be easily fixed for L009

Observing an existing final new line parse (for all dialects), we see that it is a child of the file segment, hence we can carry out the edit on the child of the file segment when fixing L009.

![image](https://user-images.githubusercontent.com/80432516/143509848-fee0a2c5-d0db-438c-b4e2-6e27c1c8be06.png)

fixing L052 to better place the final semicolon in the parse tree is more of a challenge (reasoning discussed below) but this fix corrects the issue at hand, as well as L009 in general.

### Are there any other side effects of this change that we should be aware of?
No

### Pull Request checklist
- [X] Please confirm you have completed any of the necessary steps below.

- Included test cases to demonstrate any code changes, which may be one or more of the following:
  - `.yml` rule test cases in `test/fixtures/rules/std_rule_cases`.
  - `.sql`/`.yml` parser test cases in `test/fixtures/dialects` (note YML files can be auto generated with `python test/generate_parse_fixture_yml.py` or by running `tox` locally).
  - Full autofix test cases in `test/fixtures/linter/autofix`.
  - Other.
- Added appropriate documentation for the change.
- Created GitHub issues for any relevant followup/future enhancements if appropriate.


Problem statement:
Nuisance: L052 (require_final_semicolon = True) does not reliably add `;` if L009 is on and EOF line does not exist already
If:
- Rule 009 is not excluded (default)
- There is no single trailing newline (as rule 009 requires)
- Rule 052 parameter `require_final_semicolon = True` (not default)

Then `sqlfluff fix` adds newline, but does not add a semicolon (`;`).

Related experiments:
- Value of another 052's parameter `semicolon_newline = True/False` does not affect the mechanics
- While ```[sqlfluff] exclude_rules = L009``` makes `;` appear always as expected

## Query
```
SELECT
    a
FROM foo
```

## Expected Behaviour
```
SELECT a
FROM foo;

```

## Observed Behaviour
```
SELECT a,
FROM foo

```

## Steps to Reproduce
I believe any setup will have such error if 
```
[sqlfluff:rules:L052] 
require_final_semicolon = True
```
added to the `.sqlfluff`

## Dialect
- bigquery

## Version
- sqlfluff, version 0.8.2

## Configuration
```
[sqlfluff]
templater = jinja
dialect = bigquery
# exclude_rules = L009

[sqlfluff:rules:L052] 
require_final_semicolon = True
```

Hints from issue discussion:
@barrywhart 
May have the same nature as my previous [Issue](https://github.com/sqlfluff/sqlfluff/issues/1881)
But this one does not corrupt SQL: only a nuisance that `fix` behaviour is not according to expectations
Will look into this, not sure it's related to the other issue
cool, I think I've found the fix for this. Similar to @barrywhart's final comment in #1808. Will pull together a PR

Commit message: resolve outstanding unit tests

Patch to review:
diff --git a/src/sqlfluff/core/parser/segments/base.py b/src/sqlfluff/core/parser/segments/base.py
index 3b8e43435fc..b8eb2d68b76 100644
--- a/src/sqlfluff/core/parser/segments/base.py
+++ b/src/sqlfluff/core/parser/segments/base.py
@@ -973,7 +973,11 @@ def apply_fixes(self, fixes):
                             if f.edit_type == "delete":
                                 # We're just getting rid of this segment.
                                 seg = None
-                            elif f.edit_type in ("edit", "create"):
+                            elif f.edit_type in (
+                                "edit",
+                                "create_before",
+                                "create_after",
+                            ):
                                 # We're doing a replacement (it could be a single segment or an iterable)
                                 if isinstance(f.edit, BaseSegment):
                                     seg_buffer.append(f.edit)  # pragma: no cover TODO?
@@ -981,9 +985,12 @@ def apply_fixes(self, fixes):
                                     for s in f.edit:
                                         seg_buffer.append(s)
 
-                                if f.edit_type == "create":
-                                    # in the case of a creation, also add this segment on the end
+                                if f.edit_type == "create_before":
+                                    # in the case of a creation before, also add this segment on the end
                                     seg_buffer.append(seg)
+                                elif f.edit_type == "create_after":
+                                    # in the case of a creation after, also add this segment to the start
+                                    seg_buffer.insert(0, seg)
                             else:  # pragma: no cover
                                 raise ValueError(
                                     "Unexpected edit_type: {!r} in {!r}".format(
diff --git a/src/sqlfluff/core/rules/base.py b/src/sqlfluff/core/rules/base.py
index 5d633b8d144..8192d009260 100644
--- a/src/sqlfluff/core/rules/base.py
+++ b/src/sqlfluff/core/rules/base.py
@@ -113,7 +113,12 @@ class LintFix:
     """
 
     def __init__(self, edit_type, anchor: BaseSegment, edit=None):
-        if edit_type not in ["create", "edit", "delete"]:  # pragma: no cover
+        if edit_type not in (
+            "create_before",
+            "create_after",
+            "edit",
+            "delete",
+        ):  # pragma: no cover
             raise ValueError(f"Unexpected edit_type: {edit_type}")
         self.edit_type = edit_type
         if not anchor:  # pragma: no cover
@@ -153,7 +158,7 @@ def is_trivial(self):
 
         Removing these makes the routines which process fixes much faster.
         """
-        if self.edit_type == "create":
+        if self.edit_type in ("create_before", "create_after"):
             if isinstance(self.edit, BaseSegment):
                 if len(self.edit.raw) == 0:  # pragma: no cover TODO?
                     return True
@@ -166,7 +171,7 @@ def is_trivial(self):
     def __repr__(self):
         if self.edit_type == "delete":
             detail = f"delete:{self.anchor.raw!r}"
-        elif self.edit_type in ("edit", "create"):
+        elif self.edit_type in ("edit", "create_before", "create_after"):
             if hasattr(self.edit, "raw"):
                 new_detail = self.edit.raw  # pragma: no cover TODO?
             else:
diff --git a/src/sqlfluff/rules/L003.py b/src/sqlfluff/rules/L003.py
index eae869ad0bb..7d08e80d822 100644
--- a/src/sqlfluff/rules/L003.py
+++ b/src/sqlfluff/rules/L003.py
@@ -200,7 +200,7 @@ def _coerce_indent_to(
         elif len("".join(elem.raw for elem in current_indent_buffer)) == 0:
             fixes = [
                 LintFix(
-                    "create",
+                    "create_before",
                     current_anchor,
                     WhitespaceSegment(
                         raw=desired_indent,
@@ -697,7 +697,7 @@ def _process_current_line(
                             # Add in an extra bit of whitespace for the indent
                             fixes=[
                                 LintFix(
-                                    "create",
+                                    "create_before",
                                     trigger_segment,
                                     WhitespaceSegment(
                                         raw=self._make_indent(
@@ -718,7 +718,7 @@ def _process_current_line(
                         ),
                         fixes=[
                             LintFix(
-                                "create",
+                                "create_before",
                                 trigger_segment,
                                 WhitespaceSegment(
                                     # Make the minimum indent for it to be ok.
diff --git a/src/sqlfluff/rules/L006.py b/src/sqlfluff/rules/L006.py
index b14161fb1e8..c1775964566 100644
--- a/src/sqlfluff/rules/L006.py
+++ b/src/sqlfluff/rules/L006.py
@@ -164,7 +164,7 @@ def _eval(self, context: RuleContext) -> EvalResultType:
                             ),
                             fixes=[
                                 LintFix(
-                                    "create",
+                                    "create_before",
                                     # NB the anchor here is always in the parent and not anchor
                                     anchor=sub_seg,
                                     edit=WhitespaceSegment(raw=" "),
@@ -191,7 +191,7 @@ def _eval(self, context: RuleContext) -> EvalResultType:
                             ),
                             fixes=[
                                 LintFix(
-                                    "create",
+                                    "create_before",
                                     # NB the anchor here is always in the parent and not anchor
                                     anchor=next_seg,
                                     edit=WhitespaceSegment(raw=" "),
diff --git a/src/sqlfluff/rules/L009.py b/src/sqlfluff/rules/L009.py
index 5f54644a6e5..d909c8668f0 100644
--- a/src/sqlfluff/rules/L009.py
+++ b/src/sqlfluff/rules/L009.py
@@ -91,15 +91,17 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             # We can't fail on a meta segment
             return None
         else:
-            # So this looks like the end of the file, but we
-            # need to check that each parent segment is also the last.
-            # We do this with reference to the templated file, because it's
-            # the best we can do given the information available.
-            file_len = len(context.segment.pos_marker.templated_file.templated_str)
-            pos = context.segment.pos_marker.templated_slice.stop
-            # Does the length of the file equal the end of the templated position?
-            if file_len != pos:
-                return None
+            # We know we are at a leaf of the tree but not necessarily at the end of the tree.
+            # Therefore we look backwards up the parent stack and ask if any of the parent segments
+            # have another non-meta child segment after the current one.
+            child_segment = context.segment
+            for parent_segment in context.parent_stack[::-1]:
+                possible_children = [
+                    s for s in parent_segment.segments if not s.is_meta
+                ]
+                if len(possible_children) > possible_children.index(child_segment) + 1:
+                    return None
+                child_segment = parent_segment
 
         # Include current segment for complete stack.
         complete_stack: List[BaseSegment] = list(context.raw_stack)
@@ -121,13 +123,19 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             # No need for fix if single new line exists.
             return None
         elif len(eof_newline_segments) == 0:
-            # We're going to make an edit because we're appending to the end and there's
-            # no segment after it to match on. Otherwise we would never get a match!
+            # We make an edit to create this segment after the child of the FileSegment.
+            if len(context.parent_stack) == 1:
+                edit_segment = context.segment
+            else:
+                edit_segment = context.parent_stack[1]
+
             return LintResult(
                 anchor=anchor_segment,
                 fixes=[
                     LintFix(
-                        "edit", context.segment, [context.segment, NewlineSegment()]
+                        "create_after",
+                        edit_segment,
+                        [NewlineSegment()],
                     )
                 ],
             )
diff --git a/src/sqlfluff/rules/L011.py b/src/sqlfluff/rules/L011.py
index 4b39c19fed8..cb99b58bd4e 100644
--- a/src/sqlfluff/rules/L011.py
+++ b/src/sqlfluff/rules/L011.py
@@ -98,7 +98,11 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                     return LintResult(
                         anchor=context.segment,
                         fixes=[
-                            LintFix("create", context.segment.segments[0], insert_buff)
+                            LintFix(
+                                "create_before",
+                                context.segment.segments[0],
+                                insert_buff,
+                            )
                         ],
                     )
         return None
diff --git a/src/sqlfluff/rules/L015.py b/src/sqlfluff/rules/L015.py
index 23eae0ba03d..a5f19c2b5f8 100644
--- a/src/sqlfluff/rules/L015.py
+++ b/src/sqlfluff/rules/L015.py
@@ -68,7 +68,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             ):
                 fixes.append(
                     LintFix(
-                        "create",
+                        "create_before",
                         first_element,
                         WhitespaceSegment(),
                     )
diff --git a/src/sqlfluff/rules/L016.py b/src/sqlfluff/rules/L016.py
index 00be028a3ed..86e4163a3ee 100644
--- a/src/sqlfluff/rules/L016.py
+++ b/src/sqlfluff/rules/L016.py
@@ -143,7 +143,7 @@ def generate_fixes_to_coerce(
                     # Create a newline and a similar indent
                     fixes.append(
                         LintFix(
-                            "create",
+                            "create_before",
                             create_anchor,
                             [
                                 NewlineSegment(),
@@ -164,7 +164,7 @@ def generate_fixes_to_coerce(
                     # Create a newline, create an indent of the relevant size
                     fixes.append(
                         LintFix(
-                            "create",
+                            "create_before",
                             create_anchor,
                             [
                                 NewlineSegment(),
@@ -539,7 +539,9 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                 # Create a newline before this one with the existing comment, an
                 # identical indent AND a terminating newline, copied from the current
                 # target segment.
-                create_buffer = [LintFix("create", this_line[0], create_elements)]
+                create_buffer = [
+                    LintFix("create_before", this_line[0], create_elements)
+                ]
                 return LintResult(
                     anchor=context.segment, fixes=delete_buffer + create_buffer
                 )
diff --git a/src/sqlfluff/rules/L018.py b/src/sqlfluff/rules/L018.py
index 86f4b402b99..10bed5c1f16 100644
--- a/src/sqlfluff/rules/L018.py
+++ b/src/sqlfluff/rules/L018.py
@@ -102,7 +102,7 @@ def indent_size_up_to(segs):
                                 anchor=seg,
                                 fixes=[
                                     LintFix(
-                                        "create",
+                                        "create_before",
                                         seg,
                                         WhitespaceSegment(" " * (-indent_diff)),
                                     )
@@ -125,7 +125,7 @@ def indent_size_up_to(segs):
                                 # We can move it back, it's all whitespace
                                 fixes = [
                                     LintFix(
-                                        "create",
+                                        "create_before",
                                         seg,
                                         [WhitespaceSegment(with_indent_str)],
                                     )
@@ -137,7 +137,7 @@ def indent_size_up_to(segs):
                                 # We have to move it to a newline
                                 fixes = [
                                     LintFix(
-                                        "create",
+                                        "create_before",
                                         seg,
                                         [
                                             NewlineSegment(),
diff --git a/src/sqlfluff/rules/L019.py b/src/sqlfluff/rules/L019.py
index 74448127f1e..5a3e5d1fb2b 100644
--- a/src/sqlfluff/rules/L019.py
+++ b/src/sqlfluff/rules/L019.py
@@ -185,7 +185,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                                 for d in memory["whitespace_deletions"]
                             ],
                             LintFix(
-                                "create",
+                                "create_before",
                                 anchor=self._get_following_seg(
                                     context.raw_stack, last_code_seg
                                 ),
@@ -220,7 +220,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                         fixes=[
                             LintFix("delete", anchor=last_comma_seg),
                             LintFix(
-                                "create",
+                                "create_before",
                                 anchor=context.segment,
                                 edit=[last_comma_seg, new_whitespace_seg],
                             ),
diff --git a/src/sqlfluff/rules/L022.py b/src/sqlfluff/rules/L022.py
index a8300947e65..b1f5746ee0c 100644
--- a/src/sqlfluff/rules/L022.py
+++ b/src/sqlfluff/rules/L022.py
@@ -132,7 +132,7 @@ def _eval(self, context: RuleContext) -> Optional[List[LintResult]]:
 
                     # Based on the current location of the comma we insert newlines
                     # to correct the issue.
-                    fix_type = "create"  # In most cases we just insert newlines.
+                    fix_type = "create_before"  # In most cases we just insert newlines.
                     if comma_style == "oneline":
                         # Here we respect the target comma style to insert at the relevant point.
                         if self.comma_style == "trailing":
diff --git a/src/sqlfluff/rules/L023.py b/src/sqlfluff/rules/L023.py
index e0f0553644d..e8aad35ccd1 100644
--- a/src/sqlfluff/rules/L023.py
+++ b/src/sqlfluff/rules/L023.py
@@ -71,7 +71,7 @@ def _eval(self, context: RuleContext) -> Optional[List[LintResult]]:
                                 # There's nothing between. Just add a whitespace
                                 fixes = [
                                     LintFix(
-                                        "create",
+                                        "create_before",
                                         seg,
                                         [WhitespaceSegment()],
                                     )
diff --git a/src/sqlfluff/rules/L036.py b/src/sqlfluff/rules/L036.py
index 6318dbc140c..d6f84111e27 100644
--- a/src/sqlfluff/rules/L036.py
+++ b/src/sqlfluff/rules/L036.py
@@ -148,7 +148,7 @@ def _eval_multiple_select_target_elements(self, select_targets_info, segment):
                     loop_while=lambda s: s.is_type("whitespace", "comma") or s.is_meta,
                 )
                 fixes += [LintFix("delete", ws) for ws in ws_to_delete]
-                fixes.append(LintFix("create", select_target, NewlineSegment()))
+                fixes.append(LintFix("create_before", select_target, NewlineSegment()))
 
             # If we are at the last select target check if the FROM clause
             # is on the same line, and if so move it to its own line.
@@ -165,7 +165,9 @@ def _eval_multiple_select_target_elements(self, select_targets_info, segment):
                     )
                     fixes.append(
                         LintFix(
-                            "create", select_targets_info.from_segment, NewlineSegment()
+                            "create_before",
+                            select_targets_info.from_segment,
+                            NewlineSegment(),
                         )
                     )
 
@@ -406,7 +408,9 @@ def _eval_single_select_target_element(
                 ]
                 fixes.append(
                     LintFix(
-                        "create", select_targets_info.from_segment, NewlineSegment()
+                        "create_before",
+                        select_targets_info.from_segment,
+                        NewlineSegment(),
                     )
                 )
                 return LintResult(anchor=select_clause, fixes=fixes)
diff --git a/src/sqlfluff/rules/L037.py b/src/sqlfluff/rules/L037.py
index b587d3d3768..32e3998b72f 100644
--- a/src/sqlfluff/rules/L037.py
+++ b/src/sqlfluff/rules/L037.py
@@ -102,7 +102,7 @@ def _eval(self, context: RuleContext) -> Optional[List[LintResult]]:
                     # Since ASC is default in SQL, add in ASC for fix
                     lint_fixes.append(
                         LintFix(
-                            "create",
+                            "create_before",
                             col_info.separator,
                             [WhitespaceSegment(), KeywordSegment("ASC")],
                         )
diff --git a/src/sqlfluff/rules/L041.py b/src/sqlfluff/rules/L041.py
index 80d3cd38c1a..cb8c5092136 100644
--- a/src/sqlfluff/rules/L041.py
+++ b/src/sqlfluff/rules/L041.py
@@ -78,7 +78,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                 # E.g. "\n" -> " DISTINCT\n.
                 LintFix("delete", newline_between),
                 LintFix(
-                    "create",
+                    "create_before",
                     context.segment.segments[newline_idx + 1],
                     replace_newline_with,
                 ),
diff --git a/src/sqlfluff/rules/L051.py b/src/sqlfluff/rules/L051.py
index 903be7904c3..75e028200e3 100644
--- a/src/sqlfluff/rules/L051.py
+++ b/src/sqlfluff/rules/L051.py
@@ -48,7 +48,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             context.segment.segments[0],
             fixes=[
                 LintFix(
-                    "create",
+                    "create_before",
                     context.segment.segments[0],
                     [KeywordSegment("INNER"), WhitespaceSegment()],
                 )
diff --git a/src/sqlfluff/rules/L052.py b/src/sqlfluff/rules/L052.py
index 74536d35608..a141b319168 100644
--- a/src/sqlfluff/rules/L052.py
+++ b/src/sqlfluff/rules/L052.py
@@ -89,7 +89,9 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
 
                 if len(newline_deletions) == 0:
                     # Create missing newline.
-                    fixes.append(LintFix("create", context.segment, NewlineSegment()))
+                    fixes.append(
+                        LintFix("create_before", context.segment, NewlineSegment())
+                    )
                 if len(newline_deletions) > 1:
                     # Remove excess newlines.
                     fixes.extend(LintFix("delete", d) for d in newline_deletions[1:])
@@ -114,15 +116,20 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                 # We can't fail on a meta segment
                 return None
             else:
-                # So this looks like the end of the file, but we
-                # need to check that each parent segment is also the last.
-                # We do this with reference to the templated file, because it's
-                # the best we can do given the information available.
-                file_len = len(context.segment.pos_marker.templated_file.templated_str)
-                pos = context.segment.pos_marker.templated_slice.stop
-                # Does the length of the file equal the end of the templated position?
-                if file_len != pos:
-                    return None
+                # We know we are at a leaf of the tree but not necessarily at the end of the tree.
+                # Therefore we look backwards up the parent stack and ask if any of the parent segments
+                # have another non-meta child segment after the current one.
+                child_segment = context.segment
+                for parent_segment in context.parent_stack[::-1]:
+                    possible_children = [
+                        s for s in parent_segment.segments if not s.is_meta
+                    ]
+                    if (
+                        len(possible_children)
+                        > possible_children.index(child_segment) + 1
+                    ):
+                        return None
+                    child_segment = parent_segment
 
             # Include current segment for complete stack.
             complete_stack: List[BaseSegment] = list(context.raw_stack)
diff --git a/test/core/rules/config_test.py b/test/core/rules/config_test.py
index d062cb1ecb6..836ca68eb60 100644
--- a/test/core/rules/config_test.py
+++ b/test/core/rules/config_test.py
@@ -29,7 +29,7 @@ def _eval(self, context):
         if context.segment.is_type("star"):
             return LintResult(
                 anchor=context.segment,
-                fixes=[LintFix("create", context.segment, NewlineSegment())],
+                fixes=[LintFix("create_before", context.segment, NewlineSegment())],
             )
 
 
diff --git a/test/rules/std_L009_L052_test.py b/test/rules/std_L009_L052_test.py
new file mode 100644
index 00000000000..eb2804010f3
--- /dev/null
+++ b/test/rules/std_L009_L052_test.py
@@ -0,0 +1,23 @@
+"""Tests the python routines within L009 and L052."""
+from sqlfluff.core import FluffConfig
+from sqlfluff.core import Linter
+
+
+def test__rules__std_L009_and_L052_interaction() -> None:
+    """Test interaction between L009 and L052 doesn't stop L052 from being applied."""
+    # Test sql with no final newline and no final semicolon.
+    sql = "SELECT foo FROM bar"
+
+    # Ensure final semicolon requirement is active.
+    cfg = FluffConfig()
+    cfg.set_value(config_path=["rules", "L052", "require_final_semicolon"], val=True)
+    linter = Linter(config=cfg)
+
+    # Return linted/fixed file.
+    linted_file = linter.lint_string(sql, fix=True)
+
+    # Check expected lint errors are raised.
+    assert set([v.rule.code for v in linted_file.violations]) == {"L009", "L052"}
+
+    # Check file is fixed.
+    assert linted_file.fix_string()[0] == "SELECT foo FROM bar;\n"


Code review comments:
diff_hunk: @@ -91,15 +91,17 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             # We can't fail on a meta segment
             return None
         else:
-            # So this looks like the end of the file, but we
-            # need to check that each parent segment is also the last.
-            # We do this with reference to the templated file, because it's
-            # the best we can do given the information available.
-            file_len = len(context.segment.pos_marker.templated_file.templated_str)
-            pos = context.segment.pos_marker.templated_slice.stop
-            # Does the length of the file equal the end of the templated position?
-            if file_len != pos:
-                return None
+            # We know we are at a leaf of the tree but not necessarily at the end of the tree.
path: src/sqlfluff/rules/L009.py
text: @user1:
Is this the same code that was added to L052? I wonder if this should be turned into a reusable function, e.g. on the rule context or base rule classes.

@author:
Moved the logic to `BaseRule` and it looks much neater now 👍