pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.34k stars 1.14k forks source link

Instance of '' has no 'Topic' member (no-member) for boto3.resource('sns').Topic #3134

Closed crbunney closed 4 years ago

crbunney commented 5 years ago

Since upgrading to the latest pylint/astroid versions, we've started getting this error reported. We don't get the error when we use

astroid==2.2.5
pylint==2.3.1

Steps to reproduce

  1. pylint_test.py:
    """module docstring"""
    import boto3
    boto3.resource('sns').Topic('foo')
  2. pylint pylint_test.py

Current behavior

This error is reported: pytlint_test.py:3:0: E1101: Instance of '' has no 'Topic' member (no-member)

Expected behavior

No error is reported

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

2.3.0

also, pylint==2.4.1

crbunney commented 5 years ago

Apologies, I had the pages for both pylint and astroid open, and thought I was raising an issue on the pylint project. Is it possible to move it?

PCManticore commented 5 years ago

Can confirm the issue, thanks.

heikkilavanti commented 5 years ago

Thanks for the great tools!

You most likely don't require any assistance in debugging, but I ran into this issue as well and dug around for a while. It seems to me that this issue was introduced as a side-effect of https://github.com/PyCQA/pylint/issues/2841.

mcallaghan-bsm commented 5 years ago

TIP: we did some digging and (we suspect) this is an issue with objects that dynamically construct their member methods.

https://github.com/boto/botocore/blob/develop/botocore/client.py#L330 , boto constructs methods dynamically based on the data model

mcallaghan-bsm commented 5 years ago

Another example (for reproduction) could be:

import boto3
dynamodb = boto3.resource('dynamodb', region_name='us-west-2')
device_table = dynamodb.create_table( ... <stuff> ...)

Pylint complains about lines that do that now in the latest version

(app) root@b005dca43e4d:/app# pylint foo_file.py 
************* Module foo_file
foo_file.py:78:23: E1101: Instance of '' has no 'create_table' member (no-member)
mcallaghan-bsm commented 5 years ago

What is the suggested workaround at this time? Apart from littering the code base with pylint: disable=no-member throughout (ugly/messy!). Is there a reasonable global ignore we can do in the interim?

EDIT: for now in .pylintrc, we are doing:

generated-members=.*dynamodb.*

, and whatever other class paths are affected , with comments referencing this ticket

kirintwn commented 5 years ago

Hi @PCManticore , is there any update on this? thanks.

PCManticore commented 5 years ago

@mcallaghan-bsm generated-members sound like the best approach so far. You can also try with --ignored-classes and/or --ignored-modules to ignore the module altogether.

@kirintwn No updates yet. There are ~500 issues on this tracker and we're volunteers on this project, so it could take us a while to get to it. Unfortunately I don't have many hours available per week to work on this project, and I do what I can to make its maintenance manageable. What I'm saying is if this is an important problem for you, investigating why it happens or working on a patch or anything that can help us speed it up would be tremendous.

sodul commented 4 years ago

We can confirm that the bug is in astroids as upgrading from astroids 2.2.5 to 2.3.0 triggers the issue with pylint 2.3.1.

We cloned the astroid repo, did pip3 install -e astroid checked out the astroid-2.3.0 tag and iterated over the commits to find the culprit:

BAD: March 29 2019 70ed2cfd68e71a98697b1c13c337499732a801dc
BAD: March 28 2019 79d5a3a783cf0b5a729e4e467508e955a0cca55f
OK: March 27 2019 44bbb980f30ac0d68fa5c862ac61473fdb2ab777

The regression was introduced by this change for #2841, as mentioned above: https://github.com/PyCQA/astroid/commit/79d5a3a783cf0b5a729e4e467508e955a0cca55f

We updated astroid/brain/brain_builtin_inference.py and were able to fix the regression:

From:

def _container_generic_transform(arg, klass, iterables, build_elts):
    if isinstance(arg, klass):
        return arg
    elif isinstance(arg, iterables):
        if all(isinstance(elt, nodes.Const) for elt in arg.elts):
            elts = [elt.value for elt in arg.elts]
        else:
            # TODO: Does not handle deduplication for sets.
            elts = filter(None, map(helpers.safe_infer, arg.elts))

To:

def _container_generic_transform(arg, klass, iterables, build_elts):
    if isinstance(arg, klass):
        return arg
    elif isinstance(arg, iterables):
        if all(isinstance(elt, nodes.Const) for elt in arg.elts):
            elts = [elt.value for elt in arg.elts]
        else:
            raise UseInferenceDefault()

Of course we have no familiarity with this codebase so we might have introduced an other side effect.

@PCManticore I hope this will help you get a fix. Since this is a regression it should probably be prioritized over other open issues.

sodul commented 4 years ago

We are upgrading our systems to Python 3.8 and unfortunately the older version of pylint is incompatible and we worked around the issue by applying this patch to astroid/brain/brain_builtin_inference.py:

180c180,182
<             elts = filter(None, map(helpers.safe_infer, arg.elts))
---
>             # Patching because of https://github.com/PyCQA/pylint/issues/3134
>             # elts = filter(None, map(helpers.safe_infer, arg.elts))
>             raise UseInferenceDefault()

Everything seems to work well with pylint 2.4.4, astroid 2.3.3, the patch and Python 3.8.1.

I'll prepare a pull request on the Astroid side.

PCManticore commented 4 years ago

I had some time to investigate this.

With the aforementioned "revert" PR, astroid is not capable of inferring boto3.resource(), inferring it as Uninferable as seen with:

from astroid import extract_node
n = extract_node('''
"""module docstring"""
import boto3
boto3.resource('sns')
''')
i = next(n.infer())
print(i) # Uninferable

Without the PR, with the code from master that is inferred as Instance of boto3.resources.factory.

Unfortunately, accessing Topic will raise an InferenceError because the attribute does not exist in the corresponding class, since it's set via some mumbo-jumbo meta-programming as linked here https://github.com/PyCQA/pylint/issues/3134#issuecomment-537201659.

Because ServiceRequest itself does not indicate that it generates members dynamically, via the presence of __getattr__ or __getattribute__, pylint assumes that the member simply does not exist, leading to the no-member error you are all seeing.

Even mypy needs to resort to https://github.com/alliefitter/boto3_type_annotations to fully understand the types created by boto3 and as you can see, there's quite a lot of modules that needed to be written for static analysis to properly work. Given these tools need to parse files and interpret them, I'm not entirely sure they can properly understand meta-programming as mentioned above.

Now a solution would be to revert the aforementioned commit, but I disagree that's needed. It simply removes a functionality just to appease the meta-programming of this library.

Another solution would be to use boto3_type_annotations but pylint currently does not have the capability to load typeshed modules (but maybe it should. I'll create an issue for that)

Another solution is https://github.com/PyCQA/astroid/pull/763 which makes ServiceRequest dynamic class, preventing pylint from emitting no-member ever again for this class. Right now it's a good solution even though it prevents false negatives, at least until we have support for loading typeshed-like packages.