Skip to content
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

Pylint incorrectly flags as unused-import things that are used as variable type annotations #1063

Closed
fabianhjr opened this issue Aug 6, 2016 · 15 comments
Labels

Comments

@fabianhjr
Copy link

Steps to reproduce

from typing import Optional

a = None  # type: Optional[str]

Current behavior

pylint flags Optional as an unused-import

Expected behavior

pylint shouldn't flag Optional as unused-import

pylint --version output

pylint 1.6.4,
astroid 1.4.8
Python 3.5.2 (default, Jun 28 2016, 08:46:01)
[GCC 6.1.1 20160602]

Supporting documentation:
https://www.python.org/dev/peps/pep-0484/

@PCManticore
Copy link
Contributor

Yes, we do not currently process type comments. A planned support for PEP 484 is in our roadmap, but I am not sure how fast we will reach it.

@PCManticore PCManticore added this to the 4K milestone Aug 11, 2016
@petrpulc
Copy link
Contributor

Partially fixed in #1231, but only if they are used as a sting annotations for function attributes.

The full support is in question, as I don't personally see a way how to extract comments from AST(eroid) representation of the module... Any help or advice will be very appreciated.

@PCManticore
Copy link
Contributor

You can't extract comments from astroid.

@euresti
Copy link

euresti commented Dec 30, 2016

Hi. I built a pylint extension inspired by https://gist.github.com/PCManticore/eca887488248b7594d37 to handle precisely this for our code base. With it all the unused imports go away, and it'll even warn you of stuff you haven't imported.

It also adds some inferences for special typing things like NamedTuple, NewType, TypeVar and handles subscripts on Generic types so that things like Employee = NamedTuple('Employee', [('name', str), ('id', int)]) won't generate an Invalid Constant Name error.

So if you think this is valuable I can work on cleaning up the code and making a pull request. I imagine this would be an extension that users can enable if they want to. Also suggestions on the name are welcome. Mine is called mypy_lint_enhancer.py but that's a really terrible name. Maybe extensions/pep484.py or extensions/typing.py.

Cheers!

@PCManticore
Copy link
Contributor

@euresti That sounds pretty neat! Sure thing, please send a pull request. Depending on the additions, it might be something enabled by default.

euresti pushed a commit to euresti/pylint that referenced this issue Dec 31, 2016
…as comments.

For now let's call this an RFC don't merge as is.

Basically this adds a plugin that adds a couple of transforms.

   the body as if they were real code. This silences unused-import and
   will trigger undefined-variable on missing imports.  Since these
   lines get inserted in the same line as the happen
   multiple-statements is disable on those lines.

   creating new types by subscripting existing ones.

   intended to silence invalid-name.

Also fixed a small bug in _supports_protocol where only metaclasses
were looked at and not the metaclasses of the base classes.

Tests pass!

Fixes pylint-dev#1063
euresti pushed a commit to euresti/pylint that referenced this issue Dec 31, 2016
…as comments.

For now let's call this an RFC don't merge as is.

Basically this adds a plugin that adds a couple of transforms.

   the body as if they were real code. This silences unused-import and
   will trigger undefined-variable on missing imports.  Since these
   lines get inserted in the same line as the happen
   multiple-statements is disable on those lines.

   creating new types by subscripting existing ones.

   intended to silence invalid-name.

Also fixed a small bug in _supports_protocol where only metaclasses
were looked at and not the metaclasses of the base classes.

Tests pass!

Fixes pylint-dev#1063
euresti pushed a commit to euresti/pylint that referenced this issue Dec 31, 2016
…as comments.

For now let's call this an RFC don't merge as is.

Basically this adds a plugin that adds a couple of transforms.

   the body as if they were real code. This silences unused-import and
   will trigger undefined-variable on missing imports.  Since these
   lines get inserted in the same line as the happen
   multiple-statements is disable on those lines.

   creating new types by subscripting existing ones.

   intended to silence invalid-name.

Also fixed a small bug in _supports_protocol where only metaclasses
were looked at and not the metaclasses of the base classes.

Tests pass!

Fixes pylint-dev#1063
euresti pushed a commit to euresti/pylint that referenced this issue Jan 1, 2017
…as comments.

For now let's call this an RFC don't merge as is.

Basically this adds a plugin that adds a couple of transforms.

   the body as if they were real code. This silences unused-import and
   will trigger undefined-variable on missing imports.  Since these
   lines get inserted in the same line as the happen
   multiple-statements is disable on those lines.

   creating new types by subscripting existing ones.

   intended to silence invalid-name.

Also fixed a small bug in _supports_protocol where only metaclasses
were looked at and not the metaclasses of the base classes.

Tests pass!

Fixes pylint-dev#1063
euresti pushed a commit to euresti/pylint that referenced this issue Jan 2, 2017
…as comments.

For now let's call this an RFC don't merge as is.

Basically this adds a plugin that adds a couple of transforms.

   the body as if they were real code. This silences unused-import and
   will trigger undefined-variable on missing imports.  Since these
   lines get inserted in the same line as the happen
   multiple-statements is disable on those lines.

   creating new types by subscripting existing ones.

   intended to silence invalid-name.

Also fixed a small bug in _supports_protocol where only metaclasses
were looked at and not the metaclasses of the base classes.

Tests pass!

Fixes pylint-dev#1063
@PCManticore PCManticore modified the milestones: 1.7.0, 4K Apr 12, 2017
@PCManticore PCManticore modified the milestones: 1.8.0, 1.7.0 Apr 13, 2017
@PCManticore PCManticore modified the milestones: 1.8.0, 2.0 Dec 12, 2017
@gaborbernat
Copy link

so what's the status with this? @euresti code did make it or not?

@euresti
Copy link

euresti commented Mar 21, 2018

It never made it in. My code is here:
#1253
and here
pylint-dev/astroid#392

I don't use pylint anymore nor type comments anymore though. So I don't really have much need for this. Sorry.

@Daenyth
Copy link

Daenyth commented Mar 21, 2018

Pyflakes on python3 makes a vastly better experience IMO

@ohemorange
Copy link

Certbot would love to see this!

@PCManticore PCManticore modified the milestones: 2.0, 2.1 Apr 25, 2018
@gsemet
Copy link

gsemet commented May 3, 2018

that would be very handy indeed!

@PCManticore
Copy link
Contributor

We recently switched to typed_ast from Dropbox, which has support for extracting type comments. This is fixed in the master branch, please give it a go when you get a chance!

@gaborbernat
Copy link

@PCManticore thanks, any non-dev release plans for it? 😄

@PCManticore
Copy link
Contributor

Probably right after Python 3.7 comes out. We still have some issues to flesh out until then (check the 2.0 milestone from both pylint and astroid).

ahitrin added a commit to ahitrin/SiebenApp that referenced this issue Aug 27, 2018
Pylint knows nothing about types used in comments and raises false
warnings about them [1]. A fix probably will not be released until
Python 3.7. Currently, the only way to live with it is to supppress
"unused import" warning.

> gaborbernat commented on 24 May
> @PCManticore thanks, any non-dev release plans for it?
>
> PCManticore commented on 24 May
> Probably right after Python 3.7 comes out. We still have some issues
> to flesh out until then (check the 2.0 milestone from both pylint and
> astroid).

[1]: pylint-dev/pylint#1063
@PCManticore
Copy link
Contributor

@gaborbernat Already launched, forgot to reply.

@dvogelbacher
Copy link

Are there any plans to also fix this for python 2? (pylint 1.9)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants