Repository: sqlfluff/sqlfluff
Pull request: Allow L059 to be configured to always prefer quoted identifiers
### Brief summary of the change made
fixes #2527 by providing a configuration option on the L059 rule.

### Are there any other side effects of this change that we should be aware of?
No side effects expected as the default values for the newly added configuration are respecting the previous L059 behavior.

### Pull Request checklist
- [x] Included test cases to demonstrate any code changes:  `.yml` rule test cases in `test/fixtures/rules/std_rule_cases`.
- [x] Added appropriate documentation for the change.
- [x] Created GitHub issues for any relevant followup/future enhancements if appropriate.


Even though I think I have fulfilled the requirements in terms of development, tests, documentation, please do not hesitate to help me ensuring this PR is OK, as I didn't wrote anything in python since a decade (literally), and I'm not 100% sure I respected the code of conduct (and if I failed, I'm sorry, I'll fix my mistakes).


Problem statement:
L059: Allow prefered quoted for identifiers
The current rule L059 is raising an error when some unnecessary quoted identifier has been detected.

Although it is not recommended to quote identifiers when it's not needed, this is a convention I am using for automatic business script that are parsing my SQLs. Therefore, I would like to use the backtick as a mandatory wrapper for all identifiers, no matter if they need to be quoted.

I could ignore the rule L059, but I would prefer to let the user allow, through the configuration, which quote to use and have a configuration to actually force it everywhere.

I propose 2 configurations for the L059 rule:
- force_quote_identifier: [True,False] (Defaults: False) # Force every identifiers to be quoted, even when unnecessary if True.
- preferred_quote_identifier: (any character) (Default: ") # Preferred quote identifier to use.

### With default configuration
#### Anti-pattern
```sql
SELECT `foo`, "bar", `sum`
FROM table_a;
```
#### Pattern
```sql
SELECT foo, bar, "sum"
FROM table_a;
```

### With user-defined configuration
#### sqlfluff.cfg
```cfg
[sqlfluff:rules:L059]
force_quote_identifier: True
preferred_quote_identifier: `
```
#### Anti-pattern
```sql
SELECT foo, "bar", "sum"
FROM table_a;
```
#### Pattern
```sql
SELECT `foo`, `bar`, `sum`
FROM table_a;
```


This doesn't break the current behavior of the L059 rule, but allows a better user-configuration for this rule.

What do you think about?

Thanks :+1:

Hints from issue discussion:
Sounds OK, though not sure of priority (unless you want to work on this?).

One thing to be careful off is for mixed quotes (who would use these?!?).

For example you have an identifier called "barry's comments" and a `preferred_quote_identifier` of single quotes. What should happen in this case? Leave as is? Or can you escape the single quote acting as an apostrophe (or would you even want to if you can)?
Last time I spoke python was almost 10 years ago, so I'll definitly needs a refresh to start working at this, although I could try to propose a PR if I can find some time (this is not top priority as, like I said, I can exclude L059 until this feature is available).

The more I think about mixed quotes (yeah, definitly I pray for no one using this...), the more I assume that if there's an occurrence of the quote identifier inside the identifier name, we should ignore this rule on this specific case, for now. 

I saw an issue talking about criticity of errors, and maybe having levels like "error", "warning" and "info" could help us in setting this kind of (awesomly dirty) mixed quotes use cases in "info" level, like `L059: Declared preferred quote identifier is "…" but this character is used in identifier named "…".` But, to me, this is going very far for a second thought, and probably considering this very specific use case should be ignored first is a good approach.

Commit message: Fix unit tests for L059

Fix tests as default dialect is ansi and backticks are not parsed. Added bigquery dialect tests to ensure this is working also with backticks when dialect is supporting them.

Patch to review:
diff --git a/src/sqlfluff/core/default_config.cfg b/src/sqlfluff/core/default_config.cfg
index 8a9c5c91960..9284d18bf8b 100644
--- a/src/sqlfluff/core/default_config.cfg
+++ b/src/sqlfluff/core/default_config.cfg
@@ -144,6 +144,10 @@ quoted_identifiers_policy = all
 allow_space_in_identifier = False
 additional_allowed_characters = ""
 
+[sqlfluff:rules:L059]
+# Policy on quoted and unquoted identifiers
+force_quote_identifier = False
+
 [sqlfluff:rules:L062]
 # Comma separated list of blocked words that should not be used
 blocked_words = None
diff --git a/src/sqlfluff/core/rules/config_info.py b/src/sqlfluff/core/rules/config_info.py
index dd70b57c734..374bb203b75 100644
--- a/src/sqlfluff/core/rules/config_info.py
+++ b/src/sqlfluff/core/rules/config_info.py
@@ -137,6 +137,13 @@
             "in addition to alphanumerics (A-Z, a-z, 0-9) and underscores."
         ),
     },
+    "force_quote_identifier": {
+        "validation": [True, False],
+        "definition": (
+            "If ``True``, forces every identifier to be quoted. "
+            "Defaults to ``False``."
+        ),
+    },
     "blocked_words": {
         "definition": (
             "Optional comma-separated list of blocked words which should not be used "
diff --git a/src/sqlfluff/rules/L059.py b/src/sqlfluff/rules/L059.py
index 1f3a75b4d8a..f3adf50baa3 100644
--- a/src/sqlfluff/rules/L059.py
+++ b/src/sqlfluff/rules/L059.py
@@ -6,13 +6,23 @@
 
 from sqlfluff.core.parser.segments.raw import CodeSegment
 from sqlfluff.core.rules.base import BaseRule, LintFix, LintResult, RuleContext
-from sqlfluff.core.rules.doc_decorators import document_fix_compatible
+from sqlfluff.core.rules.doc_decorators import (
+    document_configuration,
+    document_fix_compatible,
+)
 
 
+@document_configuration
 @document_fix_compatible
 class Rule_L059(BaseRule):
     """Unnecessary quoted identifier.
 
+    This rule will fail if the quotes used to quote an identifier are (un)necessary
+    depending on the ``force_quote_identifier`` configuration.
+
+    When ``force_quote_identifier = False`` (default behavior), the quotes are
+    unnecessary, except for reserved keywords and special charcters in identifiers.
+
     | **Anti-pattern**
     | In this example, a valid unquoted identifier,
     | that is also not a reserved keyword, is needlessly quoted.
@@ -28,14 +38,63 @@ class Rule_L059(BaseRule):
 
         SELECT 123 as foo
 
+    When ``force_quote_identifier = True``, the quotes are always necessary, no
+    matter if the identifier is valid, a reserved keyword, or contains special
+    characters.
+
+    | **Anti-pattern**
+    | In this example, a valid unquoted identifier,
+    | that is also not a reserved keyword, is required to be quoted.
+
+    .. code-block:: sql
+
+        SELECT 123 as foo
+
+    | **Best practice**
+    | Use quoted identifiers.
+
+    .. code-block:: sql
+
+        SELECT 123 as "foo" -- For ANSI, ...
+        -- or
+        SELECT 123 as `foo` -- For BigQuery, MySql, ...
+
     """
 
+    config_keywords = [
+        "force_quote_identifier"
+    ]
+
     def _eval(self, context: RuleContext) -> Optional[LintResult]:
         """Unnecessary quoted identifier."""
-        # We only care about quoted identifiers.
-        if context.segment.name != "quoted_identifier":
+        # Config type hints
+        self.force_quote_identifier: bool
+
+        if self.force_quote_identifier:
+            context_policy = ("naked_identifier")
+            identifier_contents = context.segment.raw
+        else:
+            context_policy = ("quoted_identifier")
+            identifier_contents = context.segment.raw[1:-1]
+
+        # Ignore the segments that are not of the same type as the defined policy above.
+        if context.segment.name not in context_policy:
             return None
 
+        # Manage cases of quoted identifiers must be forced first.
+        # Naked identifiers are _de facto_ making this rule fail as configuration forces
+        # them to be quoted.
+        # In this case, it cannot be fixed as quote to use is dialect and DBMS
+        # configuration dependent.
+        if self.force_quote_identifier:
+            return LintResult(
+                context.segment,
+                description=f"Missing quoted identifier {identifier_contents}.",
+            )
+
+        # Now we only deal with NOT forced quoted identifiers configuration
+        # (meaning force_quote_identifier=False).
+
         # Extract contents of outer quotes.
         quoted_identifier_contents = context.segment.raw[1:-1]
 
diff --git a/test/fixtures/rules/std_rule_cases/L059.yml b/test/fixtures/rules/std_rule_cases/L059.yml
index 4f55ec97f12..eb9094da06a 100644
--- a/test/fixtures/rules/std_rule_cases/L059.yml
+++ b/test/fixtures/rules/std_rule_cases/L059.yml
@@ -1,5 +1,38 @@
 rule: L059
 
+# Configuration test matrix
+# Pass/Fail Forced  Test
+# -- Identifiers must be unquoted (default configuration)
+# Pass      False   test_pass_column_reference
+# Fail      False   test_fail_column_reference
+# Pass      False   test_pass_table_reference
+# Fail      False   test_fail_table_reference
+# Pass      False   test_pass_multiple_references
+# Fail      False   test_fail_multiple_references
+# Pass      False   test_pass_whitespace
+# Pass      False   test_pass_special_symbols
+# Pass      False   test_pass_reserved_keyword
+# -- Identifiers must be quoted and expected quote is double-quotes (SQL-92 ANSI, ...)
+# Pass      True    test_pass_column_reference_forced_ansi
+# Fail      True    test_fail_column_reference_forced_ansi
+# Pass      True    test_pass_table_reference_forced_ansi
+# Fail      True    test_fail_table_reference_forced_ansi
+# Pass      True    test_pass_multiple_references_forced_ansi
+# Fail      True    test_fail_multiple_references_forced_ansi
+# Pass      True    test_pass_whitespace_forced_ansi
+# Pass      True    test_pass_special_symbols_forced_ansi
+# Pass      True    test_pass_reserved_keyword_forced_ansi
+# -- Identifiers must be quoted and expected quote is backtick (BigQuery, MySql, ...)
+# Pass      True    test_pass_column_reference_forced_backticks
+# Fail      True    test_fail_column_reference_forced_backticks
+# Pass      True    test_pass_table_reference_forced_backticks
+# Fail      True    test_fail_table_reference_forced_backticks
+# Pass      True    test_pass_multiple_references_forced_backticks
+# Fail      True    test_fail_multiple_references_forced_backticks
+# Pass      True    test_pass_whitespace_forced_backticks
+# Pass      True    test_pass_special_symbols_forced_backticks
+# Pass      True    test_pass_reserved_keyword_forced_backticks
+
 test_pass_column_reference:
   pass_str: |
     SELECT 123 AS foo;
@@ -47,3 +80,189 @@ test_pass_special_symbols:
 test_pass_reserved_keyword:
   pass_str: |
     SELECT 123 AS "SELECT"
+
+test_pass_column_reference_forced_ansi:
+  pass_str: |
+    SELECT 123 AS "foo";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_column_reference_forced_ansi:
+  fail_str: |
+    SELECT 123 AS foo;
+  pass_str: |
+    SELECT 123 AS "foo";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_table_reference_forced_ansi:
+  pass_str: |
+    SELECT "foo"
+    FROM "bar";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_table_reference_forced_ansi:
+  fail_str: |
+    SELECT "foo"
+    FROM bar;
+  pass_str: |
+    SELECT "foo"
+    FROM "bar";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_multiple_references_forced_ansi:
+  pass_str: |
+    SELECT "foo"
+    FROM "bar"."baz";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_multiple_references_forced_ansi:
+  fail_str: |
+    SELECT "foo"
+    FROM bar.baz;
+  pass_str: |
+    SELECT "foo"
+    FROM "bar"."baz";
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_whitespace_forced_ansi:
+  pass_str: |
+    SELECT 123 AS "I cannot be unquoted"
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_special_symbols_forced_ansi:
+  pass_str: |
+    SELECT 123 AS "I-c@nn0t-be~un-quoted"
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_reserved_keyword_forced_ansi:
+  pass_str: |
+    SELECT 123 AS "SELECT"
+  configs:
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_column_reference_forced_backticks:
+  pass_str: |
+    SELECT 123 AS `foo`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_column_reference_forced_backticks:
+  fail_str: |
+    SELECT 123 AS foo;
+  pass_str: |
+    SELECT 123 AS `foo`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_table_reference_forced_backticks:
+  pass_str: |
+    SELECT `foo`
+    FROM `bar`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_table_reference_forced_backticks:
+  fail_str: |
+    SELECT `foo`
+    FROM bar;
+  pass_str: |
+    SELECT `foo`
+    FROM `bar`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_multiple_references_forced_backticks:
+  pass_str: |
+    SELECT `foo`
+    FROM `bar`.`baz`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_fail_multiple_references_forced_backticks:
+  fail_str: |
+    SELECT `foo`
+    FROM bar.baz;
+  pass_str: |
+    SELECT `foo`
+    FROM `bar`.`baz`;
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_whitespace_forced_backticks:
+  pass_str: |
+    SELECT 123 AS `I cannot be unquoted`
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_special_symbols_forced_backticks:
+  pass_str: |
+    SELECT 123 AS `I-c@nn0t-be~un-quoted`
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True
+
+test_pass_reserved_keyword_forced_backticks:
+  pass_str: |
+    SELECT 123 AS `SELECT`
+  configs:
+    core:
+      dialect: bigquery
+    rules:
+      L059:
+        force_quote_identifier: True


Code review comments:
diff_hunk: @@ -28,14 +38,63 @@ class Rule_L059(BaseRule):
 
         SELECT 123 as foo
 
+    When ``force_quote_identifier = True``, the quotes are always necessary, no
+    matter if the identifier is valid, a reserved keyword, or contains special
+    characters.
+
+    | **Anti-pattern**
+    | In this example, a valid unquoted identifier,
+    | that is also not a reserved keyword, is required to be quoted.
+
+    .. code-block:: sql
+
+        SELECT 123 as foo
+
+    | **Best practice**
+    | Use quoted identifiers.
+
+    .. code-block:: sql
+
+        SELECT 123 as "foo" -- For ANSI, ...
+        -- or
+        SELECT 123 as `foo` -- For BigQuery, MySql, ...
+
     """
 
+    config_keywords = [
path: src/sqlfluff/rules/L059.py
text: @author:
@user1 [Linting tests](https://github.com/sqlfluff/sqlfluff/runs/5037642948?check_suite_focus=true) are outputting this error on this line:
```
./src/sqlfluff/rules/L059.py:64:24: BLK100 Black would make changes.
```

I don't understand what it means. 
Could you help me please?

@user1:
Just run black:
```
black src/sqlfluff/rules/L059.py
```

And it'll magically fix your issues.

I would guess it's saying you can fix this on one line, so black is asking why you are splittng it when you don't need it:

```py
config_keywords = ["force_quote_identifier"]
```

@author:
To be honest, I don't have any python/lib on my workspace, I'm editing files through GitHub editor :worried: so I don't have black installed. I'll give it a try the "one-line" proposal.

Thanks.

@user1:
Yikes! Not sure how you can code like that. You;'re a better developer than me! ;-)

Ran `black` for you there and committed.

@author:
I'm PHP oriented and my workspace is dedicated to PHP then, not Python :wink: . I usually don't work editing files 1 by 1 :laughing: .

Thanks for running `black` for me!

Hope this will be ok then.