[ruff] add fix safety section (RUF013)#17759
Conversation
|
|
Hey @dylwil3 it was hard to find an example were the fix was unsafe, or to understand the reasons why is unsafe. But I think the main reasons is the one explained above. |
|
I think this fix might just be safe. This is a creative reason it might not be safe:
but I think that example looks like a bug fix or desired change to me - the argument really is optional. My guess is that - as we've seen in other instances for this documentation issue - the fix was originally marked as unsafe because there were some false positives, but then during code review the implementation was changed to either skip or correctly handle those cases. I've made a separate PR to make the fix safe in preview #17836 . Assuming I'm not missing something and that gets merged I'll close this PR. Thank you for the investigative work! Really it's helpful either way if we end up documenting the fix or learning that it has a safe fix. |
|
RUF013 can change runtime behavior: $ cat >ruf013.py <<'# EOF'
def foo(arg: int = None):
pass
print(foo.__annotations__["arg"])
# EOF
$ python ruf013.py
<class 'int'>
$ ruff --isolated check ruf013.py --select RUF013 --unsafe-fixes --fix
Found 1 error (1 fixed, 0 remaining).
$ python ruf013.py
typing.Optional[int]It is conceivable that some code might inspect annotations at runtime, expecting normal |
|
@VascoSch92 - could you revise the fix safety section to reflect the comments given by Micha here #17836 (comment) and @dscorbett above? |
dylwil3
left a comment
There was a problem hiding this comment.
Thank you, great way to summarize the issues brought up!
The PR add the fix safety section for rule `RUF013` (#15584 ) The fix was introduced here #4831 The rule as a lot of False Negative (as it is explained in the docs of the rule). The main reason because the fix is unsafe is that it could change code generation tools behaviour, as in the example here: ```python def generate_api_docs(func): hints = get_type_hints(func) for param, hint in hints.items(): if is_optional_type(hint): print(f"Parameter '{param}' is optional") else: print(f"Parameter '{param}' is required") # Before fix def create_user(name: str, roles: list[str] = None): pass # After fix def create_user(name: str, roles: Optional[list[str]] = None): pass # Generated docs would change from "roles is required" to "roles is optional" ```
The PR add the fix safety section for rule `RUF013` (astral-sh#15584 ) The fix was introduced here astral-sh#4831 The rule as a lot of False Negative (as it is explained in the docs of the rule). The main reason because the fix is unsafe is that it could change code generation tools behaviour, as in the example here: ```python def generate_api_docs(func): hints = get_type_hints(func) for param, hint in hints.items(): if is_optional_type(hint): print(f"Parameter '{param}' is optional") else: print(f"Parameter '{param}' is required") # Before fix def create_user(name: str, roles: list[str] = None): pass # After fix def create_user(name: str, roles: Optional[list[str]] = None): pass # Generated docs would change from "roles is required" to "roles is optional" ```
The PR add the fix safety section for rule
RUF013(#15584 )The fix was introduced here #4831
The rule as a lot of False Negative (as it is explained in the docs of the rule).
The main reason because the fix is unsafe is that it could change code generation tools behaviour, as in the example here: