[flake8-simplify] Preserve f-string source verbatim in SIM101 fix#25061
Conversation
|
Thank you. Fixing the following snippet creates a syntax error on this branch types = (int,)
isinstance(x, (*types,)) or isinstance(x, ()) |
…rred element Empty tuple and starred-only tuple operands could collapse to a single starred element after splicing, which is a syntax error (e.g. `isinstance(x, (*types,)) or isinstance(x, ())` was rewritten to `isinstance(x, (*types))`). Detect this case up front and suppress the autofix; the diagnostic still fires.
|
Fixed in a199ab5. The merger now flattens the type expressions up front and suppresses the autofix when that would collapse to a single starred element. The diagnostic still fires. types = (int,)
isinstance(x, (*types,)) or isinstance(x, ())now reports |
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
This now generates invalid syntax when the expression is parenthesized
((isinstance(x, int)) or isinstance(x, str))| @@ -411,70 +411,52 @@ pub(crate) fn duplicate_isinstance_call(checker: &Checker, expr: &Expr) { | |||
| }) | |||
| .collect(); | |||
There was a problem hiding this comment.
Do we still need this collect?
The replacement range only spanned the first and last operand expressions themselves, so parentheses that wrapped just one operand within the `BoolOp` were left dangling, producing invalid syntax for inputs like `((isinstance(x, int)) or isinstance(x, str))`. Extend the range with `parenthesized_range` to absorb those parens.
|
The replacement range only covered Dropped the intermediate |
|
…stral-sh#25061) Co-authored-by: Micha Reiser <micha@reiser.io>
…stral-sh#25061) Co-authored-by: Micha Reiser <micha@reiser.io>
3 participants
Closes #19601.
The previous fix built a new
isinstance(target, (t1, t2))AST and ran the codegen generator over the entire surroundingBoolOp. Re-rendering through the generator normalizes the source — escape sequences in f-string format specs (\\x7d→}) get materialized as literal characters, lambdas inside f-string interpolations get re-spelled, and the result is either a syntax error (the rule's own fix-validator refuses to apply it) or a behavior change.This switches the fix to splice the original target and type expressions out of the source via the locator, then build the replacement string and replace just the consecutive duplicate-call range, leaving the rest of the
BoolOpsource untouched. New regression fixtures cover f-strings containing lambdas, lambdas in format specs, and the two escape-sequence cases from the issue body.