URI - file system path conversion fixes (#447)

* fixes and improvements to URI - file system path translation

tests and implementation basing on https://github.com/microsoft/vscode-uri
adds support for UNC paths
adds downcasing for Windows drive letters
fixes character escaping for paths with URI control characters (?, #, : etc)
don't translate non file: URIs to paths

* fix invalid URIs int tests

file://project/file.ex is a path to /file.ex on project server
valid UNIX path URI to /project/file.ex is file:///project/file.ex

* reintroduce relative path expand

* change dubious logic

* fix tests on windows

* fix tests

* add .formatter.exs in apps

* fix formatter tests

* fix potential crashes when URI is not in file scheme

* fix tests on elixir < 1.10

* fix tests after merge

Co-authored-by: Łukasz Samson <[email protected]>

.formatter.exs

@@ -1,8 +1,11 @@
 [
   inputs: [
     "*.exs",
-    "config/**/*.exs",
-    "apps/*/{config,lib,test}/**/*.{ex,exs}",
-    "apps/*/mix.exs"
+    "config/**/*.exs"
+  ],
+  subdirectories: [
+    "apps/elixir_ls_utils",
+    "apps/elixir_ls_debugger",
+    "apps/language_server"
   ]
 ]

apps/elixir_ls_debugger/.formatter.exs

@@ -0,0 +1,6 @@
+[
+  inputs: [
+    "*.exs",
+    "{lib,test,config}/**/*.{ex,exs}"
+  ]
+]

apps/elixir_ls_utils/.formatter.exs

@@ -0,0 +1,8 @@
+[
+  inputs: [
+    "*.exs",
+    "{lib,config}/**/*.{ex,exs}",
+    "test/*.exs",
+    "test/support/**/*.ex"
+  ]
+]

apps/language_server/.formatter.exs

@@ -0,0 +1,6 @@
+[
+  inputs: [
+    "*.exs",
+    "{lib,test,config}/**/*.{ex,exs}"
+  ]
+]

apps/language_server/lib/language_server/providers/code_lens/test.ex

@@ -16,7 +16,7 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.Test do
 
   @run_test_command "elixir.lens.test.run"
 
-  def code_lens(uri, text) do
+  def code_lens(uri = "file:" <> _, text) do
     with {:ok, buffer_file_metadata} <- parse_source(text) do
       source_lines = SourceFile.lines(text)
 
@@ -48,6 +48,8 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.Test do
     end
   end
 
+  def code_lens(_uri, _text), do: {:ok, []}
+
   defp get_test_lenses(test_blocks, file_path) do
     args = fn block ->
       %{

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

@@ -36,13 +36,15 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
 
   # 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, project_dir) do
+  defp can_format?(file_uri = "file:" <> _, project_dir) do
     file_path = file_uri |> SourceFile.path_from_uri() |> Path.absname()
 
-    not String.starts_with?(file_path, project_dir) or
+    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
+
   def should_format?(file_uri, project_dir, inputs) when is_list(inputs) do
     file_path = file_uri |> SourceFile.path_from_uri() |> Path.absname()
 

apps/language_server/lib/language_server/server.ex

@@ -118,7 +118,7 @@ defmodule ElixirLS.LanguageServer.Server do
   end
 
   @impl GenServer
-  def handle_call({:suggest_contracts, uri}, from, state) do
+  def handle_call({:suggest_contracts, uri = "file:" <> _}, from, state) do
     case state do
       %{analysis_ready?: true, source_files: %{^uri => %{dirty?: false}}} ->
         {:reply, Dialyzer.suggest_contracts([SourceFile.path_from_uri(uri)]), state}
@@ -130,6 +130,10 @@ defmodule ElixirLS.LanguageServer.Server do
     end
   end
 
+  def handle_call({:suggest_contracts, _uri}, _from, state) do
+    {:reply, [], state}
+  end
+
   @impl GenServer
   def handle_cast({:build_finished, {status, diagnostics}}, state)
       when status in [:ok, :noop, :error] and is_list(diagnostics) do
@@ -400,7 +404,7 @@ defmodule ElixirLS.LanguageServer.Server do
           # deleted file still open in editor, keep dirty flag
           acc
 
-        %{"uri" => uri}, acc ->
+        %{"uri" => uri = "file:" <> _}, acc ->
           # file created/updated - set dirty flag to false if file contents are equal
           case acc[uri] do
             %SourceFile{text: source_file_text, dirty?: true} = source_file ->

apps/language_server/lib/language_server/source_file.ex

@@ -62,28 +62,101 @@ defmodule ElixirLS.LanguageServer.SourceFile do
   @doc """
   Returns path from URI in a way that handles windows file:///c%3A/... URLs correctly
   """
-  def path_from_uri(uri) do
-    uri_path = URI.decode(URI.parse(uri).path)
+  def path_from_uri(%URI{scheme: "file", path: path, authority: authority}) do
+    uri_path =
+      cond do
+        path == nil ->
+          # treat no path as root path
+          "/"
+
+        authority not in ["", nil] and path not in ["", nil] ->
+          # UNC path
+          "//#{URI.decode(authority)}#{URI.decode(path)}"
+
+        true ->
+          decoded_path = URI.decode(path)
+
+          if match?({:win32, _}, :os.type()) and
+               String.match?(decoded_path, ~r/^\/[a-zA-Z]:/) do
+            # Windows drive letter path
+            # drop leading `/` and downcase drive letter
+            <<_, letter, path_rest::binary>> = decoded_path
+            <<downcase(letter), path_rest::binary>>
+          else
+            decoded_path
+          end
+      end
 
     case :os.type() do
-      {:win32, _} -> String.trim_leading(uri_path, "/")
-      _ -> uri_path
+      {:win32, _} ->
+        # convert path separators from URI to Windows
+        String.replace(uri_path, ~r/\//, "\\")
+
+      _ ->
+        uri_path
     end
   end
 
+  def path_from_uri(%URI{scheme: scheme}) do
+    raise ArgumentError, message: "unexpected URI scheme #{inspect(scheme)}"
+  end
+
+  def path_from_uri(uri) do
+    uri |> URI.parse() |> path_from_uri
+  end
+
   def path_to_uri(path) do
-    uri_path =
-      path
-      |> Path.expand()
-      |> URI.encode()
-      |> String.replace(":", "%3A")
+    path = Path.expand(path)
 
-    case :os.type() do
-      {:win32, _} -> "file:///" <> uri_path
-      _ -> "file://" <> uri_path
-    end
+    path =
+      case :os.type() do
+        {:win32, _} ->
+          # convert path separators from Windows to URI
+          String.replace(path, ~r/\\/, "/")
+
+        _ ->
+          path
+      end
+
+    {authority, path} =
+      case path do
+        "//" <> rest ->
+          # UNC path - extract authority
+          case String.split(rest, "/", parts: 2) do
+            [_] ->
+              # no path part, use root path
+              {rest, "/"}
+
+            [a, ""] ->
+              # empty path part, use root path
+              {a, "/"}
+
+            [a, p] ->
+              {a, "/" <> p}
+          end
+
+        "/" <> _rest ->
+          {"", path}
+
+        other ->
+          # treat as relative to root path
+          {"", "/" <> other}
+      end
+
+    %URI{
+      scheme: "file",
+      authority: authority |> URI.encode(),
+      # file system paths allow reserved URI characters that need to be escaped
+      # the exact rules are complicated but for simplicity we escape all reserved except `/`
+      # that's what https://github.com/microsoft/vscode-uri does
+      path: path |> URI.encode(&(&1 == ?/ or URI.char_unreserved?(&1)))
+    }
+    |> URI.to_string()
   end
 
+  defp downcase(char) when char >= ?A and char <= ?Z, do: char + 32
+  defp downcase(char), do: char
+
   def full_range(source_file) do
     lines = lines(source_file)
     last_line = List.last(lines)
@@ -244,7 +317,7 @@ defmodule ElixirLS.LanguageServer.SourceFile do
   end
 
   @spec formatter_opts(String.t()) :: {:ok, keyword()} | :error
-  def formatter_opts(uri) do
+  def formatter_opts(uri = "file:" <> _) do
     path = path_from_uri(uri)
 
     try do
@@ -263,6 +336,8 @@ defmodule ElixirLS.LanguageServer.SourceFile do
     end
   end
 
+  def formatter_opts(_), do: :error
+
   defp format_code(code, opts) do
     try do
       {:ok, Code.format_string!(code, opts)}

apps/language_server/test/providers/code_lens/test_test.exs

@@ -1,6 +1,7 @@
 defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
   use ExUnit.Case
 
+  import ElixirLS.LanguageServer.Test.PlatformTestHelpers
   alias ElixirLS.LanguageServer.Providers.CodeLens
 
   setup context do
@@ -16,7 +17,7 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
   end
 
   test "returns all module code lenses" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -32,13 +33,17 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
 
     assert lenses ==
              [
-               build_code_lens(0, :module, "/file.ex", %{"module" => MyModule}),
-               build_code_lens(4, :module, "/file.ex", %{"module" => MyModule2})
+               build_code_lens(0, :module, maybe_convert_path_separators("/project/file.ex"), %{
+                 "module" => MyModule
+               }),
+               build_code_lens(4, :module, maybe_convert_path_separators("/project/file.ex"), %{
+                 "module" => MyModule2
+               })
              ]
   end
 
   test "returns all nested module code lenses" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -54,13 +59,17 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
 
     assert lenses ==
              [
-               build_code_lens(0, :module, "/file.ex", %{"module" => MyModule}),
-               build_code_lens(3, :module, "/file.ex", %{"module" => MyModule.MyModule2})
+               build_code_lens(0, :module, maybe_convert_path_separators("/project/file.ex"), %{
+                 "module" => MyModule
+               }),
+               build_code_lens(3, :module, maybe_convert_path_separators("/project/file.ex"), %{
+                 "module" => MyModule.MyModule2
+               })
              ]
   end
 
   test "does not return lenses for modules that don't import ExUnit.case" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -73,7 +82,7 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
   end
 
   test "returns lenses for all describe blocks" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -91,17 +100,21 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
 
     assert Enum.member?(
              lenses,
-             build_code_lens(3, :describe, "/file.ex", %{"describe" => "describe1"})
+             build_code_lens(3, :describe, maybe_convert_path_separators("/project/file.ex"), %{
+               "describe" => "describe1"
+             })
            )
 
     assert Enum.member?(
              lenses,
-             build_code_lens(6, :describe, "/file.ex", %{"describe" => "describe2"})
+             build_code_lens(6, :describe, maybe_convert_path_separators("/project/file.ex"), %{
+               "describe" => "describe2"
+             })
            )
   end
 
   test "returns lenses for all test blocks" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -119,17 +132,21 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
 
     assert Enum.member?(
              lenses,
-             build_code_lens(3, :test, "/file.ex", %{"testName" => "test1"})
+             build_code_lens(3, :test, maybe_convert_path_separators("/project/file.ex"), %{
+               "testName" => "test1"
+             })
            )
 
     assert Enum.member?(
              lenses,
-             build_code_lens(6, :test, "/file.ex", %{"testName" => "test2"})
+             build_code_lens(6, :test, maybe_convert_path_separators("/project/file.ex"), %{
+               "testName" => "test2"
+             })
            )
   end
 
   test "given test blocks inside describe blocks, should return code lenses with the test and describe name" do
-    uri = "file://project/file.ex"
+    uri = "file:///project/file.ex"
 
     text = """
     defmodule MyModule do
@@ -146,7 +163,7 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
 
     assert Enum.member?(
              lenses,
-             build_code_lens(4, :test, "/file.ex", %{
+             build_code_lens(4, :test, maybe_convert_path_separators("/project/file.ex"), %{
                "testName" => "test1",
                "describe" => "describe1"
              })
@@ -281,26 +298,26 @@ defmodule ElixirLS.LanguageServer.Providers.CodeLens.TestTest do
     end
 
     test "returns module lens on the module declaration line", %{text: text} do
-      uri = "file://project/file.ex"
+      uri = "file:///project/file.ex"
 
       {:ok, lenses} = CodeLens.Test.code_lens(uri, text)
 
       assert Enum.member?(
                lenses,
-               build_code_lens(0, :module, "/file.ex", %{
+               build_code_lens(0, :module, maybe_convert_path_separators("/project/file.ex"), %{
                  "module" => ElixirLS.LanguageServer.DiagnosticsTest
                })
              )
     end
 
     test "returns test lenses with describe info", %{text: text} do
-      uri = "file://project/file.ex"
+      uri = "file:///project/file.ex"
 
       {:ok, lenses} = CodeLens.Test.code_lens(uri, text)
 
       assert Enum.member?(
                lenses,
-               build_code_lens(5, :test, "/file.ex", %{
+               build_code_lens(5, :test, maybe_convert_path_separators("/project/file.ex"), %{
                  "testName" => "extract the stacktrace from the message and format it",
                  "describe" => "normalize/2"
                })