Repository: sqlfluff/sqlfluff

Issue:
Trailing commas in select statement should raise linting error
#354 allows parsing of trailing commas in select statements. This is a bigquery feature only (and so technically should raise a *parsing* error), but on balance I think it's more _useful_ that the query parses and then this can be raised as an issue.

I think even in dialects where this is supported, this should be regarded as an anti-pattern. We should have a linting rule which catches this.

Discussion:

Patch:
diff --git a/src/sqlfluff/core/default_config.cfg b/src/sqlfluff/core/default_config.cfg
index befc4eb37..b6915f401 100644
--- a/src/sqlfluff/core/default_config.cfg
+++ b/src/sqlfluff/core/default_config.cfg
@@ -50,3 +50,6 @@ ignore_comment_lines = False
 
 [sqlfluff:rules:L030]
 capitalisation_policy = consistent
+
+[sqlfluff:rules:L038]
+select_clause_trailing_comma = forbid
diff --git a/src/sqlfluff/core/rules/base.py b/src/sqlfluff/core/rules/base.py
index 0c06cf0ff..bb0cd69e0 100644
--- a/src/sqlfluff/core/rules/base.py
+++ b/src/sqlfluff/core/rules/base.py
@@ -18,7 +18,7 @@ import copy
 import logging
 from collections import namedtuple
 
-from ..parser import RawSegment, KeywordSegment, BaseSegment
+from ..parser import RawSegment, KeywordSegment, BaseSegment, SymbolSegment
 from ..errors import SQLLintError
 
 # The ghost of a rule (mostly used for testing)
@@ -420,6 +420,16 @@ class BaseCrawler:
         # At the moment we let the rule dictate *case* here.
         return kws(raw=raw, pos_marker=pos_marker)
 
+    @classmethod
+    def make_symbol(cls, raw, pos_marker, seg_type, name=None):
+        """Make a symbol segment."""
+        # For the name of the segment, we force the string to lowercase.
+        symbol_seg = SymbolSegment.make(
+            raw.lower(), name=name or seg_type, type=seg_type
+        )
+        # At the moment we let the rule dictate *case* here.
+        return symbol_seg(raw=raw, pos_marker=pos_marker)
+
 
 class RuleSet:
     """Class to define a ruleset.
diff --git a/src/sqlfluff/core/rules/config_info.py b/src/sqlfluff/core/rules/config_info.py
index 7745fa7af..3a797518e 100644
--- a/src/sqlfluff/core/rules/config_info.py
+++ b/src/sqlfluff/core/rules/config_info.py
@@ -58,6 +58,12 @@ STANDARD_CONFIG_INFO_DICT = {
             " such as `{{blah}}` have their indentation linted."
         ),
     },
+    "select_clause_trailing_comma": {
+        "validation": ["forbid", "require"],
+        "definition": (
+            "Should trailing commas within select clauses be required or forbidden."
+        ),
+    },
     "ignore_comment_lines": {
         "validation": [True, False],
         "definition": (
diff --git a/src/sqlfluff/core/rules/std/L038.py b/src/sqlfluff/core/rules/std/L038.py
new file mode 100644
index 000000000..d704a1055
--- /dev/null
+++ b/src/sqlfluff/core/rules/std/L038.py
@@ -0,0 +1,66 @@
+"""Implementation of Rule L038."""
+
+from ..base import BaseCrawler, LintFix, LintResult
+from ..doc_decorators import document_fix_compatible
+
+
+@document_fix_compatible
+class Rule_L038(BaseCrawler):
+    """Trailing commas within select clause.
+
+    For some database backends this is allowed. For some users
+    this may be something they wish to enforce (in line with
+    python best practice). Many database backends regard this
+    as a syntax error, and as such the sqlfluff default is to
+    forbid trailing commas in the select clause.
+
+    | **Anti-pattern**
+
+    .. code-block::
+
+        SELECT
+            a, b,
+        FROM foo
+
+    | **Best practice**
+
+    .. code-block::
+
+        SELECT
+            a, b
+        FROM foo
+    """
+
+    config_keywords = ["select_clause_trailing_comma"]
+
+    def _eval(self, segment, parent_stack, **kwargs):
+        """Trailing commas within select clause."""
+        if segment.is_type("select_clause"):
+            # Iterate content to find last element
+            last_content = None
+            for seg in segment.segments:
+                if seg.is_code:
+                    last_content = seg
+
+            # What mode are we in?
+            if self.select_clause_trailing_comma == "forbid":
+                # Is it a comma?
+                if last_content.is_type("comma"):
+                    return LintResult(
+                        anchor=last_content,
+                        fixes=[LintFix("delete", last_content)],
+                        description="Trailing comma in select statement forbidden",
+                    )
+            elif self.select_clause_trailing_comma == "require":
+                if not last_content.is_type("comma"):
+                    new_comma = self.make_symbol(
+                        ",", last_content.get_end_pos_marker(), seg_type="comma"
+                    )
+                    return LintResult(
+                        anchor=last_content,
+                        fixes=[
+                            LintFix("edit", last_content, [last_content, new_comma])
+                        ],
+                        description="Trailing comma in select statement required",
+                    )
+        return None


Test patch:
diff --git a/test/core/rules/test_cases/L038.yml b/test/core/rules/test_cases/L038.yml
new file mode 100644
index 000000000..d8ab7cf73
--- /dev/null
+++ b/test/core/rules/test_cases/L038.yml
@@ -0,0 +1,32 @@
+rule: L038
+
+test_require_pass:
+  pass_str: SELECT a, b, FROM foo
+  configs:
+    rules:
+      L038:
+        select_clause_trailing_comma: require
+
+test_require_fail:
+  fail_str: SELECT a, b FROM foo
+  fix_str: SELECT a, b, FROM foo
+  configs:
+    rules:
+      L038:
+        select_clause_trailing_comma: require
+
+
+test_forbid_pass:
+  pass_str: SELECT a, b FROM foo
+  configs:
+    rules:
+      L038:
+        select_clause_trailing_comma: forbid
+
+test_forbid_fail:
+  fail_str: SELECT a, b, FROM foo
+  fix_str: SELECT a, b FROM foo
+  configs:
+    rules:
+      L038:
+        select_clause_trailing_comma: forbid