Repository: sqlfluff/sqlfluff
Pull request: L043 refinement: Add autofix for common use of CASE to fill NULL values.
<!--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
<!--If there is an open issue for this, then please include `fixes #XXXX` or `closes #XXXX` replacing `XXXX` with the issue number and it will automatically close the issue when the pull request is merged. Alternatively if not fully closed you can say `makes progress on #XXXX` to create a link on that issue without closing it.-->
This PR enhances L043 to fix #1797. I have also tidied up the docstring and added the corresponding unit tests.

### Are there any other side effects of this change that we should be aware of?
L043 should now flag and fix more cases of unnecessary use of CASE. 

### 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 `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:
New rule: COALESCE instead of CASE/IF when possible
In all dialects that have `COALESCE` code such as `CASE WHEN num_cats IS NULL THEN 0 ELSE num_cats END` are bad practices because they are very long and need to be simplified as `COALESCE(num_cats, 0)` which is much shorter and a lot more readable.

Hints from issue discussion:
Is this different to [Rule_L043](https://docs.sqlfluff.com/en/stable/rules.html#sqlfluff.core.rules.Rule_L043)?
@tunetheweb Yes. Here CASE does not necessarily actually return a boolean. However we may expand the scope of L043 to include it.
Looking into this, agree it should be a refinement of L043

Commit message: Merge branch 'main' into L043_refinement

Patch to review:
diff --git a/src/sqlfluff/rules/L043.py b/src/sqlfluff/rules/L043.py
index b666aaf360e..b2a27c92b8c 100644
--- a/src/sqlfluff/rules/L043.py
+++ b/src/sqlfluff/rules/L043.py
@@ -1,11 +1,12 @@
 """Implementation of Rule L043."""
-from typing import Optional
+from typing import List, Optional
 
 from sqlfluff.core.parser import (
     WhitespaceSegment,
     SymbolSegment,
     KeywordSegment,
 )
+from sqlfluff.core.parser.segments.base import BaseSegment
 
 from sqlfluff.core.rules.base import BaseRule, LintFix, LintResult, RuleContext
 from sqlfluff.core.rules.doc_decorators import document_fix_compatible
@@ -13,142 +14,248 @@
 
 @document_fix_compatible
 class Rule_L043(BaseRule):
-    """Unnecessary case when statement. Use the "when" condition itself.
-
-    If a case when else statement returns booleans, we can reduce it to the
-    "when" condition.
+    """Unnecessary CASE statement. Use COALESCE function instead.
 
     | **Anti-pattern**
+    | CASE statement returns booleans.
 
     .. code-block:: sql
+        :force:
+
+        select
+            case
+                when fab > 0 then true
+                else false
+            end as is_fab
+        from fancy_table
+
+        -- We can also simplify CASE statements that aim to fill NULL values.
 
         select
             case
-                when fab > 0 then true else false end as is_fab
+                when fab is null then 0
+                else fab
+            end as fab_clean
         from fancy_table
 
     | **Best practice**
-    |   Reduce to "when" condition. Wrap with "coalesce". If necessary, add
-    |   a "not" operator at the beginning.
+    | Reduce to WHEN condition within COALESCE function.
 
     .. code-block:: sql
+        :force:
 
         select
             coalesce(fab > 0, false) as is_fab
         from fancy_table
 
+        -- To fill NULL values.
+
+        select
+            coalesce(fab, 0) as fab_clean
+        from fancy_table
+
     """
 
-    def _eval(self, context: RuleContext) -> Optional[LintResult]:
-        """Find rule violations and provide fixes.
-
-        0. Look for a case expression
-        1. Find the first expression and "then"
-        2. Determine if the "then" is followed by a boolean
-        3. If so, determine if the first then-bool is followed by an else-bool
-        4. If so, delete everything but the first expression
-        5a. If then-true-else-false
-            * return deletions
-            * wrap with coalesce
-        5b. If then-false-else-true
-            * return deletions
-            * add a not condition
-            * wrap with parenthesis and coalesce
+    @staticmethod
+    def _coalesce_fix_list(
+        context: RuleContext,
+        coalesce_arg_1: BaseSegment,
+        coalesce_arg_2: BaseSegment,
+        coalesce_arg_1_idx: int,
+        preceding_not: bool = False,
+    ) -> List[LintFix]:
+        """Generate list of fixes to convert CASE statement to COALESCE function.
+
+        Returns:
+            :obj:`List[LintFix]`.
         """
-        # Look for a case expression
+        # Generate list of segments to delete.
+        # Everything but the column reference segment.
+        delete_segments = []
+        for s in context.segment.segments:
+            if s != coalesce_arg_1:
+                delete_segments.append(s)
+        # Add coalesce and opening parenthesis.
+        edits = []
+        if preceding_not:
+            edits.extend(
+                [
+                    KeywordSegment("not"),
+                    WhitespaceSegment(),
+                ]
+            )
+        edits.extend(
+            [
+                KeywordSegment("coalesce"),
+                SymbolSegment("(", name="start_bracket", type="start_bracket"),
+            ]
+        )
+        edit_coalesce_target = context.segment.segments[0]
+        fixes = []
+        fixes.append(
+            LintFix(
+                "edit",
+                edit_coalesce_target,
+                edits,
+            )
+        )
+        # Add comma, bool, closing parenthesis.
+        closing_parenthesis = [
+            SymbolSegment(",", name="comma", type="comma"),
+            WhitespaceSegment(),
+            coalesce_arg_2,
+            SymbolSegment(")", name="end_bracket", type="end_bracket"),
+        ]
+        fixes.append(
+            LintFix(
+                "edit",
+                context.segment.segments[coalesce_arg_1_idx + 1],
+                closing_parenthesis,
+            )
+        )
+        # Generate a "delete" action for each segment in
+        # delete_segments EXCEPT the one being edited to become a call
+        # to "coalesce(". Deleting and editing the same segment has
+        # unpredictable behavior.
+        fixes += [
+            LintFix("delete", s)
+            for s in delete_segments
+            if s is not edit_coalesce_target
+        ]
+        return fixes
+
+    def _eval(self, context: RuleContext) -> Optional[LintResult]:
+        """Unnecessary CASE statement. Use COALESCE function instead."""
+        # Look for CASE expression.
         if (
             context.segment.is_type("case_expression")
             and context.segment.segments[0].name == "case"
         ):
-            # Find the first expression and "then"
+            # We can't reduce CASE expressions with multiple WHEN clauses.
+            if (
+                len(
+                    [
+                        segment
+                        for segment in context.segment.segments
+                        if segment.name == "when"
+                    ]
+                )
+                > 1
+            ):
+                return None
+
+            # Find condition expression.
             idx = 0
             while context.segment.segments[idx].name != "then":
                 if context.segment.segments[idx].is_type("expression"):
-                    expression_idx = idx
+                    condition_expression_idx = idx
                 idx += 1
-            # Determine if "then" is followed by a boolean
-            then_bool_type = None
+
+            # Find THEN result expression.
             while context.segment.segments[idx].name not in ["when", "else", "end"]:
-                if context.segment.segments[idx].raw_upper in ["TRUE", "FALSE"]:
-                    then_bool_type = context.segment.segments[idx].raw_upper
+                if context.segment.segments[idx].is_type("expression"):
+                    then_expression_idx = idx
+                idx += 1
+
+            # Find ELSE result expression.
+            else_found = False
+            while context.segment.segments[idx].name not in ["when", "end"]:
+                if context.segment.segments[idx].name == "else":
+                    else_found = True
+                if context.segment.segments[idx].is_type("expression"):
+                    else_expression_idx = idx
                 idx += 1
-            if then_bool_type:
-                # Determine if the first then-bool is followed by an else-bool
-                while context.segment.segments[idx].name != "else":
-                    # If the first then-bool is followed by a "WHEN" or "END", exit
-                    if context.segment.segments[idx].name in ["when", "end"]:
-                        return None
-                    idx += 1  # pragma: no cover
-                # Determine if "else" is followed by a boolean
-                else_bool_type = None
-                while context.segment.segments[idx].name != "end":
-                    if context.segment.segments[idx].raw_upper in ["TRUE", "FALSE"]:
-                        else_bool_type = context.segment.segments[idx].raw_upper
-                    idx += 1
-            # If then-bool-else-bool, return fixes
+
+            # If no ELSE segment is found then we cannot reduce the expression.
+            if not else_found:
+                return None
+
+            condition_expression = context.segment.segments[condition_expression_idx]
+            then_expression = context.segment.segments[then_expression_idx]
+            else_expression = context.segment.segments[else_expression_idx]
+
+            # Method 1: Check if THEN/ELSE expressions are both Boolean and can therefore be reduced.
+            upper_bools = ["TRUE", "FALSE"]
             if (
-                then_bool_type is not None
-                and else_bool_type is not None
-                and then_bool_type != else_bool_type
+                (then_expression.raw_upper in upper_bools)
+                and (else_expression.raw_upper in upper_bools)
+                and (then_expression.raw_upper != else_expression.raw_upper)
             ):
-                # Generate list of segments to delete -- everything but the
-                # first expression.
-                delete_segments = []
-                for s in context.segment.segments:
-                    if s != context.segment.segments[expression_idx]:
-                        delete_segments.append(s)
-                # If then-false, add "not" and space
-                edits = []
-                if then_bool_type == "FALSE":
-                    edits.extend(
-                        [
-                            KeywordSegment("not"),
-                            WhitespaceSegment(),
-                        ]
-                    )
-                # Add coalesce and parenthesis
-                edits.extend(
+                coalesce_arg_1 = condition_expression
+                coalesce_arg_2 = KeywordSegment("false")
+                coalesce_arg_1_idx = condition_expression_idx
+                preceding_not = True if then_expression.raw_upper == "FALSE" else False
+
+                fixes = self._coalesce_fix_list(
+                    context,
+                    coalesce_arg_1,
+                    coalesce_arg_2,
+                    coalesce_arg_1_idx,
+                    preceding_not,
+                )
+
+                return LintResult(
+                    anchor=context.segment.segments[condition_expression_idx],
+                    fixes=fixes,
+                )
+
+            # Method 2: Check if the condition expression is comparing a column reference to NULL
+            # and whether that column reference is also in either the THEN/ELSE expression.
+            # We can only apply this method when there is only one condition in the condition expression.
+            condition_expression_segments_raw = {
+                segment.raw_upper for segment in condition_expression.segments
+            }
+            if {"IS", "NULL"}.issubset(condition_expression_segments_raw) and (
+                len(
                     [
-                        KeywordSegment("coalesce"),
-                        SymbolSegment("(", name="start_bracket", type="start_bracket"),
+                        segment
+                        for segment in condition_expression.segments
+                        if segment.type == "column_reference"
                     ]
                 )
-                edit_coalesce_target = context.segment.segments[0]
-                fixes = []
-                fixes.append(
-                    LintFix(
-                        "edit",
-                        edit_coalesce_target,
-                        edits,
-                    )
+                == 1
+            ):
+                # Check if the comparison is to NULL or NOT NULL.
+                if "NOT" in condition_expression_segments_raw:
+                    is_not_prefix = True
+                else:
+                    is_not_prefix = False
+
+                # Locate column reference in condition expression.
+                column_reference_segment = next(
+                    segment
+                    for segment in condition_expression.segments
+                    if segment.type == "column_reference"
                 )
-                # Add comma, bool, closing parenthesis
-                expression = context.segment.segments[expression_idx + 1]
-                closing_parenthesis = [
-                    SymbolSegment(",", name="comma", type="comma"),
-                    WhitespaceSegment(),
-                    KeywordSegment("false"),
-                    SymbolSegment(")", name="end_bracket", type="end_bracket"),
-                ]
-                fixes.append(
-                    LintFix(
-                        "edit",
-                        expression,
-                        closing_parenthesis,
-                    )
+
+                # Check if we can reduce the CASE expression to a single coalesce function.
+                if (
+                    not is_not_prefix
+                    and column_reference_segment.raw_upper == else_expression.raw_upper
+                ):
+                    coalesce_arg_1 = else_expression
+                    coalesce_arg_2 = then_expression
+                    coalesce_arg_1_idx = else_expression_idx
+                elif (
+                    is_not_prefix
+                    and column_reference_segment.raw_upper == then_expression.raw_upper
+                ):
+                    coalesce_arg_1 = then_expression
+                    coalesce_arg_2 = else_expression
+                    coalesce_arg_1_idx = then_expression_idx
+                else:
+                    return None
+
+                fixes = self._coalesce_fix_list(
+                    context,
+                    coalesce_arg_1,
+                    coalesce_arg_2,
+                    coalesce_arg_1_idx,
                 )
-                # Generate a "delete" action for each segment in
-                # delete_segments EXCEPT the one being edited to become a call
-                # to "coalesce(". Deleting and editing the same segment has
-                # unpredictable behavior.
-                fixes += [
-                    LintFix("delete", s)
-                    for s in delete_segments
-                    if s is not edit_coalesce_target
-                ]
+
                 return LintResult(
-                    anchor=context.segment.segments[expression_idx],
+                    anchor=context.segment.segments[condition_expression_idx],
                     fixes=fixes,
-                    description="Case when returns booleans.",
                 )
+
         return None
diff --git a/test/fixtures/rules/std_rule_cases/L043.yml b/test/fixtures/rules/std_rule_cases/L043.yml
index de88ea19e21..5e37704ef5d 100644
--- a/test/fixtures/rules/std_rule_cases/L043.yml
+++ b/test/fixtures/rules/std_rule_cases/L043.yml
@@ -33,6 +33,26 @@ test_pass_case_cannot_be_reduced_6:
           when fab < 0 then 'not a bool'
         end as fab_category
     from fancy_table
+test_pass_case_cannot_be_reduced_7:
+  pass_str: |
+    select
+        foo,
+        case
+            when
+                bar is null then bar
+            else '123'
+        end as test
+    from baz;
+test_pass_case_cannot_be_reduced_8:
+  pass_str: |
+    select
+        foo,
+        case
+            when
+                bar is not null then '123'
+            else bar
+        end as test
+    from baz;
 test_fail_unnecessary_case_1:
   fail_str: |
     select
@@ -105,3 +125,33 @@ test_fail_unnecessary_case_6:
         perks.received_at as perk_received_at
 
     from subscriptions_xf
+test_fail_unnecessary_case_7:
+  fail_str: |
+    select
+        foo,
+        case
+            when
+                bar is null then '123'
+            else bar
+        end as test
+    from baz;
+  fix_str: |
+    select
+        foo,
+        coalesce(bar, '123') as test
+    from baz;
+test_fail_unnecessary_case_8:
+  fail_str: |
+    select
+        foo,
+        case
+            when
+                bar is not null then bar
+            else '123'
+        end as test
+    from baz;
+  fix_str: |
+    select
+        foo,
+        coalesce(bar, '123') as test
+    from baz;


Code review comments:
diff_hunk: @@ -1,154 +1,261 @@
 """Implementation of Rule L043."""
-from typing import Optional
+from typing import List, Optional
 
 from sqlfluff.core.parser import (
     WhitespaceSegment,
     SymbolSegment,
     KeywordSegment,
 )
+from sqlfluff.core.parser.segments.base import BaseSegment
 
 from sqlfluff.core.rules.base import BaseRule, LintFix, LintResult, RuleContext
 from sqlfluff.core.rules.doc_decorators import document_fix_compatible
 
 
 @document_fix_compatible
 class Rule_L043(BaseRule):
-    """Unnecessary case when statement. Use the "when" condition itself.
-
-    If a case when else statement returns booleans, we can reduce it to the
-    "when" condition.
+    """Unnecessary CASE statement. Use COALESCE function instead.
 
     | **Anti-pattern**
+    | CASE statement returns booleans.
 
     .. code-block:: sql
+        :force:
+
+        select
+            case
+                when fab > 0 then true
+                else false
+            end as is_fab
+        from fancy_table
+
+        -- We can also simplify CASE statements that aim to fill NULL values.
 
         select
             case
-                when fab > 0 then true else false end as is_fab
+                when fab is null then 0
+                else fab
+            end as fab_clean
         from fancy_table
 
     | **Best practice**
-    |   Reduce to "when" condition. Wrap with "coalesce". If necessary, add
-    |   a "not" operator at the beginning.
+    | Reduce to WHEN condition within COALESCE function.
 
     .. code-block:: sql
+        :force:
 
         select
             coalesce(fab > 0, false) as is_fab
         from fancy_table
 
+        -- To fill NULL values.
+
+        select
+            coalesce(fab, 0) as fab_clean
+        from fancy_table
+
     """
 
-    def _eval(self, context: RuleContext) -> Optional[LintResult]:
-        """Find rule violations and provide fixes.
-
-        0. Look for a case expression
-        1. Find the first expression and "then"
-        2. Determine if the "then" is followed by a boolean
-        3. If so, determine if the first then-bool is followed by an else-bool
-        4. If so, delete everything but the first expression
-        5a. If then-true-else-false
-            * return deletions
-            * wrap with coalesce
-        5b. If then-false-else-true
-            * return deletions
-            * add a not condition
-            * wrap with parenthesis and coalesce
+    @staticmethod
+    def _coalesce_fix_list(
+        context: RuleContext,
+        coalesce_arg_1: BaseSegment,
+        coalesce_arg_2: BaseSegment,
+        coalesce_arg_1_idx: int,
+        preceding_not: bool = False,
+    ) -> List[LintFix]:
+        """Generate list of fixes to convert CASE statement to COALESCE function.
+
+        Returns:
+            :obj:`List[LintFix]`.
         """
-        # Look for a case expression
+        # Generate list of segments to delete.
+        # Everything but the column reference segment.
+        delete_segments = []
+        for s in context.segment.segments:
+            if s != coalesce_arg_1:
+                delete_segments.append(s)
+        # Add coalesce and opening parenthesis.
+        edits = []
+        if preceding_not:
+            edits.extend(
+                [
+                    KeywordSegment("not"),
+                    WhitespaceSegment(),
+                ]
+            )
+        edits.extend(
+            [
+                KeywordSegment("coalesce"),
+                SymbolSegment("(", name="start_bracket", type="start_bracket"),
+            ]
+        )
+        edit_coalesce_target = context.segment.segments[0]
+        fixes = []
+        fixes.append(
+            LintFix(
+                "edit",
+                edit_coalesce_target,
+                edits,
+            )
+        )
+        # Add comma, bool, closing parenthesis.
+        closing_parenthesis = [
+            SymbolSegment(",", name="comma", type="comma"),
+            WhitespaceSegment(),
+            coalesce_arg_2,
+            SymbolSegment(")", name="end_bracket", type="end_bracket"),
+        ]
+        fixes.append(
+            LintFix(
+                "edit",
+                context.segment.segments[coalesce_arg_1_idx + 1],
+                closing_parenthesis,
+            )
+        )
+        # Generate a "delete" action for each segment in
+        # delete_segments EXCEPT the one being edited to become a call
+        # to "coalesce(". Deleting and editing the same segment has
+        # unpredictable behavior.
+        fixes += [
+            LintFix("delete", s)
+            for s in delete_segments
+            if s is not edit_coalesce_target
+        ]
+        return fixes
+
+    def _eval(self, context: RuleContext) -> Optional[LintResult]:
+        """Unnecessary CASE statement. Use COALESCE function instead."""
+        # Look for CASE expression.
         if (
             context.segment.is_type("case_expression")
             and context.segment.segments[0].name == "case"
         ):
-            # Find the first expression and "then"
+            # We can't reduce CASE expressions with multiple WHEN clauses.
+            if (
+                len(
+                    [
+                        segment
+                        for segment in context.segment.segments
+                        if segment.name == "when"
+                    ]
+                )
+                > 1
+            ):
+                return None
+
+            # Find condition expression.
             idx = 0
             while context.segment.segments[idx].name != "then":
                 if context.segment.segments[idx].is_type("expression"):
-                    expression_idx = idx
+                    condition_expression_idx = idx
                 idx += 1
-            # Determine if "then" is followed by a boolean
-            then_bool_type = None
+
+            # Find THEN result expression.
             while context.segment.segments[idx].name not in ["when", "else", "end"]:
-                if context.segment.segments[idx].raw_upper in ["TRUE", "FALSE"]:
-                    then_bool_type = context.segment.segments[idx].raw_upper
+                if context.segment.segments[idx].is_type("expression"):
+                    then_expression_idx = idx
+                idx += 1
+
+            # Find ELSE result expression.
+            else_found = False
+            while context.segment.segments[idx].name not in ["when", "end"]:
+                if context.segment.segments[idx].name == "else":
+                    else_found = True
+                if context.segment.segments[idx].is_type("expression"):
+                    else_expression_idx = idx
                 idx += 1
-            if then_bool_type:
-                # Determine if the first then-bool is followed by an else-bool
-                while context.segment.segments[idx].name != "else":
-                    # If the first then-bool is followed by a "WHEN" or "END", exit
-                    if context.segment.segments[idx].name in ["when", "end"]:
-                        return None
-                    idx += 1  # pragma: no cover
-                # Determine if "else" is followed by a boolean
-                else_bool_type = None
-                while context.segment.segments[idx].name != "end":
-                    if context.segment.segments[idx].raw_upper in ["TRUE", "FALSE"]:
-                        else_bool_type = context.segment.segments[idx].raw_upper
-                    idx += 1
-            # If then-bool-else-bool, return fixes
+
+            # If no ELSE segment is found then we cannot reduce the expression.
+            if not else_found:
+                return None
+
+            condition_expression = context.segment.segments[condition_expression_idx]
+            then_expression = context.segment.segments[then_expression_idx]
+            else_expression = context.segment.segments[else_expression_idx]
+
+            # Method 1: Check if THEN/ELSE expressions are both Boolean and can therefore be reduced.
+            upper_bools = ["TRUE", "FALSE"]
             if (
-                then_bool_type is not None
-                and else_bool_type is not None
-                and then_bool_type != else_bool_type
+                (then_expression.raw_upper in upper_bools)
+                and (else_expression.raw_upper in upper_bools)
+                and (then_expression.raw_upper != else_expression.raw_upper)
             ):
-                # Generate list of segments to delete -- everything but the
-                # first expression.
-                delete_segments = []
-                for s in context.segment.segments:
-                    if s != context.segment.segments[expression_idx]:
-                        delete_segments.append(s)
-                # If then-false, add "not" and space
-                edits = []
-                if then_bool_type == "FALSE":
-                    edits.extend(
-                        [
-                            KeywordSegment("not"),
-                            WhitespaceSegment(),
-                        ]
-                    )
-                # Add coalesce and parenthesis
-                edits.extend(
+                coalesce_arg_1 = condition_expression
+                coalesce_arg_2 = KeywordSegment("false")
+                coalesce_arg_1_idx = condition_expression_idx
+                preceding_not = True if then_expression.raw_upper == "FALSE" else False
path: src/sqlfluff/rules/L043.py
text: @user1:
```suggestion
                preceding_not = (then_expression.raw_upper == "FALSE")
```

@author:
@user1 🤦 wow I definitely had a brain lapse there, thanks!