Skip to content

[ruff] Make fix for implicit-optional (RUF013) safe in preview#17836

Closed
dylwil3 wants to merge 3 commits intoastral-sh:mainfrom
dylwil3:implicit-optional-safe
Closed

[ruff] Make fix for implicit-optional (RUF013) safe in preview#17836
dylwil3 wants to merge 3 commits intoastral-sh:mainfrom
dylwil3:implicit-optional-safe

Conversation

@dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented May 4, 2025

The fix for implicit-optional (RUF013), introduced in #4831, was marked as unsafe, but I can't actually see a reason why it would be. So this PR makes the fix safe when preview is enabled.

Some potential reasons why it is marked as unsafe:

  1. When the PR was opened, there were some false positives - but those are now simply skipped.
  2. The fix may have to import Optional - but we use try_set_fix to abort if that wasn't possible for some reason.
  3. One may be worried that the pipe notation is used for the union which could introduce a syntax error - but we are careful to check the target version.

But it's possible I'm missing something here so please let me know!

@dylwil3 dylwil3 added fixes Related to suggested fixes for violations preview Related to preview mode features labels May 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +62 -0 fixes in 3 projects; 52 projects unchanged)

RasaHQ/rasa (+0 -0 violations, +6 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

- stubs/aio_pika/robust_channel.pyi:15:18: RUF013 PEP 484 prohibits implicit `Optional`
+ stubs/aio_pika/robust_channel.pyi:15:18: RUF013 [*] PEP 484 prohibits implicit `Optional`
- stubs/aio_pika/robust_channel.pyi:19:20: RUF013 PEP 484 prohibits implicit `Optional`
+ stubs/aio_pika/robust_channel.pyi:19:20: RUF013 [*] PEP 484 prohibits implicit `Optional`
- stubs/socketio.pyi:51:64: RUF013 PEP 484 prohibits implicit `Optional`
+ stubs/socketio.pyi:51:64: RUF013 [*] PEP 484 prohibits implicit `Optional`

binary-husky/gpt_academic (+0 -0 violations, +50 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

- crazy_functions/vector_fns/general_file_loader.py:10:58: RUF013 PEP 484 prohibits implicit `Optional`
+ crazy_functions/vector_fns/general_file_loader.py:10:58: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_all.py:1538:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_all.py:1538:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_chatgpt.py:128:115: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_chatgpt.py:128:115: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_chatgpt.py:227:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_chatgpt.py:227:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_cohere.py:128:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_cohere.py:128:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_cohere.py:71:115: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_cohere.py:71:115: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_google_gemini.py:57:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_google_gemini.py:57:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_moonshot.py:150:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_moonshot.py:150:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_openrouter.py:122:115: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_openrouter.py:122:115: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_openrouter.py:209:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_openrouter.py:209:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_taichu.py:43:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_taichu.py:43:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/bridge_zhipu.py:48:84: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/bridge_zhipu.py:48:84: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/chatglmoonx.py:188:37: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/chatglmoonx.py:188:37: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/com_google.py:62:51: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/com_google.py:62:51: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/edge_gpt_free.py:443:18: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/edge_gpt_free.py:443:18: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/edge_gpt_free.py:658:18: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/edge_gpt_free.py:658:18: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/edge_gpt_free.py:684:18: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/edge_gpt_free.py:684:18: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/embed_models/openai_embed.py:36:35: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/embed_models/openai_embed.py:36:35: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/embed_models/openai_embed.py:42:63: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/embed_models/openai_embed.py:42:63: RUF013 [*] PEP 484 prohibits implicit `Optional`
- request_llms/local_llm_class.py:266:88: RUF013 PEP 484 prohibits implicit `Optional`
+ request_llms/local_llm_class.py:266:88: RUF013 [*] PEP 484 prohibits implicit `Optional`
... 10 additional changes omitted for project

python/typeshed (+0 -0 violations, +6 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

- stdlib/_sitebuiltins.pyi:9:30: RUF013 PEP 484 prohibits implicit `Optional`
+ stdlib/_sitebuiltins.pyi:9:30: RUF013 [*] PEP 484 prohibits implicit `Optional`
- stdlib/lib2to3/main.pyi:29:19: RUF013 PEP 484 prohibits implicit `Optional`
+ stdlib/lib2to3/main.pyi:29:19: RUF013 [*] PEP 484 prohibits implicit `Optional`
- stdlib/pickle.pyi:214:26: RUF013 PEP 484 prohibits implicit `Optional`
+ stdlib/pickle.pyi:214:26: RUF013 [*] PEP 484 prohibits implicit `Optional`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF013 62 0 0 62 0

}

// https://github.com/astral-sh/ruff/pull/17836
pub(crate) const fn is_implicit_optional_fix_safe(settings: &LinterSettings) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we can keep the _postfix. It helps readers to easily recognize this as a preview check

@MichaReiser
Copy link
Member

I think the reason why the fix is unsafe is because it's a possibility that the default value is wrong (a leftover from changing the signature from int | None to None. A user has to explicitly express their intention.

@dylwil3 dylwil3 closed this May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixes Related to suggested fixes for violations preview Related to preview mode features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants