[ty] Add 'remove unused ignore comment' code action (#21582)

## Summary

This PR adds a code action to remove unused ignore comments.

This PR also includes some infrastructure boilerplate to set up code
actions in the editor:

* Extend `snapshot-diagnostics` to render fixes
* Render fixes when using `--output-format=full`
* Hook up edits and the code action request in the LSP
* Add the `Unnecessary` tag to `unused-ignore-comment` diagnostics
* Group multiple unused codes into a single diagnostic

The same fix can be used on the CLI once we add `ty fix` 

Note: `unused-ignore-comment` is currently disabled by default.


https://github.com/user-attachments/assets/f9e21087-3513-4156-85d7-a90b1a7a3489
This commit is contained in:
Micha Reiser 2025-11-25 14:08:21 +01:00 committed by GitHub
parent eddb9ad38d
commit 15cb41c1f9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 751 additions and 65 deletions

2
Cargo.lock generated
View File

@ -4474,6 +4474,7 @@ dependencies = [
"quickcheck_macros",
"ruff_annotate_snippets",
"ruff_db",
"ruff_diagnostics",
"ruff_index",
"ruff_macros",
"ruff_memory_usage",
@ -4519,6 +4520,7 @@ dependencies = [
"lsp-types",
"regex",
"ruff_db",
"ruff_diagnostics",
"ruff_macros",
"ruff_notebook",
"ruff_python_ast",

View File

@ -2120,7 +2120,7 @@ old_func() # emits [deprecated] diagnostic
Default level: <a href="../rules.md#rule-levels" title="This lint has a default level of 'warn'."><code>warn</code></a> ·
Added in <a href="https://github.com/astral-sh/ty/releases/tag/0.0.1-alpha.1">0.0.1-alpha.1</a> ·
<a href="https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20ignore-comment-unknown-rule" target="_blank">Related issues</a> ·
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L43" target="_blank">View source</a>
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L47" target="_blank">View source</a>
</small>
@ -2151,7 +2151,7 @@ a = 20 / 0 # ty: ignore[division-by-zero]
Default level: <a href="../rules.md#rule-levels" title="This lint has a default level of 'warn'."><code>warn</code></a> ·
Added in <a href="https://github.com/astral-sh/ty/releases/tag/0.0.1-alpha.1">0.0.1-alpha.1</a> ·
<a href="https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-ignore-comment" target="_blank">Related issues</a> ·
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L68" target="_blank">View source</a>
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L72" target="_blank">View source</a>
</small>
@ -2536,7 +2536,7 @@ print(x) # NameError: name 'x' is not defined
Default level: <a href="../rules.md#rule-levels" title="This lint has a default level of 'ignore'."><code>ignore</code></a> ·
Added in <a href="https://github.com/astral-sh/ty/releases/tag/0.0.1-alpha.1">0.0.1-alpha.1</a> ·
<a href="https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unused-ignore-comment" target="_blank">Related issues</a> ·
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L18" target="_blank">View source</a>
<a href="https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L22" target="_blank">View source</a>
</small>

View File

@ -313,7 +313,8 @@ impl MainLoop {
let terminal_settings = db.project().settings(db).terminal();
let display_config = DisplayDiagnosticConfig::default()
.format(terminal_settings.output_format.into())
.color(colored::control::SHOULD_COLORIZE.should_colorize());
.color(colored::control::SHOULD_COLORIZE.should_colorize())
.show_fix_diff(true);
if check_revision == revision {
if db.project().files(db).is_empty() {

View File

@ -13,6 +13,7 @@ license = { workspace = true }
[dependencies]
ruff_db = { workspace = true }
ruff_annotate_snippets = { workspace = true }
ruff_diagnostics = { workspace = true }
ruff_index = { workspace = true, features = ["salsa"] }
ruff_macros = { workspace = true }
ruff_memory_usage = { workspace = true }

View File

@ -312,6 +312,15 @@ info[unused-ignore-comment]: Unused blanket `type: ignore` directive
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
73 | ): ...
|
help: Remove the unused suppression comment
69 | class D(
70 | A,
71 | # error: [unused-ignore-comment]
- A, # type: ignore[duplicate-base]
72 + A,
73 | ): ...
74 |
75 | # error: [duplicate-base]
```
@ -356,5 +365,13 @@ info[unused-ignore-comment]: Unused blanket `type: ignore` directive
82 |
83 | # fmt: on
|
help: Remove the unused suppression comment
78 | A
79 | ):
80 | # error: [unused-ignore-comment]
- x: int # type: ignore[duplicate-base]
81 + x: int
82 |
83 | # fmt: on
```

View File

@ -0,0 +1,109 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: ty_ignore.md - Suppressing errors with `ty: ignore` - Multiple unused comments
mdtest path: crates/ty_python_semantic/resources/mdtest/suppressions/ty_ignore.md
---
# Python source files
## mdtest_snippet.py
```
1 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive"
2 | a = 10 / 2 # ty: ignore[division-by-zero, unresolved-reference]
3 |
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
6 | a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
9 | a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
```
# Diagnostics
```
info[unused-ignore-comment]: Unused `ty: ignore` directive
--> src/mdtest_snippet.py:2:13
|
1 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive"
2 | a = 10 / 2 # ty: ignore[division-by-zero, unresolved-reference]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3 |
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
|
help: Remove the unused suppression comment
1 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive"
- a = 10 / 2 # ty: ignore[division-by-zero, unresolved-reference]
2 + a = 10 / 2
3 |
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
```
```
info[unused-ignore-comment]: Unused `ty: ignore` directive: 'invalid-assignment'
--> src/mdtest_snippet.py:6:26
|
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
6 | a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
| ^^^^^^^^^^^^^^^^^^
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
|
help: Remove the unused suppression code
3 |
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
- a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
6 + a = 10 / 0 # ty: ignore[division-by-zero, unresolved-reference]
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
9 | a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
```
```
info[unused-ignore-comment]: Unused `ty: ignore` directive: 'unresolved-reference'
--> src/mdtest_snippet.py:6:64
|
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
6 | a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
| ^^^^^^^^^^^^^^^^^^^^
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
|
help: Remove the unused suppression code
3 |
4 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
5 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
- a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
6 + a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero]
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
9 | a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
```
```
info[unused-ignore-comment]: Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'
--> src/mdtest_snippet.py:9:26
|
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
9 | a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: Remove the unused suppression codes
6 | a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
7 |
8 | # error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
- a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
9 + a = 10 / 0 # ty: ignore[division-by-zero]
```

View File

@ -0,0 +1,72 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: type_ignore.md - Suppressing errors with `type: ignore` - Nested comments
mdtest path: crates/ty_python_semantic/resources/mdtest/suppressions/type_ignore.md
---
# Python source files
## mdtest_snippet.py
```
1 | # fmt: off
2 | a = test /
3 | + 2 # fmt: skip # type: ignore
4 |
5 | a = test /
6 | + 2 # type: ignore # fmt: skip
7 |
8 | a = (3
9 | # error: [unused-ignore-comment]
10 | + 2) # ty:ignore[division-by-zero] # fmt: skip
11 |
12 | a = (3
13 | # error: [unused-ignore-comment]
14 | + 2) # fmt: skip # ty:ignore[division-by-zero]
```
# Diagnostics
```
info[unused-ignore-comment]: Unused `ty: ignore` directive
--> src/mdtest_snippet.py:10:9
|
8 | a = (3
9 | # error: [unused-ignore-comment]
10 | + 2) # ty:ignore[division-by-zero] # fmt: skip
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11 |
12 | a = (3
|
help: Remove the unused suppression comment
7 |
8 | a = (3
9 | # error: [unused-ignore-comment]
- + 2) # ty:ignore[division-by-zero] # fmt: skip
10 + + 2) # fmt: skip
11 |
12 | a = (3
13 | # error: [unused-ignore-comment]
```
```
info[unused-ignore-comment]: Unused `ty: ignore` directive
--> src/mdtest_snippet.py:14:21
|
12 | a = (3
13 | # error: [unused-ignore-comment]
14 | + 2) # fmt: skip # ty:ignore[division-by-zero]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: Remove the unused suppression comment
11 |
12 | a = (3
13 | # error: [unused-ignore-comment]
- + 2) # fmt: skip # ty:ignore[division-by-zero]
14 + + 2) # fmt: skip
```

View File

@ -117,6 +117,6 @@ from typing import no_type_check
@no_type_check
def test():
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive"
return x + 5 # ty: ignore[unresolved-reference]
```

View File

@ -18,7 +18,7 @@ a = 4 + test # ty: ignore[unresolved-reference]
```py
test = 10
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'possibly-unresolved-reference'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive"
a = test + 3 # ty: ignore[possibly-unresolved-reference]
```
@ -26,7 +26,7 @@ a = test + 3 # ty: ignore[possibly-unresolved-reference]
```py
# error: [unresolved-reference]
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'possibly-unresolved-reference'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive"
a = test + 3 # ty: ignore[possibly-unresolved-reference]
```
@ -50,17 +50,20 @@ a = 10 / 0 # ty: ignore[division-by-zero, unused-ignore-comment]
## Multiple unused comments
Today, ty emits a diagnostic for every unused code. We might want to group the codes by comment at
some point in the future.
ty groups unused codes that are next to each other.
<!-- snapshot-diagnostics -->
```py
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'division-by-zero'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive"
a = 10 / 2 # ty: ignore[division-by-zero, unresolved-reference]
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment'"
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'unresolved-reference'"
a = 10 / 0 # ty: ignore[invalid-assignment, division-by-zero, unresolved-reference]
# error: [unused-ignore-comment] "Unused `ty: ignore` directive: 'invalid-assignment', 'unresolved-reference'"
a = 10 / 0 # ty: ignore[invalid-assignment, unresolved-reference, division-by-zero]
```
## Multiple suppressions

View File

@ -96,6 +96,8 @@ a = test # type: ignore[name-defined]
## Nested comments
<!-- snapshot-diagnostics -->
```py
# fmt: off
a = test \
@ -103,6 +105,14 @@ a = test \
a = test \
+ 2 # type: ignore # fmt: skip
a = (3
# error: [unused-ignore-comment]
+ 2) # ty:ignore[division-by-zero] # fmt: skip
a = (3
# error: [unused-ignore-comment]
+ 2) # fmt: skip # ty:ignore[division-by-zero]
```
## Misspelled `type: ignore`

View File

@ -1,20 +1,24 @@
use crate::diagnostic::DiagnosticGuard;
use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus};
use crate::types::TypeCheckDiagnostics;
use crate::{Db, declare_lint, lint::LintId};
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, IntoDiagnosticMessage, Severity, Span,
};
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
use ruff_python_parser::TokenKind;
use ruff_python_trivia::Cursor;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use smallvec::{SmallVec, smallvec};
use std::error::Error;
use std::fmt;
use std::fmt::Formatter;
use std::fmt::Write as _;
use thiserror::Error;
use crate::diagnostic::DiagnosticGuard;
use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus};
use crate::types::TypeCheckDiagnostics;
use crate::{Db, declare_lint, lint::LintId};
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, IntoDiagnosticMessage, Severity, Span,
};
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
use ruff_diagnostics::{Edit, Fix};
use ruff_python_parser::TokenKind;
use ruff_python_trivia::Cursor;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
declare_lint! {
/// ## What it does
/// Checks for `type: ignore` or `ty: ignore` directives that are no longer applicable.
@ -109,7 +113,7 @@ pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions {
for comment in parser {
match comment {
Ok(comment) => {
builder.add_comment(&comment, TextRange::new(line_start, token.end()));
builder.add_comment(comment, TextRange::new(line_start, token.end()));
}
Err(error) => match error.kind {
ParseErrorKind::NotASuppression
@ -222,38 +226,132 @@ fn check_unused_suppressions(context: &mut CheckSuppressionsContext) {
unused.push(suppression);
}
let unused = unused.iter().filter(|suppression| {
// This looks silly but it's necessary to check again if a `unused-ignore-comment` is indeed unused
// in case the "unused" directive comes after it:
// ```py
// a = 10 / 2 # ty: ignore[unused-ignore-comment, division-by-zero]
// ```
!context.is_suppression_used(suppression.id())
});
let mut unused_iter = unused
.iter()
.filter(|suppression| {
// This looks silly but it's necessary to check again if a `unused-ignore-comment` is indeed unused
// in case the "unused" directive comes after it:
// ```py
// a = 10 / 2 # ty: ignore[unused-ignore-comment, division-by-zero]
// ```
!context.is_suppression_used(suppression.id())
})
.peekable();
for suppression in unused {
match suppression.target {
let source = source_text(context.db, context.file);
while let Some(suppression) = unused_iter.next() {
let mut diag = match suppression.target {
SuppressionTarget::All => {
let Some(diag) =
context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.range)
else {
continue;
};
diag.into_diagnostic(format_args!(
"Unused blanket `{}` directive",
suppression.kind
))
}
SuppressionTarget::Lint(lint) => {
// A single code in a `ty: ignore[<code1>, <code2>, ...]` directive
// Is this the first code directly after the `[`?
let includes_first_code = source[..suppression.range.start().to_usize()]
.trim_end()
.ends_with('[');
let mut current = suppression;
let mut unused_codes = Vec::new();
// Group successive codes together into a single diagnostic,
// or report the entire directive if all codes are unused.
while let Some(next) = unused_iter.peek() {
if let SuppressionTarget::Lint(next_lint) = next.target
&& next.comment_range == current.comment_range
&& source[TextRange::new(current.range.end(), next.range.start())]
.chars()
.all(|c| c.is_whitespace() || c == ',')
{
unused_codes.push(next_lint);
current = *next;
unused_iter.next();
} else {
break;
}
}
// Is the last suppression code the last code before the closing `]`.
let includes_last_code = source[current.range.end().to_usize()..]
.trim_start()
.starts_with(']');
// If only some codes are unused
if !includes_first_code || !includes_last_code {
let mut codes = format!("'{}'", lint.name());
for code in &unused_codes {
let _ = write!(&mut codes, ", '{code}'", code = code.name());
}
if let Some(diag) = context.report_unchecked(
&UNUSED_IGNORE_COMMENT,
TextRange::new(suppression.range.start(), current.range.end()),
) {
let mut diag = diag.into_diagnostic(format_args!(
"Unused `{kind}` directive: {codes}",
kind = suppression.kind
));
diag.primary_annotation_mut()
.unwrap()
.push_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary);
// Delete everything up to the start of the next code.
let trailing_len: TextSize = source[current.range.end().to_usize()..]
.chars()
.take_while(|c: &char| c.is_whitespace() || *c == ',')
.map(TextLen::text_len)
.sum();
// If we delete the last codes before `]`, ensure we delete any trailing comma
let leading_len: TextSize = if includes_last_code {
source[..suppression.range.start().to_usize()]
.chars()
.rev()
.take_while(|c: &char| c.is_whitespace() || *c == ',')
.map(TextLen::text_len)
.sum()
} else {
TextSize::default()
};
let fix_range = TextRange::new(
suppression.range.start() - leading_len,
current.range.end() + trailing_len,
);
diag.set_fix(Fix::safe_edit(Edit::range_deletion(fix_range)));
if unused_codes.is_empty() {
diag.help("Remove the unused suppression code");
} else {
diag.help("Remove the unused suppression codes");
}
}
continue;
}
// All codes are unused
let Some(diag) =
context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.range)
context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.comment_range)
else {
continue;
};
diag.into_diagnostic(format_args!(
"Unused `{kind}` directive: '{code}'",
kind = suppression.kind,
code = lint.name()
"Unused `{kind}` directive",
kind = suppression.kind
))
}
SuppressionTarget::Empty => {
@ -268,6 +366,12 @@ fn check_unused_suppressions(context: &mut CheckSuppressionsContext) {
))
}
};
diag.primary_annotation_mut()
.unwrap()
.push_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary);
diag.set_fix(remove_comment_fix(suppression, &source));
diag.help("Remove the unused suppression comment");
}
}
@ -469,12 +573,22 @@ pub(crate) struct Suppression {
target: SuppressionTarget,
kind: SuppressionKind,
/// The range of this specific suppression.
/// This is the same as `comment_range` except for suppression comments that suppress multiple
/// codes. For those, the range is limited to the specific code.
/// The range of the code in this suppression.
///
/// This is the same as the `comment_range` for the
/// targets [`SuppressionTarget::All`] and [`SuppressionTarget::Empty`].
range: TextRange,
/// The range of the suppression comment.
///
/// This isn't the range of the entire comment if this is a nested comment:
///
/// ```py
/// a # ty: ignore # fmt: off
/// ^^^^^^^^^^^^^
/// ```
///
/// It doesn't include the range of the nested `# fmt: off` comment.
comment_range: TextRange,
/// The range for which this suppression applies.
@ -572,7 +686,7 @@ impl<'a> SuppressionsBuilder<'a> {
}
}
fn add_comment(&mut self, comment: &SuppressionComment, line_range: TextRange) {
fn add_comment(&mut self, comment: SuppressionComment, line_range: TextRange) {
// `type: ignore` comments at the start of the file apply to the entire range.
// > A # type: ignore comment on a line by itself at the top of a file, before any docstrings,
// > imports, or other executable code, silences all errors in the file.
@ -595,7 +709,7 @@ impl<'a> SuppressionsBuilder<'a> {
}
};
match comment.codes.as_deref() {
match comment.codes {
// `type: ignore`
None => {
push_type_ignore_suppression(Suppression {
@ -621,7 +735,7 @@ impl<'a> SuppressionsBuilder<'a> {
}
// `ty: ignore[]`
Some([]) => {
Some(codes) if codes.is_empty() => {
self.line.push(Suppression {
target: SuppressionTarget::Empty,
kind: comment.kind,
@ -634,25 +748,20 @@ impl<'a> SuppressionsBuilder<'a> {
// `ty: ignore[a, b]`
Some(codes) => {
for code_range in codes {
let code = &self.source[*code_range];
let range = if codes.len() == 1 {
comment.range
} else {
*code_range
};
let code = &self.source[code_range];
match self.lint_registry.get(code) {
Ok(lint) => {
self.line.push(Suppression {
target: SuppressionTarget::Lint(lint),
kind: comment.kind,
range,
range: code_range,
comment_range: comment.range,
suppressed_range,
});
}
Err(error) => self.unknown.push(UnknownSuppression {
range,
range: code_range,
comment_range: comment.range,
reason: error,
}),
@ -795,8 +904,6 @@ impl<'src> SuppressionParser<'src> {
self.eat_whitespace();
if !self.cursor.eat_char(',') {
self.eat_whitespace();
if self.cursor.eat_char(']') {
break Ok(Some(codes));
}
@ -963,6 +1070,37 @@ enum ParseErrorKind {
CodesMissingClosingBracket(SuppressionKind),
}
fn remove_comment_fix(suppression: &Suppression, source: &str) -> Fix {
let comment_end = suppression.comment_range.end();
let comment_start = suppression.comment_range.start();
let after_comment = &source[comment_end.to_usize()..];
if !after_comment.starts_with(['\n', '\r']) {
// For example: `# ty: ignore # fmt: off`
// Don't remove the trailing whitespace up to the `ty: ignore` comment
return Fix::safe_edit(Edit::range_deletion(suppression.comment_range));
}
// Remove any leading whitespace before the comment
// to avoid unnecessary trailing whitespace once the comment is removed
let before_comment = &source[..comment_start.to_usize()];
let mut leading_len = TextSize::default();
for c in before_comment.chars().rev() {
match c {
'\n' | '\r' => break,
c if c.is_whitespace() => leading_len += c.text_len(),
_ => break,
}
}
Fix::safe_edit(Edit::range_deletion(TextRange::new(
comment_start - leading_len,
comment_end,
)))
}
#[cfg(test)]
mod tests {
use crate::suppression::{SuppressionComment, SuppressionParser};

View File

@ -12,6 +12,7 @@ license = { workspace = true }
[dependencies]
ruff_db = { workspace = true, features = ["os"] }
ruff_diagnostics = { workspace = true }
ruff_macros = { workspace = true }
ruff_notebook = { workspace = true }
ruff_python_ast = { workspace = true }

View File

@ -1,11 +1,12 @@
use lsp_types::{
ClientCapabilities, CompletionOptions, DeclarationCapability, DiagnosticOptions,
DiagnosticServerCapabilities, HoverProviderCapability, InlayHintOptions,
InlayHintServerCapabilities, MarkupKind, NotebookCellSelector, NotebookSelector, OneOf,
RenameOptions, SelectionRangeProviderCapability, SemanticTokensFullOptions,
SemanticTokensLegend, SemanticTokensOptions, SemanticTokensServerCapabilities,
ServerCapabilities, SignatureHelpOptions, TextDocumentSyncCapability, TextDocumentSyncKind,
TextDocumentSyncOptions, TypeDefinitionProviderCapability, WorkDoneProgressOptions,
ClientCapabilities, CodeActionKind, CodeActionOptions, CompletionOptions,
DeclarationCapability, DiagnosticOptions, DiagnosticServerCapabilities,
HoverProviderCapability, InlayHintOptions, InlayHintServerCapabilities, MarkupKind,
NotebookCellSelector, NotebookSelector, OneOf, RenameOptions, SelectionRangeProviderCapability,
SemanticTokensFullOptions, SemanticTokensLegend, SemanticTokensOptions,
SemanticTokensServerCapabilities, ServerCapabilities, SignatureHelpOptions,
TextDocumentSyncCapability, TextDocumentSyncKind, TextDocumentSyncOptions,
TypeDefinitionProviderCapability, WorkDoneProgressOptions,
};
use crate::PositionEncoding;
@ -376,6 +377,13 @@ pub(crate) fn server_capabilities(
ServerCapabilities {
position_encoding: Some(position_encoding.into()),
code_action_provider: Some(types::CodeActionProviderCapability::Options(
CodeActionOptions {
code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]),
..CodeActionOptions::default()
},
)),
execute_command_provider: Some(types::ExecuteCommandOptions {
commands: SupportedCommand::all()
.map(|command| command.identifier().to_string())

View File

@ -32,6 +32,9 @@ pub(super) fn request(req: server::Request) -> Task {
match req.method.as_str() {
requests::ExecuteCommand::METHOD => sync_request_task::<requests::ExecuteCommand>(req),
requests::CodeActionRequestHandler::METHOD => background_document_request_task::<
requests::CodeActionRequestHandler,
>(req, BackgroundSchedule::Worker),
requests::DocumentDiagnosticRequestHandler::METHOD => background_document_request_task::<
requests::DocumentDiagnosticRequestHandler,
>(

View File

@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::hash::{DefaultHasher, Hash as _, Hasher as _};
use lsp_types::notification::PublishDiagnostics;
@ -5,18 +6,21 @@ use lsp_types::{
CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag,
NumberOrString, PublishDiagnosticsParams, Url,
};
use ruff_diagnostics::Applicability;
use ruff_text_size::Ranged;
use rustc_hash::FxHashMap;
use ruff_db::diagnostic::{Annotation, Severity, SubDiagnostic};
use ruff_db::files::{File, FileRange};
use ruff_db::system::SystemPathBuf;
use serde::{Deserialize, Serialize};
use ty_project::{Db as _, ProjectDatabase};
use crate::Db;
use crate::document::{FileRangeExt, ToRangeExt};
use crate::session::DocumentHandle;
use crate::session::client::Client;
use crate::system::{AnySystemPath, file_to_url};
use crate::{DIAGNOSTIC_NAME, Db};
use crate::{PositionEncoding, Session};
pub(super) struct Diagnostics {
@ -351,6 +355,8 @@ pub(super) fn to_lsp_diagnostic(
);
}
let data = DiagnosticData::try_from_diagnostic(db, diagnostic, encoding);
(
url,
Diagnostic {
@ -359,10 +365,10 @@ pub(super) fn to_lsp_diagnostic(
tags,
code: Some(NumberOrString::String(diagnostic.id().to_string())),
code_description,
source: Some("ty".into()),
source: Some(DIAGNOSTIC_NAME.into()),
message: diagnostic.concise_message().to_string(),
related_information: Some(related_information),
data: None,
data: serde_json::to_value(data).ok(),
},
)
}
@ -402,3 +408,49 @@ fn sub_diagnostic_to_related_information(
message: diagnostic.concise_message().to_string(),
})
}
#[derive(Serialize, Deserialize)]
pub(crate) struct DiagnosticData {
pub(crate) fix_title: String,
pub(crate) edits: HashMap<Url, Vec<lsp_types::TextEdit>>,
}
impl DiagnosticData {
fn try_from_diagnostic(
db: &dyn Db,
diagnostic: &ruff_db::diagnostic::Diagnostic,
encoding: PositionEncoding,
) -> Option<Self> {
let fix = diagnostic
.fix()
.filter(|fix| fix.applies(Applicability::Unsafe))?;
let primary_span = diagnostic.primary_span()?;
let file = primary_span.expect_ty_file();
let mut lsp_edits: HashMap<Url, Vec<lsp_types::TextEdit>> = HashMap::new();
for edit in fix.edits() {
let location = edit
.range()
.to_lsp_range(db, file, encoding)?
.to_location()?;
lsp_edits
.entry(location.uri)
.or_default()
.push(lsp_types::TextEdit {
range: location.range,
new_text: edit.content().unwrap_or_default().to_string(),
});
}
Some(Self {
fix_title: diagnostic
.first_help_text()
.map(ToString::to_string)
.unwrap_or_else(|| format!("Fix {}", diagnostic.id())),
edits: lsp_edits,
})
}
}

View File

@ -1,3 +1,4 @@
mod code_action;
mod completion;
mod diagnostic;
mod doc_highlights;
@ -19,6 +20,7 @@ mod signature_help;
mod workspace_diagnostic;
mod workspace_symbols;
pub(super) use code_action::CodeActionRequestHandler;
pub(super) use completion::CompletionRequestHandler;
pub(super) use diagnostic::DocumentDiagnosticRequestHandler;
pub(super) use doc_highlights::DocumentHighlightRequestHandler;

View File

@ -0,0 +1,82 @@
use std::borrow::Cow;
use lsp_types::{self as types, Url, request as req};
use ty_project::ProjectDatabase;
use types::{CodeActionKind, CodeActionOrCommand};
use crate::DIAGNOSTIC_NAME;
use crate::server::Result;
use crate::server::api::RequestHandler;
use crate::server::api::diagnostics::DiagnosticData;
use crate::server::api::traits::{BackgroundDocumentRequestHandler, RetriableRequestHandler};
use crate::session::DocumentSnapshot;
use crate::session::client::Client;
pub(crate) struct CodeActionRequestHandler;
impl RequestHandler for CodeActionRequestHandler {
type RequestType = req::CodeActionRequest;
}
impl BackgroundDocumentRequestHandler for CodeActionRequestHandler {
fn document_url(params: &types::CodeActionParams) -> Cow<'_, Url> {
Cow::Borrowed(&params.text_document.uri)
}
fn run_with_snapshot(
_db: &ProjectDatabase,
_snapshot: &DocumentSnapshot,
_client: &Client,
params: types::CodeActionParams,
) -> Result<Option<types::CodeActionResponse>> {
let diagnostics = params.context.diagnostics;
let mut actions = Vec::new();
for mut diagnostic in diagnostics.into_iter().filter(|diagnostic| {
diagnostic.source.as_deref() == Some(DIAGNOSTIC_NAME)
&& range_intersect(&diagnostic.range, &params.range)
}) {
let Some(data) = diagnostic.data.take() else {
continue;
};
let data: DiagnosticData = match serde_json::from_value(data) {
Ok(data) => data,
Err(err) => {
tracing::warn!("Failed to deserialize diagnostic data: {err}");
continue;
}
};
actions.push(CodeActionOrCommand::CodeAction(lsp_types::CodeAction {
title: data.fix_title,
kind: Some(CodeActionKind::QUICKFIX),
diagnostics: Some(vec![diagnostic]),
edit: Some(lsp_types::WorkspaceEdit {
changes: Some(data.edits),
document_changes: None,
change_annotations: None,
}),
is_preferred: Some(true),
command: None,
disabled: None,
data: None,
}));
}
if actions.is_empty() {
return Ok(None);
}
Ok(Some(actions))
}
}
fn range_intersect(range: &lsp_types::Range, other: &lsp_types::Range) -> bool {
let start = range.start.max(other.start);
let end = range.end.min(other.end);
end >= start
}
impl RetriableRequestHandler for CodeActionRequestHandler {}

View File

@ -0,0 +1,117 @@
use anyhow::Result;
use lsp_types::{Position, Range, request::CodeActionRequest};
use ruff_db::system::SystemPath;
use crate::TestServerBuilder;
#[test]
fn code_action() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = "\
x = 20 / 2 # ty: ignore[division-by-zero]
";
let ty_toml = SystemPath::new("ty.toml");
let ty_toml_content = "\
[rules]
unused-ignore-comment = \"warn\"
";
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(ty_toml, ty_toml_content)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(true)
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(foo, &foo_content, 1);
// Wait for diagnostics to be computed.
let diagnostics = server.document_diagnostic_request(foo, None);
// Get code actions for the line with the unused ignore comment.
let code_action_params = lsp_types::CodeActionParams {
text_document: lsp_types::TextDocumentIdentifier {
uri: server.file_uri(foo),
},
range: Range::new(Position::new(0, 0), Position::new(0, 43)),
context: lsp_types::CodeActionContext {
diagnostics: match diagnostics {
lsp_types::DocumentDiagnosticReportResult::Report(
lsp_types::DocumentDiagnosticReport::Full(report),
) => report.full_document_diagnostic_report.items,
_ => panic!("Expected full diagnostic report"),
},
only: None,
trigger_kind: None,
},
work_done_progress_params: lsp_types::WorkDoneProgressParams::default(),
partial_result_params: lsp_types::PartialResultParams::default(),
};
let code_action_id = server.send_request::<CodeActionRequest>(code_action_params);
let code_actions = server.await_response::<CodeActionRequest>(&code_action_id);
insta::assert_json_snapshot!(code_actions);
Ok(())
}
#[test]
fn no_code_action_for_non_overlapping_range_on_same_line() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = "\
x = 20 / 2 # ty: ignore[division-by-zero]
";
let ty_toml = SystemPath::new("ty.toml");
let ty_toml_content = "\
[rules]
unused-ignore-comment = \"warn\"
";
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(ty_toml, ty_toml_content)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(true)
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(foo, &foo_content, 1);
// Wait for diagnostics to be computed.
let diagnostics = server.document_diagnostic_request(foo, None);
// Get code actions for a range that doesn't overlap with the diagnostic.
// The diagnostic is at characters 12-42, so we request actions for characters 0-10.
let code_action_params = lsp_types::CodeActionParams {
text_document: lsp_types::TextDocumentIdentifier {
uri: server.file_uri(foo),
},
range: Range::new(Position::new(0, 0), Position::new(0, 10)),
context: lsp_types::CodeActionContext {
diagnostics: match diagnostics {
lsp_types::DocumentDiagnosticReportResult::Report(
lsp_types::DocumentDiagnosticReport::Full(report),
) => report.full_document_diagnostic_report.items,
_ => panic!("Expected full diagnostic report"),
},
only: None,
trigger_kind: None,
},
work_done_progress_params: lsp_types::WorkDoneProgressParams::default(),
partial_result_params: lsp_types::PartialResultParams::default(),
};
let code_action_id = server.send_request::<CodeActionRequest>(code_action_params);
let code_actions = server.await_response::<CodeActionRequest>(&code_action_id);
// Should return None because the range doesn't overlap with the diagnostic.
assert_eq!(code_actions, None);
Ok(())
}

View File

@ -27,6 +27,7 @@
//! [`await_request`]: TestServer::await_request
//! [`await_notification`]: TestServer::await_notification
mod code_actions;
mod commands;
mod initialize;
mod inlay_hints;
@ -740,7 +741,7 @@ impl TestServer {
self.initialize_response.as_ref()
}
fn file_uri(&self, path: impl AsRef<SystemPath>) -> Url {
pub(crate) fn file_uri(&self, path: impl AsRef<SystemPath>) -> Url {
Url::from_file_path(self.test_context.root().join(path.as_ref()).as_std_path())
.expect("Path must be a valid URL")
}

View File

@ -0,0 +1,55 @@
---
source: crates/ty_server/tests/e2e/code_actions.rs
expression: code_actions
---
[
{
"title": "Remove the unused suppression comment",
"kind": "quickfix",
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 12
},
"end": {
"line": 0,
"character": 42
}
},
"severity": 2,
"code": "unused-ignore-comment",
"codeDescription": {
"href": "https://ty.dev/rules#unused-ignore-comment"
},
"source": "ty",
"message": "Unused `ty: ignore` directive",
"relatedInformation": [],
"tags": [
1
]
}
],
"edit": {
"changes": {
"file://<temp_dir>/src/foo.py": [
{
"range": {
"start": {
"line": 0,
"character": 10
},
"end": {
"line": 0,
"character": 42
}
},
"newText": ""
}
]
}
},
"isPreferred": true
}
]

View File

@ -43,6 +43,11 @@ expression: initialization_result
"documentHighlightProvider": true,
"documentSymbolProvider": true,
"workspaceSymbolProvider": true,
"codeActionProvider": {
"codeActionKinds": [
"quickfix"
]
},
"declarationProvider": true,
"executeCommandProvider": {
"commands": [

View File

@ -43,6 +43,11 @@ expression: initialization_result
"documentHighlightProvider": true,
"documentSymbolProvider": true,
"workspaceSymbolProvider": true,
"codeActionProvider": {
"codeActionKinds": [
"quickfix"
]
},
"declarationProvider": true,
"executeCommandProvider": {
"commands": [

View File

@ -641,7 +641,9 @@ fn create_diagnostic_snapshot(
test: &parser::MarkdownTest,
diagnostics: impl IntoIterator<Item = Diagnostic>,
) -> String {
let display_config = DisplayDiagnosticConfig::default().color(false);
let display_config = DisplayDiagnosticConfig::default()
.color(false)
.show_fix_diff(true);
let mut snapshot = String::new();
writeln!(snapshot).unwrap();