Repository: sqlfluff/sqlfluff
Pull request: Add rule L035 with fix, 'redundant ELSE NULL in CASE WHEN'

Problem statement:
Redundant ELSE NULL in CASE WHEN
Came across

```sql
select
    id,
    case
        when category = x and category_b = a then x_name
        when category_c = y then y_name
        when category_d = z then z_name
        else null
    end as name
from table_a
```

"else NULL" is redundant

Hints from issue discussion:

Commit message: Remove dbt_utils added by mistake

Patch to review:
diff --git a/src/sqlfluff/core/errors.py b/src/sqlfluff/core/errors.py
index 75e17428120..7625d047f95 100644
--- a/src/sqlfluff/core/errors.py
+++ b/src/sqlfluff/core/errors.py
@@ -219,6 +219,8 @@ def check_tuple(self):
         )
 
     def __repr__(self):
-        return "<SQLLintError: rule {0} pos:{1!r}, #fixes: {2}>".format(
-            self.rule_code(), self.pos_marker(), len(self.fixes)
+        return (
+            "<SQLLintError: rule {0} pos:{1!r}, #fixes: {2}, description: {3}>".format(
+                self.rule_code(), self.pos_marker(), len(self.fixes), self.description
+            )
         )
diff --git a/src/sqlfluff/core/rules/std.py b/src/sqlfluff/core/rules/std.py
index 98fbf578693..7fa3622d68e 100644
--- a/src/sqlfluff/core/rules/std.py
+++ b/src/sqlfluff/core/rules/std.py
@@ -3244,3 +3244,71 @@ def _eval(self, segment, **kwargs):
                 self.violation_buff[-1].fixes = fixes
 
         return self.violation_buff or None
+
+
+@std_rule_set.document_fix_compatible
+@std_rule_set.register
+class Rule_L035(BaseCrawler):
+    """Do not specify "else null" in a case when statement (redundant).
+
+    | **Anti-pattern**
+
+    .. code-block:: sql
+
+        select
+            case
+                when name like '%cat%' then 'meow'
+                when name like '%dog%' then 'woof'
+                else null
+            end
+        from x
+
+
+    | **Best practice**
+    |  Omit "else null"
+
+    .. code-block:: sql
+
+        select
+            case
+                when name like '%cat%' then 'meow'
+                when name like '%dog%' then 'woof'
+            end
+        from x
+
+    """
+
+    def _eval(self, segment, **kwargs):
+        """Find rule violations and provide fixes.
+
+        0. Look for a case expression
+        1. Look for "ELSE"
+        2. Mark "ELSE" for deletion (populate "fixes")
+        3. Backtrack and mark all newlines/whitespaces for deletion
+        4. Look for a raw "NULL" segment
+        5.a. The raw "NULL" segment is found, we mark it for deletion and return
+        5.b. We reach the end of case when without matching "NULL": the rule passes
+        """
+        if segment.is_type("case_expression"):
+            fixes = []
+            for idx, seg in enumerate(segment.segments):
+                # When we find ELSE we delete
+                # everything up to NULL
+                if fixes:
+                    fixes.append(LintFix("delete", seg))
+                    # Safe to look for NULL, as an expression
+                    # would contain NULL but not be == NULL
+                    if seg.raw_upper == "NULL":
+                        return LintResult(anchor=segment, fixes=fixes)
+
+                if not fixes and seg.name == "ELSE":
+                    fixes.append(LintFix("delete", seg))
+                    # Walk back to remove indents/whitespaces
+                    walk_idx = idx - 1
+                    while (
+                        segment.segments[walk_idx].name == "whitespace"
+                        or segment.segments[walk_idx].name == "newline"
+                        or segment.segments[walk_idx].is_meta
+                    ):
+                        fixes.append(LintFix("delete", segment.segments[walk_idx]))
+                        walk_idx = walk_idx - 1
diff --git a/test/core/rules/std_test.py b/test/core/rules/std_test.py
index b82921d7dd4..6a729108d03 100644
--- a/test/core/rules/std_test.py
+++ b/test/core/rules/std_test.py
@@ -4,6 +4,7 @@
 import pytest
 
 from sqlfluff.core import Linter, FluffConfig
+from sqlfluff.core.errors import SQLParseError
 from sqlfluff.core.rules.base import BaseCrawler, LintResult, LintFix
 from sqlfluff.core.rules.std import std_rule_set
 
@@ -17,6 +18,7 @@ class RuleTestCase(NamedTuple):
     fail_str: str = None
     fix_str: str = None
     configs: dict = None
+    skip: str = None
 
 
 def get_rule_from_set(code, config):
@@ -35,6 +37,9 @@ def assert_rule_fail_in_sql(code, sql, configs=None):
     linted = Linter(config=cfg).lint_string(sql, fix=True)
     lerrs = linted.get_violations()
     print("Errors Found: {0}".format(lerrs))
+    parse_errors = list(filter(lambda v: type(v) == SQLParseError, lerrs))
+    if parse_errors:
+        pytest.fail(f"Found the following parse errors in test case: {parse_errors}")
     if not any(v.rule.code == code for v in lerrs):
         pytest.fail(
             "No {0} failures found in query which should fail.".format(code),
@@ -64,6 +69,9 @@ def rules__test_helper(test_case):
 
     Optionally, also test the fixed string if provided in the test case.
     """
+    if test_case.skip:
+        pytest.skip(test_case.skip)
+
     if test_case.pass_str:
         assert_rule_pass_in_sql(
             test_case.rule,
diff --git a/test/core/rules/test_cases/L035.yml b/test/core/rules/test_cases/L035.yml
new file mode 100644
index 00000000000..4364a489150
--- /dev/null
+++ b/test/core/rules/test_cases/L035.yml
@@ -0,0 +1,58 @@
+rule: L035
+
+no_redundant_else_null:
+  pass_str: |
+    select
+        case
+            when name like '%cat%' then 'meow'
+            when name like '%dog%' then 'woof'
+        end
+    from x
+
+redundant_else_null:
+  fail_str: |
+    select
+        case
+            when name like '%cat%' then 'meow'
+            when name like '%dog%' then 'woof'
+            else null
+        end
+    from x
+
+  fix_str: |
+    select
+        case
+            when name like '%cat%' then 'meow'
+            when name like '%dog%' then 'woof'
+        end
+    from x
+
+alternate_case_when_syntax:
+  skip: Alternate case when syntax is not parse-able right now
+
+  fail_str: |
+    select
+        case name
+            when like '%cat%' then 'meow'
+            when like '%dog%' then 'woof'
+            else null
+        end
+    from x
+
+  fix_str: |
+    select
+        case name
+            when like '%cat%' then 'meow'
+            when like '%dog%' then 'woof'
+        end
+    from x
+
+else_expression:
+  pass_str: |
+    select
+        case name
+            when like '%cat%' then 'meow'
+            when like '%dog%' then 'woof'
+            else iff(wing_type is not null, 'tweet', 'invalid')
+        end
+    from x
diff --git a/tox.ini b/tox.ini
index 076ea537113..7c3a453d2be 100644
--- a/tox.ini
+++ b/tox.ini
@@ -18,7 +18,7 @@ deps =
 # tox -e py35 -- project/tests/test_file.py::TestClassName::test_method
 commands =
     python util.py clean-tests
-    pytest -vv --cov {posargs}
+    pytest -vv -rs --cov="" {posargs}
 setenv =
     COVERAGE_FILE = .coverage.{envname}
 


Code review comments:
diff_hunk: @@ -18,7 +18,7 @@ deps =
 # tox -e py35 -- project/tests/test_file.py::TestClassName::test_method
 commands =
     python util.py clean-tests
-    pytest -vv --cov {posargs}
+    pytest -vv -rs --cov="" {posargs}
path: tox.ini
text: @author:
@user1 when I tried running only one file with the current tox I couldn't:

```bash
tox -e py38 -- test/core/rules/rule_test_cases_test.py::test__rule_test_case
```

passing `""` to `--cov` fixed it

the `-rs` is to display "Skipped" messaged

@user2:
I found this too and resorted to just calling pytest directly. Thanks!

@user1:
I've just done some testing, this this current change breaks test coverage.

The solution which achieves both aims is:
```
pytest -vv -rs --cov=sqlfluff {posargs}
```

...or after merging in master it should probably be.
```
pytest -vv -rs --cov=sqlfluff {posargs:-m "not dbt"}
```