Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add errors for redundant definitions. #3626

Merged
merged 3 commits into from Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/sqlfluff/core/dialects/base.py
Expand Up @@ -154,6 +154,12 @@ def replace(self, **kwargs: DialectElementType):
cls = kwargs[n]
if self._library[n] is cls:
continue
elif self._library[n] == cls:
# Check for replacement with a new but identical class.
# This would be a sign of redundant definitions in the dialect.
raise ValueError(
f"Attempted unnecessary identical redefinition of {n!r} in {self!r}"
) # pragma: no cover

# To replace a segment, the replacement must either be a
# subclass of the original, *or* it must have the same
Expand Down
19 changes: 12 additions & 7 deletions src/sqlfluff/core/parser/grammar/base.py
Expand Up @@ -85,6 +85,7 @@ class BaseGrammar(Matchable):
# Are we allowed to refer to keywords as strings instead of only passing
# grammars or segments?
allow_keyword_string_refs = True
equality_kwargs: Tuple[str, ...] = ("optional", "allow_gaps")

@staticmethod
def _resolve_ref(elem):
Expand Down Expand Up @@ -713,14 +714,18 @@ def __repr__(self):
def __eq__(self, other):
"""Two grammars are equal if their elements and types are equal.

NOTE: This could potentially mean that two grammars with
the same elements but _different configuration_ will be
classed as the same. If this matters for your use case,
consider extending this function.

e.g. `OneOf(foo) == OneOf(foo, optional=True)`
NOTE: We use the equality_kwargs tuple on the class to define
other kwargs which should also be checked so that things like
"optional" is also taken into account in considering equality.
"""
return type(self) is type(other) and self._elements == other._elements
return (
type(self) is type(other)
and self._elements == other._elements
and all(
getattr(self, k, None) == getattr(other, k, None)
for k in self.equality_kwargs
)
)

def copy(
self,
Expand Down
9 changes: 9 additions & 0 deletions src/sqlfluff/core/parser/grammar/delimited.py
Expand Up @@ -24,6 +24,15 @@ class Delimited(OneOf):
as different options of what can be delimited, rather than a sequence.
"""

equality_kwargs = (
"optional",
"allow_gaps",
"delimiter",
"allow_trailing",
"terminator",
"min_delimiters",
)

def __init__(
self,
*args,
Expand Down
2 changes: 1 addition & 1 deletion src/sqlfluff/dialects/dialect_redshift.py
Expand Up @@ -2249,7 +2249,7 @@ class TableExpressionSegment(ansi.TableExpressionSegment):

match_grammar = ansi.TableExpressionSegment.match_grammar.copy(
insert=[Ref("ObjectUnpivotSegment", optional=True)],
before=Ref("TableReferenceSegment", optional=True),
before=Ref("TableReferenceSegment"),
)


Expand Down
2 changes: 1 addition & 1 deletion src/sqlfluff/dialects/dialect_sparksql.py
Expand Up @@ -1460,7 +1460,7 @@ class SelectStatementSegment(ansi.SelectStatementSegment):
Ref("DistributeByClauseSegment", optional=True),
Ref("SortByClauseSegment", optional=True),
],
before=Ref("LimitClauseSegment"),
before=Ref("LimitClauseSegment", optional=True),
)


Expand Down