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,124 @@ 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. The classification one column to the
+  # left only matters when the cursor did NOT land on a real, in-range, non-dot symbol/operator (i.e.
+  # `r0` is `:none`, a remote-call `:dot`, or a lexical fallback that does not cover the cursor), so
+  # it is computed lazily and:
+  #
+  #   * when the cursor is a remote-call `:dot` and the character just left of it is the last of an
+  #     ALIAS leaf, that alias wins - so the end of `Foo` in `Foo.bar()` resolves to the module `Foo`
+  #     (#1027). Restricted to aliases so an atom/var LHS such as `:timer` in `:timer.sleep(...)`
+  #     keeps resolving the function, and an operator such as `Foo+1` keeps resolving the operator;
+  #   * otherwise, when 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)
+
+    cond do
+      column <= 1 ->
+        r0
+
+      # Fast path: the cursor sits inside a real, in-range, non-dot result (a symbol under the caret,
+      # an operator, ...). No trailing-edge rule can apply, so skip the second AST descent. `covers?`
+      # excludes a lexical `Code.Fragment` fallback that points at a token the cursor is not on.
+      r0 != :none and not dot_context?(r0) and covers?(r0, position) ->
+        r0
+
+      # Otherwise the trailing edge may matter (`r0` is `:none`, a remote-call `:dot`, or a fallback
+      # that does not cover the cursor) - classify one column left lazily.
+      true ->
+        r1 = classify_at(ast, source, {line, column - 1})
+
+        cond do
+          # #1027: the cursor sits at the end of an alias - resolve the module. This wins over an
+          # enclosing remote-call dot (`Foo.bar()` -> `Foo`) and over a fallback that does not cover
+          # the cursor, but not over a real symbol/operator the caret is on (handled by the fast path
+          # above). Restricted to aliases so `:timer` in `:timer.sleep(...)` keeps the function.
+          alias_trailing_edge?(r1, position) -> r1
+          # keep the cursor's own result (preserving parity with `Code.Fragment`, whose begin/end can
+          # be degenerate for some shapes) - only the alias override above is allowed to replace it
+          r0 != :none -> r0
+          # #1038: the cursor covered nothing - retry one column left onto a genuine navigation target
+          # whose range ENDS at the cursor (a `key:` / operator / sigil trailing edge stays `:none`).
+          navigable_result?(r1) and trailing_edge?(r1, position) -> r1
+          true -> :none
+        end
+    end
+  end
+
+  defp dot_context?(%{context: {:dot, _, _}}), do: true
+  defp dot_context?(_), do: false
+
+  # The result's range contains the cursor (`begin <= position <= end`). A lexical `Code.Fragment`
+  # fallback can point at a token the cursor is not on (`begin` past the cursor); such a result does
+  # not cover it. `:none` never covers.
+  defp covers?(%{begin: begin_pos, end: end_pos}, position),
+    do: before_or_at?(begin_pos, position) and before_or_at?(position, end_pos)
+
+  defp covers?(_r0, _position), do: false
+
+  # The cursor sits exactly at the result's trailing edge (`begin <= position == end`).
+  defp trailing_edge?(%{begin: begin_pos, end: end_pos}, position),
+    do: end_pos == position and before_or_at?(begin_pos, position)
+
+  defp trailing_edge?(_r1, _position), do: false
+
+  defp before_or_at?({line1, col1}, {line2, col2}),
+    do: line1 < line2 or (line1 == line2 and col1 <= col2)
+
+  # `r1` is an ALIAS leaf whose trailing edge is at `position` (so the cursor sits one column past its
+  # last character, i.e. on the `.` that follows it). Restricted to alias shapes (`{:alias, _}` and
+  # the qualified `{:alias, {:local_or_var | :module_attribute | ..., _}, _}` form) - that is exactly
+  # what #1027 / elixir-lang/elixir#13150 is about: the end of `Foo` in `Foo.bar()`, or of
+  # `__MODULE__.Foo` in `__MODULE__.Foo.bar`, should be the module. It must NOT fire for a bare-atom /
+  # var / attribute LHS of a dot (e.g. `:timer` in `:timer.sleep(...)`), which keeps the function.
+  defp alias_trailing_edge?(%{context: {:alias, _}} = r1, position),
+    do: trailing_edge?(r1, position)
+
+  defp alias_trailing_edge?(%{context: {:alias, _, _}} = r1, position),
+    do: trailing_edge?(r1, position)
+
+  defp alias_trailing_edge?(_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,76 @@ 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 "#1027 override applies to qualified aliases too" do
+      # end of `__MODULE__.Foo` (on the `.` before `bar`) resolves the qualified module
+      assert %{context: {:alias, {:local_or_var, ~c"__MODULE__"}, ~c"Foo"}} =
+               Toxic.surround_context("__MODULE__.Foo.bar", {1, 15})
+    end
+
+    test "the override is limited to the dot case: operators and atoms are preserved" do
+      # `Foo` ends at column 4, but the caret is on an operator, not a `.` - keep the operator
+      assert %{context: {:operator, ~c"+"}} = Toxic.surround_context("Foo+1", {1, 4})
+      assert %{context: {:operator, ~c"=="}} = Toxic.surround_context("Foo==Bar", {1, 4})
+      # a bare-atom LHS of a dot keeps the remote call (the function), not the module
+      assert %{context: {:dot, {:unquoted_atom, ~c"timer"}, ~c"sleep"}} =
+               Toxic.surround_context(":timer.sleep(1000)", {1, 7})
+    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