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

class attrs should not emit assigning-non-slot msg #7987

Merged
merged 4 commits into from
Dec 30, 2022

Conversation

clavedeluna
Copy link
Collaborator

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Pylint was raising assigning-non-slot when a class attribute was re-assigned, which I believe is incorrect given that __slots__ is meant for instance attrs.

Closes #6001

@coveralls
Copy link

coveralls commented Dec 26, 2022

Pull Request Test Coverage Report for Build 3787058444

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • 21 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 95.334%

Files with Coverage Reduction New Missed Lines %
pylint/checkers/classes/class_checker.py 21 92.83%
Totals Coverage Status
Change from base Build 3786035584: -0.1%
Covered Lines: 17653
Relevant Lines: 18517

πŸ’› - Coveralls

Copy link
Collaborator

@DanielNoord DanielNoord 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 add INFERENCE confidence while we're here.

Also, could you add the example from the issue just to be sure?

Rest LGTM!

@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code backport maintenance/2.15.x labels Dec 27, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.10 milestone Dec 27, 2022
@github-actions

This comment has been minimized.

@DanielNoord DanielNoord reopened this Dec 27, 2022
@codecov
Copy link

codecov bot commented Dec 27, 2022

Codecov Report

Merging #7987 (d316609) into main (57d959a) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7987      +/-   ##
==========================================
- Coverage   95.48%   95.41%   -0.08%     
==========================================
  Files         176      176              
  Lines       18511    18516       +5     
==========================================
- Hits        17676    17667       -9     
- Misses        835      849      +14     
Impacted Files Coverage Ξ”
pylint/checkers/classes/class_checker.py 93.56% <100.00%> (-1.51%) ⬇️
pylint/checkers/utils.py 95.33% <100.00%> (+0.02%) ⬆️

@Pierre-Sassoulas
Copy link
Member

I think we need an actual rebase on main to see the real codecov result @DanielNoord

@DanielNoord
Copy link
Collaborator

I think we need an actual rebase on main to see the real codecov result @DanielNoord

Yeah, sadly. Oh well, that's a small price to pay.

Sorry for the churn @clavedeluna, you'll need to rebase to get CI to pass πŸ˜„

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@DanielNoord
Copy link
Collaborator

Is the test failure genuine?

@Pierre-Sassoulas
Copy link
Member

I swear the latest pypy 3.8 release is nothing but trouble :(

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Small nits πŸ˜„

pylint/checkers/classes/class_checker.py Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

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

  1. assigning-non-slot:
    Assigning to attribute 'type' not defined in class slots
    https://github.com/getsentry/sentry/blob/77f33b1fdceeae6c4ebb96cc099d010a1d94d5da/src/sentry/analytics/event.py#L30

This comment was generated for commit d316609

@DanielNoord DanielNoord merged commit 1baa10e into pylint-dev:main Dec 30, 2022
@github-actions
Copy link
Contributor

The backport to maintenance/2.15.x failed:

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

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-7987-to-maintenance/2.15.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1baa10e250406d740d0c83f2902e949d84c1ecd8
# Push it to GitHub
git push --set-upstream origin backport-7987-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-7987-to-maintenance/2.15.x.

@Pierre-Sassoulas Pierre-Sassoulas added the Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer label Dec 30, 2022
@DanielNoord
Copy link
Collaborator

@clavedeluna Do you want to try this back port yourself?

clavedeluna added a commit to clavedeluna/pylint that referenced this pull request Dec 31, 2022
@clavedeluna clavedeluna mentioned this pull request Dec 31, 2022
Pierre-Sassoulas added a commit that referenced this pull request Jan 2, 2023
* class attrs should not emit assigning-non-slot msg (#7987)

* Create `TERMINATING_FUNCS_QNAMES` (#7825)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
@Pierre-Sassoulas Pierre-Sassoulas removed the Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer label Jan 8, 2023
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 "assigning-non-slot" error for inherited descriptor when slots are used
4 participants