Extend `repeated-equality-comparison-target` to check for mixed orderings and Yoda conditions. by tjkuson · Pull Request #6691 · astral-sh/ruff

tjkuson · 2023-08-19T14:09:55Z

Summary

Closes #5987. I am not convinced this is the most efficient way to do this, but it works.

Test Plan

cargo test

github-actions · 2023-08-19T14:28:06Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+16, -0, 0 error(s))

airflow (+8, -0)

+ airflow/providers/common/sql/operators/sql.py:973:16: PLR1714 Consider merging multiple comparisons: `0 in (cur, ref)`. Use a `set` if the elements are hashable.
+ airflow/providers/databricks/hooks/databricks_base.py:498:16: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ airflow/providers/databricks/hooks/databricks_base.py:507:16: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ airflow/providers/databricks/hooks/databricks_base.py:526:16: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ airflow/providers/databricks/hooks/databricks_base.py:535:16: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ airflow/providers/google/cloud/hooks/bigquery.py:3281:16: PLR1714 Consider merging multiple comparisons: `0 in (cur, ref)`. Use a `set` if the elements are hashable.
+ airflow/providers/google/cloud/triggers/bigquery.py:358:22: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ tests/providers/celery/executors/test_celery_executor.py:179:24: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.

bokeh (+4, -0)

+ src/bokeh/io/export.py:126:8: PLR1714 Consider merging multiple comparisons: `0 in (image.width, image.height)`. Use a `set` if the elements are hashable.
+ src/bokeh/layouts.py:450:20: PLR1714 Consider merging multiple comparisons: `0 not in (child.nrows, child.ncols)`. Use a `set` if the elements are hashable.
+ src/bokeh/server/util.py:236:12: PLR1714 Consider merging multiple comparisons: `p in (h, '*')`. Use a `set` if the elements are hashable.
+ src/bokeh/sphinxext/bokeh_sampledata_xref.py:176:16: PLR1714 Consider merging multiple comparisons: `node.subfolder in ('all', sp[-2])`. Use a `set` if the elements are hashable.

zulip (+4, -0)

+ zerver/actions/message_edit.py:348:9: PLR1714 Consider merging multiple comparisons. Use a `set` if the elements are hashable.
+ zerver/forms.py:96:8: PLR1714 Consider merging multiple comparisons: `"-" in (subdomain[0], subdomain[-1])`. Use a `set` if the elements are hashable.
+ zerver/lib/domains.py:15:8: PLR1714 Consider merging multiple comparisons: `"." in (domain[0], domain[-1])`. Use a `set` if the elements are hashable.
+ zerver/lib/domains.py:20:12: PLR1714 Consider merging multiple comparisons: `"-" in (subdomain[0], subdomain[-1])`. Use a `set` if the elements are hashable.

Rules changed: 1

Rule	Changes	Additions	Removals
PLR1714	16	16	0

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      3.8±0.10ms    10.7 MB/sec    1.03      3.9±0.07ms    10.4 MB/sec
formatter/numpy/ctypeslib.py               1.00   809.7±20.43µs    20.6 MB/sec    1.03   833.9±14.49µs    20.0 MB/sec
formatter/numpy/globals.py                 1.01     89.0±2.02µs    33.1 MB/sec    1.00     88.3±2.78µs    33.4 MB/sec
formatter/pydantic/types.py                1.00  1598.0±50.51µs    16.0 MB/sec    1.02  1627.9±31.52µs    15.7 MB/sec
linter/all-rules/large/dataset.py          1.06     12.3±0.19ms     3.3 MB/sec    1.00     11.6±0.19ms     3.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      3.2±0.06ms     5.2 MB/sec    1.00      3.2±0.06ms     5.2 MB/sec
linter/all-rules/numpy/globals.py          1.01    451.4±9.25µs     6.5 MB/sec    1.00   449.0±10.21µs     6.6 MB/sec
linter/all-rules/pydantic/types.py         1.02      6.3±0.13ms     4.1 MB/sec    1.00      6.1±0.10ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.07      6.7±0.10ms     6.1 MB/sec    1.00      6.3±0.09ms     6.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.04  1462.8±30.28µs    11.4 MB/sec    1.00  1409.9±19.34µs    11.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    165.2±4.54µs    17.9 MB/sec    1.00    165.2±4.15µs    17.9 MB/sec
linter/default-rules/pydantic/types.py     1.05      3.0±0.05ms     8.4 MB/sec    1.00      2.9±0.04ms     8.8 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      4.4±0.19ms     9.2 MB/sec    1.02      4.5±0.17ms     9.1 MB/sec
formatter/numpy/ctypeslib.py               1.00   910.6±31.93µs    18.3 MB/sec    1.00   908.5±25.44µs    18.3 MB/sec
formatter/numpy/globals.py                 1.00     94.1±2.55µs    31.4 MB/sec    1.01     95.2±4.36µs    31.0 MB/sec
formatter/pydantic/types.py                1.00  1830.2±64.12µs    13.9 MB/sec    1.02  1866.8±60.21µs    13.7 MB/sec
linter/all-rules/large/dataset.py          1.00     15.4±0.52ms     2.6 MB/sec    1.01     15.5±0.45ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.19ms     4.0 MB/sec    1.02      4.3±0.21ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00   528.0±21.73µs     5.6 MB/sec    1.22  642.5±352.85µs     4.6 MB/sec
linter/all-rules/pydantic/types.py         1.00      8.4±0.53ms     3.1 MB/sec    1.21     10.1±3.47ms     2.5 MB/sec
linter/default-rules/large/dataset.py      1.00      8.5±0.20ms     4.8 MB/sec    1.05      8.9±0.96ms     4.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1810.1±44.79µs     9.2 MB/sec    1.00  1811.3±49.11µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    214.7±7.28µs    13.7 MB/sec    1.00   215.4±10.31µs    13.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.8±0.10ms     6.7 MB/sec    1.03      3.9±0.21ms     6.5 MB/sec

tjkuson · 2023-08-19T15:03:48Z

The results from the ecosystem check seem sensible, unless people object to writing stuff like 0 in (foo, bar).

charliermarsh · 2023-08-20T14:58:26Z

crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs

    }

-    for (left, (count, comparators)) in left_to_comparators {
+    for (value, (count, comparators)) in value_to_comparators {


@tjkuson - What do you think of this? It's a bit closer to the previous version: we just track the comparators with the thing they're compared against (and this time, including the RHS), then raise if any comparator is used multiple times.

charliermarsh · 2023-08-20T14:59:30Z

crates/ruff/resources/test/fixtures/pylint/repeated_equality_comparison_target.py

+
+foo != "a" and "b" != foo and foo != "c"
+
+foo == "a" or foo == "b" or "c" == bar or "d" == bar  # Multiple targets


We have odd behavior for this kind of thing on main already:

49 │+repeated_equality_comparison_target.py:40:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable. 50 │+ | 51 │+38 | foo == bar == "b" or foo == "c" # Multiple comparisons. 52 │+39 | 53 │+40 | foo == "a" or foo == "b" or bar == "c" or bar == "d" # Multiple targets 54 │+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714 55 │+ | 56 │+ 57 │+repeated_equality_comparison_target.py:40:1: PLR1714 Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable. 58 │+ | 59 │+38 | foo == bar == "b" or foo == "c" # Multiple comparisons. 60 │+39 | 61 │+40 | foo == "a" or foo == "b" or bar == "c" or bar == "d" # Multiple targets 62 │+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714

It's not awful because it's not like we autofix this and remove the comparisons, but it'd be nice to have a more targeted range. Anyway...

tjkuson · 2023-08-23T18:32:37Z

Apologies for not getting back to you on this, I was/am on holiday and forgot to add a GitHub notification!

tjkuson · 2023-08-23T18:33:15Z

Apologies for not getting back to you on this, I was/am on holiday away from my computer and forgot to add a GitHub notification!

charliermarsh · 2023-08-23T18:46:02Z

No prob! Enjoy your holiday :) I just biased towards merging after a few days with the thinking that we can always revisit later.

tjkuson added 3 commits August 19, 2023 14:59

Check Yodas

a380270

Extend tests

e587e3b

Extend tests even further

2d04529

Re-add >1 check

45cdbb7

Run cargo fmt

be6f574

charliermarsh self-requested a review August 20, 2023 14:43

charliermarsh added 3 commits August 20, 2023 10:43

Merge branch 'main' into yoda-cond

9f1721a

Minor refactors

252b135

Add tests

bc65d4e

charliermarsh reviewed Aug 20, 2023

View reviewed changes

charliermarsh added the rule Implementing or modifying a lint rule label Aug 20, 2023

Merge branch 'main' into yoda-cond

bebbc20

charliermarsh enabled auto-merge (squash) August 23, 2023 03:39

charliermarsh merged commit 1cb1bd7 into astral-sh:main Aug 23, 2023

chenrui333 mentioned this pull request Aug 26, 2023

ruff 0.0.286 Homebrew/homebrew-core#140518

Merged

tjkuson deleted the yoda-cond branch August 29, 2023 10:44

tjkuson mentioned this pull request Sep 6, 2023

Rename PLR1714 to repeated-equality-comparison #7182

Merged

renovate bot mentioned this pull request Oct 11, 2023

⬆️ deps: Update dependencies (non-major) cattidea/doc-pipe#22

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend `repeated-equality-comparison-target` to check for mixed orderings and Yoda conditions.#6691

Extend `repeated-equality-comparison-target` to check for mixed orderings and Yoda conditions.#6691
charliermarsh merged 9 commits intoastral-sh:mainfrom
tjkuson:yoda-cond

tjkuson commented Aug 19, 2023

Uh oh!

github-actions bot commented Aug 19, 2023 •

edited

Loading

Uh oh!

tjkuson commented Aug 19, 2023

Uh oh!

charliermarsh Aug 20, 2023

Uh oh!

charliermarsh Aug 20, 2023

Uh oh!

tjkuson commented Aug 23, 2023

Uh oh!

tjkuson commented Aug 23, 2023

Uh oh!

charliermarsh commented Aug 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		foo != "a" and "b" != foo and foo != "c"

		foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets

Conversation

tjkuson commented Aug 19, 2023

Summary

Test Plan

Uh oh!

github-actions bot commented Aug 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

tjkuson commented Aug 19, 2023

Uh oh!

charliermarsh Aug 20, 2023

Choose a reason for hiding this comment

Uh oh!

charliermarsh Aug 20, 2023

Choose a reason for hiding this comment

Uh oh!

tjkuson commented Aug 23, 2023

Uh oh!

tjkuson commented Aug 23, 2023

Uh oh!

charliermarsh commented Aug 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Aug 19, 2023 •

edited

Loading