[feat] Group functions by name + type + arity in document symbols (#833)

If a module had multiple heads for a function that only differed in
parameter pattern matches, document symbols was very cluttered. This
change groups functions in a module by their name and arity, with each
implementation nested in that grouping. This allows editors to
collapse function groups to save space.

This change also removes any whitespace or linebreaks from function names.

Author: scohen

apps/remote_control/lib/lexical/remote_control/code_intelligence/symbols.ex

@@ -1,5 +1,6 @@
 defmodule Lexical.RemoteControl.CodeIntelligence.Symbols do
   alias Lexical.Document
+  alias Lexical.Document.Range
   alias Lexical.RemoteControl.CodeIntelligence.Symbols
   alias Lexical.RemoteControl.Search
   alias Lexical.RemoteControl.Search.Indexer
@@ -71,10 +72,8 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols do
         children =
           entries_by_block_id
           |> rebuild_structure(document, entry.id)
-          |> Enum.sort_by(fn %Symbols.Document{} = symbol ->
-            start = symbol.range.start
-            {start.line, start.character}
-          end)
+          |> Enum.sort_by(&sort_by_start/1)
+          |> group_functions()
 
         Symbols.Document.from(document, entry, children)
       else
@@ -86,4 +85,41 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols do
       _ -> []
     end
   end
+
+  defp group_functions(children) do
+    {functions, other} = Enum.split_with(children, &match?({:function, _}, &1.original_type))
+
+    grouped_functions =
+      functions
+      |> Enum.group_by(fn symbol ->
+        symbol.subject |> String.split(".") |> List.last() |> String.trim()
+      end)
+      |> Enum.map(fn
+        {_name_and_arity, [definition]} ->
+          definition
+
+        {name_and_arity, [first | _] = defs} ->
+          last = List.last(defs)
+          [type, _] = String.split(first.name, " ", parts: 2)
+          name = "#{type} #{name_and_arity}"
+
+          children =
+            Enum.map(defs, fn child ->
+              [_, rest] = String.split(child.name, " ", parts: 2)
+              %Symbols.Document{child | name: rest}
+            end)
+
+          range = Range.new(first.range.start, last.range.end)
+          %Symbols.Document{first | name: name, range: range, children: children}
+      end)
+
+    grouped_functions
+    |> Enum.concat(other)
+    |> Enum.sort_by(&sort_by_start/1)
+  end
+
+  defp sort_by_start(%Symbols.Document{} = symbol) do
+    start = symbol.range.start
+    {start.line, start.character}
+  end
 end

apps/remote_control/lib/lexical/remote_control/code_intelligence/symbols/document.ex

@@ -3,7 +3,7 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols.Document do
   alias Lexical.Formats
   alias Lexical.RemoteControl.Search.Indexer.Entry
 
-  defstruct [:name, :type, :range, :detail_range, :detail, children: []]
+  defstruct [:name, :type, :range, :detail_range, :detail, :original_type, :subject, children: []]
 
   def from(%Document{} = document, %Entry{} = entry, children \\ []) do
     case name_and_type(entry.type, entry, document) do
@@ -16,7 +16,9 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols.Document do
            type: type,
            range: range,
            detail_range: entry.range,
-           children: children
+           children: children,
+           original_type: entry.type,
+           subject: entry.subject
          }}
 
       _ ->
@@ -28,7 +30,10 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols.Document do
 
   defp name_and_type({:function, type}, %Entry{} = entry, %Document{} = document)
        when type in [:public, :private, :delegate] do
-    fragment = Document.fragment(document, entry.range.start, entry.range.end)
+    fragment =
+      document
+      |> Document.fragment(entry.range.start, entry.range.end)
+      |> remove_line_breaks_and_multiple_spaces()
 
     prefix =
       case type do
@@ -87,4 +92,8 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Symbols.Document do
   defp name_and_type(type, %Entry{} = entry, _document) do
     {to_string(entry.subject), type}
   end
+
+  defp remove_line_breaks_and_multiple_spaces(string) do
+    string |> String.split(~r/\s/) |> Enum.reject(&match?("", &1)) |> Enum.join(" ")
+  end
 end

apps/remote_control/test/lexical/remote_control/code_intelligence/symbols_test.exs

@@ -220,6 +220,151 @@ defmodule Lexical.RemoteControl.CodeIntelligence.SymbolsTest do
       assert function.name == "defp my_fn"
     end
 
+    test "multiple arity functions are grouped" do
+      {[module], doc} =
+        ~q[
+        defmodule Module do
+          def function_arity(:foo), do: :ok
+          def function_arity(:bar), do: :ok
+          def function_arity(:baz), do: :ok
+        end
+        ]
+        |> document_symbols()
+
+      assert [parent] = module.children
+      assert parent.name == "def function_arity/1"
+
+      expected_range =
+        """
+          «def function_arity(:foo), do: :ok
+          def function_arity(:bar), do: :ok
+          def function_arity(:baz), do: :ok»
+        """
+        |> String.trim_trailing()
+
+      assert decorate(doc, parent.range) =~ expected_range
+      assert [first, second, third] = parent.children
+
+      assert first.name == "function_arity(:foo)"
+      assert decorate(doc, first.range) =~ "«def function_arity(:foo), do: :ok»"
+      assert decorate(doc, first.detail_range) =~ "def «function_arity(:foo)», do: :ok"
+
+      assert second.name == "function_arity(:bar)"
+      assert decorate(doc, second.range) =~ "«def function_arity(:bar), do: :ok»"
+      assert decorate(doc, second.detail_range) =~ "def «function_arity(:bar)», do: :ok"
+
+      assert third.name == "function_arity(:baz)"
+      assert decorate(doc, third.range) =~ "«def function_arity(:baz), do: :ok»"
+      assert decorate(doc, third.detail_range) =~ "def «function_arity(:baz)», do: :ok"
+    end
+
+    test "multiple arity private functions are grouped" do
+      {[module], doc} =
+        ~q[
+        defmodule Module do
+          defp function_arity(:foo), do: :ok
+          defp function_arity(:bar), do: :ok
+          defp function_arity(:baz), do: :ok
+        end
+        ]
+        |> document_symbols()
+
+      assert [parent] = module.children
+      assert parent.name == "defp function_arity/1"
+
+      expected_range =
+        """
+          «defp function_arity(:foo), do: :ok
+          defp function_arity(:bar), do: :ok
+          defp function_arity(:baz), do: :ok»
+        """
+        |> String.trim_trailing()
+
+      assert decorate(doc, parent.range) =~ expected_range
+      assert [first, second, third] = parent.children
+
+      assert first.name == "function_arity(:foo)"
+      assert decorate(doc, first.range) =~ "«defp function_arity(:foo), do: :ok»"
+      assert decorate(doc, first.detail_range) =~ "defp «function_arity(:foo)», do: :ok"
+
+      assert second.name == "function_arity(:bar)"
+      assert decorate(doc, second.range) =~ "«defp function_arity(:bar), do: :ok»"
+      assert decorate(doc, second.detail_range) =~ "defp «function_arity(:bar)», do: :ok"
+
+      assert third.name == "function_arity(:baz)"
+      assert decorate(doc, third.range) =~ "«defp function_arity(:baz), do: :ok»"
+      assert decorate(doc, third.detail_range) =~ "defp «function_arity(:baz)», do: :ok"
+    end
+
+    test "groups public and private functions separately" do
+      {[module], _doc} =
+        ~q[
+        defmodule Module do
+          def fun_one(:foo), do: :ok
+          def fun_one(:bar), do: :ok
+
+          defp fun_one(:foo, :bar), do: :ok
+          defp fun_one(:bar, :baz), do: :ok
+        end
+        ]
+        |> document_symbols()
+
+      assert [first, second] = module.children
+      assert first.name == "def fun_one/1"
+      assert second.name == "defp fun_one/2"
+    end
+
+    test "line breaks are stripped" do
+      {[module], _doc} =
+        ~q[
+        defmodule Module do
+          def long_function(
+               arg_1,
+               arg_2,
+               arg_3) do
+          end
+        end
+        ]
+        |> document_symbols()
+
+      assert [function] = module.children
+      assert function.name == "def long_function( arg_1, arg_2, arg_3)"
+    end
+
+    test "line breaks are stripped for grouped functions" do
+      {[module], _doc} =
+        ~q[
+        defmodule Module do
+          def long_function(
+               :foo,
+               arg_2,
+               arg_3) do
+          end
+
+          def long_function(
+               :bar,
+               arg_2,
+               arg_3) do
+          end
+          def long_function(
+               :baz,
+               arg_2,
+               arg_3) do
+          end
+
+        end
+        ]
+        |> document_symbols()
+
+      assert [function] = module.children
+      assert function.name == "def long_function/3"
+
+      assert [first, second, third] = function.children
+      assert first.name == "long_function( :foo, arg_2, arg_3)"
+      assert second.name == "long_function( :bar, arg_2, arg_3)"
+      assert third.name == "long_function( :baz, arg_2, arg_3)"
+    end
+
     test "struct definitions are found" do
       {[module], doc} =
         ~q{