Pylint: Inappropriate assignment-from-none error

[sam-s]

Steps to reproduce

class MyApp(metaclass=abc.ABCMeta):
    ...
    def lock_name(self):
        "Lock file name, mutex apps should redefine this to return a string."
        return None

    def lock_path(self):
        "Lock file path, based on lock_name()."
        lock_path = self.lock_name()
        if lock_path is None:
            return None
        ...

Current behavior

 /.../application.py:109: refactor (R0201, no-self-use, MyApp.lock_name) Method could be a function
 /.../application.py:115: error (E1128, assignment-from-none, MyApp.lock_path) Assigning to function call which only returns None

Expected behavior

The no-self-use message is, arguably, appropriate (although one could argue that since the class MyApp is _abstract_, the method _cannot_ be a function.

The message assignment-from-none is definitely wrong because _some_ descendants of MyApp _will_ redefine lock_name so that it will not _always_ return None.

pylint --version output

pylint 2.1.1
astroid 2.0.4
Python 3.7.0 (default, Jun 29 2018, 20:13:13) 
[Clang 9.1.0 (clang-902.0.39.2)]

[PCManticore]

@sam-s thanks for creating an issue! I think a solution here would be to mark lock_name as an abstract method with abc.abstractmethod, as currently this check doesn't know if that method is supposed to be overridden in child classes.

(the no-self-use could be a separate issue)

[sam-s]

No, lock_name is _not_ an abstract method.
If it were abstract, I would have raised NotImplementedError there.
It is overridden by derived classes that want locking functionality, but not all derived classes want it.
Please reopen this issue.
Thanks.

[PCManticore]

@sam-s then there's no way for pylint to know that the method in question is supposed to return None or not, without a cue to indicate its status and return type. Given we don't look for annotations, it won't care as well if you annotate the method with Optional[ExpectedType].

[sam-s]

I don't think you can actually infer that a _method_ always return None since it can always be overridden.

[earonesty]

In this setup, lock_path seems like an alias for lock_name, there are better ways to do aliases.

[sam-s]

In this setup, lock_path seems like an alias for lock_name, there are better ways to do aliases.

no it is not.

[earonesty]

OK, I get it, the lock path function continues on afterward (...). @PCManticore In general, abstract base classes should be exempt from the no-self-use and unused-argument rules anyway. pylint just needs a way to know that a class was derived from or metaclasses as an ABC... and then skip some obviously incorrect rules.

Abstract base classes are clearly exempt from lots of pylint assumptions, in the same way mixin's shouldn't worry about no-member.

[colindayechtra]

Here's a simpler case why this should not be an error. The class filter is not abstract, it provides a meaningful default implementation that can be re-used by the application, and subclasses can be created to provided specialized filters. In this example, the filter is a trival None or string ... but could be anything.

class filter(object):
    def getFilter(self):
        return None  # default to "no filter"

class myFilter(filter):
    def getFilter(self):
        return "My special filter string"

# example use
defaultFilter = filter()
myFilter = myFilter()
processFiles(filter=defaultFilter)
processMoreFiles(filter=myFilter)