Skip to content

fix(reformatter): ensure idempotent formatting for dictionary values with line continuation - #1320

Open
jdymitarai wants to merge 1 commit into
google:mainfrom
jdymitarai:fix-continuation-space-idempotence
Open

fix(reformatter): ensure idempotent formatting for dictionary values with line continuation#1320
jdymitarai wants to merge 1 commit into
google:mainfrom
jdymitarai:fix-continuation-space-idempotence

Conversation

@jdymitarai

Copy link
Copy Markdown

Summary

Fixes #1217 where yapf repeatedly added spaces before a line continuation backslash (\) in dictionary values on subsequent formatting runs (under styles enabling INDENT_DICTIONARY_VALUE, such as google). Also resolves an AssertionError when dictionary values consisting of a single leaf were preceded by a line continuation backslash.

Cause & Analysis

  1. Continuation node parentage: In continuation_splicer.py, continuation nodes were inserted into pytree nodes via node.children.insert() directly instead of node.insert_child(), leaving continuation_node.parent as None. Any subsequent AST transformations or sibling queries (next_sibling, replace) on these nodes could fail or raise an AssertionError.
  2. Continuation in dictionary values: In subtype_assigner.py, _InsertPseudoParentheses only checked grammar_token.COMMENT when looking for tokens inserted before a value leaf, missing format_token.CONTINUATION. As a result, single-leaf values preceded by a backslash failed to be encompassed into an atom node alongside the continuation marker.
  3. Redundant whitespace emission: In reformatter.py, _FormatFinalLines appends a space for pseudo-parentheses if the succeeding token's whitespace_prefix does not start with whitespace. Because continuation tokens hold their leading whitespace in their token value (' \') rather than whitespace_prefix, _FormatFinalLines emitted an additional space before the continuation token on every formatting run, causing infinite drift.
  4. Token space calculation: In logical_line.py, continuation tokens were checked after pseudo-parentheses in _SpaceRequiredBetween, allowing pseudo scope opening to request an unnecessary space before continuation markers.

Solution

  • Use node.insert_child(...) in continuation_splicer.py to maintain consistent parent links.
  • Handle format_token.CONTINUATION alongside grammar_token.COMMENT in _InsertPseudoParentheses.
  • Check not tok.next_token.value.startswith(' ') in _FormatFinalLines before appending spaces for pseudo tokens.
  • Prioritize continuation token spacing checks in logical_line._SpaceRequiredBetween.
  • Add unit tests in reformatter_basic_test.py verifying multiple-pass idempotency and single-leaf dictionary value continuation handling.

…ntinuations

When formatting dictionary values preceded by a line continuation backslash under INDENT_DICTIONARY_VALUE (e.g. in the google style):
1. In continuation_splicer.py, use node.insert_child instead of node.children.insert to properly set the parent attribute on continuation nodes, preventing an AssertionError when navigating or replacing nodes.
2. In subtype_assigner.py, check for format_token.CONTINUATION alongside grammar_token.COMMENT so that dictionary values preceded by a continuation marker are correctly encompassed in an atom node when inserting pseudo-parentheses.
3. In reformatter.py, avoid appending redundant spaces in _FormatFinalLines for pseudo-parentheses when the succeeding token's value already starts with spaces (as continuation tokens do), preventing repeated space addition on each run.
4. In logical_line.py, prioritize continuation checks in _SpaceRequiredBetween before checking pseudo-parentheses.

Fixes google#1217
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

yapf adds spaces before backslash with every run

1 participant