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

Fix undefined-loop-variable with NoReturn and Never #7476

Merged
merged 11 commits into from Sep 19, 2022

Conversation

DanielNoord
Copy link
Collaborator

@DanielNoord DanielNoord commented Sep 16, 2022

  • Write a good description on what the PR does.
  • Create a news fragment with towncrier create <IssueNumber>.<type> which will be
    included in the changelog. <type> can be one of: new_check, removed_check, extension,
    false_positive, false_negative, bugfix, other, internal. If necessary you can write
    details or offer examples on how the new change is supposed to work.
  • If you used multiple emails or multiple names when contributing, add your mails
    and preferred name in script/.contributors_aliases.json

Type of Changes

Type
🐛 Bug fix

Description

Continuation of #7419

@DetachHead Sorry not sure what went wrong here. I tried to push my fix to your branch but it closed the PR... Sorry about that!

This should work 😄

Tested on 3.11 locally as well.

Closes #7311

@DanielNoord DanielNoord added Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer False Positive 🦟 A message is emitted but nothing is wrong with the code labels Sep 16, 2022
@DanielNoord DanielNoord added this to the 2.15.3 milestone Sep 16, 2022
return
):
return
# TODO: 2.16: Consider using RefactoringChecker._is_function_def_never_returning
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function doesn't actually infer but uses the attrname, which we also match not_typing.we_do_return.NoReturn. That isn't really safe..

@DanielNoord DanielNoord changed the title Detach head/main fix undefined-loop-variable with NoReturn and Never Sep 16, 2022
@github-actions

This comment has been minimized.

@jacobtylerwalls jacobtylerwalls changed the title fix undefined-loop-variable with NoReturn and Never Fix undefined-loop-variable with NoReturn and Never Sep 17, 2022
DanielNoord and others added 2 commits September 17, 2022 16:13
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Looks good!

@github-actions

This comment has been minimized.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

👍

@jacobtylerwalls
Copy link
Member

Do you know what's up with the test failure?

@DanielNoord
Copy link
Collaborator Author

Probably incorrect checking of typing_extensions. I'll have a look on Monday.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3080932997

  • 14 of 15 (93.33%) changed or added relevant lines in 3 files are covered.
  • 38 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.005%) to 95.322%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/checkers/variables.py 11 12 91.67%
Files with Coverage Reduction New Missed Lines %
pylint/checkers/variables.py 18 97.43%
pylint/checkers/classes/class_checker.py 20 94.78%
Totals Coverage Status
Change from base Build 3066552830: 0.005%
Covered Lines: 17055
Relevant Lines: 17892

💛 - Coveralls

@github-actions
Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 370ec10

@DanielNoord
Copy link
Collaborator Author

coveralls seems incorrect here..

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Let's ship it 🎉

@Pierre-Sassoulas Pierre-Sassoulas merged commit a47fbbf into pylint-dev:main Sep 19, 2022
@DanielNoord DanielNoord deleted the DetachHead/main branch September 19, 2022 12:12
@Pierre-Sassoulas Pierre-Sassoulas added Backported and removed Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer labels Sep 19, 2022
Pierre-Sassoulas pushed a commit to Pierre-Sassoulas/pylint that referenced this pull request Sep 19, 2022
…#7476)

Co-authored-by: detachhead <detachhead@users.noreply.github.com>
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
Pierre-Sassoulas pushed a commit that referenced this pull request Sep 19, 2022
Co-authored-by: detachhead <detachhead@users.noreply.github.com>
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

W0631:undefined-loop-variable false positive when for loop has function call that returns NoReturn
5 participants