Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (first item), minor (rest) — hit-testing coverage gaps in ide/ide-db, all empirically confirmed with the public Analysis API.
- Caret at the exclusive end of an identifier misses (
goto_definition.rs:37, hit_test.rs): hit_test covers start <= offset < end, and goto/hover pass the raw LSP offset with no fallback, so F12 with the caret immediately after a name (where double-click/typing leaves it) returns "no definition found". On return target();: goto at first byte → Some, last byte → Some, exclusive end → None. completions.rs:285-301 already implements the identifier-biased one-byte fallback (enclosing_hit); goto/hover do not share it. Fix: extract the fallback into ide-db next to hit_test and use it from goto_definition and hover.
use directives are not hit-testable (hit_test.rs:42): the walk starts exclusively from arena[file].defs and never considers SourceFileData::directives, so goto/hover anywhere on use lib::helper; returns None. Directive segments and braced imports are arena-backed IdentIds with real locations (nodes.rs:283-284) — the data exists, it is simply never walked.
Def::Function::type_params omitted from def_children (hit_test.rs:107): hover on the declared T' in pub fn id T'(x: T) -> T returns None; the parser lowers type params with real locations (lower.rs:374-382) but the Def::Function arm destructures with ...
- Enum variant declarations unreachable (
goto_definition.rs:355): goto/hover on the declared variant Red in enum Color { Red, Green } return None; goto_in_def covers only Function/ExternFunction args and Struct fields, and hover's ident_in_def has the same gap — unlike every other declaration name (goto_definition_name_reaches_itself).
- Function-local consts invisible (
symbols.rs:29): collect_def recurses only into Struct methods and Spec defs, never into function bodies, so a Def::Constant reached via Stmt::ConstDef is not in file_defs, and neither resolve_local/in_scope_locals (VarDef-only) nor find_def_* reach it — goto on a local-const reference silently returns None, despite the doc claiming it "Collects every definition in file".
Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (first item), minor (rest) — hit-testing coverage gaps in
ide/ide-db, all empirically confirmed with the publicAnalysisAPI.goto_definition.rs:37,hit_test.rs):hit_testcoversstart <= offset < end, and goto/hover pass the raw LSP offset with no fallback, so F12 with the caret immediately after a name (where double-click/typing leaves it) returns "no definition found". Onreturn target();: goto at first byte → Some, last byte → Some, exclusive end → None.completions.rs:285-301already implements the identifier-biased one-byte fallback (enclosing_hit); goto/hover do not share it. Fix: extract the fallback into ide-db next tohit_testand use it from goto_definition and hover.usedirectives are not hit-testable (hit_test.rs:42): the walk starts exclusively fromarena[file].defsand never considersSourceFileData::directives, so goto/hover anywhere onuse lib::helper;returns None. Directive segments and braced imports are arena-backedIdentIds with real locations (nodes.rs:283-284) — the data exists, it is simply never walked.Def::Function::type_paramsomitted fromdef_children(hit_test.rs:107): hover on the declaredT'inpub fn id T'(x: T) -> Treturns None; the parser lowers type params with real locations (lower.rs:374-382) but theDef::Functionarm destructures with...goto_definition.rs:355): goto/hover on the declared variantRedinenum Color { Red, Green }return None;goto_in_defcovers only Function/ExternFunction args and Struct fields, and hover'sident_in_defhas the same gap — unlike every other declaration name (goto_definition_name_reaches_itself).symbols.rs:29):collect_defrecurses only into Struct methods and Spec defs, never into function bodies, so aDef::Constantreached viaStmt::ConstDefis not infile_defs, and neitherresolve_local/in_scope_locals(VarDef-only) norfind_def_*reach it — goto on a local-const reference silently returns None, despite the doc claiming it "Collects every definition infile".