Skip to content

Commit

Permalink
Add errors for redundant definitions. (#3626)
Browse files Browse the repository at this point in the history
* Add errors for redundant definitions.

* appease mypy
  • Loading branch information
alanmcruickshank committed Jul 18, 2022
1 parent 6b31de4 commit 655cc7f
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 9 deletions.
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

0 comments on commit 655cc7f

Please sign in to comment.