refactor and fix tests

lukaszsamson · lukaszsamson · commit b9da0a2e98c7 · 2025-08-04T07:39:52.000+02:00
diff --git a/apps/language_server/lib/language_server/providers/execute_command/llm_docs_aggregator.ex b/apps/language_server/lib/language_server/providers/execute_command/llm_docs_aggregator.ex
@@ -76,36 +76,54 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
         {:ok, docs}
 
       _ ->
-        # Try as builtin type
-        if arity == nil or arity == 0 do
-          # TODO: doesn't work for types with arity > 0
-          case BuiltinTypes.get_builtin_type_doc(function) do
-            doc when doc != "" ->
-              {:ok,
-               %{
-                 type: "#{function}()",
-                 documentation: doc
-               }}
-
-            _ ->
-              # Check if it's a builtin function or try other modules
-              case BuiltinFunctions.get_docs({function, arity}) do
-                "" ->
-                  {:error, "Local call #{function}/#{arity || "?"} - no documentation found"}
-
-                builtin_docs when is_binary(builtin_docs) ->
-                  {
-                    :ok,
+        # Try as builtin type using the new API that returns a list
+        case BuiltinTypes.get_builtin_types_doc(function, arity || :any) do
+          [] ->
+            # No builtin types found, check if it's a builtin function using the new API
+            case BuiltinFunctions.get_builtin_functions_doc(function, arity || :any) do
+              [] ->
+                {:error, "Local call #{function}/#{arity || "?"} - no documentation found"}
+
+              builtin_functions ->
+                # Return all matching builtin functions
+                results =
+                  Enum.map(builtin_functions, fn {_function_name, func_arity, doc, specs} ->
                     %{
                       function: Atom.to_string(function),
-                      arity: arity,
-                      documentation: builtin_docs
+                      arity: func_arity,
+                      documentation: format_builtin_function_doc(doc, specs)
                     }
-                  }
-              end
-          end
-        else
-          {:error, "Local call #{function}/#{arity || "?"} - no documentation found"}
+                  end)
+
+                # Return single result if only one match, otherwise return list
+                case results do
+                  [single_result] -> {:ok, single_result}
+                  multiple_results -> {:ok, multiple_results}
+                end
+            end
+
+          builtin_types ->
+            # Return all matching builtin types
+            results =
+              Enum.map(builtin_types, fn {_type_name, type_arity, doc} ->
+                type_name =
+                  if type_arity == 0 do
+                    "#{function}()"
+                  else
+                    "#{function}/#{type_arity}"
+                  end
+
+                %{
+                  type: type_name,
+                  documentation: doc
+                }
+              end)
+
+            # Return single result if only one match, otherwise return list
+            case results do
+              [single_result] -> {:ok, single_result}
+              multiple_results -> {:ok, multiple_results}
+            end
         end
     end
   end
@@ -137,29 +155,11 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
                   module: inspect(module),
                   callback: Atom.to_string(function),
                   arity: callback_info[:arity] || arity,
-                  documentation: callback_info[:documentation] || ""
+                  documentation: callback_info[:documentation] || "",
+                  spec: callback_info[:spec],
+                  kind: callback_info[:kind],
+                  metadata: callback_info[:metadata] || %{}
                 }
-                
-                # Add spec if available
-                base_result = if callback_info[:spec] do
-                  Map.put(base_result, :spec, callback_info[:spec])
-                else
-                  base_result
-                end
-                
-                # Add kind if available  
-                base_result = if callback_info[:kind] do
-                  Map.put(base_result, :kind, callback_info[:kind])
-                else
-                  base_result
-                end
-                
-                # Add metadata if available
-                if callback_info[:metadata] && callback_info[:metadata] != %{} do
-                  Map.put(base_result, :metadata, callback_info[:metadata])
-                else
-                  base_result
-                end
               end)
 
             {:ok, results}
@@ -551,9 +551,9 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
         kind: callback_info.kind,
         metadata: callback_info.metadata
       }
-      
+
       callback_result = Map.put(callback_result, :metadata, callback_info.metadata)
-      
+
       callback_result
     end)
   end
@@ -569,15 +569,17 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
         {{spec_name, spec_arity}, _} ->
           # Check if it's a regular function match
           regular_match = spec_name == function and (arity == nil or spec_arity == arity)
-          
+
           # Check if it's a macro match (MACRO-name/arity+1)
           macro_name = String.to_atom("MACRO-#{function}")
-          macro_match = if arity == nil do
-            spec_name == macro_name
-          else
-            spec_name == macro_name and spec_arity == arity + 1
-          end
-          
+
+          macro_match =
+            if arity == nil do
+              spec_name == macro_name
+            else
+              spec_name == macro_name and spec_arity == arity + 1
+            end
+
           regular_match or macro_match
 
         _ ->
@@ -588,11 +590,11 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
     filtered_specs
     |> Map.new(fn {_key, {{name, spec_arity}, specs}} ->
       formatted_spec = Introspection.spec_to_string({{name, spec_arity}, specs}, :spec)
-      
+
       # Normalize macro names for the result key
       {display_name, display_arity} = normalize_macro_name_and_arity(name, spec_arity)
       normalized_key = {String.to_atom(display_name), display_arity}
-      
+
       # Return as a list containing the single spec for compatibility with Enum.map_join
       {normalized_key, [formatted_spec]}
     end)
@@ -654,6 +656,17 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregator do
   defp extract_doc(:none), do: nil
   defp extract_doc(_), do: nil
 
+  defp format_builtin_function_doc(doc, specs) do
+    spec_part =
+      if specs != [] do
+        "\n\n**Specs:**\n#{Enum.map_join(specs, "\n", fn s -> "```elixir\n#{s}\n```" end)}"
+      else
+        ""
+      end
+
+    "#{doc}#{spec_part}"
+  end
+
   defp format_function_sections(sections) do
     sections
     |> Enum.map(fn section ->
diff --git a/apps/language_server/test/providers/execute_command/llm_docs_aggregator_test.exs b/apps/language_server/test/providers/execute_command/llm_docs_aggregator_test.exs
@@ -402,7 +402,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       assert macro_result.function == "some_macro"
       assert macro_result.arity == 2
       assert macro_result.documentation =~ "An example macro"
-      
+
       # Check that metadata is included (since: "1.1.0")
       assert macro_result.documentation =~ "Since"
       assert macro_result.documentation =~ "1.1.0"
@@ -437,7 +437,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       assert macrocallback_result.callback == "some_macrocallback"
       assert macrocallback_result.arity == 1
       assert macrocallback_result.documentation =~ "An example callback"
-      
+
       # Check that we got the documentation (metadata may be formatted differently for callbacks)
       assert macrocallback_result.documentation
     end
@@ -459,23 +459,33 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
 
     test "verifies macro and macrocallback specs are included" do
       # Test callback spec
-      assert {:ok, result} = LlmDocsAggregator.execute([["ElixirSenseExample.ModuleWithDocs.some_callback/1"]], %{})
+      assert {:ok, result} =
+               LlmDocsAggregator.execute(
+                 [["ElixirSenseExample.ModuleWithDocs.some_callback/1"]],
+                 %{}
+               )
+
       assert Map.has_key?(result, :results)
       assert length(result.results) == 1
-      
+
       callback_result = hd(result.results)
       assert Map.has_key?(callback_result, :spec)
       assert Map.has_key?(callback_result, :kind)
       assert Map.has_key?(callback_result, :metadata)
       assert callback_result.spec == "@callback some_callback(integer()) :: atom()"
       assert callback_result.kind == :callback
       assert callback_result.metadata.since == "1.1.0"
-      
+
       # Test macrocallback spec  
-      assert {:ok, result} = LlmDocsAggregator.execute([["ElixirSenseExample.ModuleWithDocs.some_macrocallback/1"]], %{})
+      assert {:ok, result} =
+               LlmDocsAggregator.execute(
+                 [["ElixirSenseExample.ModuleWithDocs.some_macrocallback/1"]],
+                 %{}
+               )
+
       assert Map.has_key?(result, :results)
       assert length(result.results) == 1
-      
+
       macrocallback_result = hd(result.results)
       assert Map.has_key?(macrocallback_result, :spec)
       assert Map.has_key?(macrocallback_result, :kind)
@@ -499,7 +509,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       assert func_result.function == "some_fun"
       assert func_result.arity == 2
       assert func_result.documentation =~ "An example fun"
-      
+
       # Verify that specs are included in the documentation
       assert func_result.documentation =~ "**Specs:**"
       assert func_result.documentation =~ "@spec some_fun"
@@ -521,7 +531,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       assert macro_result.function == "some_macro"
       assert macro_result.arity == 2
       assert macro_result.documentation =~ "An example macro"
-      
+
       # Verify that specs are included in the macro documentation
       assert macro_result.documentation =~ "**Specs:**"
       assert macro_result.documentation =~ "@spec some_macro"
@@ -541,11 +551,13 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       # Find the result with arity 2 (which has specs)
       func_result = Enum.find(result.results, &(&1.arity == 2))
       assert func_result
-      
+
       # Verify spec formatting
       assert func_result.documentation =~ "**Specs:**"
       assert func_result.documentation =~ "```elixir"
-      assert func_result.documentation =~ "@spec some_fun(integer(), integer() | nil) :: integer()"
+
+      assert func_result.documentation =~
+               "@spec some_fun(integer(), integer() | nil) :: integer()"
     end
 
     test "verifies macro specs are properly formatted" do
@@ -560,13 +572,51 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.LlmDocsAggregatorTest
       # Find the result with arity 2 (which has specs)
       macro_result = Enum.find(result.results, &(&1.arity == 2))
       assert macro_result
-      
+
       # Verify spec formatting for macro
       assert macro_result.documentation =~ "**Specs:**"
       assert macro_result.documentation =~ "```elixir"
-      assert macro_result.documentation =~ "@spec some_macro(Macro.t(), Macro.t() | nil) :: Macro.t()"
+
+      assert macro_result.documentation =~
+               "@spec some_macro(Macro.t(), Macro.t() | nil) :: Macro.t()"
     end
 
+    test "handles builtin types with various arities" do
+      # Test builtin type without arity - returns all matching types
+      modules = ["list"]
+      assert {:ok, result} = LlmDocsAggregator.execute([modules], %{})
+      assert Map.has_key?(result, :results)
+      assert length(result.results) == 2
+
+      # Should get both list() and list/1
+      list_result = Enum.find(result.results, &(&1.type == "list()"))
+      assert list_result
+      assert list_result.documentation == "A list"
+
+      list1_result = Enum.find(result.results, &(&1.type == "list/1"))
+      assert list1_result
+      assert list1_result.documentation == "Proper list ([]-terminated)"
+
+      # Test builtin type with arity 0
+      modules = ["list/0"]
+      assert {:ok, result} = LlmDocsAggregator.execute([modules], %{})
+      assert Map.has_key?(result, :results)
+      assert length(result.results) == 1
+
+      list0_result = hd(result.results)
+      assert list0_result.type == "list()"
+      assert list0_result.documentation == "A list"
+
+      # Test builtin type with arity 1
+      modules = ["list/1"]
+      assert {:ok, result} = LlmDocsAggregator.execute([modules], %{})
+      assert Map.has_key?(result, :results)
+      assert length(result.results) == 1
+
+      list1_single = hd(result.results)
+      assert list1_single.type == "list/1"
+      assert list1_single.documentation == "Proper list ([]-terminated)"
+    end
 
     test "returns error for invalid arguments" do
       # Test with non-list argument
diff --git a/dep_versions.exs b/dep_versions.exs
@@ -1,5 +1,5 @@
 [
-  elixir_sense: "9140a547e8bcbdbe9146eb57afc286c550fea241",
+  elixir_sense: "bfefb1f7ef51fb38354183dd3e9d24d6d4d960fb",
   dialyxir_vendored: "f8f64cfb6797c518294687e7c03ae817bacbc6ee",
   jason_v: "f1c10fa9c445cb9f300266122ef18671054b2330",
   erl2ex_vendored: "073ac6b9a44282e718b6050c7b27cedf9217a12a",
diff --git a/mix.lock b/mix.lock
@@ -2,7 +2,7 @@
   "benchee": {:hex, :benchee, "1.1.0", "f3a43817209a92a1fade36ef36b86e1052627fd8934a8b937ac9ab3a76c43062", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}], "hexpm", "7da57d545003165a012b587077f6ba90b89210fd88074ce3c60ce239eb5e6d93"},
   "deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
   "dialyxir_vendored": {:git, "https://github.com/elixir-lsp/dialyxir.git", "f8f64cfb6797c518294687e7c03ae817bacbc6ee", [ref: "f8f64cfb6797c518294687e7c03ae817bacbc6ee"]},
-  "elixir_sense": {:git, "https://github.com/elixir-lsp/elixir_sense.git", "9140a547e8bcbdbe9146eb57afc286c550fea241", [ref: "9140a547e8bcbdbe9146eb57afc286c550fea241"]},
+  "elixir_sense": {:git, "https://github.com/elixir-lsp/elixir_sense.git", "bfefb1f7ef51fb38354183dd3e9d24d6d4d960fb", [ref: "bfefb1f7ef51fb38354183dd3e9d24d6d4d960fb"]},
   "erl2ex_vendored": {:git, "https://github.com/elixir-lsp/erl2ex.git", "073ac6b9a44282e718b6050c7b27cedf9217a12a", [ref: "073ac6b9a44282e718b6050c7b27cedf9217a12a"]},
   "erlex_vendored": {:git, "https://github.com/elixir-lsp/erlex.git", "c0e448db27bcbb3f369861d13e3b0607ed37048d", [ref: "c0e448db27bcbb3f369861d13e3b0607ed37048d"]},
   "jason_v": {:git, "https://github.com/elixir-lsp/jason.git", "f1c10fa9c445cb9f300266122ef18671054b2330", [ref: "f1c10fa9c445cb9f300266122ef18671054b2330"]},

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`[`
`2`		`- elixir_sense: "9140a547e8bcbdbe9146eb57afc286c550fea241",`
	`2`	`+ elixir_sense: "bfefb1f7ef51fb38354183dd3e9d24d6d4d960fb",`
`3`	`3`	`dialyxir_vendored: "f8f64cfb6797c518294687e7c03ae817bacbc6ee",`
`4`	`4`	`jason_v: "f1c10fa9c445cb9f300266122ef18671054b2330",`
`5`	`5`	`erl2ex_vendored: "073ac6b9a44282e718b6050c7b27cedf9217a12a",`