make formatter more sane with directories

remove last references to cwd
make formatter_for_file return directory with formatter.exs
remove error prone code finding formatter.exs directory

Author: lukaszsamson

apps/language_server/lib/language_server/mix_tasks/format.ex

@@ -333,7 +333,7 @@ defmodule Mix.Tasks.ElixirLSFormat do
 
     formatter_opts_and_subs = load_plugins(formatter_opts_and_subs)
 
-    find_formatter_and_opts_for_file(Path.expand(file, cwd), formatter_opts_and_subs)
+    find_formatter_and_opts_for_file(Path.expand(file, cwd), formatter_opts_and_subs, cwd)
   end
 
   @doc """
@@ -530,12 +530,12 @@ defmodule Mix.Tasks.ElixirLSFormat do
       if file == :stdin do
         stdin_filename = Path.expand(Keyword.get(opts, :stdin_filename, "stdin.exs"), cwd)
 
-        {formatter, _opts} =
-          find_formatter_and_opts_for_file(stdin_filename, {formatter_opts, subs})
+        {formatter, _opts, _dir} =
+          find_formatter_and_opts_for_file(stdin_filename, {formatter_opts, subs}, cwd)
 
         {file, formatter}
       else
-        {formatter, _opts} = find_formatter_and_opts_for_file(file, {formatter_opts, subs})
+        {formatter, _opts, _dir} = find_formatter_and_opts_for_file(file, {formatter_opts, subs}, cwd)
         {file, formatter}
       end
     end
@@ -601,19 +601,22 @@ defmodule Mix.Tasks.ElixirLSFormat do
     if plugins != [], do: plugins, else: nil
   end
 
-  defp find_formatter_and_opts_for_file(file, formatter_opts_and_subs) do
-    formatter_opts = recur_formatter_opts_for_file(file, formatter_opts_and_subs)
-    {find_formatter_for_file(file, formatter_opts), formatter_opts}
+  defp find_formatter_and_opts_for_file(file, formatter_opts_and_subs, cwd) do
+    {formatter_opts, dir} = recur_formatter_opts_for_file(file, formatter_opts_and_subs) |> dbg
+    {find_formatter_for_file(file, formatter_opts), formatter_opts, dir || cwd}
   end
 
   defp recur_formatter_opts_for_file(file, {formatter_opts, subs}) do
-    Enum.find_value(subs, formatter_opts, fn {sub, formatter_opts_and_subs} ->
+    Enum.find_value(subs, {formatter_opts, nil}, fn {sub, formatter_opts_and_subs} ->
       size = byte_size(sub)
 
       case file do
         <<prefix::binary-size(size), dir_separator, _::binary>>
         when prefix == sub and dir_separator in [?\\, ?/] ->
-          recur_formatter_opts_for_file(file, formatter_opts_and_subs)
+          case recur_formatter_opts_for_file(file, formatter_opts_and_subs) do
+            {nested_formatter_opts, nil} -> {nested_formatter_opts, sub}
+            {nested_formatter_opts, nested_sub} -> {nested_formatter_opts, nested_sub}
+          end
 
         _ ->
           nil

apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex

@@ -57,8 +57,8 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ApplySpec do
       try do
         target_line_length =
           case SourceFile.formatter_for(uri, state.project_dir) do
-            {:ok, {_, opts}} -> Keyword.get(opts, :line_length, @default_target_line_length)
-            :error -> @default_target_line_length
+            {:ok, {_, opts, _formatter_exs_dir}} -> Keyword.get(opts, :line_length, @default_target_line_length)
+            {:error, _} -> @default_target_line_length
           end
 
         target_line_length = target_line_length - String.length(indentation)

apps/language_server/lib/language_server/providers/formatting.ex

@@ -2,27 +2,29 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
   import ElixirLS.LanguageServer.Protocol, only: [range: 4]
   alias ElixirLS.LanguageServer.Protocol.TextEdit
   alias ElixirLS.LanguageServer.SourceFile
+  alias ElixirLS.LanguageServer.JsonRpc
 
   def format(%SourceFile{} = source_file, uri = "file:" <> _, project_dir)
       when is_binary(project_dir) do
-    if can_format?(uri, project_dir) do
+    file_path = SourceFile.Path.absolute_from_uri(uri)
+    if SourceFile.Path.path_in_dir?(file_path, project_dir) do
       case SourceFile.formatter_for(uri, project_dir) do
-        {:ok, {formatter, opts}} ->
-          if should_format?(uri, project_dir, opts[:inputs]) do
+        {:ok, {formatter, opts, formatter_exs_dir}} ->
+          if should_format?(uri, formatter_exs_dir, opts[:inputs]) do
             do_format(source_file, formatter, opts)
           else
+            JsonRpc.show_message(:info, "formatter.exs not found for #{file_path}")
             {:ok, []}
           end
 
-        :error ->
-          {:error, :internal_error, "Unable to fetch formatter options", true}
+        {:error, message} ->
+          JsonRpc.show_message(:error, "Unable to fetch formatter options for #{file_path}")
+          {:error, :internal_error, message, true}
       end
     else
-      msg =
-        "Cannot format file from current directory " <>
-          "(Currently in #{Path.relative_to(File.cwd!(), project_dir)})"
+      JsonRpc.show_message(:warning, "Cannot format file #{file_path} outside of project dir #{project_dir}")
 
-      {:error, :internal_error, msg, true}
+      {:ok, []}
     end
   end
 
@@ -53,46 +55,16 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
     IO.iodata_to_binary([Code.format_string!(text, opts), ?\n])
   end
 
-  # If in an umbrella project, the cwd might be set to a sub-app if it's being compiled. This is
-  # fine if the file we're trying to format is in that app. Otherwise, we return an error.
-  defp can_format?(file_uri = "file:" <> _, project_dir) do
+  defp should_format?(file_uri, formatter_exs_dir, inputs) when is_list(inputs) do
     file_path = SourceFile.Path.absolute_from_uri(file_uri)
 
-    String.starts_with?(file_path, Path.absname(project_dir)) or
-      String.starts_with?(file_path, File.cwd!())
-  end
-
-  defp can_format?(_uri, _project_dir), do: false
-
-  defp should_format?(file_uri, project_dir, inputs) when is_list(inputs) do
-    file_path = SourceFile.Path.absolute_from_uri(file_uri)
-    formatter_dir = find_formatter_dir(project_dir, Path.dirname(file_path))
-
     Enum.any?(inputs, fn input_glob ->
-      glob = Path.join(formatter_dir, input_glob)
+      glob = Path.join(formatter_exs_dir, input_glob)
       PathGlobVendored.match?(file_path, glob, match_dot: true)
     end)
   end
 
-  defp should_format?(_file_uri, _project_dir, _inputs), do: true
-
-  # Finds the deepest directory that contains file_path, that also contains a
-  # .formatter.exs. It's possible, though unlikely, that the .formatter.exs we
-  # find is not actually linked to the project_dir via the :subdirectories
-  # option in the top-level .formatter.exs. Currently, that edge case is
-  # glossed over.
-  defp find_formatter_dir(project_dir, dir) do
-    cond do
-      dir == project_dir ->
-        project_dir
-
-      Path.join(dir, ".formatter.exs") |> File.exists?() ->
-        dir
-
-      true ->
-        find_formatter_dir(project_dir, Path.dirname(dir))
-    end
-  end
+  defp should_format?(_file_uri, _formatter_exs_dir, _inputs), do: true
 
   defp myers_diff_to_text_edits(myers_diff) do
     myers_diff_to_text_edits(myers_diff, {0, 0}, [])

apps/language_server/lib/language_server/server.ex

@@ -973,8 +973,8 @@ defmodule ElixirLS.LanguageServer.Server do
 
     locals_without_parens =
       case SourceFile.formatter_for(uri, state.project_dir) do
-        {:ok, {_, opts}} -> Keyword.get(opts, :locals_without_parens, [])
-        :error -> []
+        {:ok, {_, opts, _formatter_exs_dir}} -> Keyword.get(opts, :locals_without_parens, [])
+        {:error, _} -> []
       end
       |> MapSet.new()
 

apps/language_server/lib/language_server/source_file.ex

@@ -229,14 +229,14 @@ defmodule ElixirLS.LanguageServer.SourceFile do
     """
   end
 
-  @spec formatter_for(String.t(), String.t() | nil) :: {:ok, {function | nil, keyword()}} | :error
+  @spec formatter_for(String.t(), String.t() | nil) :: {:ok, {function | nil, keyword(), String.t}} | :error
   def formatter_for(uri = "file:" <> _, project_dir) do
     path = __MODULE__.Path.from_uri(uri)
 
     try do
       true = Code.ensure_loaded?(Mix.Tasks.ElixirLSFormat)
 
-      if project_dir && Version.match?(System.version(), ">= 1.15.0") do
+      if project_dir do
         {:ok, Mix.Tasks.ElixirLSFormat.formatter_for_file(path, root: project_dir)}
       else
         {:ok, Mix.Tasks.ElixirLSFormat.formatter_for_file(path)}
@@ -248,11 +248,11 @@ defmodule ElixirLS.LanguageServer.SourceFile do
 
         Logger.warning("Unable to get formatter options for #{path}: #{message}")
 
-        :error
+        {:error, message}
     end
   end
 
-  def formatter_for(_, _), do: :error
+  def formatter_for(_, _), do: {:error, :project_dir_not_set}
 
   defp format_code(code, opts) do
     try do

apps/language_server/lib/language_server/source_file/path.ex

@@ -116,4 +116,19 @@ defmodule ElixirLS.LanguageServer.SourceFile.Path do
   defp os_type do
     :os.type()
   end
+
+  def path_in_dir?(file, dir) do
+    case String.starts_with?(file, dir) do
+      true ->
+        # Get the grapheme after the directory in the file path
+        next_char_index = String.length(dir)
+        next_char = String.slice(file, next_char_index, 1)
+
+        # If the next character is either "" (end of string) or a "/", it's a valid match
+        next_char in ["", "/"]
+
+      false ->
+        false
+    end
+  end
 end