[refurb] Fix starred expressions fix (FURB161) #16550
Conversation
|
ntBre
added
bug
|
@ntBre could you take a look at this PR |
| // If is a starred expression extract just the value, e.g., `123` in `bin(*[123])`. | ||
| let literal_text = match arg { | ||
| Expr::Starred(starred) => { | ||
| if let Expr::List(list) = &*starred.value { | ||
| if list.elts.len() == 1 { | ||
| checker.locator().slice(&list.elts[0]) | ||
| } else { | ||
| // If the list is empty or has more than | ||
| // one element, we cannot fix it as is a Python error. | ||
| return; | ||
| } | ||
| } else { | ||
| // If it is not the unpacking of a list | ||
| return; | ||
| } | ||
| } | ||
| _ => checker.locator().slice(arg), | ||
| }; |
There was a problem hiding this comment.
I kind of think we should just return immediately if we see a starred expression instead of trying to do this unpacking. We would also need to do this for tuples. However, this only works for the exact case of a literal list or tuple, and I don't think code like *[123] or *(123,) is very likely in real scenarios (you would just write 123 directly). I was going to say we need to handled starred variables, but that's a syntax error:
>>> (*lst).bit_count()
File "<python-input-7>", line 1
(*lst).bit_count()
^^^^
SyntaxError: cannot use starred expression here
>>>In short, I think it's better to return early for any starred expression to save us the work of unpacking expressions that should be very rare in practice.
Thank you for the feedback. I don't have strong opinions on that matter. I agree that returning in the case of starred expressions makes perfect sense. I have updated the code accordingly. Please let me know if it aligns with your suggestion |
The PR partially solves issue #16457
Specifically, it solves the following problem:
Now starred expressions are corrected handled.