Fix: The discovery hypothesis (missing look-ahead in... - #364
Open
M001N wants to merge 1 commit into
Open
Conversation
… more statements useless_pass_line_numbers() detected a redundant leading 'pass' by comparing token row numbers with strict adjacency (start_row - 1 == last_pass_row). This broke whenever a comment or blank line separated the pass from the following statement, since those lines shift the row gap to more than 1 without the check accounting for them. As a result, filter_unused_variable()'s 'pass' placeholder for a removed literal/name assignment was never cleaned up in that case, even though the block still had a following statement. Replace the row-arithmetic check with a small state machine that tracks whether only comments/blank lines have been seen since the last pass, so the leading-pass look-ahead now correctly spans intervening comments and blank lines. Fixes PyCQA#82
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
1 participant
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.
Summary
Replaced the strict
start_row - 1 == last_pass_rowadjacency check in useless_pass_line_numbers() with a small state flag (pass_pending_leading_check) that stays set across COMMENT/NL/NEWLINE/INDENT/DEDENT tokens (i.e. blank lines and comments) following a pass, and is cleared as soon as any other real code token is seen. This makes the 'pass immediately followed by another statement in the same block' look-ahead tolerant of intervening comments and blank lines, while preserving all existing semantics (indentation must still match, and a pass that truly is the sole statement in its block is left alone).Problem
PyCQA/autoflake issue reference: #82
Root Cause
The discovery hypothesis (missing look-ahead in filter_unused_variable/filter_code) was incorrect. filter_unused_variable() always turns a literal/name-RHS assignment into 'pass' unconditionally -- that part is by design. The actual clean-up happens in a separate post-processing pass, useless_pass_line_numbers() (autoflake.py ~line 809), which tokenizes the whole file and detects a 'leading pass' (a pass immediately followed by another statement at the same indentation) so it can be stripped out again as redundant. That detection used raw token-row arithmetic:
start_row - 1 == last_pass_row. A comment-only line or a blank line between the pass and the next statement emits tokenize.COMMENT/NL tokens that are not tracked, so the row gap becomes 2+ instead of 1 and the equality check silently fails -- the redundant 'pass' is therefore never removed, even though the block clearly is not empty.Testing
PASS - all 179 tests in test_autoflake.py pass, including the 5 new regression tests covering both of the issue's exact reproducers, the low-level tokenizer function, and the pre-existing sole-statement-gets-pass behavior (unregressed).
Related Issue
#82