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

Regression on # pylint: disable comment #2297

Closed
xavfernandez opened this issue Jul 16, 2018 · 9 comments
Closed

Regression on # pylint: disable comment #2297

xavfernandez opened this issue Jul 16, 2018 · 9 comments
Labels
Milestone

Comments

@xavfernandez
Copy link

Steps to reproduce

$ pip list | egrep "pylint|astroid"
astroid                  2.0       
pylint                   2.0.0
$ cat fail.py 
import os  # foobar # pylint: disable=unused-import
$ pylint --disable all --enable unused-import fail.py 
************* Module fail
fail.py:1:0: W0611: Unused import os (unused-import)

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

while

$ pip show pylint astroid | grep Version
Version: 1.9.2
Version: 1.6.5
$ cat fail.py 
import os  # foobar # pylint: disable=unused-import
$ pylint --disable all --enable unused-import fail.py 
No config file found, using default configuration

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

I can confirm the same bug, in fact I meant to report his just now. My solution was to remove the second "#" comment indicator, which was there only due to formatting.

@mattsb42-aws
Copy link

My solution was to remove the second "#" comment indicator, which was there only due to formatting.

Unfortunately, this is not possible if the other comment is there to disable mypy checks because it currently only works if it is the first comment block.

Ex:

line to ignore  # type: ignore # pylint: disable=check-to-ignore

This used to work. Now, the two cannot be combined on the same line.

@PCManticore
Copy link
Contributor

I can also confirm this bug, thanks for raising it! It seems it was introduced by #1743.

@PCManticore PCManticore added this to the Next bug fix release milestone Jul 17, 2018
@sambarluc
Copy link

Confirmed here as well.

@nedbat
Copy link
Contributor

nedbat commented Sep 17, 2018

I can add my voice to the growing chorus:

    def run_TheCode_and_report_it(self):
        """A helper for the next few tests."""
        cov = coverage.Coverage()
        cov.start()
        import TheCode  # pragma: nested # pylint: disable=import-error, unused-variable
        cov.stop()      # pragma: nested
        return self.get_report(cov)

With Pylint 1.9.2, I get no warnings. With 2.1.1, I get:

tests/test_summary.py:598: Unable to import 'TheCode' (import-error)
tests/test_summary.py:598: Unused variable 'TheCode' (unused-variable)

@PCManticore
Copy link
Contributor

Hey folks, this should be fixed in the master branch, please give it a go and let me know if it works for you!

@nedbat
Copy link
Contributor

nedbat commented Sep 18, 2018

This still seems to be a problem, though it's shifted a bit. This file:

"""A pylint problem"""

def something():
    """A helper for the next few tests."""
    import TheCode  # pragma: nested # pylint: disable=import-error, unused-variable

with no pylintrc produces:

************* Module pylint_2297
pylint_2297.py:5:4: W0611: Unused import TheCode (unused-import)

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

@PCManticore
Copy link
Contributor

@nedbat Notice the disable uses unused-variable while the emitted error is unused-import. This changed recently as it didn't make sense for pylint to emit unused-variable for imports.

@nedbat
Copy link
Contributor

nedbat commented Sep 18, 2018

oops! my mistake :)

DevynCJohnson added a commit to DevynCJohnson/pylint that referenced this issue Oct 7, 2018
DevynCJohnson added a commit to DevynCJohnson/pylint that referenced this issue Oct 7, 2018
DevynCJohnson added a commit to DevynCJohnson/pylint that referenced this issue Oct 7, 2018
DevynCJohnson added a commit to DevynCJohnson/pylint that referenced this issue Oct 7, 2018
DevynCJohnson added a commit to DevynCJohnson/pylint that referenced this issue Oct 7, 2018
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.

6 participants