Pylint false positives

[Jon Ribbens]

On 2018-08-15, Steven D'Aprano <steve+comp.lang.python at pearwood.info> wrote:
> On Tue, 14 Aug 2018 15:18:13 +0000, Jon Ribbens wrote:
>> On 2018-08-14, Steven D'Aprano <steve+comp.lang.python at pearwood.info>
>> wrote:
>>>     # === process abstract methods en masse === 
>>>     for name in "method_a method_b method_c method_d".split():
>>>         @abstractmethod
>>>         def inner(self):
>>>             raise NotImplementedError
>>>         inner.__name__ = name
>>>         # This is okay, writing to locals works inside the class body.
>>>         locals()[name] = inner
>>>
>>>     del inner, name  # Clean up the class namespace.
>> 
>> You have a peculiar idea of "good style"...
>
> Yes, very peculiar. It's called "factor out common operations" and "Don't 
> Repeat Yourself" :-)
>
> In a world full of people who write:
>
>     d[1] = None
>     d[2] = None
>     d[3] = None
>     d[4] = None
>
> I prefer to write:
>
>     for i in range(1, 5):
>        d[i] = None
>
> Shocking, I know.

Obviously what I'm objecting to is not the presence of a loop,
it's the presence of many obscure, bodgey and/or broken features
all lumped together:

  * code running directly under the class definition
  * creating a method then changing its name with foo.__name__
  * poking things into to the class namespace with locals()
  * using @abstractmethod without metaclass=ABCMeta
  * dynamically adding @abstractmethod methods to a class

(Not to mention your code means the methods cannot have meaningful
docstrings.)

I would refuse a pull request containing code such as the above,
unless the number of methods being dynamically created was much
larger than 4, in which case I would refuse it because the design
of a class requiring huge numbers of dynamically created methods is
almost certainly fundamentally broken.

>> No - if you think about it, there's no way Pylint could possibly know
>> that the above class has methods method_a, method_b, etc. 
>
> Well, if a human reader can do it, a sufficiently advanced source-code 
> analyser could do it too... *wink*

If we get there, we won't need to do computer programming since we'll
have strong AI and we'll just say "computer, obey my wishes" :-)

> Yes, of course you are right, in practical terms I think it is extremely 
> unlikely that PyLint or any other linter is smart enough to recognise 
> that locals()[name] = inner is equivalent to setting attributes method_a 
> etc. I actually knew that... "although to be honest I'm not sure" is an 
> understated way of saying "It isn't" :-)

It's not so much the locals()[name] thing as much as that Pylint would
have to know what the computed values of `name` would be.

>> It also doesn't like the `del inner, name` because theoretically
>> neither of those names might be defined, if the loop executed zero
>> times.
>
> That's a limitation of the linter. Don't blame me if it is too stupid to 
> recognise that looping over a non-empty string literal cannot possibly 
> loop zero times :-)

Well, it's not looping over a string literal, it's looping over the
output of a method call on a string literal. You could *maybe* argue
that "literal words here".split() is sufficiently idiomatic that
Pylint should understand it, and the Pylint issues page would be an
excellent place to do so ;-)