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

Duplicates found in MROs (OrderedSet) #905

Closed
hippo91 opened this issue Feb 21, 2021 · 8 comments · Fixed by #916 or PennyDreadfulMTG/Penny-Dreadful-Tools#8447
Closed

Duplicates found in MROs (OrderedSet) #905

hippo91 opened this issue Feb 21, 2021 · 8 comments · Fixed by #916 or PennyDreadfulMTG/Penny-Dreadful-Tools#8447

Comments

@hippo91
Copy link
Contributor

hippo91 commented Feb 21, 2021

Steps to reproduce

This issue is directly linked to pylint-dev/pylint#4093

  1. Write the following module:
from ordered_set import OrderedSet

class RegistryOrderedSet:
    def __init__(self, clazz, type_name="Object"):
        self._objects_dict = defaultdict(OrderedSet)
  1. Lint it

Current behavior

Traceback (most recent call last):
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/decorators.py", line 34, in cached
    return cache[func]
KeyError: <bound method ClassDef.slots of <ClassDef.OrderedSet l.55 at 0x7f2517b98dc0>>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/peillexg/PROGRAMMATION/Python/pylint_sandbox/bug_pylint_4093/test/bin/pylint", line 7, in <module>
    exec(compile(f.read(), __file__, 'exec'))
  File "/home/peillexg/PROGRAMMATION/Python/pylint/bin/pylint", line 5, in <module>
    run_pylint()
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/__init__.py", line 22, in run_pylint
    PylintRun(sys.argv[1:])
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/run.py", line 358, in __init__
    linter.check(args)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/pylinter.py", line 862, in check
    self._check_files(
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/pylinter.py", line 896, in _check_files
    self._check_file(get_ast, check_astroid_module, name, filepath, modname)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/pylinter.py", line 922, in _check_file
    check_astroid_module(ast_node)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/pylinter.py", line 1054, in check_astroid_module
    retval = self._check_astroid_module(
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/lint/pylinter.py", line 1099, in _check_astroid_module
    walker.walk(ast_node)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/utils/ast_walker.py", line 72, in walk
    callback(astroid)
  File "/home/peillexg/PROGRAMMATION/Python/pylint/pylint/checkers/variables.py", line 1182, in visit_importfrom
    module = node.do_import_module(name_parts[0])
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/mixins.py", line 99, in do_import_module
    return mymodule.import_module(
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 646, in import_module
    return MANAGER.ast_from_module_name(absmodname)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/manager.py", line 191, in ast_from_module_name
    return self.ast_from_file(found_spec.location, modname, fallback=False)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/manager.py", line 100, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/builder.py", line 139, in file_build
    return self._post_build(module, encoding)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/builder.py", line 159, in _post_build
    self.delayed_assattr(delayed)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/builder.py", line 235, in delayed_assattr
    if not _can_assign_attr(inferred, node.attrname):
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/builder.py", line 60, in _can_assign_attr
    slots = node.slots()
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/decorators.py", line 36, in cached
    cache[func] = result = func(*args, **kwargs)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 2838, in slots
    slots = list(grouped_slots())
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 2823, in grouped_slots
    for cls in self.mro()[:-1]:
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 2909, in mro
    return self._compute_mro(context=context)
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 2898, in _compute_mro
    unmerged_mro = list(clean_duplicates_mro(unmerged_mro, self, context))
  File "/home/peillexg/PROGRAMMATION/Python/astroid/astroid/scoped_nodes.py", line 110, in clean_duplicates_mro
    raise exceptions.DuplicateBasesError(
astroid.exceptions.DuplicateBasesError: Duplicates found in MROs (OrderedSet), (_GenericAlias, _Final, object), (_GenericAlias, _Final, object), (_GenericAlias, _GenericAlias) for <ClassDef.OrderedSet l.55 at 0x7f2517b98dc0>

Expected behavior

No error. It was the case with astroid 2.4.2. A bisection leads to commit 7f1d513.

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

2.5

@cdce8p
Copy link
Member

cdce8p commented Feb 22, 2021

Found a similar issue with aiohttp.

pylint 2.7.0
astroid 1.5.0

Interestingly the error occurred when using Python 3.8. However, 3.9 was fine.

# test_pylint.py

# pylint: disable=missing-class-docstring,inherit-non-class,too-few-public-methods
# pylint: disable=no-value-for-parameter,unused-import,missing-module-docstring
# pylint: disable=super-init-not-called
import abc
from typing import Sized, Iterable


class AbstractRoute(abc.ABC):
    pass


class AbstractResource(Sized, Iterable["AbstractRoute"]):
    pass


class IndexView(AbstractResource):
    def __init__(self):
        self.var = 1

Running pylint test.py results in

Traceback (most recent call last):
  File ".../astroid/astroid/decorators.py", line 34, in cached
    return cache[func]
KeyError: <bound method ClassDef.slots of <ClassDef.IndexView l.16 at 0x7fdfae20e460>>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../pylint/pylint/lint/pylinter.py", line 1030, in get_ast
    return MANAGER.ast_from_file(filepath, modname, source=True)
  File ".../astroid/astroid/manager.py", line 100, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
  File ".../astroid/astroid/builder.py", line 138, in file_build
    return self._post_build(module, encoding)
  File ".../astroid/astroid/builder.py", line 158, in _post_build
    self.delayed_assattr(delayed)
  File ".../astroid/astroid/builder.py", line 234, in delayed_assattr
    if not _can_assign_attr(inferred, node.attrname):
  File ".../astroid/astroid/builder.py", line 59, in _can_assign_attr
    slots = node.slots()
  File ".../astroid/astroid/decorators.py", line 36, in cached
    cache[func] = result = func(*args, **kwargs)
  File ".../astroid/astroid/scoped_nodes.py", line 2834, in slots
    slots = list(grouped_slots())
  File ".../astroid/astroid/scoped_nodes.py", line 2819, in grouped_slots
    for cls in self.mro()[:-1]:
  File ".../astroid/astroid/scoped_nodes.py", line 2905, in mro
    return self._compute_mro(context=context)
  File ".../astroid/astroid/scoped_nodes.py", line 2882, in _compute_mro
    mro = base._compute_mro(context=context)
  File ".../astroid/astroid/scoped_nodes.py", line 2894, in _compute_mro
    unmerged_mro = list(clean_duplicates_mro(unmerged_mro, self, context))
  File ".../astroid/astroid/scoped_nodes.py", line 109, in clean_duplicates_mro
    raise exceptions.DuplicateBasesError(
astroid.exceptions.DuplicateBasesError: Duplicates found in MROs (AbstractResource), (_GenericAlias, _Final, object), (_GenericAlias, _Final, object), (_GenericAlias, _GenericAlias) for <ClassDef.AbstractResource l.12 at 0x7fdfae20e250>.
************* Module test_pylint
test_pylint.py:1:0: F0002: <class 'astroid.exceptions.DuplicateBasesError'>: Duplicates found in MROs (AbstractResource), (_GenericAlias, _Final, object), (_GenericAlias, _Final, object), (_GenericAlias, _GenericAlias) for <ClassDef.AbstractResource l.12 at 0x7fdfae20e250>. (astroid-error)

@cdce8p
Copy link
Member

cdce8p commented Feb 22, 2021

One solution might be to add an additional condition to the check in clean_duplicates_mro to filter for typing._GenericAlias:
https://github.com/PyCQA/astroid/blob/5192907ff22fc0383206c0e5bbe3e1635d6c7132/astroid/scoped_nodes.py#L102-L110

Replacing L108 with

        if (
            names
            and names[0] is not None
            and last_index[names[0]] != 0
            and names[0][1] != "typing._GenericAlias"
        ):

does work. I just don't know enough about the way astroid works to know if that's a good fix.

--

As for Python 3.9: There it seems to be typing._BaseGenericAlias and typing._SpecialGenericAlias which previously was just typing._GenericAlias.

@cdce8p
Copy link
Member

cdce8p commented Feb 23, 2021

@hippo91 I opened #910 to hopefully fix this issue.

cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 23, 2021
commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 27, 2021
commit 80fac24471980dffc5831153fb5faac6690c4adf
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sat Feb 27 15:46:18 2021 +0100

    Use flag to guard DuplicateBasesError

commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 27, 2021
commit bc872b73dcdefd87959e08f3942fe8c0a4a746d8
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sat Feb 27 15:46:18 2021 +0100

    Use flag to guard DuplicateBasesError

commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 27, 2021
commit 3f9089a924224be472ab4856a4cdb53df9b947a3
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sat Feb 27 15:46:18 2021 +0100

    Use flag to guard DuplicateBasesError

commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 27, 2021
commit 00b9fdb81405063afe825c7a687ea82de020a41d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sat Feb 27 15:46:18 2021 +0100

    Use flag to guard DuplicateBasesError

commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
cdce8p added a commit to cdce8p/astroid that referenced this issue Feb 27, 2021
commit b151e80
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Sat Feb 27 15:46:18 2021 +0100

    Use flag to guard DuplicateBasesError

commit 3bbf4d2
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Tue Feb 23 01:48:29 2021 +0100

    Add test cases for duplicate bases error

commit 934264d
Author: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Date:   Mon Feb 22 11:00:48 2021 +0100

    Fix duplicate bases error for typing._GenericAlias

    * Fixes pylint-dev#905
@hippo91 hippo91 mentioned this issue Feb 27, 2021
2 tasks
@cdce8p cdce8p mentioned this issue Feb 28, 2021
2 tasks
Pierre-Sassoulas pushed a commit that referenced this issue Feb 28, 2021
* Adds inference support for all typing types that are defined through _alias function
* Instead of creating a new class (by the mean of TYPING_TYPE_TEMPLATE) infer the origin class : i.e MutableSet = _alias(collections.MutableSet ...) origin is the class in collections module. Needs to add __getitem method on its metaclass so that is support indexing (MutableSet[T]).
* Enable _alias mocking and testing only if python version is at least 3.7

Co-authored-by: hippo91 <guillaume.peillex@gmail.com>
@cdce8p
Copy link
Member

cdce8p commented Feb 28, 2021

@hippo91 We did our best do today to merge the fix. I believe we got it. Since you have some more experience with astroid, would you mind looking over my changes in #916? It was the first time I attempted such a complex task in astroid and there might be things I overlooked or didn't know about that could cause issues later.

Since everything seems to be fine at the moment, there is no rush. Just let me know once you did so I know what to do better next time. Thanks for your help getting this fix over the finish line.

@hippo91
Copy link
Contributor Author

hippo91 commented Mar 1, 2021

@cdce8p @Pierre-Sassoulas thanks a lot and congrats for having been able to release so fast.
@cdce8p I will have a look at #916 but it won t be before next week end. I have got almost no free time during the week. Thanks again.

@hippo91
Copy link
Contributor Author

hippo91 commented Mar 5, 2021

@cdce8p i took a look at #906 and overall it looks good. I just have two interrogations:
- what is the interest of the *_typing intermediate class? It introduces a slight difference with the mro returned by the interpreter.
- what is the interest of adding an intermediate metaclass? I supposed it is for the case where the typing classes are C coded and thus do not have ABC metaclass?

All in all it is a good job. Thanks again for taking time to achieve it.

@cdce8p
Copy link
Member

cdce8p commented Mar 5, 2021

@hippo91 An issue I noticed without the intermediate class / metaclass was that collections.abc.Iterable (and others) would suddenly become subscriptable in Python 3.7 / 3.8. That only happened if the typing module was imported as well. Although I don't know it for sure, I guess it had something to do with the way astroid caches the AST. Since I couldn't add it to the original parent class, I needed to create an intermediate one. Same for the metaclass. In the end that seemed to solve the issue.

The one thing I'm not so sure about is the MRO for

import collections.abc
class Derived(collections.abc.Iterator[int]):
    pass

During testing I noticed that it was only [Derived]. That seems a bit odd, but I didn't investigate it further. (Test case)

@hippo91
Copy link
Contributor Author

hippo91 commented Mar 7, 2021

@cdce8p it makes sense. Thanks for your explanations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants