[PLW1507] Mark fix unsafe#16343
Conversation
|
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
Thank you
We should add some documentation explaining why the fix is unsafe. You can search for ## Fix safety to see similar comments for other rules.
| # Test case where the proposed fix is wrong, i.e., unsafe fix | ||
| # Ref: https://github.com/astral-sh/ruff/issues/16274#event-16423475135 | ||
|
|
||
| import copy | ||
| import os | ||
|
|
||
| os.environ["X"] = "0" | ||
| env = copy.copy(os.environ) | ||
| os.environ["X"] = "1" |
There was a problem hiding this comment.
I think we could keep this test case in the same file but add a comment explaining what it does.
I added the explanation of the unsafe fix. I don't know if it would help to add a small example or/and quoting the issue with the example why the fix is unsafe |
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
This is perfect, thank you
* main: (38 commits) [red-knot] Use arena-allocated association lists for narrowing constraints (#16306) [red-knot] Rewrite `Type::try_iterate()` to improve type inference and diagnostic messages (#16321) Add issue templates (#16213) Normalize inconsistent markdown headings in docstrings (#16364) [red-knot] Better diagnostics for method calls (#16362) [red-knot] Add argfile and windows glob path support (#16353) [red-knot] Handle pipe-errors gracefully (#16354) Rename `venv-path` to `python` (#16347) [red-knot] Fixup some formatting in `infer.rs` (#16348) [red-knot] Restrict visibility of more things in `class.rs` (#16346) [red-knot] Add diagnostic for class-object access to pure instance variables (#16036) Add `per-file-target-version` option (#16257) [PLW1507] Mark fix unsafe (#16343) [red-knot] Add a test to ensure that `KnownClass::try_from_file_and_name()` is kept up to date (#16326) Extract class and instance types (#16337) Re-order changelog entries for 0.9.7 (#16344) [red-knot] Add support for `@classmethod`s (#16305) Update Salsa (#16338) Update Salsa part 1 (#16340) Upgrade Rust toolchain to 1.85.0 (#16339) ...
2 participants
This PR addresses issue #16274
Just marked the fix as unsafe, as it is not possible to check when a fix is safe or not (see above cited issue for example).