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.

Sign up for GitHub

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

Jump to bottom

fix(inconsistent-return-statements): Fix false positive due to pylint not seeing that a method returns NoReturn #8750

Merged
merged 6 commits into from
Jun 11, 2023
Merged

fix(inconsistent-return-statements): Fix false positive due to pylint not seeing that a method returns NoReturn #8750

merged 6 commits into from
Jun 11, 2023

Conversation

kdestin
Copy link
Contributor

@kdestin kdestin commented Jun 7, 2023

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes #8747

Pierre-Sassoulas
Pierre-Sassoulas reviewed Jun 7, 2023
View reviewed changes
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.

Looks great, thank you for contributing !

pylint/checkers/refactoring/refactoring_checker.py Show resolved Hide resolved
pylint/checkers/refactoring/refactoring_checker.py Show resolved Hide resolved
pylint/checkers/refactoring/refactoring_checker.py Show resolved Hide resolved
tests/functional/i/inconsistent/inconsistent_returns_noreturn.py Show resolved Hide resolved
tests/functional/i/inconsistent/inconsistent_returns_noreturn.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 7, 2023 •
edited
Loading

Codecov Report

Merging #8750 (d9738b7) into main (a67ffbd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8750   +/-   ##
=======================================
  Coverage   95.82%   95.82%           
=======================================
  Files         173      173           
  Lines       18386    18387    +1     
=======================================
+ Hits        17618    17619    +1     
  Misses        768      768
Impacted Files Coverage Ξ”
pylint/checkers/refactoring/refactoring_checker.py 98.22% <100.00%> (ΓΈ)

... and 1 file with indirect coverage changes

@github-actions

This comment has been minimized.

Sign in to view
@Pierre-Sassoulas Pierre-Sassoulas added Needs review πŸ” Needs to be reviewed by one or multiple more persons False Positive 🦟 A message is emitted but nothing is wrong with the code labels Jun 7, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.17.5 milestone Jun 7, 2023
@kdestin
test(inconsistent-return-statements): Document false negative when fu…
ab76794 
…nction signature claims NoReturn but body returns
Pierre-Sassoulas
Pierre-Sassoulas previously approved these changes Jun 7, 2023
View reviewed changes
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.

Great first contribution, I think it's fine to merge and backport considering the check is not using inference like we expect most of our checks to do, but ti's a pre-existing caveat. Let's see what others thinks

@github-actions

This comment has been minimized.

Sign in to view
@jacobtylerwalls jacobtylerwalls added backport maintenance/3.3.x and removed Needs review πŸ” Needs to be reviewed by one or multiple more persons labels Jun 11, 2023
jacobtylerwalls
jacobtylerwalls approved these changes Jun 11, 2023
View reviewed changes
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.

Thank you!

@jacobtylerwalls
Copy link
Member

(The documentation check failure is unrelated; I'll look into it.)

@jacobtylerwalls jacobtylerwalls merged commit 8614ccf into pylint-dev:main Jun 11, 2023
github-actions bot pushed a commit that referenced this pull request Jun 11, 2023
@kdestin @github-actions
Fix false positive inconsistent-return-statement for method marked …
9ef76ca 
…`NoReturn` (#8750)

Allow inspection of BoundMethod when checking if annotated as NoReturn

(cherry picked from commit 8614ccf)
@github-actions github-actions bot mentioned this pull request Jun 11, 2023
Merged
@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 d9738b7

Pierre-Sassoulas pushed a commit that referenced this pull request Jun 12, 2023
@github-actions @kdestin
Fix false positive inconsistent-return-statement for method marked …
80e024a 
…`NoReturn` (#8750) (#8761)

Allow inspection of BoundMethod when checking if annotated as NoReturn

(cherry picked from commit 8614ccf)

Co-authored-by: kdestin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobtylerwalls jacobtylerwalls jacobtylerwalls approved these changes

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas left review comments

Assignees
No one assigned
Labels
Backported False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Milestone
2.17.5
Development

Successfully merging this pull request may close these issues.

False positive for inconsistent-return-statements with method annotated with NoReturn return type
3 participants
@kdestin @jacobtylerwalls @Pierre-Sassoulas