Basic support for type: ignore comments#15046
Conversation
type: ignore comments
|
3e58f1f to
0191f52
Compare
0191f52 to
25fd417
Compare
| use crate::{lint::LintId, Db}; | ||
|
|
||
| #[salsa::tracked(return_ref)] | ||
| pub(crate) fn suppressions(db: &dyn Db, file: File) -> IndexVec<SuppressionIndex, Suppression> { |
There was a problem hiding this comment.
Whoops. I accidentally commited this file when I rebased https://github.com/astral-sh/ruff/pull/14956/files#diff-b1807b646317ac1945d748f7db40451ac62c582b1bbc049b174e8d98f13d3f22 Probably because it didn't get stashed with git stash. So consider all code in this file as new
crates/red_knot_python_semantic/resources/mdtest/suppressions/type-ignore.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/suppressions/type-ignore.md
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/suppressions/type-ignore.md
Show resolved
Hide resolved
Co-authored-by: Carl Meyer <[email protected]> Co-authored-by: Alex Waygood <[email protected]>
cea9ab7 to
e1d77de
Compare
e1d77de to
5e2cd37
Compare
|
I adjusted the implementation to accept However, this in itself won't be sufficient for mypy compatibility. We'd also have to match mypy's diagnostic range which we currently don't for invalid assignments (#15046 (comment)). But that's a problem out of this PR's scope |
carljm
left a comment
There was a problem hiding this comment.
Looks good, thank you for digging into the multi-line behavior here!
| cast(int, "test" + | ||
| 2 # type: ignore | ||
| ) | ||
| + "other" # TODO: expected-error[invalid-operator] |
There was a problem hiding this comment.
Could we use / 0 here and then the test would function without a TODO?
Or maybe that muddies the comparison with pyright, which doesn't error on division by zero
There was a problem hiding this comment.
It doesn't work with / 0 because Red Knot doesn't support cast yet (and without the cast it's Unknown / 0 which we seem to accept)
|
Something for tomorrow. I want to take another look why we need to test both the start and the end. Ruff only tests the start. Testing both might be a problem for a future linter if we have if True:
print("test")
else:
pass # knot: ignorebecause the ignore on the |
|
Not supporting end positions does seem unintuitive. For example, the following wouldn't work y = (
4 /
0 # type: ignore
)We should push users towards using specific error codes to mitigate the issue that I pointed out in #15046 (comment) |
And try to avoid overly-large diagnostic ranges. In particular, I suspect we should try to avoid diagnostic ranges on statements, because I think those are the cases where the end is least intuitive. |
Summary
This PR adds initial support for
type: ignore. It doesn't do anything fancy yet like:# fmt: skip # type: ignoretype: ignore [code]The goal is to add this functionality in separate PRs.
Test Plan