Skip to content

Handle when the 'ancestors' attribute is not present in an object #5647

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

Conversation

doctoryes
Copy link

Fix for issue described here: #5646

Handle when the 'ancestors' attribute is not present in an object.

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes #5646

pylint-dev#5646

Handle when the 'ancestors' attribute is not present in an object.
@DanielNoord DanielNoord self-requested a review January 7, 2022 21:53
@coveralls
Copy link

coveralls commented Jan 7, 2022

Pull Request Test Coverage Report for Build 1669502578

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

Totals Coverage Status
Change from base Build 1663696938: 0.01%
Covered Lines: 14365
Relevant Lines: 15328

πŸ’› - Coveralls

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.

Thank you for handling this !

Would it be possible to add a regression test in tests/functional/ please ? I'm not sure about what code exactly in opendx is causing the issue but if it's possible to isolate the problem if would be better as the whole file is pretty big.

Comment on lines +194 to +202
What's New in Pylint 2.12.3?
============================
Release date:

* Fixed handling of missing 'ancestors' object attribute.

Closes #5646


Copy link
Member

Choose a reason for hiding this comment

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

We don't have a bug fix branch for 2.12 currently so we do not back-port those fix. This is something that we'd need to put into place after we release 2.13 but right now this is going to be in 2.13.

Suggested change
What's New in Pylint 2.12.3?
============================
Release date:
* Fixed handling of missing 'ancestors' object attribute.
Closes #5646
* Fixed handling of missing 'ancestors' object attribute.
Closes #5646

@@ -213,5 +213,5 @@ def base_classes_of_node(instance: nodes.ClassDef) -> List[str]:
"""Return all the classes names that a ClassDef inherit from including 'object'."""
try:
return [instance.name] + [x.name for x in instance.ancestors()]
except TypeError:
except (TypeError, AttributeError):
Copy link
Member

Choose a reason for hiding this comment

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

Nice straightforward fix, I like it :)

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Jan 7, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Crash πŸ’₯ A bug that makes pylint crash label Jan 7, 2022
@DanielNoord
Copy link
Collaborator

DanielNoord commented Jan 8, 2022

Thanks for the fix @doctoryes, but after some investigation I think we need to handle this differently.

I found a reproducible example:

class MyClassWithProxy:
    @property
    def _my_property(cls):
        return {}


def func():
    xblock = MyClassWithProxy()
    assert MyClassWithProxy._my_property == {}

Turns out we're not handling properties correctly. In the original code the attribute that is failing is defined as a "lazy" property in one of the sub-sub-subclasses. This smaller example also fails. I'm working on a fix that handles these cases a little more gracefully and will open a PR soon!

@DanielNoord
Copy link
Collaborator

Hmm, fixing isn't as easy as I thought. I'll probably work on this somewhere next week.

class ParentWithProperty:
    def _parent_function(cls):
        return {}


class MyClassWithProxy(ParentWithProperty):
    @property
    def _my_property(cls):
        return {}


def func():
    xblock = MyClassWithProxy()
    assert xblock._parent_function == {}
    assert xblock._services_requested == {}

Both of these comparisons crash right now. I'll try and fix them both at the same time.

@DanielNoord
Copy link
Collaborator

Superseded by #5652.

Thanks @doctoryes for the report and initial work. This should be fixed in 2.13.0!

@DanielNoord DanielNoord closed this Jan 8, 2022
@doctoryes doctoryes deleted the juliasq/fix_attribute_error_unhandled_exception branch January 10, 2022 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crash πŸ’₯ A bug that makes pylint crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash 'FunctionDef' object has no attribute 'ancestors'
4 participants