Use unused variable detection to power incorrect-dict-iterator

[charliermarsh]

Summary

PERF102 looks for unused keys or values in dict.items() calls, and suggests instead using dict.keys() or dict.values(). Previously, this check determined usage by looking for underscore-prefixed variables. However, we can use the semantic model to actually detect whether a variable is used. This has two nice effects:

1. We avoid odd false-positives whereby underscore-prefixed variables are actually used.
2. We can catch more cases (fewer false-negatives) by detecting unused loop variables that aren't underscore-prefixed.

Closes #5692.

[charliermarsh]

Now that this rule relies on scoping, it's easier to put every case in its own scope. Otherwise, each case kind of pollutes every other case (if they're all in the module scope).

[charliermarsh]

This and the next case are new.

[charliermarsh]

This does have some weird false negatives, like:

def f():
    # Unfixable (false negative) due to usage of `bar` outside of loop.
    for foo, bar in dict.items():
        break

    bar = 1
    print(bar)

We'd consider bar used, because it's shadowed by a future bar, and that shadowed variable is used. We can fix this with better control flow logic, but it's fine for now, B007 already suffers from this.

[github-actions]

PR Check Results

Ecosystem

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

airflow (+5, -0)

+ airflow/executors/kubernetes_executor_utils.py:451:40: PERF102 [*] When using only the values of a dict use the `values()` method
+ airflow/models/dag.py:3282:21: PERF102 [*] When using only the values of a dict use the `values()` method
+ docs/conf.py:428:36: PERF102 [*] When using only the values of a dict use the `values()` method
+ docs/conf.py:438:33: PERF102 [*] When using only the values of a dict use the `values()` method
+ tests/utils/test_db_cleanup.py:287:39: PERF102 [*] When using only the values of a dict use the `values()` method

bokeh (+1, -0)

+ src/bokeh/core/property/wrappers.py:470:21: PERF102 [*] When using only the keys of a dict use the `keys()` method

zulip (+11, -0)

+ tools/setup/emoji/emoji_setup_utils.py:56:34: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/data_import/slack.py:368:25: PERF102 [*] When using only the keys of a dict use the `keys()` method
+ zerver/lib/cache.py:373:21: PERF102 [*] When using only the keys of a dict use the `keys()` method
+ zerver/lib/rate_limiter.py:285:43: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/lib/streams.py:74:40: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/lib/test_classes.py:1503:26: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/management/commands/compilemessages.py:148:31: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/models.py:1079:43: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/models.py:1218:33: PERF102 [*] When using only the values of a dict use the `values()` method
+ zerver/tests/test_events.py:1947:40: PERF102 [*] When using only the keys of a dict use the `keys()` method
+ zerver/tests/test_slack_importer.py:1348:30: PERF102 [*] When using only the values of a dict use the `values()` method

Rules changed: 1

Rule	Changes	Additions	Removals
PERF102	17	17	0

Benchmark

Linux

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.02     10.0±0.49ms     4.1 MB/sec     1.00      9.8±0.27ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.00  1980.3±102.13µs     8.4 MB/sec    1.02      2.0±0.09ms     8.3 MB/sec
formatter/numpy/globals.py                 1.00   215.4±11.30µs    13.7 MB/sec     1.04   224.3±20.52µs    13.2 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.12ms     6.1 MB/sec     1.01      4.2±0.20ms     6.0 MB/sec
linter/all-rules/large/dataset.py          1.18     17.1±0.37ms     2.4 MB/sec     1.00     14.5±0.48ms     2.8 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.11      4.1±0.07ms     4.1 MB/sec     1.00      3.7±0.12ms     4.5 MB/sec
linter/all-rules/numpy/globals.py          1.11    538.7±7.80µs     5.5 MB/sec     1.00   485.1±22.93µs     6.1 MB/sec
linter/all-rules/pydantic/types.py         1.13      7.5±0.11ms     3.4 MB/sec     1.00      6.6±0.29ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.24      8.8±0.13ms     4.6 MB/sec     1.00      7.1±0.26ms     5.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.21   1926.0±9.28µs     8.6 MB/sec     1.00  1590.5±54.38µs    10.5 MB/sec
linter/default-rules/numpy/globals.py      1.12    209.7±2.53µs    14.1 MB/sec     1.00    187.4±5.54µs    15.7 MB/sec
linter/default-rules/pydantic/types.py     1.22      3.9±0.04ms     6.5 MB/sec     1.00      3.2±0.08ms     8.0 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.02     11.2±0.14ms     3.6 MB/sec    1.00     11.0±0.15ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.2±0.04ms     7.6 MB/sec    1.00      2.2±0.11ms     7.6 MB/sec
formatter/numpy/globals.py                 1.00    247.5±4.91µs    11.9 MB/sec    1.02    251.6±9.85µs    11.7 MB/sec
formatter/pydantic/types.py                1.01      4.8±0.06ms     5.4 MB/sec    1.00      4.7±0.06ms     5.4 MB/sec
linter/all-rules/large/dataset.py          1.02     16.5±0.39ms     2.5 MB/sec    1.00     16.2±0.45ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.1±0.06ms     4.0 MB/sec    1.00      4.1±0.05ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.01   504.7±15.77µs     5.8 MB/sec    1.00   497.9±10.08µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.01      7.2±0.13ms     3.5 MB/sec    1.00      7.2±0.12ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.1±0.09ms     5.0 MB/sec    1.02      8.3±0.15ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.02  1731.6±24.37µs     9.6 MB/sec    1.00  1704.2±20.45µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.04    203.7±2.84µs    14.5 MB/sec    1.00    196.2±4.14µs    15.0 MB/sec
linter/default-rules/pydantic/types.py     1.02      3.7±0.04ms     7.0 MB/sec    1.00      3.6±0.04ms     7.1 MB/sec

[charliermarsh]

The ecosystem checks generally look right to me, although in Airflow I think there's one false positive due to a non-dictionary that implements .items().

[zanieb]

Was this filter just missing previously or... ?

[zanieb]

(I spent an embarrassing amount of time trying to figure out how this did the change described in the PR title before realizing it was a different rule)

[charliermarsh]

Yeah, sorry, the fix to incorrect-dict-iterator is based on the way this existing rule operates, and I noticed that they would both benefit from this condition. (We don't care about bindings that occur before the loop.)

[zanieb]

Cool!

Edit: Also read the ecosystem checks and agree they look fine