Repository: sqlfluff/sqlfluff
Pull request: Fix issue 646: Rule L036 misbehaves when there's trailing whitespace after "SELECT"
Addresses #646 

Problem statement:
L036 includes whitespace as a select target which should be on a new line
In the sql 
```
combined as (
    select 
        renamed.*,
        coalesce(academic_years.is_currently_active, false) as is_current_academic_year
    from renamed
    left outer join academic_years using (academic_year_id)
)
```
, there is a space after `select`. This triggers two linting rules:
```
L:  29 | P:   5 | L036 | Select targets should be on a new line unless there is
                       | only one select target.
L:  29 | P:  11 | L001 | Unnecessary trailing whitespace.
```

Whitespace is covered by L001, so I'd say it shouldn't also trigger L036. 

`python --version`
Python 3.8.3 (Windows)

`sqlfluff --version`
sqlfluff, version 0.4.0a3

Hints from issue discussion:

Commit message: Fix issue 646: Rule L036 misbehaves when there's trailing whitespace after "SELECT"

Patch to review:
diff --git a/src/sqlfluff/core/rules/std.py b/src/sqlfluff/core/rules/std.py
index c4dd1f36e26..6d4d01e02a0 100644
--- a/src/sqlfluff/core/rules/std.py
+++ b/src/sqlfluff/core/rules/std.py
@@ -3462,11 +3462,14 @@ def _get_indexes(segment):
                 cnt_select_targets += 1
                 if first_select_target_idx == -1:
                     first_select_target_idx = fname_idx
-            if seg.is_type("keyword") and seg.name == "SELECT" and select_idx - 1:
+            if seg.is_type("keyword") and seg.name == "SELECT" and select_idx == -1:
                 select_idx = fname_idx
             if seg.is_type("newline") and first_new_line_idx == -1:
                 first_new_line_idx = fname_idx
-            if seg.is_type("whitespace") and first_whitespace_idx == -1:
+            # TRICKY: Ignore whitespace prior to the first newline, e.g. if
+            # the line with "SELECT" (before any selct targets) has trailing
+            # whitespace.
+            if seg.is_type("whitespace") and first_new_line_idx != -1 and first_whitespace_idx == -1:
                 first_whitespace_idx = fname_idx
 
         eval_result = EvalResult(


Code review comments:
diff_hunk: @@ -3462,11 +3462,14 @@ def _get_indexes(segment):
                 cnt_select_targets += 1
                 if first_select_target_idx == -1:
                     first_select_target_idx = fname_idx
-            if seg.is_type("keyword") and seg.name == "SELECT" and select_idx - 1:
+            if seg.is_type("keyword") and seg.name == "SELECT" and select_idx == -1:
                 select_idx = fname_idx
             if seg.is_type("newline") and first_new_line_idx == -1:
                 first_new_line_idx = fname_idx
-            if seg.is_type("whitespace") and first_whitespace_idx == -1:
+            # TRICKY: Ignore whitespace prior to the first newline, e.g. if
+            # the line with "SELECT" (before any selct targets) has trailing
+            # whitespace.
+            if seg.is_type("whitespace") and first_new_line_idx != -1 and first_whitespace_idx == -1:
path: src/sqlfluff/core/rules/std.py
text: @author:
This is the real fix. By ignoring whitespace prior to the first newline, we avoid being fooled by trailing whitespace after the `SELECT`.

Question: Does this potentially cause other issues?