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

False positive for "bad-continuation" #289

Closed
pylint-bot opened this issue Jul 29, 2014 · 27 comments · Fixed by #3571
Closed

False positive for "bad-continuation" #289

pylint-bot opened this issue Jul 29, 2014 · 27 comments · Fixed by #3571
Labels

Comments

@pylint-bot
Copy link

Originally reported by: Steven Myint (BitBucket: myint, GitHub: @myint?)


This valid style is incorrectly labeled as bad-continuation.

def test():
    if (
        True or
        False
    ):
        pass
C:  3, 0: Wrong hanging indentation before block.
        True or
        ^   | (bad-continuation)
C:  4, 0: Wrong hanging indentation before block.
        False
        ^   | (bad-continuation)

As described in #232, there are more false positives demonstrated in E12not.py.


@pylint-bot
Copy link
Author

Original comment by Niklas Hambüchen (BitBucket: nh2, GitHub: @nh2?):


Relatedly, which of these two is correct?

#!python


for x in [
  1,
  2,
  ]:
  print x

for x in [
    1,
    2,
  ]:
  print x

Pylint 1.4 says that the upper one is correct and the lower one is wrong; can you tell me whether that's true according to PEP8?

(Of course assuming I'm working with indent-string = " " and indent-after-paren = 2.)

@pylint-bot
Copy link
Author

Original comment by Olegs Jeremejevs (BitBucket: jeremejevs, GitHub: @jeremejevs?):


Similarly, the following gets labeled as bad-continuation as well:

#!python

for x in [
    1,
    2
]:
    pass

@pylint-bot
Copy link
Author

Original comment by Steven Myint (BitBucket: myint, GitHub: @myint?):


def test():
    if (
        True or
        False
    ):
        pass

c00365b shows what is happening in more detail. It looks like it wants the thing to be doubly indented for some reason.

C:  3, 0: Wrong hanging indentation before block (add 4 spaces).
        True or
        ^   | (bad-continuation)
C:  4, 0: Wrong hanging indentation before block (add 4 spaces).
        False
        ^   | (bad-continuation)

That would look odd:

def test():
    if (
            True or
            False
    ):
        pass

@pylint-bot
Copy link
Author

Original comment by Debanshu Kundu (BitBucket: debanshuk, GitHub: @debanshuk?):


I am forced to use pylint-1.1.0 (instead of latest the 1.4.3), due to this bug.

Following is another example of false positive for 'bad-continuation':

#!python

def abc(
    arg1,
    arg2
):
    pass

Only the following should be reported as 'bad-continuation' and not the one above.

#!python

def abc(
    arg1,
    arg2
    ):
    pass

@PCManticore PCManticore added this to the 1.7.0 milestone May 10, 2016
@PCManticore PCManticore modified the milestones: 2.3, 2.2 Jun 17, 2016
@PCManticore PCManticore removed this from the 2.4 milestone Aug 11, 2016
@mjrk
Copy link

mjrk commented Dec 9, 2016

+1

At least, it would be very nice to have a way to disable these checks (so they are covered by pep8)

dalepotter added a commit to IATI/pyIATI that referenced this issue Jul 11, 2017
pylint suggests adding 4 spaces in multi-line if statement.
This may be a false positive bug though - see:
pylint-dev/pylint#289
@allan-simon
Copy link

Is this bug hard to fix, does someone has some hint on where the bug may be located ?

Just to know if it's something one's can easily try to fix, or if it requires deep knowledge of the codebase.

@fgimian
Copy link

fgimian commented Mar 22, 2018

I found that I could disable this using code C0330 and use pep8 (or flake8) for this particular check 😄

Hope this helps
Fotis

@radeklat
Copy link

This bug is really annoying. It forces me to turn off C0330. But I suppose if it hasn't been fixed in 4 year, it won't be fixed at all...

@PCManticore
Copy link
Contributor

At this point you might just use black and disable pylint's formatting altogether. Or wait until we'll get to it, or send a patch. Lots of solutions .:)

@radeklat
Copy link

I was thinking of using black but it is still in beta and it breaks some formatting as well. I wish I could help fixing this. I guess I'll just wait whatever becomes usable first and I'll stick with the unnecessary spacing until then. Thanks for replying :) Much appreciated.

@return42
Copy link

return42 commented Aug 12, 2018

I like to use Donald E. Knuth 's style which is more readable. My compromise for this if - bug is to add an extra space after/before the outer left/right parenthesis:

if ( foo is not None
     and bar is None ):
    shake_gizmo()

ATM I haven't found an compromise for expression like this one

gizmo = {
    'foo'   : foo
    , 'bar' : bar }

so I have to use disable :(

return42 added a commit to return42/linuxdoc that referenced this issue Aug 13, 2018
@PCManticore
Copy link
Contributor

@return42 We'll most likely never support that extra space.

@return42
Copy link

Sorry, I do not understand what you mean "extra space" .. it is simply a bug when linting continuation .. and what I show is just a workaround for parenthesis case which does not work for dicts (curly brace) ..
I'am wondering that it is not fixable or hard to fix .. I mean .. indentation in python is elementary .. so why is this "extra spcae"?

@PCManticore
Copy link
Contributor

@return42 This is not conforming to PEP 8. If this style is your preference, feel free to enforce it with any tool you want, but pylint is not likely to support it.

@return42
Copy link

You misunderstood me, what I want is PEP8, I prefer to write PEP8, but when I write PEP8 I got this "bad-continuation" messages.

@return42
Copy link

@PCManticore: by example ..

# test123.py
if (not True
    and False):
    pass

and then ..

$ pylint3 test123.py
No config file found, using default configuration
************* Module test123
C:  3, 0: Wrong continued indentation before block (add 4 spaces).
    and False):
    ^   | (bad-continuation)
C:  1, 0: Missing module docstring (missing-docstring)

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

@rmcfatter
Copy link

This hits me too. PEP-8 requires extra indents if the result would otherwise be visually confusing.

# Bad - no visual separation between condition and code block
if (
    condition 1
    or condition2):
    do_something()

# Good - extra indent for visual separation
if (
        condition 1
        or condition2):
    do_something()`

# Good - closing paren on separate line, visual separation
if (
    condition1
    or condition2
):
    do_something()

The latter form (which is my preferred style) is specifically allowed by PEP-8 but flagged as bad-continuation by pylint.

sbidoul added a commit to acsone/acsoo that referenced this issue Oct 30, 2018
FairphoneMirrorsBot pushed a commit to FairphoneMirrors/hiccup-server that referenced this issue Dec 24, 2018
Disable C0330 as there are currently some false positives:
pylint-dev/pylint#289

Issue: HIC-175
Change-Id: I0f6d9d446dffc72c7ea3989fe59b1adfdcc8acbf
bibz added a commit to bibz/challenge-mini-vouchers that referenced this issue Feb 9, 2019
Define the project toolset with pre-commit:

- The Black formatter;
- The style enforcer flake8;
- The linter pylint.

As described online [1], pylint raises a false positive about line
continuation when using the black formatter.  Also set the maximum
line length to what black uses (88 characters).

[1]: pylint-dev/pylint#289
akuzminsky added a commit to revenants-cie/terraform-ci that referenced this issue Dec 25, 2019
* var_file argument to terraform_apply()

By default terraform_apply() read terraform variables from a file `configuration.tfvars` in the same directory as `path`.
Argument `var_file` allows to specify where terraform should read variables from.

* Bump version: 0.8.4 → 0.9.0

* Make black and pylint happy

This is funny - black makes changes pylint doesn't like.
There is an open bug pylint-dev/pylint#289 which is unlikely to be fixed since it hasn't been closed for 5+ years.

There can be only one!
@piotr-dobrogost
Copy link

@rmcfatter

# Good - closing paren on separate line, visual separation
if (
    condition1
    or condition2
):
    do_something()

The latter form (which is my preferred style) is specifically allowed by PEP-8 but flagged as bad-continuation by pylint.

Where exactly does PEP-8 allow this?

Current version of PEP-8 has the following on the subject:

Acceptable options in this situation include, but are not limited to:

    # No extra indentation.
    if (this_is_one_thing and
        that_is_another_thing):
        do_something()

    # Add a comment, which will provide some distinction in editors
    # supporting syntax highlighting.
    if (this_is_one_thing and
        that_is_another_thing):
        # Since both conditions are true, we can frobnicate.
        do_something()

    # Add some extra indentation on the conditional continuation line.
    if (this_is_one_thing
            and that_is_another_thing):
        do_something()

@piotr-dobrogost
Copy link

piotr-dobrogost commented Jan 1, 2020

Debanshu Kundu in his comment wrote:

Following is another example of false positive for 'bad-continuation':

#!python

def abc(
    arg1,
    arg2
):
    pass

I believe this not a false positive as PEP-8 specifically says

When using a hanging indent the following should be considered; there should be no arguments on the first line and further indentation should be used to clearly distinguish itself as a continuation line.

So according to PEP-8 the only valid option of distinguishing continuation lines from the following block is to use further indentation and not to mess with the position of parenthesis.

See psf/black#1178

@allan-simon
Copy link

PEP-8 that you quote also say some lines above:

This PEP takes no explicit position on how (or whether) to further visually distinguish such
conditional lines from the nested suite inside the if-statement.
Acceptable options

so I read it as pylint considering its "acceptable options" better than black (which does not seem forbidden as per pep8)

@allan-simon
Copy link

added a request for clarity in PEP-8 python/peps#1267 I've added a request for more clarity in pep-8 , so that we're less prone to interpretation.

@piotr-dobrogost
Copy link

@allan-simon

PEP-8 that you quote also say some lines above:

Yes, but as @rmcfatter wrote (emphasis mine)

The latter form (which is my preferred style) is specifically allowed by PEP-8

you would expect to see such a formatting (closing parenthesis on its own line) given as one of examples in the PEP whereas that is not the case.

@allan-simon
Copy link

allan-simon commented Jan 2, 2020

sure but then we're no more debating that it's forbidden just that it's not clearly accepted . and on my side I've never been arguing that your proposition was wrong. Therefore that your preferred way of formatting is accepted does not prevent other form froms being acceptable. Especially as several times in PEP-8 they clearly state that they have no strong opinion on which formatting is best and gives only opinion on what could be acceptable (they specifically state "not limited to"). Moreover if you've been in programming for quite some times, you have certainly noticed that the closing character on a separate line has gained traction only recently, so I'm not surprised it's not mentioned by a document mostly written in 2001.

For that matter and in order to not have the discussion on several thread of several projects over and over, I've directly asked for clarifying pep-8 here https://discuss.python.org/t/pep-8-clarify-if-multiline-argument-list-with-a-closing-on-a-separate-line-is-acceptable/2948 , so that people with authority on PEP-8 can add either the example in "don't" or "acceptable".

PS: I do not like the style above , but as it was at first the point of the issue, note that this style

def abc(
    arg1,
    args):
    """docstring"""
    pass

is being acceptable as per the explicit example of PEP-8 :

# Add a comment, which will provide some distinction in editors
# supporting syntax highlighting.
if (this_is_one_thing and
    that_is_another_thing):
    # Since both conditions are true, we can frobnicate.
    do_something()

jameswettenhall added a commit to mytardis/mydata-python that referenced this issue Jan 10, 2020
Erawpalassalg added a commit to Erawpalassalg/bli that referenced this issue Jan 14, 2020
Mostly to prevent the conflict with black for `bad-continuation`,
see pylint's issue [#289](pylint-dev/pylint#289)
pastelmind added a commit to pastelmind/slack-craps that referenced this issue Feb 27, 2020
This is caused by a disagreement between black and pylint on indentation
of multiple chained conditions in an if-statment. See:

psf/black#48
pylint-dev/pylint#289

Since black is less compromising on the matter, let's disable pylint's
warnings instead.
@brycepg brycepg removed their assignment Jun 16, 2020
nijel added a commit to WeblateOrg/weblate that referenced this issue Jun 30, 2020
The continuation check is not compatible with what black formats, see
pylint-dev/pylint#289
chris-verclytte added a commit to ChauffeurPrive/nestor-api that referenced this issue Jul 8, 2020
chris-verclytte added a commit to ChauffeurPrive/nestor-api that referenced this issue Jul 8, 2020
tkoyama010 added a commit to tkoyama010/pyvistaqt that referenced this issue Aug 15, 2020
akaszynski pushed a commit to pyvista/pyvistaqt that referenced this issue Aug 23, 2020
* Add pylint in CI #39

* Improve rating to 8.87/10

* Fix codespell conf

* Install dependencies

* Setup python first

* Use python 3.6

* Fix workflow

* Restore workflow

* Update .github/workflows/pythonpackage.yml

* Fix C0103: invalid-name

* Fix C0103 invalid-name

* Add pylint Makefile target

* Fix C0103 invalid-name

* ♻️ fix by pylint

* Fix Makefile

* Move dragEnterEvent to function

* Fix pylint error

* Fix W0611 unused-import

* Fix W0613: unused-argument

* Fix isort

* Revert of comment

* Fix unnecessary-lambda

* Fix unnecessary-lambda

* Fix invalid name

* Fix unused variable

* Disable conflicting rule between black and pylint

See : psf/black#48 pylint-dev/pylint#289

* Fix reimported (W0404)

* Fix C0412 (ungrouped-imports)

* Fix missing-module-docstring

* Fix too-many-arguments

* Fix broad-except

* Fix invalid-name

* Fix disable to name

* Fix too-many-instance-attributes

* Fix pylint

* Fix black

* Fix invalid-name

* Fix too-many-arguments

* Update pyvistaqt/window.py

* Update pyvistaqt/dialog.py

* Fix pylint

* Fix too-few-public-method

* Fix too-few-public-methods

* Remove enable F401

* Revert "Remove enable F401"

This reverts commit c937bc5.

* Fix attribute-defined-outside-init

* Fix too-few-public-methods

* Fix unsubscriptable-object

* Fix unsubscriptable-object

* Fix pycodestyle

* Fix isort

* Fix typo

* Update pyvistaqt/plotting.py

* Fix unnecessary-lambda

* Update dialog.py

* Update pyvistaqt/dialog.py

* Update pyvistaqt/dialog.py

* Update pyvistaqt/dialog.py

* Update pyvistaqt/dialog.py

* Fix invalid-name

* Update pyvistaqt/dialog.py

* Update pyvistaqt/dialog.py

* Update pyvistaqt/dialog.py

* Fix unused-argument

* Fix at least two spaces before inline comment

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Fix invalid-name

* Update plotting.py

* Update plotting.py

* Fix no-self-use

* Fix attribute-defined-outside-init

* Fix attribute-defined-outside-init

* Fix attribute-defined-outside-init

* Fix E261 at least two spaces before inline comment

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Fix   attribute-defined-outside-init

* Update pyvistaqt/window.py

* Fix no cover

* Update pyvistaqt/plotting.py

* Update pyvistaqt/plotting.py

* Fix pylint

* Restore imports and adds docstrings

* Deal with PyQt5 ungrouped-imports

* Make isort

* Add docstring

Co-authored-by: Guillaume Favelier <guillaume.favelier@gmail.com>
msuozzo pushed a commit to msuozzo/pylint that referenced this issue Feb 18, 2022
PEP 484 specifies that they be hinted as the type of a single element,
as seen from the caller's perspective.

Closes pylint-dev#289.

Co-authored-by: Christopher Wilcox <crwilcox@google.com>
tiltingpenguin added a commit to cobbler/libcobblersignatures that referenced this issue Dec 12, 2022
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.