Skip to content

Commit

Permalink
Merge branch 'main' into bhart-issue_2713_l018_move_cte_close_paren_t…
Browse files Browse the repository at this point in the history
…o_next_line
  • Loading branch information
tunetheweb committed Feb 28, 2022
2 parents 09a7c13 + ce9cd4a commit 85a5e11
Show file tree
Hide file tree
Showing 8 changed files with 335 additions and 39 deletions.
4 changes: 4 additions & 0 deletions src/sqlfluff/core/default_config.cfg
Expand Up @@ -137,6 +137,10 @@ forbid_subquery_in = join
prefer_count_1 = False
prefer_count_0 = False

[sqlfluff:rules:L051]
# Fully qualify JOIN clause
fully_qualify_join_types = inner

[sqlfluff:rules:L052]
# Semi-colon formatting approach
multiline_newline = False
Expand Down
4 changes: 4 additions & 0 deletions src/sqlfluff/core/rules/config_info.py
Expand Up @@ -113,6 +113,10 @@
"Should alias have an explict AS or is implicit aliasing required?"
),
},
"fully_qualify_join_types": {
"validation": ["inner", "outer", "both"],
"definition": ("Which types of JOIN clauses should be fully qualified?"),
},
"multiline_newline": {
"validation": [True, False],
"definition": (
Expand Down
5 changes: 1 addition & 4 deletions src/sqlfluff/rules/L018.py
Expand Up @@ -117,10 +117,7 @@ def indent_size_up_to(segs):
# Is it all whitespace before the bracket on this line?
prev_segs_on_line = [
elem
for elem in context.segment.iter_segments(
expanding=["common_table_expression", "bracketed"],
pass_through=True,
)
for elem in context.segment.raw_segments
if elem.pos_marker.line_no == seg.pos_marker.line_no
and elem.pos_marker.line_pos < seg.pos_marker.line_pos
]
Expand Down
78 changes: 59 additions & 19 deletions src/sqlfluff/rules/L051.py
Expand Up @@ -3,17 +3,24 @@
from sqlfluff.core.parser.segments.raw import KeywordSegment, WhitespaceSegment

from sqlfluff.core.rules.base import BaseRule, LintResult, LintFix, RuleContext
from sqlfluff.core.rules.doc_decorators import document_fix_compatible
from sqlfluff.core.rules.doc_decorators import (
document_fix_compatible,
document_configuration,
)


@document_fix_compatible
@document_configuration
class Rule_L051(BaseRule):
"""``INNER JOIN`` must be fully qualified.
"""Join clauses should be fully qualified.
By default this rule is configured to enforce fully qualified ``INNER JOIN``
clauses, but not ``[LEFT/RIGHT/FULL] OUTER JOIN``. If you prefer a stricter
lint then this is configurable.
**Anti-pattern**
A join is specified without specifying the **kind** of join, i.e. the standalone
keyword ``JOIN``.
A join is specified without expliciting the **kind** of join.
.. code-block:: sql
:force:
Expand All @@ -36,23 +43,56 @@ class Rule_L051(BaseRule):
INNER JOIN baz;
"""

config_keywords = ["fully_qualify_join_types"]

def _eval(self, context: RuleContext) -> Optional[LintResult]:
"""INNER JOIN must be fully qualified."""
"""Fully qualify JOINs."""
# Config type hints
self.fully_qualify_join_types: str

# We are only interested in JOIN clauses.
if context.segment.type != "join_clause":
if not context.segment.is_type("join_clause"):
return None

# We identify non-lone JOIN by looking at first child segment.
if context.segment.segments[0].name != "join":
return None
join_clause_keywords = [
segment for segment in context.segment.segments if segment.type == "keyword"
]

# Identify LEFT/RIGHT/OUTER JOIN and if the next keyword is JOIN.
if (
self.fully_qualify_join_types in ["outer", "both"]
and join_clause_keywords[0].raw_upper in ["RIGHT", "LEFT", "FULL"]
and join_clause_keywords[1].raw_upper == "JOIN"
):
# Define basic-level OUTER capitalization based on JOIN
outer_kw = ("outer", "OUTER")[join_clause_keywords[1].raw == "JOIN"]
# Insert OUTER after LEFT/RIGHT/FULL
return LintResult(
context.segment.segments[0],
fixes=[
LintFix.create_after(
context.segment.segments[0],
[WhitespaceSegment(), KeywordSegment(outer_kw)],
)
],
)

# Identify lone JOIN by looking at first child segment.
if (
self.fully_qualify_join_types in ["inner", "both"]
and join_clause_keywords[0].raw_upper == "JOIN"
):
# Define basic-level INNER capitalization based on JOIN
inner_kw = ("inner", "INNER")[join_clause_keywords[0].raw == "JOIN"]
# Replace lone JOIN with INNER JOIN.
return LintResult(
context.segment.segments[0],
fixes=[
LintFix.create_before(
context.segment.segments[0],
[KeywordSegment(inner_kw), WhitespaceSegment()],
)
],
)

# Replace lone JOIN with INNER JOIN.
return LintResult(
context.segment.segments[0],
fixes=[
LintFix.create_before(
context.segment.segments[0],
[KeywordSegment("INNER"), WhitespaceSegment()],
)
],
)
return None
15 changes: 14 additions & 1 deletion src/sqlfluff/testing/rules.py
Expand Up @@ -91,7 +91,7 @@ def assert_rule_fail_in_sql(code, sql, configs=None, line_numbers=None):
return fixed, linted.violations


def assert_rule_pass_in_sql(code, sql, configs=None):
def assert_rule_pass_in_sql(code, sql, configs=None, msg=None):
"""Assert that a given rule doesn't fail on the given sql."""
# Configs allows overrides if we want to use them.
if configs is None:
Expand All @@ -105,6 +105,8 @@ def assert_rule_pass_in_sql(code, sql, configs=None):
rendered = linter.render_string(sql, fname="<STR>", config=cfg, encoding="utf-8")
parsed = linter.parse_rendered(rendered, recurse=True)
if parsed.violations:
if msg:
print(msg) # pragma: no cover
pytest.fail(parsed.violations[0].desc() + "\n" + parsed.tree.stringify())
print(f"Parsed:\n {parsed.tree.stringify()}")

Expand All @@ -116,6 +118,8 @@ def assert_rule_pass_in_sql(code, sql, configs=None):
lerrs = lint_result.violations
print(f"Errors Found: {lerrs}")
if any(v.rule.code == code for v in lerrs):
if msg:
print(msg) # pragma: no cover
pytest.fail(f"Found {code} failures in query which should pass.", pytrace=False)


Expand Down Expand Up @@ -193,6 +197,15 @@ def rules__test_helper(test_case):
assert res == test_case.fix_str
if test_case.violations_after_fix:
assert_violations_after_fix(test_case)
else:
assert_rule_pass_in_sql(
test_case.rule,
test_case.fix_str,
configs=test_case.configs,
msg="The SQL after fix is applied still contains rule violations. "
"To accept a partial fix, violations_after_fix must be set "
"listing the remaining, expected, violations.",
)
else:
# Check that tests without a fix_str do not apply any fixes.
assert res == test_case.fail_str, (
Expand Down
13 changes: 6 additions & 7 deletions test/core/rules/docstring_test.py
Expand Up @@ -57,13 +57,12 @@ def test_backtick_replace():
"""Test replacing docstring double backticks for lint results."""
sql = """
SELECT
foo.a,
bar.b
DISTINCT(a),
b
FROM foo
JOIN bar;
"""
result = lint(sql, rules=["L051"])
# L051 docstring looks like:
# ``INNER JOIN`` must be fully qualified.
result = lint(sql, rules=["L015"])
# L015 docstring looks like:
# ``DISTINCT`` used with parentheses.
# Check the double bacticks (``) get replaced by a single quote (').
assert result[0]["description"] == "'INNER JOIN' must be fully qualified."
assert result[0]["description"] == "'DISTINCT' used with parentheses."
17 changes: 17 additions & 0 deletions test/fixtures/rules/std_rule_cases/L018.yml
Expand Up @@ -33,3 +33,20 @@ test_fail_with_clause_closing_misaligned_negative_indentation:
with cte as (
select 1
) select * from cte
test_move_parenthesis_to_next_line:
fail_str: |
with cte_1 as (
select foo
from tbl_1) -- Foobar
select cte_1.foo
from cte_1
fix_str: |
with cte_1 as (
select foo
from tbl_1
) -- Foobar
select cte_1.foo
from cte_1

0 comments on commit 85a5e11

Please sign in to comment.