Python: Modernize py/not-named-self and py/not-named-cls queries

[joefarebrother]

Rewrites queries to not rely on pointsTo analysis and instead use API graphs and dataflow where necessary.

[github-actions]

QHelp previews:

python/ql/src/Functions/NonCls.qhelp

First parameter of a class method is not named 'cls'

The first parameter of a class method (including certain special methods such as __new__), or a method of a metaclass, should be named cls.

Recommendation

Ensure that the first parameter of class methods is named cls, as recommended by the style guidelines in PEP 8.

Example

In the following example, the first parameter of the class method make is named self instead of cls.

class Entry(object):
    @classmethod
    def make(self):
        return Entry()

References

• Python PEP 8: Function and method arguments.
• Python Tutorial: Classes.
• Python Docs: classmethod.

python/ql/src/Functions/NonSelf.qhelp

First parameter of a method is not named 'self'

Normal methods should have at least one parameter and the first parameter should be called self.

Recommendation

Ensure that the first parameter of a normal method is named self, as recommended by the style guidelines in PEP 8.

If a self parameter is unneeded, the method should be decorated with staticmethod, or moved out of the class as a regular function.

Example

In the following cases, the first argument of Point.__init__ is named val instead; whereas in Point2.__init__ it is correctly named self.

class Point:
    def __init__(val, x, y):  # BAD: first parameter is mis-named 'val'
        val._x = x
        val._y = y

class Point2:
    def __init__(self, x, y):  # GOOD: first parameter is correctly named 'self'
        self._x = x
        self._y = y

References

• Python PEP 8: Function and method arguments.
• Python Tutorial: Classes.
• Python Docs: staticmethod.

[yoff]

Generally very nice, thank you for also updating the tests and the docs, I do think they read nicer.
Only one actual gripe, where the logic could read easier. You may also be able to reuse some existing predicates.

[yoff]

These two predicates also exist in DataFlowDispatch.qll.

[yoff]

This reads a bit confusing at the moment. If you want to group metaclass method argument named self with class arguments named class, could we at least rename shouldBeClass to shouldReferToClass and firstArgShouldBeNamedClsAndIsnt to firstArgShouldReferToClsAndDosnt?

Alternatively, rename firstArgNamedSelf to something like normalFirstArgNamedSelf and treat the metaclassNamedSelf case inside firstArgShouldBeNamedSelfAndIsnt. Then you will have to update the shouldBeXXX logic, of course..

[yoff]

LGTM

[yoff]

Stage timings look quite a bit worse 6-7x, but then we are finding nearly 1000 new alerts.. Are the new results looking good?

[joefarebrother]

Wasn't certain about the stage timings.
Results in cpython look fine; but those in FreeCAD and aws-parallelcluster look like mostly the result of decorators that allows a method to have no arguments.
Would it make sense to exclude alerts on decorated no-arg methods?

[yoff]

I thought we were only going to allow the two decorators found during triage, but I think I actually like this approach better; we get a higher precision this way.