Skip to content

[flake8-simplify] Further simplify to binary in preview for if-else-block-instead-of-if-exp (SIM108)#12796

Merged
charliermarsh merged 13 commits intoastral-sh:mainfrom
dylwil3:sim108-boolean-simplify
Aug 10, 2024
Merged

[flake8-simplify] Further simplify to binary in preview for if-else-block-instead-of-if-exp (SIM108)#12796
charliermarsh merged 13 commits intoastral-sh:mainfrom
dylwil3:sim108-boolean-simplify

Conversation

@dylwil3
Copy link
Copy Markdown
Collaborator

@dylwil3 dylwil3 commented Aug 10, 2024

In most cases we should suggest a ternary operator, but there are three edge cases where a binary operator is more appropriate.

Given an if-else block of the form

if test:
    target_var = body_value
else:
    target_var = else_value

This PR updates the check for SIM108 to the following:

  • If test == body_value and preview enabled, suggest to replace with target_var = test or else_value
  • If test == not body_value and preview enabled, suggest to replace with target_var = body_value and else_value
  • If not test == body_value and preview enabled, suggest to replace with target_var = body_value and else_value
  • Otherwise, suggest to replace with target_var = body_value if test else else_value

Closes #12189.

@dylwil3
Copy link
Copy Markdown
Collaborator Author

dylwil3 commented Aug 10, 2024 •
edited
Loading

The edge cases in question are pretty rare - not sure if there is a better way to organize the code (for performance and readability) to take advantage of that.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 10, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

apache/airflow (+7 -4 violations, +0 -0 fixes) 

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

+ airflow/api/common/mark_tasks.py:311:5: SIM108 Use binary operator `start_date = dag.start_date or execution_date` instead of `if`-`else`-block
- airflow/api/common/mark_tasks.py:311:5: SIM108 Use ternary operator `start_date = dag.start_date if dag.start_date else execution_date` instead of `if`-`else`-block
+ airflow/cli/commands/celery_command.py:213:5: SIM108 Use binary operator `umask = args.umask or conf.get("celery", "worker_umask", fallback=settings.DAEMON_UMASK)` instead of `if`-`else`-block
- airflow/cli/commands/celery_command.py:213:5: SIM108 Use ternary operator `umask = args.umask if args.umask else conf.get("celery", "worker_umask", fallback=settings.DAEMON_UMASK)` instead of `if`-`else`-block
+ airflow/providers/celery/cli/celery_command.py:229:5: SIM108 Use binary operator `umask = args.umask or conf.get("celery", "worker_umask", fallback=settings.DAEMON_UMASK)` instead of `if`-`else`-block
- airflow/providers/celery/cli/celery_command.py:229:5: SIM108 Use ternary operator `umask = args.umask if args.umask else conf.get("celery", "worker_umask", fallback=settings.DAEMON_UMASK)` instead of `if`-`else`-block
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py:153:13: SIM108 Use binary operator `namespaces = self.kube_config.multi_namespace_mode_namespace_list or [None]` instead of `if`-`else`-block
+ airflow/providers/databricks/operators/databricks_sql.py:139:17: SIM108 Use binary operator `csv_params = self._csv_params or {}` instead of `if`-`else`-block
- airflow/providers/databricks/operators/databricks_sql.py:139:17: SIM108 Use ternary operator `csv_params = self._csv_params if self._csv_params else {}` instead of `if`-`else`-block
+ airflow/providers/imap/hooks/imap.py:91:13: SIM108 Use binary operator `ssl_context_string = extra_ssl_context or conf.get("imap", "SSL_CONTEXT", fallback=None)` instead of `if`-`else`-block
+ airflow/providers/smtp/hooks/smtp.py:118:13: SIM108 Use binary operator `ssl_context_string = extra_ssl_context or conf.get("smtp_provider", "SSL_CONTEXT", fallback=None)` instead of `if`-`else`-block

scikit-build/scikit-build (+1 -0 violations, +0 -0 fixes) 

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

+ skbuild/setuptools_wrap.py:532:5: SIM108 Use binary operator `cmake_install_target = cmake_install_target_from_command or cmake_install_target_from_setup` instead of `if`-`else`-block

zulip/zulip (+1 -1 violations, +0 -0 fixes) 

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

+ zerver/webhooks/bitbucket3/view.py:78:5: SIM108 Use binary operator `topic_name = include_title or "Bitbucket Server Ping"` instead of `if`-`else`-block
- zerver/webhooks/bitbucket3/view.py:78:5: SIM108 Use ternary operator `topic_name = include_title if include_title else "Bitbucket Server Ping"` instead of `if`-`else`-block

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
SIM108 14 9 5 0 0

@charliermarsh
Copy link
Copy Markdown
Member

charliermarsh commented Aug 10, 2024

Sweet, thanks!

@charliermarsh charliermarsh self-assigned this Aug 10, 2024
@charliermarsh charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels Aug 10, 2024
charliermarsh
charliermarsh approved these changes Aug 10, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

This looks good to me. I just added a check to ensure there's no effect (like a function call) in the test, since it no longer gets repeated.

@charliermarsh charliermarsh enabled auto-merge (squash) August 10, 2024 16:41
@charliermarsh charliermarsh merged commit 0c2b88f into astral-sh:main Aug 10, 2024
@dylwil3 dylwil3 deleted the sim108-boolean-simplify branch August 10, 2024 18:06
@BrewTestBot BrewTestBot mentioned this pull request Aug 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

@charliermarsh charliermarsh

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SIM108 suggested for code block that could have been simplified to an or

2 participants

@dylwil3 @charliermarsh