sourcery icon indicating copy to clipboard operation
sourcery copied to clipboard

Variables only used in convoluted runtime type hints may be removed with for-index-underscore

Open JacobHayes opened this issue 2 years ago • 1 comments

Checklist

  • [x] I have searched the Sourcery documentation for the issue, and found nothing
  • [x] I have checked there are no open bugs referencing the same bug or problem

Description

I was testing out sourcery on https://github.com/artigraph/artigraph and it broke one of the tests (but overall looks quite good!), which included some convoluted dynamic function creation that relies on runtime type hinting.

Code snippet that reproduces issue

Here's a minimal example:

from typing import get_type_hints


def test() -> None:
    for hint in [int]:
        class X:
            def f(self) -> hint:  # type: ignore[valid-type]
                return 0

        print(get_type_hints(X.f))


test()

When run as is, it works as expected:

$ python3 x.py
{'return': <class 'int'>}

Sourcery suggest renaming hint to _ (and misses that hint is referenced in the def f return hint):

$ sourcery review x.py --verbose
Reviewed x.py
──────────────────────────────────────────────── Overview ────────────────────────────────────────────────
Function test refactored with the following changes:

 • Replace unused for index with underscore (for-index-underscore)
  4  def test() -> None:
  5 -    for hint in [int]:
  6 +    for _ in [int]:
  7          class X:
  8              def f(self) -> hint:  # type: ignore[valid-type]
  9                  return 0



──────────────────────────────────────────────── Overview ────────────────────────────────────────────────

 • 1 file scanned.
 • 1 issue detected.

Issues by Type

 • 1 issue could be fixed by Sourcery. Run sourcery review --fix
 • 0 issues need to be fixed manually.

Issues by Rule ID
┏━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━┓
┃ Rule ID              ┃ Count ┃
┡━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━┩
│ for-index-underscore │     1 │
├──────────────────────┼───────┤
│ Total                │     1 │
└──────────────────────┴───────┘

Applying the change results in:

$ python3 x.py
Traceback (most recent call last):
  File "/Users/jacobhayes/src/github.com/artigraph/artigraph/x.py", line 13, in <module>
    test()
  File "/Users/jacobhayes/src/github.com/artigraph/artigraph/x.py", line 6, in test
    class X:
  File "/Users/jacobhayes/src/github.com/artigraph/artigraph/x.py", line 7, in X
    def f(self) -> hint:  # type: ignore[valid-type]
                   ^^^^
NameError: name 'hint' is not defined. Did you mean: 'int'?

Notably, this seems to only happen when the whole thing is inside a function (makes sense - if the loop was top level, nothing guarantees the variable won't be referenced elsewhere) and f is a method - if I remove the wrapping class, it doesn't suggest any changes.

Debug Information

IDE Version: NA

Sourcery Version: 1.2.0

Operating system and Version: macOS 13.3.1

JacobHayes avatar Apr 29 '23 14:04 JacobHayes

Thanks for raising this issue, it's a bug indeed. We'll look into it, but I can't promise a timeline when we'll fix it. As a workaround, my suggestion is to add a comment in such case:

  # sourcery skip: for-index-underscore

reka avatar May 09 '23 08:05 reka