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

Decorator false positive: unsubscriptable-object, no-member, unsupported-membership-test, not-an-iterable, too-many-function-args #1694

Open
ghost opened this issue Oct 6, 2017 · 5 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 6, 2017

Steps to reproduce

I use simple classproperty decorator:

class classproperty(classmethod):

    def __init__(self, fget):
        if isinstance(fget, (classmethod, staticmethod)):
            self.fget = lambda cls: fget.__get__(None, cls)()
        else:
            self.fget = fget
        self.cached = {}
        super(classproperty, self).__init__(self.fget)

    def __get__(self, instance, cls):
        if cls in self.cached:
            return self.cached[cls]
        value = self.cached[cls] = self.fget(cls)
        return value

Example of usages:

from collections import OrderedDict

from sqlalchemy.ext.declarative import declarative_base, declared_attr
from sqlalchemy.inspection import inspect

Base = declarative_base()

class Model(Base):
    __abstract__ = True

    def __init__(self, **kwargs):
        super(Model, self).__init__()
        self.set_attrs(**kwargs)

    def set_attrs(self, **attrs):
        for attr_name, attr_value in attrs.items():
            if attr_name in self.fields:
                setattr(self, attr_name, attr_value)

    def to_dict(self):
        result = {}
        for attr_name in self.fields:
            attr_value = getattr(self, attr_name)
            if attr_value is not None:
                result[attr_name] = attr_value
        return result

    @declared_attr
    def __tablename__(cls):
        # pylint: disable=no-self-argument
        return plural(decapitalize(cls.__name__))

    @classproperty
    def columns(cls):
        columns = inspect(cls).mapper.column_attrs
        columns = list(sorted(columns, key=lambda column: column.key))
        return columns

    @classproperty
    def fields(cls):
        fields = OrderedDict([(column.key, getattr(cls, column.key))
                              for column in cls.columns])
        return fields

    @classproperty
    def primary_key_columns(cls):
        columns = list(inspect(cls).mapper.primary_key)
        return columns

    @classproperty
    def primary_key_fields(cls):
        fields = OrderedDict([(column.key, getattr(cls, column.key))
                              for column in cls.primary_key_columns])
        return fields

Current behavior

There are a lot of false positive errors:

[E1135(unsupported-membership-test), Model.set_attrs] Value 'self.fields' doesn't support membership test
[E1133(not-an-iterable), Model.to_dict] Non-iterable value self.fields is used in an iterating context
[E1133(not-an-iterable), Model.fields] Non-iterable value cls.columns is used in an iterating context
[E1133(not-an-iterable), Model.primary_key_fields] Non-iterable value cls.primary_key_columns is used in an iterating context

Expected behavior

If I make interface class where declare expected value the errors are not raised:

class iface(object):
    columns = ()
    fields = {}
    primary_key_columns = ()
    primary_key_fields = {}

class Model(Base, iface):
    # ...

Expected behavior: no false-positive errors

pylint --version output

pylint 1.7.4,
astroid 1.5.3
Python 3.5.2 (default, Sep 14 2017, 22:51:06)
[GCC 5.4.0 20160609]

@PCManticore
Copy link
Contributor

This is a known issue, we don't yet support custom decorators/descriptors.

@PCManticore PCManticore added Enhancement ✨ Improvement to a component Bug 🪲 labels Oct 12, 2017
@haxwithaxe
Copy link

haxwithaxe commented Apr 6, 2018

I'd like to write this but I'm not finding any unit/functional tests for the existing decorator code. Am I missing something?
Edit: clarified question.

tovrstra added a commit to tovrstra/iodata that referenced this issue May 13, 2019
Fixes theochem#73

Not all pylint exclusions can be removed, essentially due to pylint
bugs resulting in many false negatives, see

https://stackoverflow.com/questions/47972143/using-attr-with-pylint
pylint-dev/pylint#1694

The good solution is to disable all type-checking warnings of
pylint and to use a proper type checker like mypy instead.

Related changes:

- reaction_coordinate, ipoint, npoint, istep and nstep move to extra.
- many getattr and hasattr calls are replaced by nicer code.
- ArrayTypeCheckDescriptor is replaced by two simple functions.
- Document IOData.charge
- Bug got fixed in poscar format (gvecs)
cgalibern added a commit to cgalibern/opensvc that referenced this issue Dec 24, 2020
This is a known issue, pylint don't yet (2017) support custom decorators/descriptors
pylint-dev/pylint#1694
@chrisjsewell
Copy link

chrisjsewell commented Oct 13, 2021

Heya, I assume this is still an issue?

Using the example of a classproperty decorator taken from python/mypy#2266 (comment)

"""My example"""
from typing import Any, Callable, Generic, TypeVar

ReturnType = TypeVar('ReturnType')

class classproperty(Generic[ReturnType]):  # pylint: disable=invalid-name
    """A decorator for class properties"""

    def __init__(self, getter: Callable[[Any], ReturnType]) -> None:
        self.getter = getter

    def __get__(self, instance: Any, owner: Any) -> ReturnType:
        return self.getter(owner)

class Example:
    """An example class"""

    @classproperty
    def class_property(cls) -> str:
        return 'class property'

Example.class_property.isdigit()

This works correctly for static analysers like mypy and VS Code's Pylance.
But with:

pylint 2.11.1
astroid 2.8.0
Python 3.8.12

You get:

$ pylint example.py
************* Module example
example.py:19:4: E0213: Method should have "self" as first argument (no-self-argument)
example.py:19:4: R0201: Method could be a function (no-self-use)
example.py:22:0: E1101: Method 'class_property' has no 'isdigit' member (no-member)

@chrisjsewell
Copy link

FYI, I created a quick and dirty pylint plugin to get rid of these, but any feedback/improvements welcome!

"""pylint plugin for ``aiida-core`` specific issues."""
import astroid


def register(linter):  # pylint: disable=unused-argument
    """Register linters (unused)"""


def remove_classprop_imports(import_from: astroid.ImportFrom):
    """Remove any ``classproperty`` imports (handled in ``replace_classprops``)"""
    import_from.names = [name for name in import_from.names if name[0] != 'classproperty']


def replace_classprops(func: astroid.FunctionDef):
    """Replace ``classproperty`` decorated methods.

    As discussed in https://github.com/PyCQA/pylint/issues/1694, pylint does not understand the ``@classproperty``
    decorator, and so mistakes the method as a function, rather than an attribute of the class.
    If the method is annotated, this leads to pylint issuing ``no-member`` errors.

    This transform replaces ``classproperty`` decorated methods with an annotated attribute::

        from aiida.common.lang import classproperty

        class MyClass:
            @classproperty
            def my_property(cls) -> AnnotatedType:
                return cls.my_value

        MyClass.my_property.attribute  # <-- pylint issues: Method 'my_property' has no 'attribute' member (no-member)

        class MyClass:
            my_property: AnnotatedType

    """
    # ignore methods without annotations or decorators
    if not (func.returns and func.decorators and func.decorators.nodes):
        return
    # ignore methods that are specified as abstract
    if any(isinstance(node, astroid.Name) and 'abstract' in node.name for node in func.decorators.nodes):
        return
    if any(isinstance(node, astroid.Attribute) and 'abstract' in node.attrname for node in func.decorators.nodes):
        return
    # convert methods with @classproperty decorator
    if isinstance(func.decorators.nodes[0], astroid.Name) and func.decorators.nodes[0].name == 'classproperty':
        assign = astroid.AnnAssign(lineno=func.lineno, col_offset=func.col_offset, parent=func.parent)
        assign.simple = 1
        assign.target = astroid.AssignName(func.name, lineno=assign.lineno, col_offset=assign.col_offset, parent=assign)
        assign.annotation = func.returns
        assign.annotation.parent = assign
        func.parent.locals[func.name] = [assign.target]
        return assign


astroid.MANAGER.register_transform(astroid.ImportFrom, remove_classprop_imports)
astroid.MANAGER.register_transform(astroid.FunctionDef, replace_classprops)

chrisjsewell added a commit to aiidateam/aiida-core that referenced this issue Oct 19, 2021
As discussed in #5176 and pylint-dev/pylint#1694,
pylint does not understand the `@classproperty` decorator,
and so mistakes the method as a function, rather than an attribute of the class.
This commit adds a pylint transform plugin, to remove false-positives.
(see: https://pylint.pycqa.org/en/latest/how_tos/transform_plugins.html)
hf-kklein added a commit to Hochfrequenz/mig_ahb_utility_stack that referenced this issue Feb 9, 2022
stack overflow says this is due to a known bug in pylint: pylint-dev/pylint#1694
hf-kklein added a commit to Hochfrequenz/mig_ahb_utility_stack that referenced this issue Feb 9, 2022
* Bump attrs from 21.2.0 to 21.4.0

Bumps [attrs](https://github.com/python-attrs/attrs) from 21.2.0 to 21.4.0.
- [Release notes](https://github.com/python-attrs/attrs/releases)
- [Changelog](https://github.com/python-attrs/attrs/blob/main/CHANGELOG.rst)
- [Commits](python-attrs/attrs@21.2.0...21.4.0)

---
updated-dependencies:
- dependency-name: attrs
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* replacements to use modern attrs

attr.ib ➡ attrs.field
attr.s ➡ attrs.define
attr.validators ➡ attrs.validators

* ignore pylint warnings

stack overflow says this is due to a known bug in pylint: pylint-dev/pylint#1694

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: konstantin <konstantin.klein@hochfrequenz.de>
@jacobtylerwalls
Copy link
Member

Test case from #818 involving the Voluptuous library:

"""Demonstration of no-value-for-parameter using Voluptuous"""
#
# This code shows the proper use of the python Voluptuous
# library.  IsFile() is a callable that can be passed
# as a dictionary value in a schema definition.  When called
# it validates the parameter that is passed in.
#
# It is defined as
#    def IsFile(v):
# but called from within Voluptuous as
#    IsFile()('passed param')
#
# Pylint finds
#
# E: 24,39: No value for argument 'v' in function call (no-value-for-parameter)
#
from voluptuous import Schema, IsFile, MultipleInvalid

def main():
    """Testing Voluptuous IsFile() validator"""

    data = {'filename': 'not a file.txt'}

    schema = Schema({'filename': IsFile()})

    try:
        schema(data)
    except MultipleInvalid as e:
        print("Success:", e.msg)
    else:
        print("Failure: IsFile did not work")

if __name__ == "__main__":
    main()

@Pierre-Sassoulas Pierre-Sassoulas changed the title decorator false positive: unsubscriptable-object, no-member, unsupported-membership-test, not-an-iterable, too-many-function-args Decorator false positive: unsubscriptable-object, no-member, unsupported-membership-test, not-an-iterable, too-many-function-args Jun 11, 2022
edan-bainglass pushed a commit to edan-bainglass/aiida-core that referenced this issue Dec 23, 2023
As discussed in aiidateam#5176 and pylint-dev/pylint#1694,
pylint does not understand the `@classproperty` decorator,
and so mistakes the method as a function, rather than an attribute of the class.
This commit adds a pylint transform plugin, to remove false-positives.
(see: https://pylint.pycqa.org/en/latest/how_tos/transform_plugins.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants