Ignore return, yield and raises checks in abstractmethods

[maltevesper]

Ignore some checks in abstract methods to avoid false positives.

A function is considered abstract if it is decorated with @abstractmethod,
and considered pure if it has "no implementation".

Since Python does not have entirely empty functions, all functions which contain
one of the following (and possibly a docstring) are considered empty

 - ... (Ellipsis)
 - pass
 - raise NotImplementedException
 - return NotImplementedException
 - only a docstring

Closes #54

[maltevesper]

If you need any edits, further tests etc. please let me know. There are styling bits where I am a bit unsure (i.e. using mypy)

[maltevesper]

I will have a look at the failing tests tomorrow. I was a bit to optimistic that the integration commit does not break things, and forgot to rerun pytest :/

[maltevesper]

I rewrote it, I am not sure about the location of the analysis function (new file), but think it is a bit too much to just cram all that into the function description file.

I just saw that the python < 3.8 checks fail, I will have a quick look.

[maltevesper]

I believe the 3.6 continuous Integration fail to be caused by a build issue, rather than by my changes. I will now await feedback before I take any further action.

[terrencepreilly]

Ah, there's some dependency breaking the tests on earlier python versions in master. I haven't had time to look into it. If it's passing for others, that's probably fine, as I'll look into it before I merge this into master.

[terrencepreilly]

Cool, this looks pretty good so far. Could you put this into an ast.NodeVisitor subclass, and incorporate this into the AnalysisVisitor? The main point of introducing the AnalysisVisitor was to have a clean way to incorporate new analysis.

If it's confusing how to add a class to it, I did a minimal example based on your branch in 382e2de8a56a544f62ced97af261b76d00b0808a. Just make sure to move all of the functionality into the same file, and update the tests to use the visitor.

Thanks for the help!

[maltevesper]

As you probably saw in fd58822583af0233f3d313a31a6d5573e41528c3 I tried the visitor before and thought that I would break things. I learned a new thing about pythons super(), the comment from your commit 👍 did it for me. Thanks for that :).

And I thought a PR would only help me silence the linters, now I actually learned that the mixin pattern is more powerful than I thought if applied correctly.

I noticed that the pattern using super should be rolled out to a wider codebase, I did so in 937a9ef2e404a957d885ec6cf387d1b76984143d. Please read the commit message for some details and the thought process behind it.

As it turned out I had a flaw in my reasoning which is why I had to do 425077bc3fc48aac4728e35877a9d57e3c1d8cf7. (I think the problem is more related to not calling a visit_* over visit_generic, but I had the patch written with super and thought that I did not want to commit an unnecessary super change)

[maltevesper]

I added some code deduplication, at this point in time I am not sure if the new function should or should not be a static method. On the one hand, it is a static method (no use of self), on the other hand, I would like to keep things consistent.

(There are also conflicting style guides on saying if it is not using self make it static, others saying if it is a static method just turn it into a free function).

Another bit that needs review (apart from the whole thing ;) ) is the use of functools.wraps while it is a good tool, I think it has little purpose where I added it. There are no docstrings to copy in the code base and the parameters are already tied down.

[maltevesper]

I snuck in a minor change to reindent. Not sure if it is better to suggest it here or in a separate PR (although it is a tiny change). If you feel it should go somewhere else/is a bad change you could just cherry-pick the rest.

[maltevesper]

I feel apart from the function_scoped_visitor mixin I have applied the changes requested in the first review round. If I would revert the funciton_scoped_visitor changes I run into the issue described above:

function_scoped_visitor and abstract_callable_visitor both define the same visit_* functions (i.e. visit_FunctionDef). Without the changes in function_scoped_visitor mixin , abstact_callable_visitor.visit_FunctionDef would not be called.

[terrencepreilly]

Thanks for the PR! I'll try to include it in a release this weekend.

[maltevesper]

Thanks for sticking with it. A new release of darglint would be nice.