From 8deac8be078af8f0f60a6874a803da0f4ad9721c Mon Sep 17 00:00:00 2001 From: Arham Begani Date: Thu, 15 Oct 2026 04:41:40 +0000 Subject: [PATCH 1/3] Add tests for LSP features inside quoted annotations Adds tests that show the current behavior for names inside quoted annotations such as `x: "list[Foo]"`. The checker resolves these names, but the language server treats the whole annotation as a string: hover shows only the bare type, go-to-definition finds nothing (and the name in `list["Foo"]` resolves to `list.__getitem__`), and references and rename skip occurrences inside strings. This is the bug in #4740. The next commit fixes it. Also covers strings that must stay plain strings: ordinary string values, `Literal["..."]` values and malformed annotations. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01769iN16mLYKGKoWiouacg6 --- pyrefly/lib/test/lsp/definition.rs | 102 ++++++++++++++++++++++++ pyrefly/lib/test/lsp/hover.rs | 49 ++++++++++++ pyrefly/lib/test/lsp/local_find_refs.rs | 38 +++++++++ pyrefly/lib/test/lsp/rename.rs | 43 ++++++++++ 4 files changed, 232 insertions(+) diff --git a/pyrefly/lib/test/lsp/definition.rs b/pyrefly/lib/test/lsp/definition.rs index 4480ecf6db..059024e0b2 100644 --- a/pyrefly/lib/test/lsp/definition.rs +++ b/pyrefly/lib/test/lsp/definition.rs @@ -5159,3 +5159,105 @@ object.__setattr__ "missing definition of `object.__setattr__`:\n{report}" ); } + +#[test] +fn goto_def_inside_quoted_annotation() { + // BUG: Names inside a forward-reference string have no definition, and the name in + // `list["Model"]` resolves to `list.__getitem__`. + // https://github.com/facebook/pyrefly/issues/4740 + let lib = "class Helper: ...\n"; + let code = r#" +from typing import TYPE_CHECKING +import lib + +if TYPE_CHECKING: + from lib import Helper + +class Model: ... + +a: "Model" +# ^ +b: "list[Helper]" +# ^ +c: "dict[str, list[Model]]" +# ^ +d: list["Model"] +# ^ +e: "lib.Helper" +# ^ ^ +"#; + let report = + get_batched_lsp_operations_report(&[("main", code), ("lib", lib)], get_test_report); + assert_eq!( + r#" +# main.py +10 | a: "Model" + ^ +Definition Result: None + +12 | b: "list[Helper]" + ^ +Definition Result: None + +14 | c: "dict[str, list[Model]]" + ^ +Definition Result: None + +16 | d: list["Model"] + ^ +Definition Result: +3558 | def __getitem__(self, i: SupportsIndex, /) -> _T: + ^^^^^^^^^^^ + +18 | e: "lib.Helper" + ^ +Definition Result: None + +18 | e: "lib.Helper" + ^ +Definition Result: None + + +# lib.py +"# + .trim(), + report.trim(), + ); +} + +#[test] +fn goto_def_ignores_strings_that_are_not_type_expressions() { + // Only strings the checker parsed as type expressions resolve: ordinary strings, + // `Literal` values, and malformed annotations stay plain text. + let code = r#" +from typing import Literal + +class Model: ... + +x = "Model" +# ^ +y: Literal["Model"] +# ^ +z: "list[Model" +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert_eq!( + r#" +# main.py +6 | x = "Model" + ^ +Definition Result: None + +8 | y: Literal["Model"] + ^ +Definition Result: None + +10 | z: "list[Model" + ^ +Definition Result: None +"# + .trim(), + report.trim(), + ); +} diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index e7d3b9855b..8f55e44f3c 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -4278,3 +4278,52 @@ Widget(1) ); assert!(!report.contains("from the stub"), "got: {report}"); } + +#[test] +fn hover_inside_quoted_annotation() { + // BUG: Hovering a name inside a forward-reference string shows only its bare type, + // without the name and docstring that the unquoted hover shows. + // https://github.com/facebook/pyrefly/issues/4740 + let code = r#" +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib import Helper + +class Model: + """Model docs.""" + +a: Model +# ^ +b: "Model" +# ^ +c: list[Helper] +# ^ +d: "dict[str, list[Helper]]" +# ^ +"#; + let mut env = TestEnv::new(); + env.add("main", code); + env.add("lib", "class Helper: ...\n"); + let (state, handle_for_name) = env.to_state(); + let handle = handle_for_name("main"); + let [unquoted_model, quoted_model, unquoted_helper, quoted_helper] = + extract_cursors_for_test(code)[..] + else { + panic!("expected four cursors"); + }; + let hover = |position| get_test_report(&state, &handle, position); + + assert!( + hover(unquoted_model).contains("Model docs."), + "got: {}", + hover(unquoted_model) + ); + assert_eq!(hover(quoted_model), "```python\ntype[Model]\n```"); + assert!( + hover(unquoted_helper).contains("Helper"), + "got: {}", + hover(unquoted_helper) + ); + assert_eq!(hover(quoted_helper), "```python\ntype[lib.Helper]\n```"); +} diff --git a/pyrefly/lib/test/lsp/local_find_refs.rs b/pyrefly/lib/test/lsp/local_find_refs.rs index 649748173a..1b2a6403e1 100644 --- a/pyrefly/lib/test/lsp/local_find_refs.rs +++ b/pyrefly/lib/test/lsp/local_find_refs.rs @@ -753,3 +753,41 @@ References: without_declaration.trim(), ); } + +#[test] +fn references_inside_quoted_annotations() { + // BUG: References skip names inside forward-reference strings, and finding references + // from inside one returns nothing. + // https://github.com/facebook/pyrefly/issues/4740 + let code = r#" +from typing import Literal + +class Model: ... +# ^ +a: "Model" +# ^ +b: "dict[str, list[Model]]" +c: list["Model"] +d: Literal["Model"] +e = "Model" +"#; + let report = get_batched_lsp_operations_report(&[("main", code)], |state, handle, position| { + get_test_report(state, handle, position, true) + }); + assert_eq!( + r#" +# main.py +4 | class Model: ... + ^ +References: +4 | class Model: ... + ^^^^^ + +6 | a: "Model" + ^ +References: +"# + .trim(), + report.trim(), + ); +} diff --git a/pyrefly/lib/test/lsp/rename.rs b/pyrefly/lib/test/lsp/rename.rs index 08219170e8..6b6deccdee 100644 --- a/pyrefly/lib/test/lsp/rename.rs +++ b/pyrefly/lib/test/lsp/rename.rs @@ -657,3 +657,46 @@ Rename locations: report.trim(), ); } + +#[test] +fn test_rename_from_inside_quoted_annotation() { + // BUG: Renaming from inside a forward-reference string finds nothing to rename. + // https://github.com/facebook/pyrefly/issues/4740 + let code = r#" +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib import Helper + +class Model: ... + +def make(helper: "Helper") -> "list[Model]": +# ^ + return [Model()] +x: "Helper | None" = None +# ^ +"#; + let report = get_batched_lsp_operations_report( + &[("main", code), ("lib", "class Helper: ...\n")], + get_test_report, + ); + assert_eq!( + r#" +# main.py +9 | def make(helper: "Helper") -> "list[Model]": + ^ +Rename locations: + + +12 | x: "Helper | None" = None + ^ +Rename locations: + + + +# lib.py +"# + .trim(), + report.trim(), + ); +} From 667b45b8a546d410416e6970dba3c0faf2e1c283 Mon Sep 17 00:00:00 2001 From: Arham Begani Date: Thu, 15 Oct 2026 04:50:18 +0000 Subject: [PATCH 2/3] Resolve identifiers inside quoted annotations in the LSP Part of #4740. The binder parses forward-reference strings such as `x: "list[Foo]"` and binds the names inside them at their real source ranges, but the language server located the symbol under the cursor in the original AST, where the annotation is a single string literal. Hover, go-to-definition, references and rename therefore saw a string instead of `Foo`, and `list["Foo"]` even jumped to `list.__getitem__`. When the cursor is inside a string the binder parsed as a type expression, `identifier_at` and `find_definition` now resolve against the parsed expression, and the local reference scan also looks inside such strings. A string only counts if a name in its parse has a binding key, so ordinary strings, `Literal["..."]` values and malformed annotations stay plain strings. Strings with escapes or implicit concatenation are skipped because their contents don't map exactly onto the source. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01769iN16mLYKGKoWiouacg6 --- pyrefly/lib/state/lsp.rs | 76 ++++++++++++++++++++++++- pyrefly/lib/test/lsp/definition.rs | 28 +++++---- pyrefly/lib/test/lsp/hover.rs | 16 +++--- pyrefly/lib/test/lsp/local_find_refs.rs | 17 +++++- pyrefly/lib/test/lsp/rename.rs | 16 ++++-- 5 files changed, 124 insertions(+), 29 deletions(-) diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 1b652e8336..0c2bd5fdab 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -57,6 +57,7 @@ use ruff_python_ast::ExprAttribute; use ruff_python_ast::ExprCall; use ruff_python_ast::ExprContext; use ruff_python_ast::ExprName; +use ruff_python_ast::ExprStringLiteral; use ruff_python_ast::Identifier; use ruff_python_ast::Keyword; use ruff_python_ast::ModModule; @@ -65,6 +66,7 @@ use ruff_python_ast::StmtClassDef; use ruff_python_ast::StmtImportFrom; use ruff_python_ast::UnaryOp; use ruff_python_ast::name::Name; +use ruff_python_parser::typing::parse_type_annotation; use ruff_text_size::Ranged; use ruff_text_size::TextRange; use ruff_text_size::TextSize; @@ -394,6 +396,17 @@ fn recorded_references( .collect() } +/// Parses a forward-reference string such as `"list[Foo]"` into the expression the binder +/// built from it. Only strings whose contents map exactly onto the source are parsed: escapes +/// or implicit concatenation leave no exact position for the identifiers inside. +fn parse_forward_ref(literal: &ExprStringLiteral, source: &str) -> Option { + let parsed = parse_type_annotation(literal, source).ok()?; + parsed + .kind() + .is_simple() + .then(|| parsed.expression().clone()) +} + #[derive(Clone, Debug)] pub enum DefinitionMetadata { Attribute, @@ -1087,9 +1100,40 @@ impl<'a> Transaction<'a> { ) -> Option { let mod_module = self.get_ast(handle)?; let covering_nodes = Ast::locate_node(&mod_module, position); + if let Some(forward_ref) = self.forward_ref_at(handle, &covering_nodes) { + return Self::identifier_from_covering_nodes(&Ast::locate_node_in( + AnyNodeRef::from(&forward_ref), + position, + )); + } Self::identifier_from_covering_nodes(&covering_nodes) } + /// The type expression parsed from the forward-reference string in `covering_nodes`, such + /// as `list[Foo]` from `x: "list[Foo]"`. Its nodes carry source ranges, so the identifiers + /// inside resolve through the same keys and traces as an unquoted annotation. + /// + /// The binder only parses strings in type positions, so a string counts only if a name in + /// it has a key: ordinary strings and `Literal["..."]` values stay strings. + fn forward_ref_at(&self, handle: &Handle, covering_nodes: &[AnyNodeRef]) -> Option { + fn any_name(x: &Expr, f: &impl Fn(&ExprName) -> bool) -> bool { + let mut found = matches!(x, Expr::Name(name) if f(name)); + x.recurse(&mut |x| found = found || any_name(x, f)); + found + } + let literal = covering_nodes.iter().find_map(|node| match node { + AnyNodeRef::ExprStringLiteral(literal) => Some(*literal), + _ => None, + })?; + let parsed = parse_forward_ref(literal, self.get_module_info(handle)?.contents())?; + let answers = self.get_answers(handle)?; + let bindings = answers.bindings(); + any_name(&parsed, &|name| { + bindings.is_valid_key(&Key::BoundName(ShortIdentifier::expr_name(name))) + }) + .then_some(parsed) + } + pub(crate) fn identifier_from_covering_nodes( covering_nodes: &[AnyNodeRef], ) -> Option { @@ -2999,6 +3043,12 @@ impl<'a> Transaction<'a> { return Err(EmptyResponseReason::AstNotFound); }; let covering_nodes = Ast::locate_node(&mod_module, position); + // Inside a forward-reference string, resolve against the type expression parsed from it. + let forward_ref = self.forward_ref_at(handle, &covering_nodes); + let covering_nodes = match &forward_ref { + Some(forward_ref) => Ast::locate_node_in(AnyNodeRef::from(forward_ref), position), + None => covering_nodes, + }; if covering_nodes .iter() @@ -5065,15 +5115,35 @@ impl<'a> Transaction<'a> { false } }; - fn f(x: &Expr, is_valid_use: &impl Fn(&ExprName) -> bool, res: &mut Vec) { + let module_info = self.get_module_info(handle); + // Only strings that mention the name can hold a forward reference to it. + let forward_ref = |x: &ExprStringLiteral| { + let source = module_info.as_ref()?.contents(); + if x.value.to_str().contains(expected_name.as_str()) { + parse_forward_ref(x, source) + } else { + None + } + }; + fn f( + x: &Expr, + is_valid_use: &impl Fn(&ExprName) -> bool, + forward_ref: &impl Fn(&ExprStringLiteral) -> Option, + res: &mut Vec, + ) { if let Expr::Name(x) = x && is_valid_use(x) { res.push(x.range()); } - x.recurse(&mut |x| f(x, is_valid_use, res)); + if let Expr::StringLiteral(x) = x + && let Some(parsed) = forward_ref(x) + { + f(&parsed, is_valid_use, forward_ref, res); + } + x.recurse(&mut |x| f(x, is_valid_use, forward_ref, res)); } - mod_module.visit(&mut |x| f(x, &is_valid_use, &mut references)); + mod_module.visit(&mut |x| f(x, &is_valid_use, &forward_ref, &mut references)); } Some(references) diff --git a/pyrefly/lib/test/lsp/definition.rs b/pyrefly/lib/test/lsp/definition.rs index 059024e0b2..cd4488ddab 100644 --- a/pyrefly/lib/test/lsp/definition.rs +++ b/pyrefly/lib/test/lsp/definition.rs @@ -5162,9 +5162,7 @@ object.__setattr__ #[test] fn goto_def_inside_quoted_annotation() { - // BUG: Names inside a forward-reference string have no definition, and the name in - // `list["Model"]` resolves to `list.__getitem__`. - // https://github.com/facebook/pyrefly/issues/4740 + // Each identifier inside a forward-reference string resolves like the unquoted form. let lib = "class Helper: ...\n"; let code = r#" from typing import TYPE_CHECKING @@ -5193,29 +5191,39 @@ e: "lib.Helper" # main.py 10 | a: "Model" ^ -Definition Result: None +Definition Result: +8 | class Model: ... + ^^^^^ 12 | b: "list[Helper]" ^ -Definition Result: None +Definition Result: +1 | class Helper: ... + ^^^^^^ 14 | c: "dict[str, list[Model]]" ^ -Definition Result: None +Definition Result: +8 | class Model: ... + ^^^^^ 16 | d: list["Model"] ^ Definition Result: -3558 | def __getitem__(self, i: SupportsIndex, /) -> _T: - ^^^^^^^^^^^ +8 | class Model: ... + ^^^^^ 18 | e: "lib.Helper" ^ -Definition Result: None +Definition Result: +1 | class Helper: ... + ^ 18 | e: "lib.Helper" ^ -Definition Result: None +Definition Result: +1 | class Helper: ... + ^^^^^^ # lib.py diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 8f55e44f3c..8447841e0d 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -4281,9 +4281,7 @@ Widget(1) #[test] fn hover_inside_quoted_annotation() { - // BUG: Hovering a name inside a forward-reference string shows only its bare type, - // without the name and docstring that the unquoted hover shows. - // https://github.com/facebook/pyrefly/issues/4740 + // Hovering a name inside a forward-reference string shows that name, not the string. let code = r#" from typing import TYPE_CHECKING @@ -4315,15 +4313,15 @@ d: "dict[str, list[Helper]]" let hover = |position| get_test_report(&state, &handle, position); assert!( - hover(unquoted_model).contains("Model docs."), + hover(quoted_model).contains("Model docs."), "got: {}", - hover(unquoted_model) + hover(quoted_model) ); - assert_eq!(hover(quoted_model), "```python\ntype[Model]\n```"); + assert_eq!(hover(unquoted_model), hover(quoted_model)); assert!( - hover(unquoted_helper).contains("Helper"), + hover(quoted_helper).contains("Helper"), "got: {}", - hover(unquoted_helper) + hover(quoted_helper) ); - assert_eq!(hover(quoted_helper), "```python\ntype[lib.Helper]\n```"); + assert_eq!(hover(unquoted_helper), hover(quoted_helper)); } diff --git a/pyrefly/lib/test/lsp/local_find_refs.rs b/pyrefly/lib/test/lsp/local_find_refs.rs index 1b2a6403e1..98ccd452dc 100644 --- a/pyrefly/lib/test/lsp/local_find_refs.rs +++ b/pyrefly/lib/test/lsp/local_find_refs.rs @@ -756,9 +756,6 @@ References: #[test] fn references_inside_quoted_annotations() { - // BUG: References skip names inside forward-reference strings, and finding references - // from inside one returns nothing. - // https://github.com/facebook/pyrefly/issues/4740 let code = r#" from typing import Literal @@ -782,10 +779,24 @@ e = "Model" References: 4 | class Model: ... ^^^^^ +6 | a: "Model" + ^^^^^ +8 | b: "dict[str, list[Model]]" + ^^^^^ +9 | c: list["Model"] + ^^^^^ 6 | a: "Model" ^ References: +4 | class Model: ... + ^^^^^ +6 | a: "Model" + ^^^^^ +8 | b: "dict[str, list[Model]]" + ^^^^^ +9 | c: list["Model"] + ^^^^^ "# .trim(), report.trim(), diff --git a/pyrefly/lib/test/lsp/rename.rs b/pyrefly/lib/test/lsp/rename.rs index 6b6deccdee..8b328aabf1 100644 --- a/pyrefly/lib/test/lsp/rename.rs +++ b/pyrefly/lib/test/lsp/rename.rs @@ -660,8 +660,6 @@ Rename locations: #[test] fn test_rename_from_inside_quoted_annotation() { - // BUG: Renaming from inside a forward-reference string finds nothing to rename. - // https://github.com/facebook/pyrefly/issues/4740 let code = r#" from typing import TYPE_CHECKING @@ -686,12 +684,22 @@ x: "Helper | None" = None 9 | def make(helper: "Helper") -> "list[Model]": ^ Rename locations: - +7 | class Model: ... + ^^^^^ +9 | def make(helper: "Helper") -> "list[Model]": + ^^^^^ +11 | return [Model()] + ^^^^^ 12 | x: "Helper | None" = None ^ Rename locations: - +5 | from lib import Helper + ^^^^^^ +9 | def make(helper: "Helper") -> "list[Model]": + ^^^^^^ +12 | x: "Helper | None" = None + ^^^^^^ # lib.py From 43ebb5cb3c0c056d4d43e0560082404be6806084 Mon Sep 17 00:00:00 2001 From: Arham Begani Date: Fri, 6 Nov 2026 05:33:55 +0000 Subject: [PATCH 3/3] Parse quoted annotations in the LSP with the binder's parser The LSP had two forward-reference parsers: `type_from_expression_at_impl` used `Ast::parse_type_literal`, and identifier lookup called ruff's `parse_type_annotation` directly so it could skip strings without an exact source mapping. Two parsers can disagree about ranges, and the lookup needs the ranges the binder recorded its keys under. All three cursor lookups now share `forward_ref_at`, which parses with `Ast::parse_type_literal` just as the binder does. Identifier lookup still requires a bound name in the string, so `Literal["..."]` values stay strings. Find-references is the only path that reports ranges back to the editor, so it keeps a name only where its range covers its own text. Escapes before a name shift the ranges parsed from the string, and this check keeps rename from editing the wrong text. --- pyrefly/lib/state/lsp.rs | 61 +++++++++++-------------- pyrefly/lib/test/lsp/local_find_refs.rs | 1 + 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 0c2bd5fdab..5db880cb59 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -66,7 +66,6 @@ use ruff_python_ast::StmtClassDef; use ruff_python_ast::StmtImportFrom; use ruff_python_ast::UnaryOp; use ruff_python_ast::name::Name; -use ruff_python_parser::typing::parse_type_annotation; use ruff_text_size::Ranged; use ruff_text_size::TextRange; use ruff_text_size::TextSize; @@ -396,17 +395,6 @@ fn recorded_references( .collect() } -/// Parses a forward-reference string such as `"list[Foo]"` into the expression the binder -/// built from it. Only strings whose contents map exactly onto the source are parsed: escapes -/// or implicit concatenation leave no exact position for the identifiers inside. -fn parse_forward_ref(literal: &ExprStringLiteral, source: &str) -> Option { - let parsed = parse_type_annotation(literal, source).ok()?; - parsed - .kind() - .is_simple() - .then(|| parsed.expression().clone()) -} - #[derive(Clone, Debug)] pub enum DefinitionMetadata { Attribute, @@ -1018,14 +1006,7 @@ impl<'a> Transaction<'a> { let covering_nodes = Ast::locate_node(&module, position); let mut ranges = Vec::new(); - // The binder parses forward-reference strings with `Ast::parse_type_literal`, so - // parsing again yields the same ranges under which types were recorded. - if let Some(AnyNodeRef::ExprStringLiteral(literal)) = covering_nodes - .iter() - .find(|node| matches!(node, AnyNodeRef::ExprStringLiteral(_))) - && let Some(info) = self.get_module_info(handle) - && let Ok(parsed) = Ast::parse_type_literal(literal, info.contents()) - { + if let Some(parsed) = self.forward_ref_at(handle, &covering_nodes) { ranges.extend( Ast::locate_node_in(AnyNodeRef::from(&parsed), position) .iter() @@ -1100,7 +1081,7 @@ impl<'a> Transaction<'a> { ) -> Option { let mod_module = self.get_ast(handle)?; let covering_nodes = Ast::locate_node(&mod_module, position); - if let Some(forward_ref) = self.forward_ref_at(handle, &covering_nodes) { + if let Some(forward_ref) = self.type_forward_ref_at(handle, &covering_nodes) { return Self::identifier_from_covering_nodes(&Ast::locate_node_in( AnyNodeRef::from(&forward_ref), position, @@ -1109,23 +1090,27 @@ impl<'a> Transaction<'a> { Self::identifier_from_covering_nodes(&covering_nodes) } - /// The type expression parsed from the forward-reference string in `covering_nodes`, such - /// as `list[Foo]` from `x: "list[Foo]"`. Its nodes carry source ranges, so the identifiers - /// inside resolve through the same keys and traces as an unquoted annotation. - /// - /// The binder only parses strings in type positions, so a string counts only if a name in - /// it has a key: ordinary strings and `Literal["..."]` values stay strings. + /// The string literal in `covering_nodes` parsed as a forward reference, such as `list[Foo]` + /// from `x: "list[Foo]"`. It is parsed with `Ast::parse_type_literal`, as in the binder, so + /// its nodes carry the ranges under which keys and types were recorded. fn forward_ref_at(&self, handle: &Handle, covering_nodes: &[AnyNodeRef]) -> Option { + let literal = covering_nodes.iter().find_map(|node| match node { + AnyNodeRef::ExprStringLiteral(literal) => Some(*literal), + _ => None, + })?; + Ast::parse_type_literal(literal, self.get_module_info(handle)?.contents()).ok() + } + + /// Like `forward_ref_at`, but only for strings the binder treated as types. The binder only + /// parses strings in type positions, so a string counts only if a name in it has a key: + /// ordinary strings and `Literal["..."]` values stay strings. + fn type_forward_ref_at(&self, handle: &Handle, covering_nodes: &[AnyNodeRef]) -> Option { fn any_name(x: &Expr, f: &impl Fn(&ExprName) -> bool) -> bool { let mut found = matches!(x, Expr::Name(name) if f(name)); x.recurse(&mut |x| found = found || any_name(x, f)); found } - let literal = covering_nodes.iter().find_map(|node| match node { - AnyNodeRef::ExprStringLiteral(literal) => Some(*literal), - _ => None, - })?; - let parsed = parse_forward_ref(literal, self.get_module_info(handle)?.contents())?; + let parsed = self.forward_ref_at(handle, covering_nodes)?; let answers = self.get_answers(handle)?; let bindings = answers.bindings(); any_name(&parsed, &|name| { @@ -3044,7 +3029,7 @@ impl<'a> Transaction<'a> { }; let covering_nodes = Ast::locate_node(&mod_module, position); // Inside a forward-reference string, resolve against the type expression parsed from it. - let forward_ref = self.forward_ref_at(handle, &covering_nodes); + let forward_ref = self.type_forward_ref_at(handle, &covering_nodes); let covering_nodes = match &forward_ref { Some(forward_ref) => Ast::locate_node_in(AnyNodeRef::from(forward_ref), position), None => covering_nodes, @@ -5094,8 +5079,15 @@ impl<'a> Transaction<'a> { ) -> Option> { let mut references = Vec::new(); if let Some(mod_module) = self.get_ast(handle) { + let module_info = self.get_module_info(handle); let is_valid_use = |x: &ExprName| { + // Escapes or implicit concatenation shift the ranges of names parsed from a + // forward-reference string, so a name counts only where its range covers its + // own text in the source. if x.id() == expected_name + && module_info + .as_ref() + .is_some_and(|info| info.code_at(x.range()) == expected_name.as_str()) && let Some((def_handle, Export { location, .. })) = self .find_export_for_key( handle, @@ -5115,12 +5107,11 @@ impl<'a> Transaction<'a> { false } }; - let module_info = self.get_module_info(handle); // Only strings that mention the name can hold a forward reference to it. let forward_ref = |x: &ExprStringLiteral| { let source = module_info.as_ref()?.contents(); if x.value.to_str().contains(expected_name.as_str()) { - parse_forward_ref(x, source) + Ast::parse_type_literal(x, source).ok() } else { None } diff --git a/pyrefly/lib/test/lsp/local_find_refs.rs b/pyrefly/lib/test/lsp/local_find_refs.rs index 98ccd452dc..c6e2f1aeaf 100644 --- a/pyrefly/lib/test/lsp/local_find_refs.rs +++ b/pyrefly/lib/test/lsp/local_find_refs.rs @@ -767,6 +767,7 @@ b: "dict[str, list[Model]]" c: list["Model"] d: Literal["Model"] e = "Model" +f: "\x6cist[Model]" "#; let report = get_batched_lsp_operations_report(&[("main", code)], |state, handle, position| { get_test_report(state, handle, position, true)