Repository: sqlfluff/sqlfluff
Pull request: Fix runtime error (IndexError ) when linting file with jinja "if"
### Brief summary of the change made

Fixes #1414

### 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/parser` (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:
Runtime error (IndexError ) linting file with jinja "if"
## Expected Behaviour

Get list of rule violations by line  for the file:

```sql
-- This file combines product data from individual brands into a staging table
{% set products =  [
  'tabe1',
  'table2'] %}

{% for product in products %}
SELECT
  brand,
  country_code,
  category,
  name,
  id
FROM
  {{ ref(product) }}
{% if not loop.last -%} UNION ALL {%- endif %}
{% endfor %}
```

## Observed Behaviour

Cannot make sqlfluff work with jinja for loops.

Error:

```
WARNING    Unable to lint /app/models/3_INFORMATION_MART/orders.sql due to an internal error. Please report this as an issue with your query's contents and stacktrace below!
To hide this warning, add the failing file to .sqlfluffignore
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/linter/runner.py", line 94, in run
    yield partial()
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/linter/linter.py", line 512, in lint_rendered
    parsed = cls.parse_rendered(rendered)
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/linter/linter.py", line 294, in parse_rendered
    tokens, lvs, config = cls._lex_templated_file(
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/linter/linter.py", line 127, in _lex_templated_file
    tokens, lex_vs = lexer.lex(templated_file)
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/parser/lexer.py", line 319, in lex
    segments: Tuple[RawSegment, ...] = self.elements_to_segments(
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/parser/lexer.py", line 346, in elements_to_segments
    source_slice = templated_file.templated_slice_to_source_slice(
  File "/usr/local/lib/python3.8/site-packages/sqlfluff/core/templaters/base.py", line 287, in templated_slice_to_source_slice
    start_slices = [self.sliced_file[ts_start_sf_start]]
IndexError: list index out of range

```

## Steps to Reproduce

run lint against the target file

## Dialect
dbt

## Version
0.6.4

## Configuration
```
[sqlfluff]
templater = dbt
exclude_rules = L031,L029

[sqlfluff:rules]
tab_space_size = 2

[sqlfluff:templater:dbt]
profiles_dir = .
profile = dev

[sqlfluff:rules:L010]  # Keywords
capitalisation_policy = upper

```

Hints from issue discussion:
I reproduced this error with the `jinja` templater, so it is not specific to dbt.

```
-- This file combines product data from individual brands into a staging table
{% set products =  [
  'tabe1',
  'table2'] %}

{% for product in products %}
SELECT
  brand,
  country_code,
  category,
  name,
  id
FROM
  product
{% if not loop.last -%} UNION ALL {%- endif %}
{% endfor %}
```
This line is triggering the error. The error goes away if I either:
* Add a newline prior to the `endif`
* Change `{%- endif %}` to `{% endif %}`

```
{% if not loop.last %} UNION ALL {%- endif %}
```

A recent PR (#1364) made some changes regarding how `{%-` is handled. I don't know that this bug is related, but it's at least _interesting_ that we've seen multiple bugs recently relating to Jinja "whitespace control".
This minimal example produces the same error:
```
{% set v = 0 %}

{% if not 1 %}{% endif %}
```

Commit message: Add "no cover" pragma

Patch to review:
diff --git a/src/sqlfluff/core/templaters/base.py b/src/sqlfluff/core/templaters/base.py
index e41857408bb..ed9e4e7d969 100644
--- a/src/sqlfluff/core/templaters/base.py
+++ b/src/sqlfluff/core/templaters/base.py
@@ -285,8 +285,12 @@ def templated_slice_to_source_slice(
                 ts_start_sf_stop, ts_stop_sf_stop
             )
         ]
-        if ts_start_sf_start == ts_start_sf_stop:  # pragma: no cover TODO?
-            start_slices = [self.sliced_file[ts_start_sf_start]]
+        if ts_start_sf_start == ts_start_sf_stop:
+            if ts_start_sf_start < len(self.sliced_file):
+                start_slices = [self.sliced_file[ts_start_sf_start]]  # pragma: no cover TODO?
+            else:
+                assert ts_start_sf_start == len(self.sliced_file)
+                return self.sliced_file[-1].source_slice
         else:
             start_slices = self.sliced_file[ts_start_sf_start:ts_start_sf_stop]
         if ts_stop_sf_start == ts_stop_sf_stop:  # pragma: no cover TODO?
diff --git a/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/after.sql b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/after.sql
new file mode 100644
index 00000000000..52d84549d59
--- /dev/null
+++ b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/after.sql
@@ -0,0 +1,16 @@
+-- This file combines product data from individual brands into a staging table
+{% set products =  [
+  'table1',
+  'table2'] %}
+
+{% for product in products %}
+SELECT
+    brand,
+    country_code,
+    category,
+    name,
+    id
+FROM
+    product
+{% if not loop.last -%} UNION ALL {%- endif %}
+{% endfor %}
diff --git a/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/before.sql b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/before.sql
new file mode 100644
index 00000000000..7b58f397361
--- /dev/null
+++ b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/before.sql
@@ -0,0 +1,16 @@
+-- This file combines product data from individual brands into a staging table
+{% set products =  [
+  'table1',
+  'table2'] %}
+
+{% for product in products %}
+SELECT
+  brand,
+  country_code,
+  category,
+  name,
+  id
+FROM
+  product
+{% if not loop.last -%} UNION ALL {%- endif %}
+{% endfor %}
diff --git a/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/test-config.yml b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/test-config.yml
new file mode 100644
index 00000000000..48051468f9d
--- /dev/null
+++ b/test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/test-config.yml
@@ -0,0 +1,4 @@
+test-config:
+  rules:
+    - L003
+    - L016


Code review comments:
diff_hunk: @@ -0,0 +1,16 @@
+-- This file combines product data from individual brands into a staging table
+{% set products =  [
+  'table1',
+  'table2'] %}
+
+{% for product in products %}
+SELECT
+  brand,
+  country_code,
+  category,
+  name,
+  id
+FROM
+  product
path: test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/before.sql
text: @user1:
This really should be this as per above:

```suggestion
  {{ product }}
```

However when I test this with `{% if not loop.last -%} UNION ALL {%- endif %}` syntax it _dedents_ this as part of the fix (so fixes it to `{{ product}}` instead of `••••{{ product }}`(where `•` represents a space) but it correctly indents it with `{% if not loop.last %} UNION ALL {% endif %}` (i.e. without the negative signs for Jinja pieces) so a bit confused as to why that happens. Any ideas?

diff_hunk: @@ -285,8 +285,12 @@ def templated_slice_to_source_slice(
                 ts_start_sf_stop, ts_stop_sf_stop
             )
         ]
-        if ts_start_sf_start == ts_start_sf_stop:  # pragma: no cover TODO?
-            start_slices = [self.sliced_file[ts_start_sf_start]]
+        if ts_start_sf_start == ts_start_sf_stop:
+            if ts_start_sf_start < len(self.sliced_file):
+                start_slices = [self.sliced_file[ts_start_sf_start]]  # pragma: no cover TODO?
+            else:
+                assert ts_start_sf_start == len(self.sliced_file)
+                return self.sliced_file[-1].source_slice
path: src/sqlfluff/core/templaters/base.py
text: @user1:
I'm not loving this `assert` to be honest. To me this should be take care of with an `if` statement.

Also, while this fixes the test case you have, it doesn't work if there is anything after the test case (even a semi-colon!)

So I would suggest the following instead:

```suggestion
        if ts_start_sf_start == ts_start_sf_stop:
            if ts_start_sf_start > len(self.sliced_file):  # pragma: no cover
                # We should never get here
                raise ValueError("Starting position higher than sliced file position")
            if ts_start_sf_start < len(self.sliced_file):
                start_slices = [self.sliced_file[ts_start_sf_start]]
            else:
                return self.sliced_file[-1].source_slice
```

The middle `ts_start_sf_start < len(self.sliced_file)` handles the cases when there is stuff after the templated segment, so can remove the `pragma : no cover` from the initial solution (except the first exception which definitely is a `no cover` statement rather than a `no cover TODO`), so will need a test case for this as discussed below.

@author:
Can you add the suggested test case here?
>Also, while this fixes the test case you have, it doesn't work if there is anything after the test case (even a semi-colon!)

@user1:
Happy to commit my changes to this branch now if you want?

@user1:
I went ahead and did this in https://github.com/sqlfluff/sqlfluff/pull/1430/commits/62687a79887205a0c877ccdd8bf18be44e39738e

@user1:
Resolving this comment now assuming you're happy with that extra code I committed (revert it if not).

diff_hunk: @@ -0,0 +1,16 @@
+-- This file combines product data from individual brands into a staging table
+{% set products =  [
+  'table1',
+  'table2'] %}
+
+{% for product in products %}
+SELECT
+    brand,
+    country_code,
+    category,
+    name,
+    id
+FROM
+    product
path: test/fixtures/linter/autofix/ansi/016_index_error_with_jinja_if/after.sql
text: @user1:
This to me is wrong as `product` is a variable, so this should be:

```suggestion
    {{ product }}
```

However testing this leads to another problem (discussed below)...

@author:
Oops, I just intended to use a placeholder table name. I'll change it to something else like `foo`.

@user1:
Actually I think we should use the real templated name (`{{ product }}`) since it has shown another error. Even if we don;'t fix that here. Again happy to commit my change if you want.