Make UP028 always fixable again

[InSyncWithFoo]

After #16451, UP028 is no longer always fixable. However, a different kind of fix can be offered in such cases:

for some_global, some_nonlocal in iterable:
    yield some_global, some_nonlocal

for _element in iterable:
    some_global, some_nonlocal = _element
    yield some_global, some_nonlocal

I can't think of any edge cases, other than that adding a new name to the scope might break runtime inspections:

nonlocal some_nonlocal

for some_nonlocal in iterable:
    yield some_nonlocal

print(locals())  # {'some_nonlocal': ...}

nonlocal some_nonlocal

for _element in iterable:
    some_nonlocal = _element
    yield some_nonlocal

print(locals())  # {'some_nonlocal': ..., '_element': ...}

Also consider the case where the new name is the same as an existing one:

_element = ...

for _element in iterable:
    some_global, some_nonlocal = _element
    yield some_global, some_nonlocal

print(_element)  # !!!

Generating a new, unique variable name is trivial, but it does, however, necessitate the fix being marked as unsafe to give the user a chance to review that name (along with everything else).

[MichaReiser]

I consider adding such a fix low priority as this pattern should be rare. I'm okay adding it if it has low complexity.

I believe Ruff already has a functionality to introduce new names but that can fail (because of overlaps?)

[InSyncWithFoo]

@MichaReiser Generating a new name can never fail: Ruff just needs to check all names of the form var_{number} from 0 to infinity, stopping as soon as it finds one available (I think I did this once in a PR).

The examples above are all about global/nonlocal, but the same also applies to subscripts and attributes.

[dscorbett]

The point of UP028 is to recommend yield from where possible. The fix proposed here doesn’t use yield from so it isn’t an appropriate fix for UP028: it only fixes it in that it adds an extra statement to the loop so UP028 doesn’t apply. It could be an appropriate fix for a new style rule against assigning to globals via for targets, which is unrelated to yield.

[InSyncWithFoo]

That's a much better idea. Consider this a rule request then.

[MichaReiser]

I'll close this. Feel free to open a new issue if you want to request a rule. A new issue makes it easier to find and should document its motivation