-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add exact line coverage report #1704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The output is intended to be fed into coverage.py, the Python code coverage tool.
This is a slightly cleaned-up version of what I put together for Zulip. For example, see https://coveralls.io/jobs/15343075. |
|
||
def indentation_level(self, line_number: int) -> Optional[int]: | ||
"""Return the indentation of a line of the source (specified by | ||
zero-indexed line number). Returns None for blank lines or comments.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to handle certain idioms correctly. Example:
def f():
x = (
1) # actually not indented logically
y = """
foo""" # again not indented logically
z = \
"bar" # again not indented
Not sure how serious these issues are or if it's worth fixing them now, but at least the triple-quoted string case is pretty frequent in some code I work on (though I dislike this "idiom"). We could perhaps use a Python lexer to get more reliable indent level information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's what the TODO comment above is referring to. Of these cases, the triple-quoted string concerns me most because there is a valid use for not indenting the continuation lines of the string literal. The other two cases are probably uncommon in practice because they're ugly and hopefully rejected by linters.
It wouldn't be too hard to handle these cases correctly, but if anyone knows of an existing Python lexer that we can conveniently use from mypy, I'm all ears.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider making typed_ast give you the end line/col number as well as the starting one (because that might be useful later anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking it might be useful for reporting a missing return
statement warning, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look at having typed_ast give the ending position, at least for function definitions. It doesn't look that hard, but then I realized I don't even know how to build typed_ast and I don't really want to get into that right now.
Another approach would be to just take the largest (starting) line of a node within a function as the last line of the function. This would be too conservative in some cases (function ends with a multi-line string literal, or a close parenthesis on its own line), but it would be pretty close to correct, and for coveralls (as used by Zulip) I think it wouldn't even matter since coveralls considers continuation lines to be uninteresting anyways. Unfortunately it's not totally trivial to do that either; I might add a default_visit_node
method to TraverserVisitor
that is called at the start of every method...
Just a small note, based on the analysis in https://github.com/zulip/zulip/pull/1558/files, I think that probably we should make this support a |
(Also, would love to see this merged so that Zulip can stop using a fork of mypy for just this feature...) |
I just learned about the existence of lib2to3; does using its parser sound like a reasonable approach? |
You mean could we use lib2to3's parser instead of typed_ast? I suppose, but
I don't think we have the engineering resources to try it, and I expect
that it'd be no faster than mypy's old parser. It's also not necessarily a
more compliant parse than mypy's old parser.
|
I mean just for the purpose of this line coverage report: re-parse the input file using lib2to3 just to find out exactly where functions end. I'd be happy to try to implement that as long as lib2to3 is not obviously incapable of doing it (I haven't looked into the details at all) or too unstable to use as a dependency of mypy. |
Oh, I see. I wrote lib2to3 so long ago that I'm afraid I can't help you
answer those questions any quicker than you can do the research... Sorry!
|
Closed by #2057. |
The output is intended to be fed into coverage.py, the Python code
coverage tool.