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

Duplicate Code warning occurs for multiline imports even with ignore-imports=yes #2019

Closed
bw-matthew opened this issue Apr 16, 2018 · 6 comments
Labels

Comments

@bw-matthew
Copy link

bw-matthew commented Apr 16, 2018

Steps to reproduce

  1. Define the following three files:

a.py

""" Mandatory Docstring """

def fun_a():
    """ Mandatory Docstring """
    pass

def fun_b():
    """ Mandatory Docstring """
    pass

def fun_c():
    """ Mandatory Docstring """
    pass

def fun_d():
    """ Mandatory Docstring """
    pass

b.py

""" Mandatory Docstring """

from .a import (
    fun_a,
    fun_b,
    fun_c,
    fun_d,
)

print(fun_a)
print(fun_b)
print(fun_c)
print(fun_d)

c.py

""" Mandatory Docstring """

from .a import (
    fun_a,
    fun_b,
    fun_c,
    fun_d,
)

print(fun_d)
print(fun_c)
print(fun_b)
print(fun_a)

Place them in a folder called src that contains an __init__.py file.

  1. Run pylint over them: pylint src/*.py

  2. Run pylint over them, using the ignore-imports option: pylint --ignore-imports=yes src/*.py

Current behavior

➜ pylint src/*.py
No config file found, using default configuration
************* Module src.__init__
R:  1, 0: Similar lines in 2 files
==src.b:2
==src.c:2
from .a import (
    fun_a,
    fun_b,
    fun_c,
    fun_d,
)
 (duplicate-code)

------------------------------------------------------------------
Your code has been rated at 9.44/10 (previous run: 9.44/10, +0.00)
➜ pylint --ignore-imports=yes src/*.py
No config file found, using default configuration
************* Module src.__init__
R:  1, 0: Similar lines in 2 files
==src.b:3
==src.c:3
    fun_a,
    fun_b,
    fun_c,
    fun_d,
)
 (duplicate-code)

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

Expected behavior

The duplicate lines are import statements that have been wrapped onto multiple lines.
They should not trigger the similarity checker when the --ignore-imports=yes option is used.

pylint --version output

➜ pylint --version
No config file found, using default configuration
pylint 1.8.4,
astroid 1.6.3
Python 3.6.4 (default, Jan  2 2018, 15:04:06)
[GCC 5.4.0 20160609]
@PCManticore
Copy link
Contributor

Thanks for the report! I can reproduce the issue. From a short glance, it seems we don't quite handle correctly ignore_imports, we only replace the line that has the import with blank, but that still will trigger on the following lines, while we should in fact skip the entire block.

@AraHaan
Copy link

AraHaan commented Apr 25, 2018

Perhaps what should really happen is well right before the actual code that reports it as a issue have a

if not ignore_imports:
    # report issue(s) to user.

So that way it does not clutter code with a ton of hacks. I know from experience (look at my failure repos at https://github.com/DecoraterBot-devs/).

But @PCManticore everyone in python 3.x do this sort of import and star imports from modules, I think the reports from those on python 3 code should not happen even without ignore-imports=yes, maybe python 2 but I do not think so on 3. Even some places of the python standard library uses it so I personally think it is not only useless but dumb to scan for if the resulting code is python 3.

@bw-matthew
Copy link
Author

bw-matthew commented Apr 25, 2018

The duplicate code checks cover more than imports, so suppressing all duplicate code warnings if the flag is set would be incorrect. I believe that the problem as described is that ignore_imports causes pylint to skip the line starting import but not subsequent lines even if those subsequent lines are part of the import statement.

I have not looked at the code yet however it seems like it would be very difficult to achieve linting without parsing the code. The solution that I see is to ensure that this operates over the logical import statement rather than the line starting import.

@AraHaan
Copy link

AraHaan commented May 2, 2018

Or maybe check to see if the import line has a "(" and ignore every line within that until it finds the other ")"?

@bw-matthew
Copy link
Author

That's basically implementing a parser. Pylint uses ast which is a full python parser.

@chkno
Copy link
Contributor

chkno commented Aug 19, 2018

pylint uses a parser elsewhere, but the Similar / symilar / duplicate-code / R0801 check does not use a parser. --ignore-imports is implemented in stripped_lines() in pylint/checkers/similar.py like this:

for line in lines:
...
    if ignore_imports:
        if line.startswith("import ") or line.startswith("from "):
            line = ''

I agree, it should use a real parser.

This and #1422 appear to describe the same issue.

LanderU added a commit to aliasrobotics/aztarna that referenced this issue May 1, 2019
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.

4 participants