Repository: sqlfluff/sqlfluff

Issue:
Enhance rule L036 to put all columns on separate lines if any of them are
The current description is ambiguous, but after discussion, we decided to update the rule and keep the description at least _similar_ to what it is currently.. See discussion on #769.

Discussion:

Patch:
diff --git a/src/sqlfluff/core/rules/std/L036.py b/src/sqlfluff/core/rules/std/L036.py
index 7e64bafbd..d49de4d94 100644
--- a/src/sqlfluff/core/rules/std/L036.py
+++ b/src/sqlfluff/core/rules/std/L036.py
@@ -90,21 +90,26 @@ class Rule_L036(BaseRule):
         )
 
     def _eval_multiple_select_target_elements(self, select_targets_info, segment):
-        if select_targets_info.first_new_line_idx == -1:
-            # there are multiple select targets but no new lines
-
-            # Find and delete any whitespace between "SELECT" and its targets.
-            ws_to_delete = segment.select_children(
-                start_seg=segment.segments[select_targets_info.select_idx],
-                select_if=lambda s: s.is_type("whitespace"),
-                loop_while=lambda s: s.is_type("whitespace") or s.is_meta,
+        """Multiple select targets. Ensure each is on a separate line."""
+        # Insert newline before every select target.
+        fixes = []
+        for i, select_target in enumerate(select_targets_info.select_targets):
+            base_segment = (
+                segment if not i else select_targets_info.select_targets[i - 1]
             )
-            fixes = [LintFix("delete", ws) for ws in ws_to_delete]
-            # Insert newline before the first select target.
-            ins = self.make_newline(
-                pos_marker=segment.pos_marker.advance_by(segment.raw)
-            )
-            fixes.append(LintFix("create", select_targets_info.select_targets[0], ins))
+            if base_segment.pos_marker.line_no == select_target.pos_marker.line_no:
+                # Find and delete any whitespace before the select target.
+                ws_to_delete = segment.select_children(
+                    start_seg=segment.segments[select_targets_info.select_idx]
+                    if not i
+                    else select_targets_info.select_targets[i - 1],
+                    select_if=lambda s: s.is_type("whitespace"),
+                    loop_while=lambda s: s.is_type("whitespace", "comma") or s.is_meta,
+                )
+                fixes += [LintFix("delete", ws) for ws in ws_to_delete]
+                ins = self.make_newline(pos_marker=select_target.pos_marker)
+                fixes.append(LintFix("create", select_target, ins))
+        if fixes:
             return LintResult(anchor=segment, fixes=fixes)
 
     def _eval_single_select_target_element(


Test patch:
diff --git a/test/api/simple_test.py b/test/api/simple_test.py
index b80e78816..5fe52c3f3 100644
--- a/test/api/simple_test.py
+++ b/test/api/simple_test.py
@@ -121,7 +121,14 @@ def test__api__fix_string():
     # Check return types.
     assert isinstance(result, str)
     # Check actual result
-    assert result == "SELECT\n    *, 1, blah AS foo FROM mytable\n"
+    assert (
+        result
+        == """SELECT
+    *,
+    1,
+    blah AS foo FROM mytable
+"""
+    )
 
 
 def test__api__fix_string_specific():
diff --git a/test/fixtures/rules/std_rule_cases/L036.yml b/test/fixtures/rules/std_rule_cases/L036.yml
index bfce26f1f..70e4676e0 100644
--- a/test/fixtures/rules/std_rule_cases/L036.yml
+++ b/test/fixtures/rules/std_rule_cases/L036.yml
@@ -37,7 +37,7 @@ test_multiple_select_targets_all_on_the_same_line:
   fail_str: |
     select a, b, c
     from x
-  fix_str: "select\na, b, c\nfrom x\n"
+  fix_str: "select\na,\nb,\nc\nfrom x\n"
 
 test_multiple_select_targets_trailing_whitespace_after_select:
   # TRICKY: Use explicit newlines to preserve the trailing space after "SELECT".
@@ -57,3 +57,47 @@ test_comment_between_select_and_single_select_target:
         -- This is the user's ID.
         FROM
         safe_user
+
+test_multiple_select_targets_some_newlines_missing_1:
+  fail_str: |
+    select
+      a, b, c,
+      d, e, f, g,
+      h
+    from x
+  # The spaces before a, d, and h look odd, but these are places where the
+  # select targets were already on a separate line, and the rule made no
+  # changes.
+  fix_str: |
+    select
+      a,
+    b,
+    c,
+      d,
+    e,
+    f,
+    g,
+      h
+    from x
+
+
+test_multiple_select_targets_some_newlines_missing_2:
+  fail_str: |
+    select a, b, c,
+      d, e, f, g,
+      h
+    from x
+  # The spaces before d, and h look odd, but these are places where the
+  # select targets were already on a separate line, and the rule made no
+  # changes.
+  fix_str: |
+    select
+    a,
+    b,
+    c,
+      d,
+    e,
+    f,
+    g,
+      h
+    from x