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 used-before-assignment for variable annotations guarded by TYPE_CHECKING #7810

Merged
merged 1 commit into from Nov 23, 2022

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
🐛 Bug fix

Description

Closes #7609

This part of the (terribly long) method is starting to get ugly, so if we merge this first and backport it, I could use #7806 to factor out a staticmethod.

@jacobtylerwalls jacobtylerwalls 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 Nov 20, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.15.7 milestone Nov 20, 2022
@Pierre-Sassoulas
Copy link
Member

if we merge this first and backport it, I could use #7806 to factor out a staticmethod.

We could also release 2.16.0 with the refactor so backports are easier to do after that (we're starting to have a lot of conflict in cherry-pick). There's a lot of content in 2.16.0 already, and we could make 2.17.0 the last minor of the 2.x series after all.

@coveralls
Copy link

coveralls commented Nov 20, 2022

Pull Request Test Coverage Report for Build 3514404778

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0002%) to 95.428%

Totals Coverage Status
Change from base Build 3512582852: 0.0002%
Covered Lines: 17472
Relevant Lines: 18309

💛 - Coveralls

@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.

I think the branch need to be rebased on latest main for the primer to be usable.

@jacobtylerwalls
Copy link
Member Author

Happy to do it, but the primer result looks correct -- we're fixing that many false positives.

Copy link
Collaborator

@clavedeluna clavedeluna left a comment

Choose a reason for hiding this comment

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

woah all the FP cleared by the primer

@github-actions
Copy link
Contributor

🤖 Effect of this PR on checked open source code: 🤖

Effect on pytest:
The following messages are no longer emitted:

  1. used-before-assignment:
    Using variable 'Literal' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/reports.py#L325
  2. used-before-assignment:
    Using variable 'Cache' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/stepwise.py#L61
  3. used-before-assignment:
    Using variable 'Literal' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/python.py#L1443
  4. used-before-assignment:
    Using variable 'CallSpec2' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/fixtures.py#L246
  5. used-before-assignment:
    Using variable 'Deque' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/fixtures.py#L275
  6. used-before-assignment:
    Using variable 'Session' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/fixtures.py#L711
  7. used-before-assignment:
    Using variable 'Session' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/terminal.py#L326
  8. used-before-assignment:
    Using variable 'Session' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/nodes.py#L211
  9. used-before-assignment:
    Using variable 'CaptureManager' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/debugging.py#L244
  10. used-before-assignment:
    Using variable 'Literal' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/runner.py#L376
  11. used-before-assignment:
    Using variable 'TerminalReporter' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/config/__init__.py#L1049
  12. used-before-assignment:
    Using variable '_TracebackStyle' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/config/__init__.py#L1086
  13. used-before-assignment:
    Using variable 'Item' before assignment
    https://github.com/pytest-dev/pytest/blob/791b51d0faea365aa9474bb83f9cd964fe265c21/src/_pytest/mark/__init__.py#L228

This comment was generated for commit 14cd577

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.

LGTM. Sorry, you fixed so much false positives it looked like a bug 😄

@Pierre-Sassoulas Pierre-Sassoulas added the Blocked 🚧 Blocked by a particular issue label Nov 21, 2022
@Pierre-Sassoulas
Copy link
Member

Require #7804 to be reviewed and merged in main first becuase I want to backport automatically..

@Pierre-Sassoulas Pierre-Sassoulas added backport maintenance/2.15.x and removed Blocked 🚧 Blocked by a particular issue labels Nov 21, 2022
@Pierre-Sassoulas Pierre-Sassoulas merged commit e2ef840 into pylint-dev:main Nov 23, 2022
@github-actions
Copy link
Contributor

The backport to maintenance/2.15.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-maintenance/2.15.x maintenance/2.15.x
# Navigate to the new working tree
cd .worktrees/backport-maintenance/2.15.x
# Create a new branch
git switch --create backport-7810-to-maintenance/2.15.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 e2ef840528de633fdf8b826a9d7068345d8f371f
# Push it to GitHub
git push --set-upstream origin backport-7810-to-maintenance/2.15.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-maintenance/2.15.x

Then, create a pull request where the base branch is maintenance/2.15.x and the compare/head branch is backport-7810-to-maintenance/2.15.x.

@Pierre-Sassoulas
Copy link
Member

@cdce8p it seems we do need the content: write because this job failed during the push to the backporting branch with:

   /usr/bin/git push --set-upstream origin backport-7810-to-maintenance/2.15.x
  remote: Permission to PyCQA/pylint.git denied to github-actions[bot].
  fatal: unable to access 'https://github.com/PyCQA/pylint.git/': The requested URL returned error: 403
  Error: Error: The process '/usr/bin/git' failed with exit code 128

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Nov 23, 2022
We need it to push to the backporting branch after cherry-picking

See pylint-dev#7810
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Nov 23, 2022
We need it to push to the backporting branch after cherry-picking

See pylint-dev#7810
@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 Nov 23, 2022
Pierre-Sassoulas pushed a commit to Pierre-Sassoulas/pylint that referenced this pull request Nov 23, 2022
@jacobtylerwalls jacobtylerwalls deleted the issue-7609 branch November 24, 2022 13:48
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Nov 24, 2022
We need it to push to the backporting branch after cherry-picking

See pylint-dev#7810
Pierre-Sassoulas added a commit that referenced this pull request Nov 24, 2022
* Do not run primers tests on backporting branches

* Add content: write rights for backporting job

We need it to push to the backporting branch after cherry-picking

See #7810

* [github actions] Add a version comment so the tag is clearer

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Pierre-Sassoulas added a commit that referenced this pull request Nov 24, 2022
* Do not run primers tests on backporting branches

* Add content: write rights for backporting job

We need it to push to the backporting branch after cherry-picking

See #7810

* [github actions] Add a version comment so the tag is clearer

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.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.

False positive used-before-assignment for variable annotations when using TYPE_CHECKING
4 participants