handle new parser metadata in selection ranges

improve handing of parens
fix cases where AST would produce ranges braking invariants

Author: lukaszsamson

apps/language_server/lib/language_server/ast_utils.ex

@@ -10,24 +10,40 @@ defmodule ElixirLS.LanguageServer.AstUtils do
   def node_range(node, options \\ [])
   def node_range(atom, _options) when is_atom(atom), do: nil
 
-  def node_range([{{:__block__, _, [_]} = first, _} | _] = list, _options) do
-    case List.last(list) do
-      {_, last} ->
-        case {node_range(first), node_range(last)} do
-          {range(start_line, start_character, _, _), range(_, _, end_line, end_character)} ->
-            range(start_line, start_character, end_line, end_character)
-
-          _ ->
-            nil
-        end
+  def node_range([{{:__block__, meta, [_]} = first, _} | _] = list, _options) do
+    if Keyword.get(meta, :format) == :keyword or Keyword.has_key?(meta, :assoc) or
+         Version.match?(System.version(), "< 1.18.0-dev") do
+      case List.last(list) do
+        {_, last} ->
+          case {node_range(first), node_range(last)} do
+            {range(start_line, start_character, _, _), range(_, _, end_line, end_character)} ->
+              range(start_line, start_character, end_line, end_character)
+
+            _ ->
+              nil
+          end
 
-      _ ->
-        nil
+        _ ->
+          nil
+      end
     end
   end
 
   def node_range(list, _options) when is_list(list), do: nil
 
+  def node_range({{:__block__, meta, [_]} = first, last}, _options) do
+    if Keyword.get(meta, :format) == :keyword or Keyword.has_key?(meta, :assoc) or
+         Version.match?(System.version(), "< 1.18.0-dev") do
+      case {node_range(first), node_range(last)} do
+        {range(start_line, start_character, _, _), range(_, _, end_line, end_character)} ->
+          range(start_line, start_character, end_line, end_character)
+
+        _ ->
+          nil
+      end
+    end
+  end
+
   def node_range({:__block__, meta, args} = _ast, _options) do
     line = Keyword.get(meta, :line)
     column = Keyword.get(meta, :column)
@@ -58,6 +74,18 @@ defmodule ElixirLS.LanguageServer.AstUtils do
             # 2 element tuple
             {end_location[:line] - 1, end_location[:column] - 1 + 1}
 
+          match?(kind when kind in [:atom, :keyword], Keyword.get(meta, :format)) ->
+            [literal] = args
+
+            modifier =
+              if literal in [true, false, nil] do
+                1
+              else
+                0
+              end
+
+            get_literal_end(literal, {line, column + modifier}, nil)
+
           match?([_], args) ->
             [literal] = args
             delimiter = meta[:delimiter]
@@ -163,18 +191,24 @@ defmodule ElixirLS.LanguageServer.AstUtils do
                 nil
             end
 
-          match?({:., _, [Kernel, :to_string]}, form) ->
-            {line, column}
+          match?({:., _meta, [Kernel, :to_string]}, form) ->
+            if Keyword.get(meta, :from_interpolation) or
+                 Version.match?(System.version(), "< 1.16.0-dev") do
+              {line, column}
+            end
 
-          match?({:., _, [Access, :get]}, form) and match?([_ | _], args) ->
-            [arg | _] = args
+          match?({:., _meta, [Access, :get]}, form) and match?([_ | _], args) ->
+            if Keyword.get(meta, :from_brackets) or
+                 Version.match?(System.version(), "< 1.16.0-dev") do
+              [arg | _] = args
 
-            case node_range(arg) do
-              range(line, column, _, _) ->
-                {line, column}
+              case node_range(arg) do
+                range(line, column, _, _) ->
+                  {line, column}
 
-              nil ->
-                nil
+                nil ->
+                  nil
+              end
             end
 
           match?({:., _, [_ | _]}, form) ->

apps/language_server/lib/language_server/providers/selection_ranges.ex

@@ -397,6 +397,53 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
                 acc
               end
 
+            ranges_acc =
+              case ast do
+                {_, meta, _} ->
+                  parens_ranges =
+                    for {:parens, parens_meta} <- meta,
+                        parens_start_line = Keyword.fetch!(parens_meta, :line) - 1,
+                        parens_start_character = Keyword.fetch!(parens_meta, :column) - 1,
+                        parens_meta_closing = Keyword.fetch!(parens_meta, :closing),
+                        parens_end_line = Keyword.fetch!(parens_meta_closing, :line) - 1,
+                        parens_end_character = Keyword.fetch!(parens_meta_closing, :column),
+                        (parens_start_line < line or
+                           (parens_start_line == line and parens_start_character <= character)) and
+                          (parens_end_line > line or
+                             (parens_end_line == line and parens_end_character >= character)) do
+                      # NOTE there may be multiple parens keys
+                      outer_range =
+                        range(
+                          parens_start_line,
+                          parens_start_character,
+                          parens_end_line,
+                          parens_end_character
+                        )
+
+                      if (parens_start_line < line or
+                            (parens_start_line == line and parens_start_character + 1 <= character)) and
+                           (parens_end_line > line or
+                              (parens_end_line == line and parens_end_character - 1 >= character)) do
+                        inner_range =
+                          range(
+                            parens_start_line,
+                            parens_start_character + 1,
+                            parens_end_line,
+                            parens_end_character - 1
+                          )
+
+                        [outer_range, inner_range]
+                      else
+                        [outer_range]
+                      end
+                    end
+
+                  List.flatten(parens_ranges) ++ ranges_acc
+
+                _ ->
+                  ranges_acc
+              end
+
             parent_acc =
               if match?({_, _, _}, ast) do
                 [ast | parent_ast]
@@ -417,6 +464,8 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
 
     acc
     |> sort_ranges_widest_to_narrowest()
+    |> deduplicate
+    |> fix_properties
   end
 
   def ast_node_ranges(_, _, _, _), do: []

apps/language_server/lib/language_server/range_utils.ex

@@ -180,4 +180,34 @@ defmodule ElixirLS.LanguageServer.RangeUtils do
   defp do_deduplicate([range | rest], acc) do
     do_deduplicate(rest, [range | acc])
   end
+
+  def fix_properties(ranges) do
+    ranges
+    |> Enum.reverse()
+    |> Enum.reduce([], fn
+      r, [] ->
+        [r]
+
+      range(start_line, start_character, end_line, end_character),
+      [range(last_start_line, last_start_character, last_end_line, last_end_character) | _] = acc ->
+        new_start_line = min(start_line, last_start_line)
+        new_end_line = max(end_line, last_end_line)
+
+        new_start_character =
+          if start_line < last_start_line do
+            start_character
+          else
+            min(start_character, last_start_character)
+          end
+
+        new_end_character =
+          if end_line > last_end_line do
+            end_character
+          else
+            max(end_character, last_end_character)
+          end
+
+        [range(new_start_line, new_start_character, new_end_line, new_end_character) | acc]
+    end)
+  end
 end

apps/language_server/test/ast_utils_test.exs

@@ -32,8 +32,17 @@ defmodule ElixirLS.LanguageServer.AstUtilsTest do
       assert get_range("nil") == range(0, 0, 0, 3)
     end
 
+    if Version.match?(System.version(), ">= 1.18.0") do
+      test "true as atom" do
+        assert get_range(":true") == range(0, 0, 0, 5)
+      end
+    end
+
     test "integer" do
       assert get_range("1234") == range(0, 0, 0, 4)
+
+      assert node_range({:__block__, [token: "2", line: 1, column: 10], [2]}) ==
+               range(0, 9, 0, 10)
     end
 
     test "float" do
@@ -246,8 +255,9 @@ defmodule ElixirLS.LanguageServer.AstUtilsTest do
     end
 
     # Parser is simplifying the expression and not including the parens
+    # we handle parens meta in selection ranges
     # test "nested binary operators with parens" do
-    #   assert get_range("var * 3 * (foo + x)") == range(0, 0, 0, 17)
+    #   assert get_range("var * 3 * (foo + x)") == range(0, 0, 0, 19)
     # end
 
     test "nested binary and unary operators" do

apps/language_server/test/providers/selection_ranges_test.exs

@@ -767,6 +767,12 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRangesTest do
       assert_range(ranges, range(0, 0, 1, 0))
       # full for
       assert_range(ranges, range(0, 0, 0, 56))
+
+      if Version.match?(System.version(), ">= 1.18.0") do
+        # do: x + y expression
+        assert_range(ranges, range(0, 47, 0, 56))
+      end
+
       # x + y expression
       assert_range(ranges, range(0, 51, 0, 56))
     end
@@ -994,6 +1000,14 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRangesTest do
       assert_range(ranges, range(0, 0, 3, 1))
       # full keyword
       assert_range(ranges, range(0, 6, 2, 6))
+
+      if Version.match?(System.version(), ">= 1.18.0") do
+        # key and value
+        assert_range(ranges, range(1, 2, 1, 6))
+
+        # key
+        assert_range(ranges, range(1, 2, 1, 4))
+      end
     end
   end
 
@@ -1062,4 +1076,31 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRangesTest do
       assert_range(ranges, range(0, 2, 0, 78))
     end
   end
+
+  test "expression with parens" do
+    text = """
+    1 + (2 * (3 + (s + x)) / 1)
+    """
+
+    ranges = get_ranges(text, 0, 15)
+
+    # full range
+    assert_range(ranges, range(0, 0, 1, 0))
+    # full line
+    assert_range(ranges, range(0, 0, 0, 27))
+    # outside outermost parens
+    assert_range(ranges, range(0, 4, 0, 27))
+    # inside outermost parens
+    assert_range(ranges, range(0, 5, 0, 26))
+    # outside middle parens
+    assert_range(ranges, range(0, 9, 0, 22))
+    # inside middle parens
+    assert_range(ranges, range(0, 10, 0, 21))
+    # outside innermost parens
+    assert_range(ranges, range(0, 14, 0, 21))
+    # inside innermost parens
+    assert_range(ranges, range(0, 15, 0, 20))
+    # s variable
+    assert_range(ranges, range(0, 15, 0, 16))
+  end
 end