Fixes, cleanups and type annotations for zope.interface support #336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI seems to be broken because of actions/setup-python#171. |
Inner methods are difficult for mypy. In this particular case, mypy doesn't know that 'expr' is a Call instance, because it doesn't know that the assert that establishes that will always run before the inner method is called and 'expr' won't be modified afterward.
The visit_Call_zope_interface_classImplements() method assumes that if the first argument can be resolved to an object, that object will be a class. Since that is the correct way to use classImplements(), this assumption is usually true, but it doesn't have to hold when analyzing broken code or mis-analyzing complex code.
The old warning didn't print a source location and didn't have unit test coverage.
This default was inactive because we override 'exclude_lines'.
If we don't have a name for the decorator, we can't do anything useful with it, so there is no point in storing it. Not storing it means that the code analyzing decorators later doesn't have to deal with 'base' being None. Note that analysis can fail in theory, but in practice I was unable to find any decorators for which this happens: CPython's parser for decorators seems to be quite strict and every attempt I made to use a nonsensical decorator resulted in SyntaxError being raised.
This decorator requires arguments, but in broken code it can occur without being called.
This property was used by the pydoctor.html module, which was removed over a year ago, in commit 8040e1d. Since it did have test coverage, it wasn't obvious that it was dead code. After I added annotations, mypy reported that 'c' could be an instance of ZopeInterfaceModule, which doesn't have a 'subclasses' attribute. When I tried to find any use cases that could trigger this bug, I found out that nothing uses this property.
Now that the whole package is annotated, we no longer need to exempt this package from the `disallow_untyped_defs` setting.
If we can return an existing list under a read-only type, do so. I don't expect this to make much of a difference though: in Twisted, there are zero uses of implementsOnly() and one use of moduleProvides().
mthuurne
force-pushed
the
annotate-zopeinterface
branch
from
baa93ff
to
1b0316c
Compare
Codecov Report
@@ Coverage Diff @@
## master #336 +/- ##
==========================================
+ Coverage 85.73% 85.89% +0.15%
==========================================
Files 22 22
Lines 3927 3914 -13
Branches 817 812 -5
==========================================
- Hits 3367 3362 -5
+ Misses 359 355 -4
+ Partials 201 197 -4
Continue to review full report at Codecov.
|
The other two elements of the tuple were not used anywhere.
The other two elements of the tuple were not used anywhere.
Previously, self.builder.current was used as the scope, but this will contain the class being decorated at time of evaluation, which is not the right context for the class decorators.
There are only strings in the 'bases' attribute. I guess the other types were present at some point during the port to the 'ast' module, but they didn't stick around.
This is more efficient than looking up the name. And more reliable, if for example the name was altered because it was a duplicate.
adiroiban
approved these changes
Dec 28, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After I added annotations, mypy flagged several cases where pydoctor made invalid assumptions about types. These could lead to crashes when analyzing invalid code. Since projects don't tend to keep invalid code around, this is unlikely to occur often in practice, but of course it shouldn't happen at all. In one case, the method (
allImplementationsproperty) was broken but it was never called, so it couldn't lead to problems.