Skip to content

unused-import false negative when typing.Annotated has str parameters #7547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lggruspe opened this issue Oct 1, 2022 · 5 comments · Fixed by #7678
Closed

unused-import false negative when typing.Annotated has str parameters #7547

lggruspe opened this issue Oct 1, 2022 · 5 comments · Fixed by #7678
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@lggruspe
Copy link
Contributor

lggruspe commented Oct 1, 2022

Bug description

example.py:

# pylint: disable=missing-docstring
from pathlib import Path
import typing as t

example: t.Annotated[str, "Path"] = "/foo/bar"

Configuration

No response

Command used

pylint example.py

Pylint output

Success: no issues found in 1 source file

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Expected behavior

It should report unused-import, since the parameter to Annotated does not refer to pathlib.Path.

Pylint version

pylint 2.15.3
astroid 2.12.10
Python 3.10.7 (main, Sep  7 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)]

OS / Environment

fedora 36

Additional dependencies

No response

@lggruspe lggruspe added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Oct 1, 2022
@Pierre-Sassoulas
Copy link
Member

I don't understand the example, what does the parameter of annotated refer to according to you ? I think "Path" does refer to pathlib.Path ?

@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 1, 2022
@lggruspe
Copy link
Contributor Author

lggruspe commented Oct 3, 2022

What comes after the first parameter to Annotated is just "an arbitrary list of Python values".
Its interpretation is left to the library or tool that encounters the Annotated type: https://peps.python.org/pep-0593/#consuming-annotations.

So if I understand correctly, the "Path" in the example is just a string literal rather than a reference to a type.

@Pierre-Sassoulas
Copy link
Member

Ok, I think you're right and it's a false negative. autoflake is not removing the import either, you could open the same issue in their repository if you want.

@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Oct 7, 2022
@DanielNoord
Copy link
Collaborator

Just to be sure, we should just ignore any second parameter to Annotated right? Or is there any case where it should be counted towards "usage".

@lggruspe
Copy link
Contributor Author

I think so. mypy ignores the second parameter as well.

mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Oct 26, 2022
…yping.Annotated`` was treated as a reference to an import.

Closes pylint-dev#7547
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Oct 26, 2022
…yping.Annotated`` was treated as a reference to an import.

Closes pylint-dev#7547
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Oct 26, 2022
…yping.Annotated`` was treated as a reference to an import.

Closes pylint-dev#7547
Pierre-Sassoulas pushed a commit that referenced this issue Nov 9, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Fix a false negative for ``unused-import`` when a constant inside ``typing.Annotated`` was treated as a reference to an import.

* Update with Daniël's suggestions:

- More understandable function parameter name.
- Update function parameter type: Expect a tuple of strings.

Closes #7547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
3 participants