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

"Wrong hanging indentation" with tabs #1148

Closed
xarx00 opened this issue Oct 26, 2016 · 10 comments
Closed

"Wrong hanging indentation" with tabs #1148

xarx00 opened this issue Oct 26, 2016 · 10 comments
Labels

Comments

@xarx00
Copy link

xarx00 commented Oct 26, 2016

Steps to reproduce

  1. Take the following piece code (test.py):
class MainFrame(object):
    def init_ui(self):
        self.m_notebook.ChangeSelection(
            self.m_notebook.FindPage(self.m_panelComponents))

Note, the indentation is made via tabs, not spaces.
2. pylint.exe --indent-string='\t' --indent-after-paren=1 --reports=no test.py

Current behavior

C:  4, 0: Wrong hanging indentation (add 14 spaces).
                        self.m_notebook.FindPage(self.m_panelComponents))
   ^             | (bad-continuation)

Adding more tabs or spaces at the beginning of the hanging line doesn't help.

Expected behavior

I would expect that pylint considers this hanging indentation correct. When the function is placed outside the class, the "Wrong hanging indentation" warning is not reported.

pylint --version output

No config file found, using default configuration
pylint 1.6.4,
astroid 1.4.8
Python 2.7.8 (default, Jul  2 2014, 19:48:49) [MSC v.1500 64 bit (AMD64)]
@moylop260
Copy link
Contributor

Is duplicated #638?

@xarx00
Copy link
Author

xarx00 commented Oct 27, 2016

I don't think it's identical, because

  1. in False positive bad-continuation error #638, they do not talk about hanging indentation
  2. in my case tab indentation seems to be important
    But perhaps they are related somehow.

Another example of the same problem (of mine):

class FileProcessor(object):
    def read_items(self):
        return [
            XItem(ITEM1),
            XItem(ITEM4),
        ]
C:  4, 0: Wrong hanging indentation (add 14 spaces).
                        XItem(ITEM1),
   ^             | (bad-continuation)
C:  5, 0: Wrong hanging indentation (add 14 spaces).
                        XItem(ITEM4),
   ^             | (bad-continuation)
C:  6, 0: Wrong hanging indentation.
                ]
  ^             || (bad-continuation)

@xarx00
Copy link
Author

xarx00 commented Oct 27, 2016

I'd say that the problem is caused by incorrect computation of indentation spaces in case of indentation by tabs.

pylint.exe --indent-string='\t' --indent-after-paren=1 --reports=no test.py

Pylint wants us to add 14 more spaces (Wrong hanging indentation (add 14 spaces)), three characters (tabs) are already present, which gives 17 expected characters in total. The actual number of spaces seen by pylint is: two tabs before return (in the second example), which pylint re-calculates to 16 spaces (2x8), plus 1 space comming from indent-after-paren; 17 chars in total again.

When indent-after-paren parameter is increased, the number of spaces pylint wants us to add is increased as well.

The correct computation should be as follows:
Pylint should expect two tabs (before return) plus one char (tab, or space?) for indent-after-paren, i.e. three tabs in total. And it should see three tabs, which is correct. In my opinion, tab should be treated as one character, not expanded to (8 or whatever number of) spaces.

But please, do not break the calculation of "continued indentation", which in case of tab indentation is done with spaces, and which works now:

class FileProcessor(object):
    def read_items(self):
        return [XItem(ITEM1),
                XItem(ITEM4),
        ]

@PCManticore
Copy link
Contributor

Thank you for providing this input, I will take a look when I will get some free time. If you want to try your hand at a patch, please feel free to do so.

@zacwitte
Copy link

+1
I've been annoyed by this warning as well so turned it off. But hanging indentation is a common problem so I'd like to have this as a part of my linting.

@dstromberg
Copy link

+1

1 similar comment
@ziesemer
Copy link

ziesemer commented Apr 2, 2017

+1

x539 pushed a commit to x539/pylint that referenced this issue Aug 1, 2017
A Tab is not equal to 8 spaces. So just counting tabs as 8 spaces is wrong.

Use the whole 'tabs' and/or 'spaces' string for indentation checks
instead of some imaginary number of whitespaces.
x539 pushed a commit to x539/pylint that referenced this issue Aug 1, 2017
A Tab is not equal to 8 spaces. So just counting tabs as 8 spaces is wrong.

Use the whole 'tabs' and/or 'spaces' string for indentation checks
instead of some imaginary number of whitespaces.
@irridia
Copy link

irridia commented Mar 14, 2018

+1

It would be great to get the patch above applied to mainline...

@Xcelled
Copy link

Xcelled commented Mar 15, 2018

+1. @x539, are you going to make a PR? @PCManticore any updates on this?

x539 pushed a commit to x539/pylint that referenced this issue Mar 17, 2018
x539 pushed a commit to x539/pylint that referenced this issue Mar 17, 2018
A Tab is not equal to 8 spaces. So just counting tabs as 8 spaces is wrong.

Use the whole 'tabs' and/or 'spaces' string for indentation checks
instead of some imaginary number of whitespaces.
brycepg pushed a commit that referenced this issue Mar 25, 2018
A Tab is not equal to 8 spaces. So just counting tabs as 8 spaces is wrong.

Use the whole 'tabs' and/or 'spaces' string for indentation checks
instead of some imaginary number of whitespaces.
@Pierre-Sassoulas
Copy link
Member

bad-continuation has been removed in #3571, black or another formatter can help you with this better than Pylint

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

No branches or pull requests

9 participants