[perflint] Simplify finding the loop target in PERF401#15025
[perflint] Simplify finding the loop target in PERF401#15025MichaReiser merged 2 commits intoastral-sh:mainfrom
perflint] Simplify finding the loop target in PERF401#15025Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| B018 | 1 | 0 | 1 | 0 | 0 |
Linter (preview)
✅ ecosystem check detected no linter changes.
Yep. This is the cause of a few cases of strange behavior... When the semantic model is called "as it's built" then this behavior is correct (because at the program point of interest, the delete has not yet ocurred), but it can be counter-intuitive when called (like in the current situation) on deferred scopes/for-loops/etc. |
perflint] Simplify finding the loop target in perf401perflint] Simplify finding the loop target in PERF401
| .bindings | ||
| .iter() | ||
| .find(|binding| for_stmt.target.range() == binding.range) | ||
| .unwrap(); |
There was a problem hiding this comment.
Is this unwrap safe? If so, we should use expect but I'd possibly just do an early return if this is None.
There was a problem hiding this comment.
It should be fine. The rule only applies to for-loop targets that are names; a binding must exist.
* main: [red-knot] Explicitly test diagnostics are emitted for unresolvable submodule imports (#15035) Fix stale File status in tests (#15030) [red-knot] Basic support for other legacy `typing` aliases (#14998) feat(AIR302): extend the following rules (#15015) [`perflint`] Simplify finding the loop target in `PERF401` (#15025) [red-knot] Avoid undeclared path when raising conflicting declarations (#14958)
Fixes #15012.
I'm not sure exactly why this test case panics, but I suspect the
del iremoves the binding from the semantic model's symbols.I changed the code to search for the correct binding by directly iterating through the bindings. Since we know exactly which binding we want, this should find the loop variable without any complications.