Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUF028 - Flag unused variables in tuple unpacking. #9936

Closed
wants to merge 2 commits into from

Conversation

Tsdevendra1
Copy link

Related to #8884

Summary

It was suggested in #8884 that we have a separate rule for tuple unpacking. There was a bit of back and forth so I wasn't sure what the final requirements were. The following is what I went with:

Requirements implemented:

  • Create a new rule
  • In the error message make it clear that the error can be fixed by prefixing the variable with an underscore
  • Provide an autofix which prefixes unused variables with an underscore
  • Ignore tuples which are entirely unused in favour of F841

Things I was unsure of:

  • Was F841 meant to be refactored to remove the underscore fix so that it is only performed by this new rule?
  • Should F841 be modified to allow disabling the remove left-hand side part of it?
  • If there is a tuple a, b = c, d = foo() and only a is used, c and d will be flagged for a fix. But in #8884 it was mentioned that this new rule should only flag if the entire tuple is unused. Since the c, d tuple is entirely unused, should it be ignored in this rule?

Test Plan

cargo test

@Tsdevendra1 Tsdevendra1 reopened this Feb 11, 2024
Copy link

codspeed-hq bot commented Feb 11, 2024

CodSpeed Performance Report

Merging #9936 will not alter performance

Comparing Tsdevendra1:unused-tuple-element (88c3db5) with main (a50e278)

Summary

✅ 30 untouched benchmarks

Copy link
Contributor

github-actions bot commented Feb 11, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+327 -0 violations, +0 -0 fixes in 12 projects; 31 projects unchanged)

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

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

+ rasa/telemetry.py:594:30: RUF028 Local variable `tb` is assigned to but never used. Consider renaming to `_tb`.
+ rasa/telemetry.py:594:9: RUF028 Local variable `exc_type` is assigned to but never used. Consider renaming to `_exc_type`.
+ tests/cli/test_rasa_export.py:278:5: RUF028 Local variable `events` is assigned to but never used. Consider renaming to `_events`.
+ tests/core/evaluation/test_marker.py:287:13: RUF028 Local variable `expected` is assigned to but never used. Consider renaming to `_expected`.
+ tests/core/evaluation/test_marker.py:307:13: RUF028 Local variable `expected` is assigned to but never used. Consider renaming to `_expected`.
+ tests/core/evaluation/test_marker.py:558:13: RUF028 Local variable `expected_size` is assigned to but never used. Consider renaming to `_expected_size`.
+ tests/core/evaluation/test_marker.py:583:13: RUF028 Local variable `expected_size` is assigned to but never used. Consider renaming to `_expected_size`.
+ tests/core/test_evaluation.py:152:23: RUF028 Local variable `num_stories` is assigned to but never used. Consider renaming to `_num_stories`.
+ tests/core/test_evaluation.py:319:23: RUF028 Local variable `num_stories` is assigned to but never used. Consider renaming to `_num_stories`.
+ tests/core/training/test_interactive.py:818:11: RUF028 Local variable `kwargs` is assigned to but never used. Consider renaming to `_kwargs`.
... 25 additional changes omitted for project

apache/airflow (+61 -0 violations, +0 -0 fixes)

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

+ airflow/configuration.py:683:25: RUF028 Local variable `should_continue` is assigned to but never used. Consider renaming to `_should_continue`.
+ airflow/providers/cncf/kubernetes/operators/spark_kubernetes.py:223:21: RUF028 Local variable `spark_obj_spec` is assigned to but never used. Consider renaming to `_spark_obj_spec`.
+ airflow/providers/google/cloud/log/gcs_task_handler.py:177:33: RUF028 Local variable `stackinfo` is assigned to but never used. Consider renaming to `_stackinfo`.
+ airflow/providers/google/cloud/transfers/s3_to_gcs.py:228:9: RUF028 Local variable `gcs_bucket` is assigned to but never used. Consider renaming to `_gcs_bucket`.
+ airflow/providers/google/cloud/transfers/s3_to_gcs.py:242:9: RUF028 Local variable `gcs_bucket` is assigned to but never used. Consider renaming to `_gcs_bucket`.
+ airflow/providers/google/cloud/transfers/s3_to_gcs.py:250:30: RUF028 Local variable `dest_gcs_object_prefix` is assigned to but never used. Consider renaming to `_dest_gcs_object_prefix`.
+ airflow/utils/log/task_context_logger.py:104:37: RUF028 Local variable `stackinfo` is assigned to but never used. Consider renaming to `_stackinfo`.
+ dev/breeze/src/airflow_breeze/commands/ci_image_commands.py:384:13: RUF028 Local variable `child_pid` is assigned to but never used. Consider renaming to `_child_pid`.
+ dev/breeze/src/airflow_breeze/commands/ci_image_commands.py:660:22: RUF028 Local variable `info` is assigned to but never used. Consider renaming to `_info`.
+ dev/breeze/src/airflow_breeze/commands/production_image_commands.py:590:22: RUF028 Local variable `info` is assigned to but never used. Consider renaming to `_info`.
... 51 additional changes omitted for project

bokeh/bokeh (+23 -0 violations, +0 -0 fixes)

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

+ src/bokeh/sphinxext/bokeh_model.py:121:34: RUF028 Local variable `arglist` is assigned to but never used. Consider renaming to `_arglist`.
+ src/bokeh/sphinxext/bokeh_model.py:121:43: RUF028 Local variable `retann` is assigned to but never used. Consider renaming to `_retann`.
+ src/bokeh/sphinxext/bokeh_model.py:121:9: RUF028 Local variable `name_prefix` is assigned to but never used. Consider renaming to `_name_prefix`.
+ src/bokeh/sphinxext/bokeh_options.py:108:36: RUF028 Local variable `arglist` is assigned to but never used. Consider renaming to `_arglist`.
+ src/bokeh/sphinxext/bokeh_options.py:108:45: RUF028 Local variable `retann` is assigned to but never used. Consider renaming to `_retann`.
+ src/bokeh/sphinxext/bokeh_options.py:108:9: RUF028 Local variable `name_prefix` is assigned to but never used. Consider renaming to `_name_prefix`.
+ src/bokeh/sphinxext/bokeh_plot.py:254:15: RUF028 Local variable `lineno` is assigned to but never used. Consider renaming to `_lineno`.
+ src/bokeh/sphinxext/bokeh_settings.py:84:32: RUF028 Local variable `arglist` is assigned to but never used. Consider renaming to `_arglist`.
+ src/bokeh/sphinxext/bokeh_settings.py:84:41: RUF028 Local variable `retann` is assigned to but never used. Consider renaming to `_retann`.
+ src/bokeh/sphinxext/bokeh_settings.py:84:9: RUF028 Local variable `name_prefix` is assigned to but never used. Consider renaming to `_name_prefix`.
... 13 additional changes omitted for project

ibis-project/ibis (+2 -0 violations, +0 -0 fixes)

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

+ ibis/backends/tests/test_set_ops.py:40:12: RUF028 Local variable `c` is assigned to but never used. Consider renaming to `_c`.
+ ibis/backends/tests/test_set_ops.py:40:25: RUF028 Local variable `dc` is assigned to but never used. Consider renaming to `_dc`.

milvus-io/pymilvus (+1 -0 violations, +0 -0 fixes)

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

+ examples/collection.py:144:5: RUF028 Local variable `raw_vectors` is assigned to but never used. Consider renaming to `_raw_vectors`.

pandas-dev/pandas (+75 -0 violations, +0 -0 fixes)

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

+ doc/make.py:80:9: RUF028 Local variable `base_name` is assigned to but never used. Consider renaming to `_base_name`.
+ doc/source/conf.py:615:18: RUF028 Local variable `parent` is assigned to but never used. Consider renaming to `_parent`.
+ doc/source/conf.py:615:26: RUF028 Local variable `modname` is assigned to but never used. Consider renaming to `_modname`.
+ pandas/core/_numba/kernels/mean_.py:172:23: RUF028 Local variable `comp_arr` is assigned to but never used. Consider renaming to `_comp_arr`.
+ pandas/core/_numba/kernels/sum_.py:224:23: RUF028 Local variable `comp_arr` is assigned to but never used. Consider renaming to `_comp_arr`.
+ pandas/core/arrays/datetimelike.py:1105:19: RUF028 Local variable `o_mask` is assigned to but never used. Consider renaming to `_o_mask`.
+ pandas/core/arrays/datetimelike.py:1168:19: RUF028 Local variable `o_mask` is assigned to but never used. Consider renaming to `_o_mask`.
+ pandas/core/arrays/datetimelike.py:1232:19: RUF028 Local variable `o_mask` is assigned to but never used. Consider renaming to `_o_mask`.
+ pandas/core/computation/expr.py:741:17: RUF028 Local variable `op_class` is assigned to but never used. Consider renaming to `_op_class`.
+ pandas/core/frame.py:1102:16: RUF028 Local variable `height` is assigned to but never used. Consider renaming to `_height`.
+ pandas/core/frame.py:7751:9: RUF028 Local variable `cols` is assigned to but never used. Consider renaming to `_cols`.
... 64 additional changes omitted for project

pypa/build (+3 -0 violations, +0 -0 fixes)

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

+ tests/test_main.py:119:10: RUF028 Local variable `err` is assigned to but never used. Consider renaming to `_err`.
+ tests/test_main.py:306:13: RUF028 Local variable `stderr` is assigned to but never used. Consider renaming to `_stderr`.
+ tests/test_projectbuilder.py:417:10: RUF028 Local variable `err` is assigned to but never used. Consider renaming to `_err`.

pypa/cibuildwheel (+3 -0 violations, +0 -0 fixes)

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

+ unit_test/architecture_test.py:31:5: RUF028 Local variable `platform_name` is assigned to but never used. Consider renaming to `_platform_name`.
+ unit_test/architecture_test.py:55:5: RUF028 Local variable `platform_name` is assigned to but never used. Consider renaming to `_platform_name`.
+ unit_test/architecture_test.py:71:5: RUF028 Local variable `platform_name` is assigned to but never used. Consider renaming to `_platform_name`.

python-poetry/poetry (+4 -0 violations, +0 -0 fixes)

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

+ src/poetry/repositories/link_sources/base.py:80:19: RUF028 Local variable `ext` is assigned to but never used. Consider renaming to `_ext`.
+ src/poetry/utils/helpers.py:258:10: RUF028 Local variable `type` is assigned to but never used. Consider renaming to `_type`.
+ tests/console/test_application.py:126:10: RUF028 Local variable `name` is assigned to but never used. Consider renaming to `_name`.
+ tests/console/test_application.py:126:16: RUF028 Local variable `args` is assigned to but never used. Consider renaming to `_args`.

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

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

+ tests/test_command_line.py:28:10: RUF028 Local variable `err` is assigned to but never used. Consider renaming to `_err`.
+ tests/test_command_line.py:36:10: RUF028 Local variable `err` is assigned to but never used. Consider renaming to `_err`.
+ tests/test_command_line.py:45:10: RUF028 Local variable `err` is assigned to but never used. Consider renaming to `_err`.

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

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

+ analytics/management/commands/populate_analytics_db.py:291:23: RUF028 Local variable `created` is assigned to but never used. Consider renaming to `_created`.
+ analytics/tests/test_counts.py:1722:17: RUF028 Local variable `recipient` is assigned to but never used. Consider renaming to `_recipient`.
+ analytics/tests/test_counts.py:952:9: RUF028 Local variable `stream2` is assigned to but never used. Consider renaming to `_stream2`.
+ analytics/views/stats.py:161:13: RUF028 Local variable `ignored_sub` is assigned to but never used. Consider renaming to `_ignored_sub`.
+ corporate/lib/stripe.py:3407:23: RUF028 Local variable `created` is assigned to but never used. Consider renaming to `_created`.
+ corporate/tests/test_stripe.py:1308:24: RUF028 Local variable `invoice1` is assigned to but never used. Consider renaming to `_invoice1`.
+ corporate/tests/test_stripe.py:1308:34: RUF028 Local variable `invoice2` is assigned to but never used. Consider renaming to `_invoice2`.
+ corporate/tests/test_stripe.py:2588:30: RUF028 Local variable `invoice2` is assigned to but never used. Consider renaming to `_invoice2`.
+ corporate/tests/test_stripe.py:2588:40: RUF028 Local variable `invoice3` is assigned to but never used. Consider renaming to `_invoice3`.
+ corporate/tests/test_stripe.py:2588:50: RUF028 Local variable `invoice4` is assigned to but never used. Consider renaming to `_invoice4`.
+ corporate/tests/test_stripe.py:2706:20: RUF028 Local variable `invoice1` is assigned to but never used. Consider renaming to `_invoice1`.
+ corporate/tests/test_stripe.py:2706:30: RUF028 Local variable `invoice2` is assigned to but never used. Consider renaming to `_invoice2`.
+ corporate/tests/test_stripe.py:2879:20: RUF028 Local variable `invoice1` is assigned to but never used. Consider renaming to `_invoice1`.
+ corporate/tests/test_stripe.py:2879:30: RUF028 Local variable `invoice2` is assigned to but never used. Consider renaming to `_invoice2`.
+ corporate/tests/test_stripe.py:5091:20: RUF028 Local variable `invoice1` is assigned to but never used. Consider renaming to `_invoice1`.
+ corporate/tests/test_stripe.py:5147:20: RUF028 Local variable `invoice1` is assigned to but never used. Consider renaming to `_invoice1`.
+ corporate/tests/test_stripe.py:5877:9: RUF028 Local variable `flat_discount` is assigned to but never used. Consider renaming to `_flat_discount`.
... 99 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF028 327 327 0 0 0

@charliermarsh charliermarsh self-requested a review March 9, 2024 12:50
@MichaReiser
Copy link
Member

Thanks for opening this PR and I'm sorry that it took us so long to get back to you.

We discussed this rule internally, and we haven't come to an agreement if it's the right call to split unused-variables (and changing the intent of unused-variables) to allow opting into fixing unused-tuple-elements but not other unused variables). We concluded that we need a more formal guideline on when it is okay to have multiple rules that catch the same or a very similar code smell and when it should be a single rule.

I prefer not to merge this PR today because we lack that agreement. I would rather keep the status quo than change the rule and risk having to change it back in a few months.

I'm sorry that we gave the wrong impression and that it took us so long to get back to you. Thanks again for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants