Expression-scoped ignores in Python 3.8.#6648
Expression-scoped ignores in Python 3.8.#6648ilevkivskyi merged 12 commits intopython:masterfrom brandtbucher:scoped-ignores
Conversation
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thank you for PR!
Generally looks good, but I have few comments.
|
Please let me know when this is ready for another review. |
|
Thanks for the feedback @ilevkivskyi! I've gone ahead and addressed your concerns, and am ready for another review. I've chosen to maintain a set of source comment lines as you suggested, in addition to the mapping of ignore scopes. One nice benefit is, with a change in the order of traversal, we can now avoid expensive dictionary copies at each ignored expression node. |
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks for updates! I have few more comments.
|
I'm ready for a new review @ilevkivskyi. I've updated my implementation to use a new |
brandtbucher
changed the title
brandtbucher
changed the title
|
Thanks for the guidance @ilevkivskyi! Happy to contribute. |
Fixes #3871. This change fixes a few things concerning class/function definition line numbers. For users running mypy with Python 3.8+: - Function definitions are now reported on the correct line, rather than the line of the first decorator plus the number of decorators. - Class definitions are now reported on the correct line, rather than the line of the first decorator. - **These changes are fully backward compatible with respect to `# type: ignore` lines**. In other words, existing comments will _not_ need to be moved. This is accomplished by defining an ignore "scope" just like we added to expressions with #6648. See the recent discussion at #3871 for reasoning. For other users (running mypy with Python 3.7 or earlier): - Class definition lines are reported by using the same decorator-offset estimate as functions. This is both more accurate, and necessary to get 15 or so tests to pass for both pre- and post-3.8 versions. This seems fine, since there [doesn't appear](#6539 (comment)) to be a compelling reason why it was different in the first place. - **This change is also backward-compatible with existing ignores, as above.** About 15 tests had to have their error messages / nodes moved down one line, and 4 new regression tests have been added to `check-38.test`.
2 participants
Fixes #1032. On Python 3.8, we use
end_linenoinformation to scope# type: ignorecomments to enclosing expressions. Auto-formatter users, rejoice!The
--warn-unused-ignoressemantics should be clear from the test cases. Basically, ignores in inner scopes take precedence over ignores in enclosing scopes, and the first of multiple ignores in a scope "wins".A few notes:
This changes the type ofErrors.ignored_linesfrom asetto adict, which maps ignored lines to the line of the comment they originated from. This structure is necessary in order to support--warn-unused-ignores.Context.end_line. It defaults toNonein the absence of anend_linenoor ifContextis not an expression.ErrorInfo.originnow has a third member, representing the end line of the error context. It is used to determine the range of lines to search for# type: ignorecomments. If unavailable, it defaults to the same value as the origin line.check-38.testfile has been created, and is hidden behind a version guard intestcheck.py.