-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 false negative for protected-access
on functions
#5990
Fix false negative for protected-access
on functions
#5990
Conversation
if not closest_func.is_bound(): | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves the false negative.
@staticmethod | ||
def func(light) -> None: | ||
print(light._light_internal) # [protected-access] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added another test.
if self._is_type_self_call(attribute.expr): | ||
if isinstance(attribute.expr, nodes.Call): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolving the false negative then caused a failure here in another test -- we really just need to test if it's a Call and short-circuit, because Call
doesn't have a name
attribute and will crash below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the change against Home Assistant. It does restore the state before the regression. Thanks @jacobtylerwalls for the quick fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see the speed at which these hot fixes get PRed. Great work from all involved π
I completely agree π From what I've seen, it also looks that the initial release was much more stable than some of the past ones. The primer tests seem to help a lot! |
Yes, we had average of 5 crashes opened for 100 issues (before primer from pylint 2.6 to 2.11), we just released 2.13 with 400 issues and we had 3 crashes total (one being in pylint_django a downstream library). This is night and day. It will get even better with the github actions to show the change on primer repositories that @DanielNoord has experimented with. We'll have fewer false positives too then. |
Time permitting this summer I was hoping to look at the baseline functionality. Adding baselines for the primer repos (and triggering PR comments etc) would have probably helped us catch the false negative regressions like this one. |
wooooo!!! πΊπ» |
98ceead
to
903f708
Compare
This discussion should probably take place somewhere else, but both for baseline and primer we just need to fix the |
Pull Request Test Coverage Report for Build 2045944367
π - Coveralls |
Type of Changes
Description
Fixes a false negative regression in #5662.
Closes #5989