Skip to content

Commit 6491932

Browse files
GankraGankra
authored
[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 astral-sh/ty#1748 ## Test Plan Many tests added (only the e2e test matters).
1 parent a9f2bb4 commit 6491932

File tree

7 files changed

+275
-2
lines changed

7 files changed

+275
-2
lines changed

‎crates/ty_ide/src/find_references.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,42 @@ cls = MyClass
898898
assert_snapshot!(test.references(), @"No references found");
899899
}
900900

901+
#[test]
902+
fn references_string_annotation_recursive() {
903+
let test = cursor_test(
904+
r#"
905+
ab: "a<CURSOR>b"
906+
"#,
907+
);
908+
909+
assert_snapshot!(test.references(), @r#"
910+
info[references]: Reference 1
911+
--> main.py:2:1
912+
|
913+
2 | ab: "ab"
914+
| ^^
915+
|
916+
917+
info[references]: Reference 2
918+
--> main.py:2:6
919+
|
920+
2 | ab: "ab"
921+
| ^^
922+
|
923+
"#);
924+
}
925+
926+
#[test]
927+
fn references_string_annotation_unknown() {
928+
let test = cursor_test(
929+
r#"
930+
x: "foo<CURSOR>bar"
931+
"#,
932+
);
933+
934+
assert_snapshot!(test.references(), @"No references found");
935+
}
936+
901937
#[test]
902938
fn references_match_name_stmt() {
903939
let test = cursor_test(

‎crates/ty_ide/src/goto_declaration.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,41 @@ def another_helper(path):
10731073
assert_snapshot!(test.goto_declaration(), @"No goto target found");
10741074
}
10751075

1076+
#[test]
1077+
fn goto_declaration_string_annotation_recursive() {
1078+
let test = cursor_test(
1079+
r#"
1080+
ab: "a<CURSOR>b"
1081+
"#,
1082+
);
1083+
1084+
assert_snapshot!(test.goto_declaration(), @r#"
1085+
info[goto-declaration]: Declaration
1086+
--> main.py:2:1
1087+
|
1088+
2 | ab: "ab"
1089+
| ^^
1090+
|
1091+
info: Source
1092+
--> main.py:2:6
1093+
|
1094+
2 | ab: "ab"
1095+
| ^^
1096+
|
1097+
"#);
1098+
}
1099+
1100+
#[test]
1101+
fn goto_declaration_string_annotation_unknown() {
1102+
let test = cursor_test(
1103+
r#"
1104+
x: "foo<CURSOR>bar"
1105+
"#,
1106+
);
1107+
1108+
assert_snapshot!(test.goto_declaration(), @"No goto target found");
1109+
}
1110+
10761111
#[test]
10771112
fn goto_declaration_nested_instance_attribute() {
10781113
let test = cursor_test(

‎crates/ty_ide/src/goto_type_definition.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,60 @@ mod tests {
964964
assert_snapshot!(test.goto_type_definition(), @"No goto target found");
965965
}
966966

967+
#[test]
968+
fn goto_type_string_annotation_recursive() {
969+
let test = cursor_test(
970+
r#"
971+
ab: "a<CURSOR>b"
972+
"#,
973+
);
974+
975+
assert_snapshot!(test.goto_type_definition(), @r#"
976+
info[goto-type-definition]: Type definition
977+
--> stdlib/ty_extensions.pyi:20:1
978+
|
979+
19 | # Types
980+
20 | Unknown = object()
981+
| ^^^^^^^
982+
21 | AlwaysTruthy = object()
983+
22 | AlwaysFalsy = object()
984+
|
985+
info: Source
986+
--> main.py:2:6
987+
|
988+
2 | ab: "ab"
989+
| ^^
990+
|
991+
"#);
992+
}
993+
994+
#[test]
995+
fn goto_type_string_annotation_unknown() {
996+
let test = cursor_test(
997+
r#"
998+
x: "foo<CURSOR>bar"
999+
"#,
1000+
);
1001+
1002+
assert_snapshot!(test.goto_type_definition(), @r#"
1003+
info[goto-type-definition]: Type definition
1004+
--> stdlib/ty_extensions.pyi:20:1
1005+
|
1006+
19 | # Types
1007+
20 | Unknown = object()
1008+
| ^^^^^^^
1009+
21 | AlwaysTruthy = object()
1010+
22 | AlwaysFalsy = object()
1011+
|
1012+
info: Source
1013+
--> main.py:2:5
1014+
|
1015+
2 | x: "foobar"
1016+
| ^^^^^^
1017+
|
1018+
"#);
1019+
}
1020+
9671021
#[test]
9681022
fn goto_type_match_name_stmt() {
9691023
let test = cursor_test(

‎crates/ty_ide/src/hover.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,60 @@ mod tests {
10891089
assert_snapshot!(test.hover(), @"Hover provided no content");
10901090
}
10911091

1092+
#[test]
1093+
fn hover_string_annotation_recursive() {
1094+
let test = cursor_test(
1095+
r#"
1096+
ab: "a<CURSOR>b"
1097+
"#,
1098+
);
1099+
1100+
assert_snapshot!(test.hover(), @r#"
1101+
Unknown
1102+
---------------------------------------------
1103+
```python
1104+
Unknown
1105+
```
1106+
---------------------------------------------
1107+
info[hover]: Hovered content is
1108+
--> main.py:2:6
1109+
|
1110+
2 | ab: "ab"
1111+
| ^-
1112+
| ||
1113+
| |Cursor offset
1114+
| source
1115+
|
1116+
"#);
1117+
}
1118+
1119+
#[test]
1120+
fn hover_string_annotation_unknown() {
1121+
let test = cursor_test(
1122+
r#"
1123+
x: "foo<CURSOR>bar"
1124+
"#,
1125+
);
1126+
1127+
assert_snapshot!(test.hover(), @r#"
1128+
Unknown
1129+
---------------------------------------------
1130+
```python
1131+
Unknown
1132+
```
1133+
---------------------------------------------
1134+
info[hover]: Hovered content is
1135+
--> main.py:2:5
1136+
|
1137+
2 | x: "foobar"
1138+
| ^^^-^^
1139+
| | |
1140+
| | Cursor offset
1141+
| source
1142+
|
1143+
"#);
1144+
}
1145+
10921146
#[test]
10931147
fn hover_overload_type_disambiguated1() {
10941148
let test = CursorTest::builder()

‎crates/ty_python_semantic/src/suppression.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,15 @@ pub fn create_suppression_fix(db: &dyn Db, file: File, id: LintId, range: TextRa
413413
}
414414

415415
// Always insert a new suppression at the end of the range to avoid having to deal with multiline strings
416-
// etc.
416+
// etc. Also make sure to not pass a sub-token range to `Tokens::after`.
417417
let parsed = parsed_module(db, file).load(db);
418-
let tokens_after = parsed.tokens().after(range.end());
418+
let tokens = parsed.tokens().at_offset(range.end());
419+
let token_range = match tokens {
420+
ruff_python_ast::token::TokenAt::None => range,
421+
ruff_python_ast::token::TokenAt::Single(token) => token.range(),
422+
ruff_python_ast::token::TokenAt::Between(..) => range,
423+
};
424+
let tokens_after = parsed.tokens().after(token_range.end());
419425

420426
// Same as for `line_end` when building up the `suppressions`: Ignore newlines
421427
// in multiline-strings, inside f-strings, or after a line continuation because we can't

‎crates/ty_server/tests/e2e/code_actions.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,39 @@ html.parser
309309

310310
Ok(())
311311
}
312+
313+
/// Regression test for a panic when a code-fix diagnostic points at a string annotation
314+
#[test]
315+
fn code_action_invalid_string_annotations() -> Result<()> {
316+
let workspace_root = SystemPath::new("src");
317+
let foo = SystemPath::new("src/foo.py");
318+
let foo_content = r#"
319+
ab: "foobar"
320+
"#;
321+
322+
let ty_toml = SystemPath::new("ty.toml");
323+
let ty_toml_content = "";
324+
325+
let mut server = TestServerBuilder::new()?
326+
.with_workspace(workspace_root, None)?
327+
.with_file(ty_toml, ty_toml_content)?
328+
.with_file(foo, foo_content)?
329+
.enable_pull_diagnostics(true)
330+
.build()
331+
.wait_until_workspaces_are_initialized();
332+
333+
server.open_text_document(foo, &foo_content, 1);
334+
335+
// Wait for diagnostics to be computed.
336+
let diagnostics = server.document_diagnostic_request(foo, None);
337+
let range = full_range(foo_content);
338+
let code_action_params = code_actions_at(&server, diagnostics, foo, range);
339+
340+
// Get code actions for the line with the unused ignore comment.
341+
let code_action_id = server.send_request::<CodeActionRequest>(code_action_params);
342+
let code_actions = server.await_response::<CodeActionRequest>(&code_action_id);
343+
344+
insta::assert_json_snapshot!(code_actions);
345+
346+
Ok(())
347+
}

‎crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_invalid_string_annotations.snap‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
source: crates/ty_server/tests/e2e/code_actions.rs
3+
expression: code_actions
4+
---
5+
[
6+
{
7+
"title": "Ignore 'unresolved-reference' for this line",
8+
"kind": "quickfix",
9+
"diagnostics": [
10+
{
11+
"range": {
12+
"start": {
13+
"line": 1,
14+
"character": 5
15+
},
16+
"end": {
17+
"line": 1,
18+
"character": 11
19+
}
20+
},
21+
"severity": 1,
22+
"code": "unresolved-reference",
23+
"codeDescription": {
24+
"href": "https://ty.dev/rules#unresolved-reference"
25+
},
26+
"source": "ty",
27+
"message": "Name `foobar` used when not defined",
28+
"relatedInformation": []
29+
}
30+
],
31+
"edit": {
32+
"changes": {
33+
"file://<temp_dir>/src/foo.py": [
34+
{
35+
"range": {
36+
"start": {
37+
"line": 1,
38+
"character": 12
39+
},
40+
"end": {
41+
"line": 1,
42+
"character": 12
43+
}
44+
},
45+
"newText": " # ty:ignore[unresolved-reference]"
46+
}
47+
]
48+
}
49+
},
50+
"isPreferred": false
51+
}
52+
]

0 commit comments

Comments
 (0)