Fix `used-before-assignment` false positive for TYPE_CHECKING if/elif/else usage by zenlyj · Pull Request #8071 · pylint-dev/pylint

zenlyj · 2023-01-17T13:53:50Z

Type of Changes

	Type
✓	🐛 Bug fix
	✨ New feature
	🔨 Refactoring
	📜 Docs

Description

Extend existing logic to prevent false positive when TYPE_CHECKING is used with if/elif/else blocks.

Closes #7574

…itive_type_checking

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

codecov · 2023-01-17T14:04:28Z

Codecov Report

Merging #8071 (c24f62a) into main (8d13dbe) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8071   +/-   ##
=======================================
  Coverage   95.53%   95.54%           
=======================================
  Files         177      177           
  Lines       18622    18633   +11     
=======================================
+ Hits        17791    17802   +11     
  Misses        831      831

Impacted Files	Coverage Δ
pylint/checkers/utils.py	`95.83% <100.00%> (+0.06%)`	⬆️
pylint/checkers/variables.py	`97.37% <100.00%> (-0.01%)`	⬇️

jacobtylerwalls

Thanks for the PR!

This still misses one false positive that I think is in scope. Perhaps you would find _exhaustively_define_name_raise_or_return or _defines_name_raises_or_returns_recursive useful to handle this? ~~If so, we can (in a separate commit) promote it out of the NamesConsumer into utils.py.~~ (EDIT: no, I wouldn't move it, it's too many related methods and should be discussed separately.)

diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py
index 6e1f87131..94d28830b 100644
--- a/tests/functional/u/used/used_before_assignment_typing.py
+++ b/tests/functional/u/used/used_before_assignment_typing.py
@@ -11,7 +11,10 @@ if TYPE_CHECKING:
     import calendar
     from urllib.request import urlopen
 elif input():
-    import calendar
+    if input() + 1:
+        import calendar
+    else:
+        import calendar
 else:
     from urllib.request import urlopen

Gives:

E       AssertionError: Wrong results for file "used_before_assignment_typing":
E       
E       Unexpected in testdata:
E        124: used-before-assignment

…itive_type_checking

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

zenlyj · 2023-02-02T19:17:12Z

@jacobtylerwalls Thanks for the review! I looked into the usage _exhaustively_define_name_raise_or_return and _defines_name_raises_or_returns_recursive, but I don't think they can be used without making significant changes to them.

I took reference from those 2 and implemented a new function in utils.py. Currently, the solution should handle the mentioned nested if/else case, alongside with a few other cases (reflected in the updated functional tests).

Please let me know if I missed out on anything here!

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

jacobtylerwalls

This is amazing! Such a useful generalized solution (vastly outstripping the underlying issue in importance!) 🚀

I left some cosmetic feedback, but this looks good to go.

…itive_type_checking

github-actions · 2023-02-05T17:49:20Z

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

This comment was generated for commit c24f62a

zenlyj · 2023-02-05T18:09:57Z

@Pierre-Sassoulas this PR should be ready for final review, whenever you are free. Thank you!

Pierre-Sassoulas

Amazing, thank you !

…/else usage (#8071) (cherry picked from commit 58fce61)

…/else usage (#8071) (#8229) (cherry picked from commit 58fce61) Co-authored-by: Zen Lee <53538590+zenlyj@users.noreply.github.com>

zenlyj and others added 6 commits January 17, 2023 03:12

Fix type checking used-before-assignment false positive

0acdd1d

Add functional tests

6f85bb9

Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…

cce327a

…itive_type_checking

Add changelog

d99dd81

Revert unnecessary changes

19d2681

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

51090bd

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

This comment has been minimized.

Sign in to view

Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Jan 17, 2023

DanielNoord requested a review from jacobtylerwalls January 17, 2023 20:18

jacobtylerwalls requested changes Jan 17, 2023

View reviewed changes

zEdS15B3GCwq mentioned this pull request Jan 25, 2023

Yet another false positive E0601 used-before-assignment, now for type hints #8110

Closed

zenlyj and others added 7 commits February 3, 2023 01:08

Add utility function to detect name definition recursively

775ccc9

Update tests to handle more cases

bf72993

Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…

04a091d

…itive_type_checking

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

c594f75

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

Resolve CI errors

4aafcae

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

318f30d

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

Update comment for easier understanding

35576ea

zenlyj requested a review from jacobtylerwalls February 2, 2023 19:17

This comment has been minimized.

Sign in to view

zenlyj and others added 2 commits February 3, 2023 04:13

Debug is_defined check for except handler name

352d6de

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

d1dc39b

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

This comment has been minimized.

Sign in to view

zenlyj changed the title ~~Fix false positive for used-before-assignment~~ Fix used-before-assignment false positive for TYPE_CHECKING if/elif/else usage Feb 4, 2023

jacobtylerwalls previously approved these changes Feb 4, 2023

View reviewed changes

Comment thread pylint/checkers/utils.py

Comment thread tests/functional/u/used/used_before_assignment_typing.py Outdated

Comment thread tests/functional/u/used/used_before_assignment_typing.py Outdated

Comment thread tests/functional/u/used/used_before_assignment_typing.rc Outdated

jacobtylerwalls added this to the 2.16.2 milestone Feb 4, 2023

jacobtylerwalls added the backport maintenance/2.16.x label Feb 4, 2023

nickdrozd reviewed Feb 4, 2023

View reviewed changes

Comment thread pylint/checkers/variables.py

Comment thread pylint/checkers/variables.py Outdated

zenlyj added 6 commits February 5, 2023 21:11

[cosmetic] Alphabetize imports in functional test

0904a52

Shift walrus functional tests to dedicated py38 test file

58bdfff

[cosmetic] Rename functions in functional test

1198e5b

Minor refactor to improve code readability

4acf0f6

Remove exception handler name check in utils.is_defined

2fc8fda

Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…

c24f62a

…itive_type_checking

zenlyj dismissed jacobtylerwalls’s stale review via c24f62a February 5, 2023 16:48

jacobtylerwalls approved these changes Feb 5, 2023

View reviewed changes

Pierre-Sassoulas approved these changes Feb 5, 2023

View reviewed changes

jacobtylerwalls mentioned this pull request Feb 6, 2023

used-before-assignment false negative with TYPE_CHECKING #8198

Closed

Pierre-Sassoulas merged commit 58fce61 into pylint-dev:main Feb 7, 2023

github-actions Bot mentioned this pull request Feb 7, 2023

[Backport maintenance/2.16.x] Fix used-before-assignment false positive for TYPE_CHECKING if/elif/else usage #8229

Merged

github-actions Bot pushed a commit that referenced this pull request Feb 7, 2023

Fix used-before-assignment false positive for TYPE_CHECKING if/elif…

2af8457

…/else usage (#8071) (cherry picked from commit 58fce61)

zenlyj mentioned this pull request Feb 8, 2023

Minor refactor to util function #8243

Merged

Pierre-Sassoulas added Backported and removed backport maintenance/2.16.x labels Mar 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix `used-before-assignment` false positive for TYPE_CHECKING if/elif/else usage#8071

Fix `used-before-assignment` false positive for TYPE_CHECKING if/elif/else usage#8071
Pierre-Sassoulas merged 21 commits intopylint-dev:mainfrom
zenlyj:false_positive_type_checking

zenlyj commented Jan 17, 2023

Uh oh!

codecov Bot commented Jan 17, 2023 •

edited

Loading

Uh oh!

This comment has been minimized.

jacobtylerwalls left a comment •

edited

Loading

Uh oh!

zenlyj commented Feb 2, 2023 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

jacobtylerwalls left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 5, 2023

Uh oh!

zenlyj commented Feb 5, 2023

Uh oh!

Pierre-Sassoulas left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

zenlyj commented Jan 17, 2023

Type of Changes

Description

Uh oh!

codecov Bot commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

jacobtylerwalls left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zenlyj commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

jacobtylerwalls left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 5, 2023

Uh oh!

zenlyj commented Feb 5, 2023

Uh oh!

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jan 17, 2023 •

edited

Loading

jacobtylerwalls left a comment •

edited

Loading

zenlyj commented Feb 2, 2023 •

edited

Loading