surround_context: resolve trailing edge of a symbol (#1038 / #1027)

lib/elixir_sense/core/surround_context/toxic.ex

@@ -24,12 +24,19 @@ defmodule ElixirSense.Core.SurroundContext.Toxic do
   # var in an unspaced step range `a..b//c` stays a `:local_or_var`. These are kept because they
   # drive better navigation than matching the lexical oracle would.
   #
+  # It also intentionally resolves the TRAILING EDGE of a symbol - the cursor one column past the
+  # last character, where `Code.Fragment` returns `:none` (bare symbol) or the enclosing remote call
+  # (an alias before a `.`). Both broke navigation: goto-definition/references/hover failed at the
+  # end of a name (elixir-lsp/elixir-ls#1038) and an alias resolved to the function rather than the
+  # module (#1027 / the reverted elixir-lang/elixir#13150). Because every node carries an exact
+  # end-exclusive range we can do this precisely; see `resolve/3`.
+  #
   # NOTE: completion (`Code.Fragment.cursor_context` / `container_cursor_to_quoted`) is out of
   # scope and stays on `Code.Fragment`.
 
   @spec surround_context(String.t(), {pos_integer, pos_integer}) :: :none | map()
   def surround_context(source, position) when is_binary(source) do
-    classify_at(parse(source), source, position)
+    resolve(parse(source), source, position)
   rescue
     _ -> Code.Fragment.surround_context(source, position)
   catch
@@ -41,13 +48,87 @@ defmodule ElixirSense.Core.SurroundContext.Toxic do
   # reuses the AST across every cursor position instead of triggering a fresh parse per call.
   @spec surround_context(Macro.t(), String.t(), {pos_integer, pos_integer}) :: :none | map()
   def surround_context(ast, source, position) when is_binary(source) do
-    classify_at(ast, source, position)
+    resolve(ast, source, position)
   rescue
     _ -> Code.Fragment.surround_context(source, position)
   catch
     _, _ -> Code.Fragment.surround_context(source, position)
   end
 
+  # Trailing-edge resolution. `Code.Fragment.surround_context/2` (and, mirroring it, `classify_at/3`)
+  # returns `:none` when the cursor sits one column PAST the last character of a bare symbol, and
+  # returns the enclosing remote call when it sits at the end of the alias before a `.`. Both make
+  # navigation (goto-definition / references / hover) fail or point at the wrong thing when the user
+  # places the caret at the visual end of a name - elixir-lsp/elixir-ls#1038 and #1027 (upstream
+  # elixir-lang/elixir#13150, whose lexical fix was reverted). Because toxic2 gives every node an
+  # exact end-exclusive range, we can resolve this precisely: classify at the cursor and one column
+  # to its left, and
+  #
+  #   * if the character just left of the cursor is the last character of an ALIAS leaf, that alias
+  #     wins even over an enclosing dot - so the end of `Foo` in `Foo.bar()` resolves to the module
+  #     `Foo`, not the remote call (#1027). Restricted to aliases so an atom/var LHS such as `:timer`
+  #     in `:timer.sleep(...)` keeps resolving the function;
+  #   * otherwise, if the cursor itself resolves to nothing, retry one column left (#1038 - the
+  #     trailing edge of a bare symbol or of a whole remote call, e.g. before `,` / `)` / end-of-line).
+  #
+  # This only ever reaches the token whose range ENDS exactly at the cursor: if `classify_at/3` is
+  # `:none` at the cursor, any node found one column left cannot extend past the cursor (contiguous
+  # integer ranges), so it must end there. It never reaches across whitespace or a separator.
+  defp resolve(ast, source, {line, column} = position) do
+    r0 = classify_at(ast, source, position)
+
+    if column > 1 do
+      r1 = classify_at(ast, source, {line, column - 1})
+
+      cond do
+        # #1027: at the trailing edge of a name leaf, the leaf wins even over an enclosing dot
+        trailing_name_leaf?(r1, position) -> r1
+        # otherwise keep whatever the cursor itself resolved to
+        r0 != :none -> r0
+        # #1038: the cursor resolved to nothing - retry one column left, but only surface a genuine
+        # navigation target (a `key:` / operator / sigil trailing edge stays `:none`, as before)
+        navigable_result?(r1) -> r1
+        true -> :none
+      end
+    else
+      r0
+    end
+  end
+
+  # `r1` is an ALIAS leaf whose source range ends exactly at `position` (so the cursor sits one column
+  # past its last character). This is the only leaf that gets to override a non-`:none` result at the
+  # cursor, and it is deliberately restricted to `:alias` (uppercase Elixir modules) - that is exactly
+  # what #1027 / elixir-lang/elixir#13150 is about: the end of `Foo` in `Foo.bar()` should be the
+  # module `Foo`. It must NOT fire for a bare-atom / var / attribute LHS of a dot (e.g. `:timer` in
+  # `:timer.sleep(...)`, where the user wants the function), so those only resolve via the `:none`
+  # retry below - i.e. when they are NOT shadowing a remote call.
+  defp trailing_name_leaf?(%{context: {:alias, _}, end: ending}, position)
+       when ending == position,
+       do: true
+
+  defp trailing_name_leaf?(_r1, _position), do: false
+
+  # The context shapes the navigation providers can actually act on (via `SurroundContext.to_binding`).
+  # Lexical-only shapes (`:key`, `:keyword`, `:operator`, `:sigil`) are excluded so the `:none` retry
+  # never turns an empty position into a non-navigable result.
+  defp navigable_result?(%{context: context}) do
+    case context do
+      {:alias, _} -> true
+      {:alias, _, _} -> true
+      {:dot, _, _} -> true
+      {:local_or_var, _} -> true
+      {:local_call, _} -> true
+      {:local_arity, _} -> true
+      {:capture_arg, _} -> true
+      {:module_attribute, _} -> true
+      {:unquoted_atom, _} -> true
+      {:struct, _} -> true
+      _ -> false
+    end
+  end
+
+  defp navigable_result?(_r1), do: false
+
   defp classify_at(ast, source, {line, column} = position) do
     case deepest_at(ast, {line, column}) do
       {node, parent} ->

test/elixir_sense/core/surround_context/toxic_test.exs

@@ -84,13 +84,18 @@ defmodule ElixirSense.Core.SurroundContext.ToxicTest do
 
   # regressions found by adversarial review (gpt-5.5)
   describe "surround_context/2 multi-line / lexical edge cases" do
-    # the dot end must come from the callee position (line+col), not the dot - otherwise a remote
-    # call whose name is on the next line gets an impossible same-line end and breaks arity lookup.
-    test "multi-line remote call, cursor on the dot" do
+    # At the end of the alias `A` (the cursor sits on the dot, one past `A`) the trailing-edge rule
+    # resolves the module `A`, not the remote call (elixir-lsp/elixir-ls#1027). On the name `bar`
+    # itself it is still the remote call, whose end comes from the callee position (line+col) - so a
+    # name on the next line does not get an impossible same-line end that would break arity lookup.
+    test "multi-line remote call: end of alias is the module, on the name is the remote call" do
       source = "A.\n  bar\n"
 
-      assert Toxic.surround_context(source, {1, 2}) ==
-               Code.Fragment.surround_context(source, {1, 2})
+      assert %{context: {:alias, ~c"A"}, begin: {1, 1}, end: {1, 2}} =
+               Toxic.surround_context(source, {1, 2})
+
+      assert %{context: {:dot, {:alias, ~c"A"}, ~c"bar"}, begin: {1, 1}, end: {2, 6}} =
+               Toxic.surround_context(source, {2, 4})
     end
 
     # `@ attr` (space) is the unary `@` operator on a local var, not a module attribute.
@@ -101,12 +106,14 @@ defmodule ElixirSense.Core.SurroundContext.ToxicTest do
                Code.Fragment.surround_context(source, {5, 7})
     end
 
-    # the slash of remote arity `A.bar/1` must defer to Code.Fragment (:none), not be a `/` operator.
-    test "remote arity slash defers to Code.Fragment" do
+    # The slash of `A.bar/1` is not misclassified as a `/` operator. Since the cursor sits one past
+    # `bar` the trailing-edge retry resolves the remote call `A.bar` (a navigable target), which is
+    # what the user wants when the caret is at its end.
+    test "remote arity slash resolves the remote call at the trailing edge" do
       source = "A.bar/1\n"
 
-      assert Toxic.surround_context(source, {1, 6}) ==
-               Code.Fragment.surround_context(source, {1, 6})
+      assert %{context: {:dot, {:alias, ~c"A"}, ~c"bar"}, begin: {1, 1}, end: {1, 6}} =
+               Toxic.surround_context(source, {1, 6})
     end
 
     # `a..b//c` lowers to a single ternary `:..//` node, but Code.Fragment classifies the `..`
@@ -183,4 +190,61 @@ defmodule ElixirSense.Core.SurroundContext.ToxicTest do
       end
     end
   end
+
+  # Cursor placed at the *end* of a symbol (one column past its last character). `Code.Fragment`
+  # (and, mirroring it, the mid-token classification) returns `:none`, or the enclosing remote call
+  # for an alias before a `.`, which breaks goto-definition/references/hover (elixir-lsp/elixir-ls
+  # #1038) and points an alias at the function instead of the module (#1027 / elixir-lang/elixir
+  # #13150). The trailing-edge rule resolves these from the exact toxic2 ranges.
+  describe "surround_context/2 trailing edge (#1038 / #1027)" do
+    test "#1038: a bare symbol resolves at its end (one past the last char)" do
+      assert %{context: {:local_or_var, ~c"foo_bar"}, begin: {1, 1}, end: {1, 8}} =
+               Toxic.surround_context("foo_bar", {1, 8})
+
+      assert %{context: {:alias, ~c"Test"}, begin: {1, 1}, end: {1, 5}} =
+               Toxic.surround_context("Test", {1, 5})
+
+      assert %{context: {:module_attribute, ~c"my_attr"}} =
+               Toxic.surround_context("@my_attr", {1, 9})
+
+      assert %{context: {:unquoted_atom, ~c"my_atom"}} =
+               Toxic.surround_context(":my_atom", {1, 9})
+    end
+
+    test "#1038: the end of a whole remote call resolves to the call" do
+      assert %{context: {:dot, {:alias, ~c"Enum"}, ~c"count"}} =
+               Toxic.surround_context("x = Enum.count", {1, 15})
+    end
+
+    test "#1027: end of an alias before `.` is the module, not the remote call" do
+      # cursor one past `Test`, i.e. on the `.` / `,` / `)` / `/` that follows it
+      for {source, col} <- [
+            {"Test.test()", 5},
+            {"&Test.test/0", 6},
+            {"[Test, :abc]", 6},
+            {"List.wrap(Test)", 15}
+          ] do
+        assert %{context: {:alias, ~c"Test"}} = Toxic.surround_context(source, {1, col}),
+               "expected module Test at #{inspect(source)} @#{col}"
+      end
+    end
+
+    test "compound alias resolves at its end" do
+      assert %{context: {:alias, ~c"Foo.Bar"}} =
+               Toxic.surround_context("defmodule Foo.Bar", {1, 18})
+    end
+
+    test "no false positives: whitespace, separators and non-navigable trailing edges stay :none" do
+      # two columns past the symbol (a gap) - must not reach back to it
+      assert :none = Toxic.surround_context("foo", {1, 5})
+      # cursor in whitespace between two symbols
+      assert :none = Toxic.surround_context("foo   bar", {1, 5})
+      # end of a keyword key (`:` column) is a non-navigable `:key`, so stays :none
+      assert :none = Toxic.surround_context("[key: 1]", {1, 5})
+      # trailing edge of a number literal is not navigable
+      assert :none = Toxic.surround_context("x = 123", {1, 8})
+      # past end of line / empty
+      assert :none = Toxic.surround_context("", {1, 1})
+    end
+  end
 end