Fix crash on # fmt: skip after an opening bracket with a standalone comment#5161
Open
sarathfrancis90 wants to merge 1 commit into
Open
Fix crash on # fmt: skip after an opening bracket with a standalone comment#5161sarathfrancis90 wants to merge 1 commit into
# fmt: skip after an opening bracket with a standalone comment#5161sarathfrancis90 wants to merge 1 commit into
Conversation
A `# fmt: skip` line placed directly after an opening bracket, combined with a standalone comment among the bracket's contents, crashed with `AttributeError: 'Leaf' object has no attribute 'bracket_depth'`. The crash happened in `is_line_short_enough`. When the rendered line spans multiple physical lines, the function walks the leaves and reads `leaf.bracket_depth` on each of them. However, a closing bracket that is continued from a previous line break (a leftover `)` / `]` left on its own line) is intentionally skipped by `BracketTracker.mark` and never receives a `bracket_depth` attribute. Accessing it therefore raised. Such a leaf is not inside any bracket that opened on the current line, so its effective depth is 0. Read the attribute defensively with that default, which leaves behaviour unchanged for all already-marked leaves and stops the crash, producing valid and stable output instead. Added a regression test covering the import, call, subscript and inline `# fmt: skip` variants.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
blackcrashes withAttributeError: 'Leaf' object has no attribute 'bracket_depth'on valid Python when a# fmt: skipline is placed directly after an opening bracket and a standalone comment also appears among the bracket's contents. For example, all of the following crash:Root cause. The crash happens in
is_line_short_enough(src/black/lines.py). When the rendered line spans multiple physical lines, the function walks every leaf and readsleaf.bracket_depth. That attribute is set byBracketTracker.mark, butmarkdeliberately early-returns for a closing bracket that is continued from a previous line break (a leftover)/]/}left on its own line) — seeBracketTracker.markinsrc/black/brackets.py. Such a leaf therefore never receives abracket_depth, and accessing it raisesAttributeError.The combination of
# fmt: skipright after an opening bracket plus a standalone comment is exactly what produces a line containing such an unmarked, continued closing bracket, sois_line_short_enoughblows up.Fix. A leaf that the bracket tracker skipped is, by construction, not inside any bracket that opened on the current line, so its effective depth is
0. Read the attribute defensively with that default (getattr(leaf, "bracket_depth", 0)). This is a no-op for every already-marked leaf (behaviour is unchanged) and removes the crash, producing valid, AST-equivalent and idempotent output. Since this only fixes a crash (it does not change formatting of any code that previously formatted successfully), it is a stable-style fix per the contributing docs.Testing. Added
tests/data/cases/fmtskip_after_bracket_with_comment.pycovering the import, call, subscript and inline# fmt: skipvariants. The test fails onmain(crash insideis_line_short_enough) and passes with this change. The produced output is AST-preserving and idempotent (formatting it a second time is a no-op).Checklist - did you ...
--previewstyle, following the stability policy? (N/A — this is a crash fix only, no formatting changes to previously-working code, so it belongs in stable style)CHANGES.mdif necessary?