-
Notifications
You must be signed in to change notification settings - Fork 1.6k
new module for parsing ranged suppressions #21441
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
|
|
Updated with logic to match disable/enable suppression comments and transform them into ranged suppression objects. Currently just looks for matching indentation and codes, to get a simplified implementation that works, since looking at tokens is complicated by where dedent tokens appear relative to own-line comments at the end of blocks (playground example). |
Yeah, I think we have to "ignore" |
Plus we still need to do indentation checking to make a reasonable guess as to which scope the comment should be associated: def foo():
if True:
pass
# here?
# here?
# or here? |
5ebddd7 to
34f19b1
Compare
7c1629a to
88a71a0
Compare
dfd4fa3 to
ef8f399
Compare
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
ce14617 to
607325e
Compare
|
Updated with most of the suggestions, including use of |
a09899f to
ea5b48d
Compare
MichaReiser
left a comment
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.
Thanks, this looks good.
I suggest keeping disable comments without an explicit comment as valid suppressions and instead creating a new ruff dedicated lint rule that warns about this. This way, it gives users a way to opt-out from this behavior.
| // for each pending comment, search for matching comments at the same indentation level, | ||
| // generate range suppressions for any matches, and then discard and unmatched comments | ||
| // from the outgoing indentation block | ||
| while comment_index < self.pending.len() { |
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 still think that this might be a nice simplification but up to you
ea5b48d to
288d431
Compare
* origin/main: [ty] Improve `@override`, `@final` and Liskov checks in cases where there are multiple reachable definitions (#21767) [ty] Extend `invalid-explicit-override` to also cover properties decorated with `@override` that do not override anything (#21756) [ty] Enable LRU collection for parsed module (#21749) [ty] Support typevar-specialized dynamic types in generic type aliases (#21730) Add token based `parenthesized_ranges` implementation (#21738) [ty] Default-specialization of generic type aliases (#21765) [ty] Suppress false positives when `dataclasses.dataclass(...)(cls)` is called imperatively (#21729) [syntax-error] Default type parameter followed by non-default type parameter (#21657) new module for parsing ranged suppressions (#21441) [ty] `type[T]` is assignable to an inferable typevar (#21766) Fix syntax error false positives for `await` outside functions (#21763) [ty] Improve diagnostics for unsupported comparison operations (#21737) Move `Token`, `TokenKind` and `Tokens` to `ruff-python-ast` (#21760) [ty] Don't confuse multiple occurrences of `typing.Self` when binding bound methods (#21754) Use our org-wide Renovate preset (#21759) Delete `my-script.py` (#21751) [ty] Move `all_members`, and related types/routines, out of `ide_support.rs` (#21695)
Summary
This adds a new
suppressionmodule to theruff_lintercrate, similar to the suppressionmodule for ty, to parse comments for ruff suppression directives, such as
# ruff: disable[CODE].This is just the piece for parsing and storing an IMR of the comments themselves. Next step
is to interpret parsed comments and map them to ranges, codes, etc that can be referenced
later when determining which diagnostics should be kept/filtered/whatever.
Test Plan
check_pathso that the parsing runs on all checked filesfor performance regression testing.
Issue #3711