Repository: sqlfluff/sqlfluff
Pull request: Update L009 to operate in raw, not templated space
<!--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
Fixes #2046 

Updates L009 to consider whether trailing newlines are in raw or templated space before reporting an issue.

### Are there any other side effects of this change that we should be aware of?


### 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 `tox -e generate-fixture-yml`).
  - 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:
dbt - L009: Files must end with a single trailing newline.
<!--If this is a parsing or linting issue, please include a minimal SQL example which reproduces the issue, along with the `sqlfluff parse` output, `sqlfluff lint` output and `sqlfluff fix` output when relevant.-->

## Expected Behaviour
No error


## Observed Behaviour
```
L:  5 | P:  60 | L009 | Files must end with a single trailing newline.    
```

## Steps to Reproduce
A dbt model that calls a macro, eg:
```
{{ config ( materialized='incremental',
   transient=false,
    ) }}

{{ generate_table('mytable') }}

```

## Dialect
snowflake

## Version
sqlfluff, version 0.8.2
python 3.9

## Configuration
```
[tool.sqlfluff.core]
templater = "dbt"
sql_file_exts = ".sql,.sql.j2,.dml,.ddl"
dialect = "snowflake"
ignore_templated_areas = true

[tool.sqlfluff.indentation]
indented_joins = false
indented_using_on = true
template_blocks_indent = false

[tool.sqlfluff.templater.dbt]
project_dir = "."
profile = "snowflake"
```

Hints from issue discussion:
@tekumara is this all that's in the file?

Please could you post the output of `sqlfluff parse <file>` here?
Yep that's all.

Here's the tail of `sqlfluff parse`:
```
[L:  8, P:  1]      |                        identifier:                           'RunID'
[L:  8, P: 44]      |                    [META] dedent:
[L:  8, P:  1]      |    newline:                                                  '\n'
[L:  8, P:  1]      |    newline:                                                  '\n'
[L:  8, P: 44]      |    newline:                                                  '\n'
```

Here's the tail of the macro:
```
ORDER BY rl.RunID

{% endmacro %}

```

If I remove the newline before `{% endmacro %}` it produces two newlines in the parse, and still errors.
Makes sense thanks! This is a known issue with the dbt templater.

In Jinja, the template lines are included as placeholders in the parse tree so it's possible to account for those
e.g.
![image](https://user-images.githubusercontent.com/80432516/145007044-7760e2cc-458f-46de-8468-fe1eac22a9f0.png)

However, as you can see from your parse tree, certain dbt templates don't show up in the parse tree. So as far as the rule can see the file finishes with 3 newlines. We encountered a similar error in L050 (no newlines at start of the file) https://github.com/sqlfluff/sqlfluff/pull/1840#issuecomment-962682902. I ended up adding some logic to infer the presence of those missing template blocks so I'll try and add some similar logic to L009.

I'll also create a separate issue to see if one of the dbt contributors can get those templates to show up in the parse tree as a more long term solution.
See this [comment](https://github.com/sqlfluff/sqlfluff/issues/2066#issuecomment-988800699) on #2066 for more details on what's happening. TL;DR: This is actually not a dbt templater issue; addressing issues like this systematically will require a parser change.
Another update: we decided not to change the parser, but the work in PR #2126, once it is merged, will provide nicer ways of working with collections of segments and raw/templated slices, and that's what is needed to fix this issue. (To be clear, it's possible to fix it now, but it'll be much easier then.)
This issue can be worked on now. I suggest trying to use the new classes and functions in `sqlfluff.core.rules.functional`, which should provide more convenient access & conversions from the parser segments to the related raw and/or templated file slices.

Commit message: More comments

Patch to review:
diff --git a/src/sqlfluff/core/rules/functional/__init__.py b/src/sqlfluff/core/rules/functional/__init__.py
index 9ab43488147..8e7f84dd363 100644
--- a/src/sqlfluff/core/rules/functional/__init__.py
+++ b/src/sqlfluff/core/rules/functional/__init__.py
@@ -12,7 +12,8 @@
 use one or the other for readability.
 """
 
-__all__ = ("Segments", "sp")
+__all__ = ("Segments", "rsp", "sp")
 
 from sqlfluff.core.rules.functional.segments import Segments
+import sqlfluff.core.rules.functional.raw_file_slice_predicates as rsp
 import sqlfluff.core.rules.functional.segment_predicates as sp
diff --git a/src/sqlfluff/core/rules/functional/segment_predicates.py b/src/sqlfluff/core/rules/functional/segment_predicates.py
index 7eeeeddb273..833cc3a6abc 100644
--- a/src/sqlfluff/core/rules/functional/segment_predicates.py
+++ b/src/sqlfluff/core/rules/functional/segment_predicates.py
@@ -8,9 +8,11 @@
 requirements. Rule authors can define their own predicates as needed, either
 as regular functions, `lambda`, etc.
 """
-from typing import Callable
+from typing import Callable, Optional
 
 from sqlfluff.core.parser import BaseSegment
+from sqlfluff.core.rules.functional.raw_file_slices import RawFileSlices
+from sqlfluff.core.templaters.base import TemplatedFile
 
 
 def is_type(*seg_type: str) -> Callable[[BaseSegment], bool]:
@@ -133,3 +135,18 @@ def _(segment: BaseSegment):
         return not fn(segment)
 
     return _
+
+
+def raw_slices(
+    segment: BaseSegment,
+    templated_file: Optional[TemplatedFile],
+) -> RawFileSlices:
+    """Returns raw slices for a segment."""
+    if not templated_file:
+        raise ValueError('raw_slices: "templated_file" parameter is required.')
+    return RawFileSlices(
+        *templated_file.raw_slices_spanning_source_slice(
+            segment.pos_marker.source_slice
+        ),
+        templated_file=templated_file
+    )
diff --git a/src/sqlfluff/rules/L009.py b/src/sqlfluff/rules/L009.py
index ad3dc632028..b5c03afca1e 100644
--- a/src/sqlfluff/rules/L009.py
+++ b/src/sqlfluff/rules/L009.py
@@ -5,8 +5,7 @@
 
 from sqlfluff.core.rules.base import BaseRule, LintResult, LintFix, RuleContext
 from sqlfluff.core.rules.doc_decorators import document_fix_compatible
-from sqlfluff.core.rules.functional import Segments
-from sqlfluff.core.rules.functional import segment_predicates as sp
+from sqlfluff.core.rules.functional import Segments, rsp, sp
 
 
 @document_fix_compatible
@@ -99,10 +98,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
             loop_while=sp.or_(sp.is_whitespace(), sp.is_type("dedent")),
         )
 
-        if len(trailing_newlines) == 1:
-            # No need for fix if single new line exists.
-            return None
-        elif len(trailing_newlines) == 0:
+        if not trailing_newlines:
             # We make an edit to create this segment after the child of the FileSegment.
             if len(parent_stack) == 1:
                 fix_anchor_segment = context.segment
@@ -119,8 +115,20 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]:
                 ],
             )
         else:
-            # There are excess newlines so delete all bar one.
-            return LintResult(
-                anchor=context.segment,
-                fixes=[LintFix.delete(d) for d in trailing_newlines[1:]],
+            # There are one or more trailing newlines in templated space.
+            # For any excess newlines, translate to "raw" space to determine if
+            # they are literal (rather than templated). Delete any extras.
+            # (Note that 'trailing_newlines' is ordered in reverse, i.e. from
+            # the end of the file *backwards*.)
+            extra_newlines = trailing_newlines[1:].select(
+                loop_while=lambda seg: sp.raw_slices(seg, context.templated_file).all(
+                    rsp.is_slice_type("literal")
+                )
             )
+            if extra_newlines:
+                return LintResult(
+                    anchor=context.segment,
+                    fixes=[LintFix.delete(d) for d in extra_newlines],
+                )
+            # Single newline, no need for fix.
+            return None
diff --git a/test/fixtures/rules/std_rule_cases/L009.yml b/test/fixtures/rules/std_rule_cases/L009.yml
index 0da55a7b5ee..b708d5d639d 100644
--- a/test/fixtures/rules/std_rule_cases/L009.yml
+++ b/test/fixtures/rules/std_rule_cases/L009.yml
@@ -10,3 +10,7 @@ test_fail_no_final_newline:
 test_fail_multiple_final_newlines:
   fail_str: "SELECT foo FROM bar\n\n"
   fix_str: "SELECT foo FROM bar\n"
+
+test_pass_templated_plus_raw_newlines:
+  pass_str: |
+    {{ "\n\n" }}


Code review comments:
diff_hunk: @@ -10,3 +10,7 @@ test_fail_no_final_newline:
 test_fail_multiple_final_newlines:
   fail_str: "SELECT foo FROM bar\n\n"
   fix_str: "SELECT foo FROM bar\n"
+
+test_pass_templated_plus_raw_newlines:
+  pass_str: |
+    {{ "\n\n" }}
path: test/fixtures/rules/std_rule_cases/L009.yml
text: @user1:
Why is this a valid pass_str? Should it be `{{ "\n\n" }}\n`?

@author:
The vertical bar (`|`) character tells YAML this is a multiline string, and that we want to preserve newlines at the end of lines. Most other rules use multiline format, but this one mostly uses quoted strings. I'll change the test to the equivalent quoted string, as in your example above. 🙏🏽