Implement UnnecessaryListAllocationForFirstElement

[evanrittenhouse]

Summary

Fixes #5503. Ready for final review as the mkdocs issue involving SSH keys is fixed.

Note that this will only throw on a Name - it will be refactorable once we have a type-checker. This means that this is the only sort of input that will throw.

x = range(10)
list(x)[0]

I thought it'd be confusing if we supported direct function results. Consider this example, assuming we support direct results:

# throws
list(range(10))[0]

def createRange(bound):
    return range(bound)

# "why doesn't this throw, but a direct `range(10)` call does?"
list(createRange(10))[0]

If it's necessary, I can go through the list of built-ins and find those which produce iterables, then add them to the throwing list.

Test Plan

Added a new fixture, then ran cargo t

[github-actions]

PR Check Results

Ecosystem

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

scikit-build (+1, -0)

+ skbuild/setuptools_wrap.py:248:17: RUF015 [*] Prefer `next(iter(plat_names))` over `list(plat_names)[0]`

zulip (+1, -0)

+ zerver/actions/streams.py:435:39: RUF015 [*] Prefer `next(iter(altered_user_ids))` over `list(altered_user_ids)[0]`

Rules changed: 1

Rule	Changes	Additions	Removals
RUF015	2	2	0

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.5±0.01ms     4.8 MB/sec    1.00      8.4±0.01ms     4.8 MB/sec
formatter/numpy/ctypeslib.py               1.00   1812.9±5.91µs     9.2 MB/sec    1.00   1805.3±4.70µs     9.2 MB/sec
formatter/numpy/globals.py                 1.01    199.6±0.66µs    14.8 MB/sec    1.00    197.9±0.38µs    14.9 MB/sec
formatter/pydantic/types.py                1.02      4.1±0.01ms     6.2 MB/sec    1.00      4.0±0.01ms     6.3 MB/sec
linter/all-rules/large/dataset.py          1.00     14.0±0.02ms     2.9 MB/sec    1.01     14.2±0.06ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.02ms     4.7 MB/sec    1.00      3.5±0.01ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    366.8±1.41µs     8.0 MB/sec    1.00    365.1±1.25µs     8.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.2±0.01ms     4.1 MB/sec    1.02      6.3±0.01ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.00      7.1±0.01ms     5.7 MB/sec    1.00      7.1±0.01ms     5.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1473.9±4.55µs    11.3 MB/sec    1.00   1473.1±1.69µs    11.3 MB/sec
linter/default-rules/numpy/globals.py      1.01    158.4±0.26µs    18.6 MB/sec    1.00    157.3±0.29µs    18.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.01ms     8.0 MB/sec    1.00      3.2±0.01ms     8.0 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.6±0.07ms     4.2 MB/sec    1.00      9.6±0.06ms     4.2 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.0±0.02ms     8.2 MB/sec    1.00      2.0±0.02ms     8.2 MB/sec
formatter/numpy/globals.py                 1.00    220.5±2.48µs    13.4 MB/sec    1.00    221.6±5.05µs    13.3 MB/sec
formatter/pydantic/types.py                1.02      4.7±0.04ms     5.4 MB/sec    1.00      4.6±0.02ms     5.5 MB/sec
linter/all-rules/large/dataset.py          1.01     15.9±0.11ms     2.6 MB/sec    1.00     15.8±0.26ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.03ms     4.0 MB/sec    1.00      4.2±0.03ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.01    435.9±6.02µs     6.8 MB/sec    1.00    431.1±5.81µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.02      7.2±0.09ms     3.6 MB/sec    1.00      7.0±0.06ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.1±0.04ms     5.0 MB/sec    1.01      8.2±0.07ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1681.4±11.77µs     9.9 MB/sec    1.01  1696.9±18.50µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.02    182.0±5.47µs    16.2 MB/sec    1.00    179.0±1.46µs    16.5 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.7±0.02ms     6.9 MB/sec    1.00      3.6±0.04ms     7.0 MB/sec

[evanrittenhouse]

@charliermarsh was wondering if I could grab a review on this so I can avoid needing to rename in case RUF015 gets taken soon :)

[charliermarsh]

Haven't reviewed the code yet (but I will soon), but note that this will introduce at least one behavior change, which is that the exception thrown in the empty case is different:

>>> next(iter([]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
StopIteration
>>> [][0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range

[dhruvmanila]

Do we need to test whether we're in a slice or not? Are there any examples where the node is Expr::Slice but in code it's something else?

[evanrittenhouse]

@dhruvmanila sorry, could you please provide an example? A bit unclear on what you mean

[dhruvmanila]

Sorry for being unclear. Here, I'm assuming you're checking whether the node is a slice element ([1:]) as oppose to a constant ([1]). The heuristic you're using is to check if the upper and step element aren't present. But, we've already identified that the node is a slice using the match above.

Examples:

• [1] will be a Constant node
• [1:] will be a Slice node with upper and step as None

I'm just trying to figure out the motivation for using this heuristic to check if the node is Slice or not as I think we could remove it.

[evanrittenhouse]

Ah, I see your point - yep, this is an unnecessary heuristic and a relic of old changes. Thanks for the detailed review

[evanrittenhouse]

Haven't reviewed the code yet (but I will soon), but note that this will introduce at least one behavior change, which is that the exception thrown in the empty case is different:

@charliermarsh Yeah, I figured it'd be fine since it's a suggested fix. I don't think there's a way to get around it though, is there?

[charliermarsh]

Will get to this today.

[charliermarsh]

Can &mut be removed here?

[charliermarsh]

Nit: can use elt.is_name_expr here.

[charliermarsh]

Nit: can use let [generator] = generators.as_slice() else { return None; } here.

[charliermarsh]

I think this down to the end of the method is equivalent to just get_name_id(&generator.iter), right?