Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Incorrect line number on E303 errors on test files #60

Closed
cangove opened this Issue · 6 comments

2 participants

Chris Angove Florent Xicluna
Chris Angove

When running the E30.py test file I noticed an incorrect message. I boiled it down to a simple example. If you use the following example:

print
#: E303
def a():
    print


    # comment

    print
#:

When you run pep8 on it you get:

test2.py:3:1: E302 expected 2 blank lines, found 0
test2.py:9:5: E303 too many blank lines (2>1)

If you notice the first report is genuine but the second report says 2>1 but points to the line for print. First that is 1 blank line not 2 and second the violation is actually on 7 the comment line. When you manually remove line 6 it no longer detects the error as expected. So it seems it's a problem with the line number reported.

Chris Angove

It seems to only happen when you have a comment in there. If that line is replaces by code like 'pass' then it detects the correct line. So looking at the output I think the issue is its skipping the comments and reporting the wrong end number.

Florent Xicluna
Collaborator

It is a little annoyance.
It prints the line number of the next non-comment instruction after the blank lines.

Chris Angove

Could it be fixed not to do that? There is a pull request to allow auto-fixing and this breaks that ability because we do not know the actual line numbers of the error. As far as PEP-8 is concerned comments are just like lines of code (for the spacing/empty line rules) and should report the same, no?

Florent Xicluna
Collaborator

Well we had an issue with comments between block of codes.
For example this piece of code gave spurious errors before.

a = 42


# This function is a stub. Need more work.
def foo():
    pass

See cburroughs/pep8.py#10 for details.

However if someone provides a patch, I can review it.

Chris Angove

I think the problem is that we skip comment lines but we should really treat them the same as code, no? For example if the comment line above was just a line of code as far as pep8 was concerned it would pass, as would my example. It seems the fix was to skip comments but keep track of them which does indeed fix issue #10 but can cause confusion.

So why not just simplify and treat a comment line as any other line of code?

Florent Xicluna
Collaborator

So why not just simplify and treat a comment line as any other line of code?

There's a check to verify that top-level classes and functions are preceded with 2 blank lines, and that other classes and function definitions are preceded with 1 blank line.
For example the previous example cannot be checked properly if we treat comments as code.
Following case show the similar difficulty:

a = 42


# Now how do you check that top-level function are separated by 2 blank lines?

def foo():
    pass

See also testsuite/E30.pyand testsuite/E30not.py.
If you have a patch, it should pass these tests.

Florent Xicluna florentx removed the needs patch label
Florent Xicluna florentx modified the milestone: 1.5.x
Florent Xicluna florentx closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.