Skip to content

[red-knot] Fix diagnostic range for non-iterable unpacking assignments#15994

Merged
sharkdp merged 2 commits intomainfrom
david/unpacking-diagnostic-range
Feb 6, 2025
Merged

[red-knot] Fix diagnostic range for non-iterable unpacking assignments#15994
sharkdp merged 2 commits intomainfrom
david/unpacking-diagnostic-range

Conversation

@sharkdp
Copy link
Copy Markdown
Contributor

@sharkdp sharkdp commented Feb 6, 2025

Summary

I noticed that the diagnostic range in specific unpacking assignments is wrong. For this example

a, b = 1

we previously got (see first commit):

error: lint:not-iterable
 --> /src/mdtest_snippet.py:1:1
  |
1 | a, b = 1
  | ^^^^ Object of type `Literal[1]` is not iterable
  |

and with this change, we get:

error: lint:not-iterable
 --> /src/mdtest_snippet.py:1:8
  |
1 | a, b = 1
  |        ^ Object of type `Literal[1]` is not iterable
  |

Test Plan

New snapshot tests.

@sharkdp sharkdp added ty Multi-file analysis & type inference diagnostics Related to reporting of diagnostics. labels Feb 6, 2025
Comment on lines +24 to +25
1 | a, b = (1,) # error: [invalid-assignment]
| ^^^^ Not enough values to unpack (expected 2, got 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a case where we'd want multiple diagnostic ranges (@BurntSushi) but having the assignment target being the main range seems correct.

Copy link
Copy Markdown
Contributor Author

@sharkdp sharkdp Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just included these here to make sure that we're not generally moving the diagnostic range to the right hand side of the assignment. Just for this very specific case I fixed (where unwrap_diagnostic_range really expects the expression of the iterable, not the assignment target).

Copy link
Copy Markdown
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sharkdp sharkdp merged commit e345307 into main Feb 6, 2025
@sharkdp sharkdp deleted the david/unpacking-diagnostic-range branch February 6, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diagnostics Related to reporting of diagnostics. ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants