[ruff] Check for shadowed map before suggesting fix (RUF058)#15790
[ruff] Check for shadowed map before suggesting fix (RUF058)#15790
ruff] Check for shadowed map before suggesting fix (RUF058)#15790Conversation
ntBre
added
bug
|
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Nice! While we're here, as an optional extra: this rule is lacking docs on why the fix is sometimes marked as unsafe (the reason looks like it's because it could destroy comments in some situations). We should probably add a ## Fix safety section to the docs that explains this (see #15584)
|
Good catch! I adapted the fix safety section from RUF055: /// ## Fix safety
///
/// This rule's fix is marked as unsafe if the `starmap` or `zip` expressions contain comments that
/// would be deleted by applying the fix. Otherwise, the fix can be applied safely.
What about a |
It's true that we don't document it as often currently, but I think it's nice to document that too when we can, for sure! |
|
Sounds good! I added both new sections. I put safety first (pun somewhat intended), then availability, but happy to reverse it too. |
* main: [red-knot] Extend instance-attribute tests (#15808) Fix formatter warning message for `flake8-quotes` option (#15788) [`flake8-bugbear`] Exempt `NewType` calls where the original type is immutable (`B008`) (#15765) Add missing config docstrings (#15803) [`refurb`] Do not emit diagnostic when loop variables are used outside loop body (`FURB122`) (#15757) [`ruff`] Check for shadowed `map` before suggesting fix (`RUF058`) (#15790) [red-knot] Do not use explicit `knot_extensions.Unknown` declaration (#15787) Preserve quotes in generated byte strings (#15778) [minor] Simplify some `ExprStringLiteral` creation logic (#15775) Preserve quote style in generated code (#15726) Rename internal helper functions (#15771) [`airflow`] Extend airflow context parameter check for `BaseOperator.execute` (`AIR302`) (#15713) Implement tab autocomplete for `ruff config` (#15603)
2 participants
Summary
Fixes #15786 by not suggesting a fix if
mapdoesn't have its builtin binding.Test Plan
New test taken from the report in #15786.