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

Add forgotten-debug-statement checker #4771

Merged
merged 9 commits into from Jul 29, 2021

Conversation

DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Type of Changes

Type
✨ New feature

Description

This adds a checker for calls to breakpoint() or sys.breakpointhook().
This closes #3692

This adds a checker for calls to ``breakpoint()``
or ``sys.breakpointhook()``.
This closes pylint-dev#3692
@coveralls
Copy link

coveralls commented Jul 29, 2021

Coverage Status

Coverage increased (+0.002%) to 92.252% when pulling 07f6167 on DanielNoord:no-breakpoint into a051aa8 on PyCQA:main.

@cdce8p cdce8p self-requested a review July 29, 2021 11:59
@DanielNoord
Copy link
Collaborator Author

After thinking about this some more:
Do we want to check for redefined functions? And if so, do we want to produce the message even if they are redefined, or do we want to mute the message after redefinition?

Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check for redefined functions? And if so, do we want to produce the message even if they are redefined, or do we want to mute the message after redefinition?

That should be covered with inference. You could add an example for it to the test.

pylint/checkers/stdlib.py Outdated Show resolved Hide resolved
pylint/checkers/stdlib.py Outdated Show resolved Hide resolved
@cdce8p cdce8p added the Enhancement ✨ Improvement to a component label Jul 29, 2021
@cdce8p cdce8p added this to the 2.10.0 milestone Jul 29, 2021
@DanielNoord
Copy link
Collaborator Author

DanielNoord commented Jul 29, 2021

That should be covered with inference. You could add an example for it to the test.

I found that the following code passes without throwing the error:

def breakpoint():
    pass
breakpoint()

However, the following code does throw an error.

def overwrite():
    pass
sys.breakpointhook = overwrite
sys.breakpointhook()

I don't think anyone should ever want to overwrite functions like in the second example, but should we consider such code? And if so, much we allow it?

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
@cdce8p
Copy link
Member

cdce8p commented Jul 29, 2021

However, the following code does throw an error.

def overwrite():
    pass
sys.breakpointhook = overwrite
sys.breakpointhook()

I don't think anyone should ever want to overwrite functions like in the second example, but should we consider such code? And if so, much we allow it?

Tbh I haven't seen anybody reassign stdlib function yet. So I would just ignore that case.

--
I initially though you meant the other way round, i.e. setting an alias. That is common.

b = breakpoint
b()

That should through a no-breakpoint error and can / should be covered by the tests.

Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small last comment

Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @DanielNoord!

doc/whatsnew/2.10.rst Outdated Show resolved Hide resolved
doc/whatsnew/2.10.rst Outdated Show resolved Hide resolved
ChangeLog Outdated Show resolved Hide resolved
@Pierre-Sassoulas Pierre-Sassoulas changed the title Add no-breakpoint checker Add forgotten-debug-statement checker Jul 29, 2021
@Pierre-Sassoulas Pierre-Sassoulas merged commit 5e5f48d into pylint-dev:main Jul 29, 2021
@DanielNoord DanielNoord deleted the no-breakpoint branch July 29, 2021 21:49
cdce8p added a commit to cdce8p/pylint that referenced this pull request Aug 1, 2021
Squashed commit of the following:

commit 49c4bba
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Sun Aug 1 19:56:51 2021 +0800

    Fix crash for `unused-private-member` when there are nested attributes

commit 2ad8247
Merge: 8ceb26d 1d09bc8
Author: yushao2 <36848472+yushao2@users.noreply.github.com>
Date:   Sun Aug 1 20:13:05 2021 +0800

    Merge pull request pylint-dev#4709 from yushao2/fix-unused-private-member-4673

    [unused-private-member] add logic for checking nested functions

commit 1d09bc8
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Sun Aug 1 20:03:42 2021 +0800

    update pr based on review

commit a4198cd
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Sun Aug 1 19:21:36 2021 +0800

    Update pr based on review

commit c8b2cbb
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Sun Jul 25 05:20:42 2021 +0000

    [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

commit 4ffea0b
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Sun Jul 25 13:12:29 2021 +0800

    Update pr based on review

commit e4d6243
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sun Jul 18 17:23:31 2021 +0200

    Remove empty line

commit cce5833
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Sun Jul 18 22:40:54 2021 +0800

    update PR based on comments

commit 712dc2b
Author: Yu Shao, Pang <p.yushao2@gmail.com>
Date:   Wed Jul 14 16:42:25 2021 +0800

    [unused-private-member] add logic for checking nested functions

    also, improve error message for nested functions

commit 8ceb26d
Author: Michal Vasilek <michal@vasilek.cz>
Date:   Sun Aug 1 08:14:58 2021 +0200

    Fix IsADirectoryError in tests/lint/unittest_lint (pylint-dev#4781)

    pylintd is a directory, so os.remove throws IsADirectoryError

commit a31e6bc
Author: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Date:   Sat Jul 31 11:21:46 2021 +0200

    Add documentation for useless-suppression

    Closes pylint-dev#4757

commit b71be8a
Author: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com>
Date:   Fri Jul 30 20:21:02 2021 +0200

    Handle has-a relationships for type-hinted arguments in class diagrams (pylint-dev#4745)

    * Pyreverse - Show class has-a relationships inferred from type-hints

    Closes pylint-dev#4744

    Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

commit 5e5f48d
Author: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
Date:   Thu Jul 29 23:44:30 2021 +0200

    Add ``forgotten-debug-statement`` checker (pylint-dev#4771)

    * Add ``no-breakpoint`` checker this adds a checker for calls to ``breakpoint()``, ``pdb.set_trace()``, or ``sys.breakpointhook()``.

    Closes pylint-dev#3692

    Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
    Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Check: Warning for breakpoint calls. no-breakpoint
4 participants