Repository: sqlfluff/sqlfluff

Issue:
2.0.0a1 - Comments cause inconsistent indentation of CASE
### Search before asking

- [X] I searched the [issues](https://github.com/sqlfluff/sqlfluff/issues) and found no similar issues.


### What Happened

Given this query:

```sql
select
    case
        when a
        then b
    end as foo,
    case
        when a -- bar
        then b
    end as bar
from c
```

the fix is:

```sql
select
    case
        when a
            then b
    end as foo,
    case
        when
            a -- bar
        then b
    end as bar
from c
```

### Expected Behaviour

The comment causes a different treatment of the fix. This is true with and without implicit indents. The indentation without the comment is the "correct" one (at least as per https://github.com/dbt-labs/corp/blob/main/dbt_style_guide.md#example-sql).

### Observed Behaviour

see above.

### How to reproduce

`sqlfluff fix -r L003 -x f -d ansi test.sql --logger rules -vvvv`

### Dialect

snowflake

### Version

2.0.0a1

### Configuration

default

### Are you willing to work on and submit a PR to address the issue?

- [X] Yes I am willing to submit a PR!

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/sqlfluff/sqlfluff/blob/main/CODE_OF_CONDUCT.md)


Discussion:
coveralls: ## Pull Request Test Coverage Report for [Build 3837448641](https://coveralls.io/builds/55629543)

* **13** of **13**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* **4** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.02%**) to **99.976%**

---


|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [src/sqlfluff/utils/reflow/reindent.py](https://coveralls.io/builds/55629543/source?filename=src%2Fsqlfluff%2Futils%2Freflow%2Freindent.py#L728) | 4 | 99.18% |
<!-- | **Total:** | **4** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/55629543/badge)](https://coveralls.io/builds/55629543) |
| :-- | --: |
| Change from base [Build 3830030163](https://coveralls.io/builds/55599557): |  -0.02% |
| Covered Lines: | 16869 |
| Relevant Lines: | 16873 |

---
##### 💛  - [Coveralls](https://coveralls.io)

codecov[bot]: # [Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/4239?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) Report
Base: **100.00**% // Head: **99.97**% // Decreases project coverage by **`-0.02%`** :warning:
> Coverage data is based on head [(`1b899d0`)](https://codecov.io/gh/sqlfluff/sqlfluff/pull/4239?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) compared to base [(`e251fd8`)](https://codecov.io/gh/sqlfluff/sqlfluff/commit/e251fd84bfd728292d75f07be031b8043fb55ed9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##              main    #4239      +/-   ##
===========================================
- Coverage   100.00%   99.97%   -0.03%     
===========================================
  Files          199      199              
  Lines        15099    15109      +10     
===========================================
+ Hits         15099    15105       +6     
- Misses           0        4       +4     
```


| [Impacted Files](https://codecov.io/gh/sqlfluff/sqlfluff/pull/4239?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) | Coverage Δ | |
|---|---|---|
| [src/sqlfluff/utils/reflow/reindent.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/4239/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL3V0aWxzL3JlZmxvdy9yZWluZGVudC5weQ==) | `99.17% <100.00%> (-0.83%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/4239?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).


Patch:
diff --git a/src/sqlfluff/utils/reflow/reindent.py b/src/sqlfluff/utils/reflow/reindent.py
index 466277a0d..94c2447cd 100644
--- a/src/sqlfluff/utils/reflow/reindent.py
+++ b/src/sqlfluff/utils/reflow/reindent.py
@@ -463,15 +463,23 @@ def _crawl_indent_points(
     happens. The values returned here have a large impact on
     exactly how indentation is treated.
 
+    NOTE: If a line ends with a comment, indent impulses are pushed
+    to the point _after_ the comment rather than before to aid with
+    indentation. This saves searching for them later.
+
     TODO: Once this function *works*, there's definitely headroom
     for simplification and optimisation. We should do that.
     """
     last_line_break_idx = None
     indent_balance = 0
     untaken_indents: Tuple[int, ...] = ()
+    cached_indent_stats: Optional[IndentStats] = None
     for idx, elem in enumerate(elements):
         if isinstance(elem, ReflowPoint):
-            indent_stats = elem.get_indent_impulse(allow_implicit_indents)
+            indent_stats = cached_indent_stats or elem.get_indent_impulse(
+                allow_implicit_indents
+            )
+            cached_indent_stats = None
 
             # Is it a line break? AND not a templated one.
             if has_untemplated_newline(elem) and idx != last_line_break_idx:
@@ -496,6 +504,27 @@ def _crawl_indent_points(
                 or idx == 0
                 or elements[idx + 1].segments[0].is_type("end_of_file")
             ):
+                # If the next block contains comments, then don't yield the
+                # impulses here. Yield them afterwards. Instead generate a
+                # point here with no balance change instead. It's a point
+                # we might later add a line break - but not very interesting
+                # otherwise.
+                if "comment" in elements[idx + 1].class_types:
+                    cached_indent_stats = indent_stats
+                    yield _IndentPoint(
+                        idx,
+                        0,
+                        0,
+                        indent_balance,
+                        last_line_break_idx,
+                        False,
+                        untaken_indents,
+                    )
+                    # Stop here and continue onward. Because we're treating
+                    # this point as though it has no impulse, we don't want
+                    # to update any balance values.
+                    continue
+
                 yield _IndentPoint(
                     idx,
                     indent_stats.impulse,
@@ -723,8 +752,18 @@ def _lint_line_untaken_positive_indents(
     """Check for positive indents which should have been taken."""
     # If we don't close the line higher there won't be any.
     starting_balance = indent_line.opening_balance()
-    if indent_line.closing_balance() <= starting_balance:
-        return [], []
+    # Work back through points until we're past any comments.
+    for ip in reversed(indent_line.indent_points):
+        # Check whether it closes the opening indent.
+        if ip.initial_indent_balance + ip.indent_trough <= starting_balance:
+            return [], []
+        # Is it preceded by comments?
+        if "comment" in elements[ip.idx - 1].class_types:
+            # It is, keep searching
+            continue
+        else:
+            # It's not, we don't close out an opened indent.
+            break
 
     indent_points = indent_line.indent_points
 
@@ -737,12 +776,6 @@ def _lint_line_untaken_positive_indents(
         closing_trough = (
             indent_points[-1].initial_indent_balance + indent_points[-1].indent_impulse
         )
-    # Edge case: if closing_balance > starting balance
-    # but closing_trough isn't, then we shouldn't insert
-    # a new line. That means we just dropped back down to
-    # close the untaken newline.
-    if closing_trough <= starting_balance:
-        return [], []
 
     # On the way up we're looking for whether the ending balance
     # was an untaken indent or not. If it *was* untaken, there's
@@ -817,16 +850,11 @@ def _lint_line_untaken_negative_indents(
             # Yep, untaken.
             continue
 
-        # Edge Case: Comments. For now we don't introduce line breaks
-        # before comments, largely because a trailing comment line is
-        # probably referring to that line (and may contain noqa elements).
-        if elements[ip.idx + 1 :] and "comment" in elements[ip.idx + 1].class_types:
-            reflow_logger.debug(
-                "    Detected missing -ve line break @ line %s, before "
-                "comment. Ignoring...",
-                elements[ip.idx + 1].segments[0].pos_marker.working_line_no,
-            )
-            continue
+        # Edge Case: Comments. Since introducing the code to push indent effects
+        # to the point _after_ comments, we no longer need to detect an edge case
+        # for them here. If we change that logic again in the future, so that
+        # indent values are allowed before comments - that code should be
+        # reintroduced here.
 
         # Edge Case: Semicolons. For now, semicolon placement is a little
         # more complicated than what we do here. For now we don't (by


Test patch:
diff --git a/test/fixtures/rules/std_rule_cases/L003.yml b/test/fixtures/rules/std_rule_cases/L003.yml
index 8c62b328d..651d86561 100644
--- a/test/fixtures/rules/std_rule_cases/L003.yml
+++ b/test/fixtures/rules/std_rule_cases/L003.yml
@@ -1531,3 +1531,42 @@ test_fail_hard_templated_indents:
     {%- if true -%}
         SELECT * FROM {{ "t1" }}
     {%- endif %}
+
+test_fail_fix_consistency_around_comments:
+  # Check that comments don't make fixes inconsistent.
+  # https://github.com/sqlfluff/sqlfluff/issues/4223
+  fail_str: |
+    select
+        case
+            when a
+            then b
+        end as foo,
+        case
+            when a -- bar
+            then b
+        end as bar
+    from c
+  fix_str: |
+    select
+        case
+            when a
+                then b
+        end as foo,
+        case
+            when a -- bar
+                then b
+        end as bar
+    from c
+
+test_fail_coverage_indent_trough:
+  # This test primarily tests the handling of closing trough indents
+  fail_str: |
+    WITH bar as (SELECT 1
+        FROM foo)
+    SELECT a FROM bar
+  fix_str: |
+    WITH bar as (
+        SELECT 1
+        FROM foo
+    )
+    SELECT a FROM bar
diff --git a/test/rules/std_test.py b/test/rules/std_test.py
index 6d460f004..0b9516a05 100644
--- a/test/rules/std_test.py
+++ b/test/rules/std_test.py
@@ -76,9 +76,8 @@ from sqlfluff.utils.testing.rules import assert_rule_raises_violations_in_file
         (
             "L016",
             "block_comment_errors.sql",
-            # Two errors on line one, because we call out the missing
-            # indent *and* the long line as two issues.
-            [(1, 7), (1, 8), (2, 5), (4, 5)],
+            # Errors should flag on the first element of the line.
+            [(1, 1), (2, 5), (4, 5)],
         ),
         ("L016", "block_comment_errors_2.sql", [(1, 1), (2, 1)]),
         # Column references