mirror of https://github.com/astral-sh/ruff
[ty] Fix crash when hovering an unknown string annotation (#21782)
## Summary
I have no idea what I'm doing with the fix (all the interesting stuff is
in the second commit).
The basic problem is the compiler emits the diagnostic:
```
x: "foobar"
^^^^^^
```
Which the suppression code-action hands the end of to `Tokens::after`
which then panics because that function panics if handed an offset that
is in the middle of a token.
Fixes https://github.com/astral-sh/ty/issues/1748
## Test Plan
Many tests added (only the e2e test matters).
This commit is contained in:
Aria Desires
GitHub
parent
a9f2bb41bd
commit
6491932757
|
|
@ -898,6 +898,42 @@ cls = MyClass
|
|||
assert_snapshot!(test.references(), @"No references found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn references_string_annotation_recursive() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
ab: "a<CURSOR>b"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.references(), @r#"
|
||||
info[references]: Reference 1
|
||||
--> main.py:2:1
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^^
|
||||
|
|
||||
|
||||
info[references]: Reference 2
|
||||
--> main.py:2:6
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^^
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn references_string_annotation_unknown() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
x: "foo<CURSOR>bar"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.references(), @"No references found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn references_match_name_stmt() {
|
||||
let test = cursor_test(
|
||||
|
|
|
|||
|
|
@ -1073,6 +1073,41 @@ def another_helper(path):
|
|||
assert_snapshot!(test.goto_declaration(), @"No goto target found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_declaration_string_annotation_recursive() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
ab: "a<CURSOR>b"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.goto_declaration(), @r#"
|
||||
info[goto-declaration]: Declaration
|
||||
--> main.py:2:1
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^^
|
||||
|
|
||||
info: Source
|
||||
--> main.py:2:6
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^^
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_declaration_string_annotation_unknown() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
x: "foo<CURSOR>bar"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.goto_declaration(), @"No goto target found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_declaration_nested_instance_attribute() {
|
||||
let test = cursor_test(
|
||||
|
|
|
|||
|
|
@ -964,6 +964,60 @@ mod tests {
|
|||
assert_snapshot!(test.goto_type_definition(), @"No goto target found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_type_string_annotation_recursive() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
ab: "a<CURSOR>b"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.goto_type_definition(), @r#"
|
||||
info[goto-type-definition]: Type definition
|
||||
--> stdlib/ty_extensions.pyi:20:1
|
||||
|
|
||||
19 | # Types
|
||||
20 | Unknown = object()
|
||||
| ^^^^^^^
|
||||
21 | AlwaysTruthy = object()
|
||||
22 | AlwaysFalsy = object()
|
||||
|
|
||||
info: Source
|
||||
--> main.py:2:6
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^^
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_type_string_annotation_unknown() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
x: "foo<CURSOR>bar"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.goto_type_definition(), @r#"
|
||||
info[goto-type-definition]: Type definition
|
||||
--> stdlib/ty_extensions.pyi:20:1
|
||||
|
|
||||
19 | # Types
|
||||
20 | Unknown = object()
|
||||
| ^^^^^^^
|
||||
21 | AlwaysTruthy = object()
|
||||
22 | AlwaysFalsy = object()
|
||||
|
|
||||
info: Source
|
||||
--> main.py:2:5
|
||||
|
|
||||
2 | x: "foobar"
|
||||
| ^^^^^^
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goto_type_match_name_stmt() {
|
||||
let test = cursor_test(
|
||||
|
|
|
|||
|
|
@ -1089,6 +1089,60 @@ mod tests {
|
|||
assert_snapshot!(test.hover(), @"Hover provided no content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hover_string_annotation_recursive() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
ab: "a<CURSOR>b"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.hover(), @r#"
|
||||
Unknown
|
||||
---------------------------------------------
|
||||
```python
|
||||
Unknown
|
||||
```
|
||||
---------------------------------------------
|
||||
info[hover]: Hovered content is
|
||||
--> main.py:2:6
|
||||
|
|
||||
2 | ab: "ab"
|
||||
| ^-
|
||||
| ||
|
||||
| |Cursor offset
|
||||
| source
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hover_string_annotation_unknown() {
|
||||
let test = cursor_test(
|
||||
r#"
|
||||
x: "foo<CURSOR>bar"
|
||||
"#,
|
||||
);
|
||||
|
||||
assert_snapshot!(test.hover(), @r#"
|
||||
Unknown
|
||||
---------------------------------------------
|
||||
```python
|
||||
Unknown
|
||||
```
|
||||
---------------------------------------------
|
||||
info[hover]: Hovered content is
|
||||
--> main.py:2:5
|
||||
|
|
||||
2 | x: "foobar"
|
||||
| ^^^-^^
|
||||
| | |
|
||||
| | Cursor offset
|
||||
| source
|
||||
|
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hover_overload_type_disambiguated1() {
|
||||
let test = CursorTest::builder()
|
||||
|
|
|
|||
|
|
@ -413,9 +413,15 @@ pub fn create_suppression_fix(db: &dyn Db, file: File, id: LintId, range: TextRa
|
|||
}
|
||||
|
||||
// Always insert a new suppression at the end of the range to avoid having to deal with multiline strings
|
||||
// etc.
|
||||
// etc. Also make sure to not pass a sub-token range to `Tokens::after`.
|
||||
let parsed = parsed_module(db, file).load(db);
|
||||
let tokens_after = parsed.tokens().after(range.end());
|
||||
let tokens = parsed.tokens().at_offset(range.end());
|
||||
let token_range = match tokens {
|
||||
ruff_python_ast::token::TokenAt::None => range,
|
||||
ruff_python_ast::token::TokenAt::Single(token) => token.range(),
|
||||
ruff_python_ast::token::TokenAt::Between(..) => range,
|
||||
};
|
||||
let tokens_after = parsed.tokens().after(token_range.end());
|
||||
|
||||
// Same as for `line_end` when building up the `suppressions`: Ignore newlines
|
||||
// in multiline-strings, inside f-strings, or after a line continuation because we can't
|
||||
|
|
|
|||
|
|
@ -309,3 +309,39 @@ html.parser
|
|||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Regression test for a panic when a code-fix diagnostic points at a string annotation
|
||||
#[test]
|
||||
fn code_action_invalid_string_annotations() -> Result<()> {
|
||||
let workspace_root = SystemPath::new("src");
|
||||
let foo = SystemPath::new("src/foo.py");
|
||||
let foo_content = r#"
|
||||
ab: "foobar"
|
||||
"#;
|
||||
|
||||
let ty_toml = SystemPath::new("ty.toml");
|
||||
let ty_toml_content = "";
|
||||
|
||||
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);
|
||||
let range = full_range(foo_content);
|
||||
let code_action_params = code_actions_at(&server, diagnostics, foo, range);
|
||||
|
||||
// Get code actions for the line with the unused ignore comment.
|
||||
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(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,52 @@
|
|||
---
|
||||
source: crates/ty_server/tests/e2e/code_actions.rs
|
||||
expression: code_actions
|
||||
---
|
||||
[
|
||||
{
|
||||
"title": "Ignore 'unresolved-reference' for this line",
|
||||
"kind": "quickfix",
|
||||
"diagnostics": [
|
||||
{
|
||||
"range": {
|
||||
"start": {
|
||||
"line": 1,
|
||||
"character": 5
|
||||
},
|
||||
"end": {
|
||||
"line": 1,
|
||||
"character": 11
|
||||
}
|
||||
},
|
||||
"severity": 1,
|
||||
"code": "unresolved-reference",
|
||||
"codeDescription": {
|
||||
"href": "https://ty.dev/rules#unresolved-reference"
|
||||
},
|
||||
"source": "ty",
|
||||
"message": "Name `foobar` used when not defined",
|
||||
"relatedInformation": []
|
||||
}
|
||||
],
|
||||
"edit": {
|
||||
"changes": {
|
||||
"file://<temp_dir>/src/foo.py": [
|
||||
{
|
||||
"range": {
|
||||
"start": {
|
||||
"line": 1,
|
||||
"character": 12
|
||||
},
|
||||
"end": {
|
||||
"line": 1,
|
||||
"character": 12
|
||||
}
|
||||
},
|
||||
"newText": " # ty:ignore[unresolved-reference]"
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"isPreferred": false
|
||||
}
|
||||
]
|
||||
Loading…
Reference in New Issue