From 9bd93037fa9a8700a79a9cb95c616edba730fff7 Mon Sep 17 00:00:00 2001 From: dualfroz Date: Wed, 2 Sep 2026 12:14:20 +0200 Subject: [PATCH] fix: group dotted identifiers split by interleaved comments (issue712) Identifiers may carry comments between their tokens (e.g. name -- c\n . -- c\n obj). _group only skipped whitespace when locating the tokens around a joining token, so an interleaved comment defeated the prev/next checks and the identifier was never formed. Add an opt-in skip_comments flag to _group and enable it for group_period and group_as; the comments stay inside the resulting group. Also skip comments in get_parent_name and avoid a redundant separating space in StripCommentsFilter for the newly reachable interleaved-comment identifiers. --- sqlparse/engine/grouping.py | 26 ++++++++++++++++++++------ sqlparse/filters/others.py | 16 ++++++++++++++-- sqlparse/sql.py | 4 +++- tests/test_grouping.py | 21 +++++++++++++++++++++ 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/sqlparse/engine/grouping.py b/sqlparse/engine/grouping.py index d8cfa9e..d5714d7 100644 --- a/sqlparse/engine/grouping.py +++ b/sqlparse/engine/grouping.py @@ -188,7 +188,8 @@ def group_period(tlist): return (pidx, nidx) if valid_next else (pidx, tidx) - _group(tlist, sql.Identifier, match, valid_prev, valid_next, post) + _group(tlist, sql.Identifier, match, valid_prev, valid_next, post, + skip_comments=True) def group_as(tlist): @@ -205,7 +206,8 @@ def group_as(tlist): def post(tlist, pidx, tidx, nidx): return pidx, nidx - _group(tlist, sql.Identifier, match, valid_prev, valid_next, post) + _group(tlist, sql.Identifier, match, valid_prev, valid_next, post, + skip_comments=True) def group_assignment(tlist): @@ -481,9 +483,16 @@ def _group(tlist, cls, match, post=None, extend=True, recurse=True, - depth=0 + depth=0, + skip_comments=False ): - """Groups together tokens that are joined by a middle token. i.e. x < y""" + """Groups together tokens that are joined by a middle token. i.e. x < y + + When *skip_comments* is ``True`` comments interleaved between the joined + tokens are ignored while looking for the previous and next token, so + e.g. ``a -- c\\n . -- c\\n b`` still groups into a single identifier + (issue712). The comments themselves stay inside the resulting group. + """ if MAX_GROUPING_DEPTH is not None and depth > MAX_GROUPING_DEPTH: raise SQLParseError( f"Maximum grouping depth exceeded ({MAX_GROUPING_DEPTH})." @@ -510,10 +519,10 @@ def _group(tlist, cls, match, if recurse and token.is_group and not isinstance(token, cls): _group(token, cls, match, valid_prev, valid_next, - post, extend, True, depth + 1) + post, extend, True, depth + 1, skip_comments) if match(token): - nidx, next_ = tlist.token_next(tidx) + nidx, next_ = tlist.token_next(tidx, skip_cm=skip_comments) if prev_ and valid_prev(prev_) and valid_next(next_): from_idx, to_idx = post(tlist, pidx, tidx, nidx) grp = tlist.group_tokens(cls, from_idx, to_idx, extend=extend) @@ -522,4 +531,9 @@ def _group(tlist, cls, match, pidx, prev_ = from_idx, grp continue + if skip_comments and imt(token, t=T.Comment, i=sql.Comment): + # Keep prev_ pointing at the real previous token so an interleaved + # comment does not defeat valid_prev (issue712). + continue + pidx, prev_ = tidx, token diff --git a/sqlparse/filters/others.py b/sqlparse/filters/others.py index 95bc436..21a59eb 100644 --- a/sqlparse/filters/others.py +++ b/sqlparse/filters/others.py @@ -63,8 +63,20 @@ class StripCommentsFilter: ): # Insert a whitespace to ensure the following SQL produces # a valid SQL (see #425). - if prev_ is not None and not prev_.match(T.Punctuation, '('): - tlist.tokens.insert(tidx, _get_insert_token(token)) + insert_tok = _get_insert_token(token) + # A plain space separator is redundant when a neighbour is + # already whitespace, which happens for comments interleaved + # inside a grouped identifier (issue712). A line break carried + # by the comment is always kept (see #484). + redundant_space = ( + insert_tok.is_whitespace and not insert_tok.is_newline + and ((prev_ is not None and prev_.is_whitespace) + or (next_ is not None and next_.is_whitespace)) + ) + if (prev_ is not None + and not prev_.match(T.Punctuation, '(') + and not redundant_space): + tlist.tokens.insert(tidx, insert_tok) tlist.tokens.remove(token) tidx -= 1 else: diff --git a/sqlparse/sql.py b/sqlparse/sql.py index ec44a6d..571b3b6 100644 --- a/sqlparse/sql.py +++ b/sqlparse/sql.py @@ -387,7 +387,9 @@ class TokenList(Token): A parent object is identified by the first occurring dot. """ dot_idx, _ = self.token_next_by(m=(T.Punctuation, '.')) - _, prev_ = self.token_prev(dot_idx) + # skip_cm so a comment between the parent name and the dot (issue712) + # does not get mistaken for the parent name. + _, prev_ = self.token_prev(dot_idx, skip_cm=True) return remove_quotes(prev_.value) if prev_ is not None else None def _get_first_name(self, idx=None, reverse=False, keywords=False, diff --git a/tests/test_grouping.py b/tests/test_grouping.py index 20fab9b..96a9b06 100644 --- a/tests/test_grouping.py +++ b/tests/test_grouping.py @@ -241,6 +241,27 @@ def test_grouping_identifier_list_with_inline_comments(): assert isinstance(p.tokens[0].tokens[3], sql.Identifier) +def test_grouping_identifier_with_comments(): + # issue712: comments between the tokens of a (possibly aliased) dotted + # identifier must not prevent it from being grouped. + s = 'a\n-- c1\n.\n-- c2\nb' + p = sqlparse.parse(s)[0] + assert str(p) == s + assert len([t for t in p.tokens if not t.is_whitespace]) == 1 + assert isinstance(p.tokens[0], sql.Identifier) + assert p.tokens[0].get_parent_name() == 'a' + assert p.tokens[0].get_real_name() == 'b' + + s = 'ns -- a\n. -- b\n"tbl" -- c\nas -- d\nal' + p = sqlparse.parse(s)[0] + assert str(p) == s + assert len([t for t in p.tokens if not t.is_whitespace]) == 1 + assert isinstance(p.tokens[0], sql.Identifier) + assert p.tokens[0].get_parent_name() == 'ns' + assert p.tokens[0].get_real_name() == 'tbl' + assert p.tokens[0].get_alias() == 'al' + + def test_grouping_identifiers_with_operators(): p = sqlparse.parse('a+b as c from table where (d-e)%2= 1')[0] assert len([x for x in p.flatten() if x.ttype == T.Name]) == 5