False positives with not-an-iterable and unsubscriptable-object when using default values in base classes.

[pylint-bot]

Originally reported by: Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore)

---

The aforementioned checkers are having a lot of trouble with default values in a base class. There are multiple ways to express this:

#!python

class A:
   value = None

   def foo(self):
       for i in value: process(i)

class B(A):
   value = [1, 2, 3]

#!python

class A:
   def __init__(self):
      self.value = None
   def foo(self):
      return self.value[some_key]

class B(A):
   def __init__(self):
       super(B, self).__init__()
       self.value = [1, 2, 3]

Obviously these examples show that the code in question could have handled the situation a bit different, by defining what's expected to be set in the implementation classes as abstract methods or properties, but pylint doesn't catch this as well.

---

• Bitbucket: https://bitbucket.org/logilab/pylint/issue/701

[pylint-bot]

Original comment by BitBucket: dmand, GitHub: @dmand?:

---

I created an experimental checker for subscript operations that understands such cases (same strategy can be used for iterable checker and maybe some others). Trouble is, it's pretty ugly and uses rather unusual decision making strategy. Here's what it does:

1. Checker collects all ClassDef nodes found in project.
2. Checker collects all Subscript nodes to be analyzed later.
3. After all classes are collected, checker does check all saved subscript operations.
4. If Subscript is done over a class/instance attribute (e.g. self.values in the following code sample) and class has subclasses that override this atrtibute to something subscriptable - it skips the check.

For example, this checker doesn't emit error for the following piece of code (while the current one does):

#!python
class A(object):
    def __init__(self):
        self.values = None

    def pick(self):
        return self.values[1]

class B(A):
    def __init__(self):
        super(B, self).__init__()
        self.values = [1, 2, 3]

It's a pretty specialized solution, i understand. But it may be useful for us nonetheless, considering the number of such such cases.

So, @PCManticore , what do you think of this approach?

[pylint-bot]

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

Hey @dmand, I'll take a look closely in a couple of days, I want to fix the new release first.

[BrianCarter76]

Hi,

There is an equivalent problem for unsupported-membership-test. Should I raise a new bug for this against PyLint?

"""Docstring (avoid PyLint error)"""

class Aclass:
    """Docstring (avoid PyLint error)"""
    value = None

    def foo_fn(self, i):
        """Docstring (avoid PyLint error)"""
        if i in self.value:
            self.foo_fn2(i)

    def foo_fn2(self, i):
        """Docstring (avoid PyLint error)"""
        print("i: {} is in {}".format(i, self.value))

class Bclass(Aclass):
    """Docstring (avoid PyLint error)"""
    value = set([1, 2, 3])

[PCManticore]

Just leave like this for now, another issue won't help. Unfortunately, fixing this isn't an easy task and I didn't got to do it just yet.

[lovetox]

You can also see this false-positive with Gtk Widgets

from gi.repository import Gtk
a = Gtk.TreeStore()
for b in a:
    pass

[JulienPalard]

For the record I'm having the "unsupported-membership-test" false positive on:

$ pylint -rn bizarerie.py 
************* Module bizarerie
bizarerie.py:6:20: E1135: Value 'subprocess.check_output(['ls'])' doesn't support membership test (unsupported-membership-test)

----------------------------------------------------------------------
Your code has been rated at -15.00/10 (previous run: -15.00/10, +0.00)

$ cat bizarerie.py 
"""Hello pylint
"""

import subprocess

print(b"foo" not in subprocess.check_output(["ls"]))

$ python3 bizarerie.py 
True

[vtgn]

Hi! I can't believe this bug is still present more than 9 years after its notification!

Here is my case:

class A:

    class_att: List[int] | None = None

    @staticmethod
    def f():
        A.class_att = [1, 2, 3]

        for i in A.class_att: # ERROR: Non-iterable value A.att is used in an iterating context Pylint(E1133:not-an-iterable)
            print(i)

I see there are a lot of bugs notified with the E1133:not-an-iterable rule for almost 10 years. You're not supposed to be a static typing linter, so stick to your intended functionality instead of overstepping and causing gross issues like this one, especially when you haven't fixed it after 10 years.

You should better delete this rule and let this check to the real static typing linters.

If the Pylint developers still want to fix these rules about typing, so then Pylint must become a static typing linter.

Regards.

[vtgn]

@JulienPalard What do you dislike in my comment? This issue is open since almost 10 years, it clearly requires a complete static typing analysis of the objects, do you realize how hard it is for a tool that is not a typing linter? If you want this issue to be fixed, Pylint must implement a complete static typing analysis, like Mypy: do you understand how hard and how much time it takes to implement this? Pylint is even ignoring the typing assertions, like the assert method, what could fix this error, but even with this, it is hard to implement because the tool should be able to understand the check, and to know that the type in the check is iterable. 10 years later, nothing has been done, this is totally logical as I say.

[nickdrozd]

@vtgn FWIW there is a workaround for your code:

class A:
    class_att: list[int] | None = None

    @staticmethod
    def f() -> None:
        A.class_att = [1, 2, 3]

        if A.class_att is not None:  # <-- workaround
            for i in A.class_att:
                print(i)

Not ideal, but it shows Pylint inference is just somewhat buggy rather than hopelessly busted. Conceivably it could be fixed.

[vtgn]

@nickdrozd Thanks for this tips. But I'm still convinced it will be very hard to fix it. Maybe this workaround works just because Pylint takes into account the declared type of the attribute, its default value, and its last value in the current method (even badly), but it's not enough to fix all the cases without a complete typing analysis everywhere in the code and from the imported modules. And if Pylint implements this complete analysis of these iterable objects, it could become a static typing linter because this complete analysis could be used for all the objects of the code.

And because Pylint doesn't even take into account the assert method checks, it is a proof it is not supposed to be a static typing linter. The developers of Pylint have to choose if Pylint must become a static typing linter or not. You can't fix this rule for all cases if you don't want to be a static typing linter.

[Pierre-Sassoulas]

A fair point but imo the check not being perfect doesn't mean it's completely useless either. If you don't do any "crazy dynamic things" it just works ™. I don't think removing it entirely would be well received by all users. (You can disable a check but it's impossible to enable something we remove).

[vtgn]

@Pierre-Sassoulas Cases that don't work are not "crazy dynamic things". I said that, because 10 years later, the issue is still open, and because a complete fix requires a complete static typing analysis including imported modules, which is a huge work and not an objective for pylint. And this is not the only one Pylint rule which has the same problem.