Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting does not work with phoenix and neovim (with vim-lsp) #235

Closed
wingyplus opened this issue May 7, 2020 · 14 comments · Fixed by #345
Closed

Formatting does not work with phoenix and neovim (with vim-lsp) #235

wingyplus opened this issue May 7, 2020 · 14 comments · Fixed by #345

Comments

@wingyplus
Copy link
Contributor

wingyplus commented May 7, 2020

Environment

  • Elixir & Erlang versions (elixir --version): Elixir 1.10.2 (compiled with Erlang/OTP 22)
  • ElixirLS version: v0.3.3
  • Operating system: macOS
  • Editor or IDE name (e.g. Emacs/VSCode): neovim with vim-lsp
  • Phoenix version: v1.4.16

Description

The bug happens when I generate phoenix project. After I open file inside project and try formatting with LspDocumentFormat. vim-lsp returns an errors (described in Reproduce Steps below).

Reproduce Steps

  • Create project with mix phx.new --no-html --no-webpack --no-gettext lsp_formatter and fetch and install dependencies.
  • Open lib/lsp_formatter.ex with nvim. Waiting elixir-ls to compile.
  • Call LspDocumentFormat, received an error that capture from vim lsp log:
Thu May  7 23:35:24 2020:["--->", 3, "elixir-ls", {"method": "textDocument/formatting", "on_notification": "---funcref---", "sync": 0, "params": {"options": {"insertSpaces": true, "tabSize"
: 2}, "textDocument": {"uri": "file:///Users/thanabodee/src/github.com/wingyplus/lsp_formatter/lib/lsp_formatter.ex"}}}]
Thu May  7 23:35:24 2020:["<---", 3, "elixir-ls", {"response": {"id": 2, "jsonrpc": "2.0", "error": {"code": -32000, "message": "an exception was raised:\n    ** (Mix.Error) Unknown depende
ncy :ecto_sql given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :test\n        (mix 1.10.2) lib/mix.ex:392: Mix.raise/1\n
     (mix 1.10.2) lib/mix/tasks/format.ex:246: anonymous fn/3 in Mix.Tasks.Format.eval_deps_opts/2\n        (elixir 1.10.2) lib/enum.ex:2111: Enum.\"-reduce/3-lists^foldl/2-0-\"/3\n
(mix 1.10.2) lib/mix/tasks/format.ex:245: Mix.Tasks.Format.eval_deps_opts/2\n        (mix 1.10.2) lib/mix/tasks/format.ex:201: anonymous fn/5 in Mix.Tasks.Format.eval_deps_and_subdirectories/4\n        (mix 1.10.2) lib/mix/tasks/format.ex:276: anonymous fn/2 in Mix.Tasks.Format.eval_subs_opts/3\n        (elixir 1.10.2) lib/enum.ex:1094: anonymous fn/3 in Enum.flat_map_reduce/
3\n        (elixir 1.10.2) lib/enum.ex:3686: Enumerable.List.reduce/3"}}, "request": {"id": 2, "jsonrpc": "2.0", "method": "textDocument/formatting", "params": {"options": {"insertSpaces":
true, "tabSize": 2}, "textDocument": {"uri": "file:///Users/thanabodee/src/github.com/wingyplus/lsp_formatter/lib/lsp_formatter.ex"}}}}]
Thu May  7 23:35:24 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "\n23:35:24.508 [error] Process #PID<0.4583.0> raised
an exception\n** (Mix.Error) Unknown dependency :ecto_sql given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :test\n    (mix
1.10.2) lib/mix.ex:392: Mix.raise/1\n    (mix 1.10.2) lib/mix/tasks/format.ex:246: anonymous fn/3 in Mix.Tasks.Format.eval_deps_opts/2\n    (elixir 1.10.2) lib/enum.ex:2111: Enum.\"-reduce/
3-lists^foldl/2-0-\"/3\n    (mix 1.10.2) lib/mix/tasks/format.ex:245: Mix.Tasks.Format.eval_deps_opts/2\n    (mix 1.10.2) lib/mix/tasks/format.ex:201: anonymous fn/5 in Mix.Tasks.Format.eva
l_deps_and_subdirectories/4\n    (mix 1.10.2) lib/mix/tasks/format.ex:276: anonymous fn/2 in Mix.Tasks.Format.eval_subs_opts/3\n    (elixir 1.10.2) lib/enum.ex:1094: anonymous fn/3 in Enum.
flat_map_reduce/3\n    (elixir 1.10.2) lib/enum.ex:3686: Enumerable.List.reduce/3", "type": 4}}}]
  • Try comment subdirectories in .formatter.exs:
[
  import_deps: [:ecto, :phoenix],
  inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"],
  # subdirectories: ["priv/*/migrations"]
]
  • Restart editor (open the same file) and try formatting. It received the same kind of errors but different lib (previously, it's :ecto_sql. the current is :ecto)
Thu May  7 23:38:07 2020:["--->", 3, "elixir-ls", {"method": "textDocument/formatting", "on_notification": "---funcref---", "sync": 0, "params": {"options": {"insertSpaces": true, "tabSize"
: 2}, "textDocument": {"uri": "file:///Users/thanabodee/src/github.com/wingyplus/lsp_formatter/lib/lsp_formatter.ex"}}}]
Thu May  7 23:38:07 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "[ElixirLS Dialyzer] Done writing manifest in 1561 mil
liseconds.", "type": 3}}}]
Thu May  7 23:38:08 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "[ElixirLS WorkspaceSymbols] Indexing...", "type": 3}}
}]
Thu May  7 23:38:08 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "Compiling with Mix env test", "type": 4}}}]
Thu May  7 23:38:08 2020:["<---", 3, "elixir-ls", {"response": {"id": 2, "jsonrpc": "2.0", "error": {"code": -32000, "message": "an exception was raised:\n    ** (Mix.Error) Unknown depende
ncy :ecto given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :test\n        (mix 1.10.2) lib/mix.ex:392: Mix.raise/1\n
 (mix 1.10.2) lib/mix/tasks/format.ex:246: anonymous fn/3 in Mix.Tasks.Format.eval_deps_opts/2\n        (elixir 1.10.2) lib/enum.ex:2111: Enum.\"-reduce/3-lists^foldl/2-0-\"/3\n        (mix
 1.10.2) lib/mix/tasks/format.ex:245: Mix.Tasks.Format.eval_deps_opts/2\n        (mix 1.10.2) lib/mix/tasks/format.ex:201: anonymous fn/5 in Mix.Tasks.Format.eval_deps_and_subdirectories/4\
n        (mix 1.10.2) lib/mix/tasks/format.ex:160: Mix.Tasks.Format.formatter_opts_for_file/2\n        (language_server 0.3.3) lib/language_server/providers/formatting.ex:12: ElixirLS.Langu
ageServer.Providers.Formatting.format/3\n        (language_server 0.3.3) lib/language_server/server.ex:535: anonymous fn/3 in ElixirLS.LanguageServer.Server.handle_request_async/2"}}, "requ
est": {"id": 2, "jsonrpc": "2.0", "method": "textDocument/formatting", "params": {"options": {"insertSpaces": true, "tabSize": 2}, "textDocument": {"uri": "file:///Users/thanabodee/src/gith
ub.com/wingyplus/lsp_formatter/lib/lsp_formatter.ex"}}}}]
Thu May  7 23:38:08 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "\n23:38:08.864 [error] Process #PID<0.285.0> raised a
n exception\n** (Mix.Error) Unknown dependency :ecto given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :test\n    (mix 1.10.
2) lib/mix.ex:392: Mix.raise/1\n    (mix 1.10.2) lib/mix/tasks/format.ex:246: anonymous fn/3 in Mix.Tasks.Format.eval_deps_opts/2\n    (elixir 1.10.2) lib/enum.ex:2111: Enum.\"-reduce/3-lis
ts^foldl/2-0-\"/3\n    (mix 1.10.2) lib/mix/tasks/format.ex:245: Mix.Tasks.Format.eval_deps_opts/2\n    (mix 1.10.2) lib/mix/tasks/format.ex:201: anonymous fn/5 in Mix.Tasks.Format.eval_dep
s_and_subdirectories/4\n    (mix 1.10.2) lib/mix/tasks/format.ex:160: Mix.Tasks.Format.formatter_opts_for_file/2\n    (language_server 0.3.3) lib/language_server/providers/formatting.ex:12:
 ElixirLS.LanguageServer.Providers.Formatting.format/3\n    (language_server 0.3.3) lib/language_server/server.ex:535: anonymous fn/3 in ElixirLS.LanguageServer.Server.handle_request_async/
2", "type": 4}}}]
  • Next step, try to comment import_deps in .formatter.exs:
[
  # import_deps: [:ecto, :phoenix],
  inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"],
  # subdirectories: ["priv/*/migrations"]
]
  • Restart editor and try formatting. It's works!! 🎉

More information

I try it with vscode without any comments in .formatter.exs. It's works fine.
Run mix format. It's works fine.

@axelson
Copy link
Member

axelson commented May 8, 2020

I think this is some sort of race condition around how the CWD is managed. I think it affects vscode-elixir-ls as well. I think that to resolve this ElixirLS needs to be smarter about choosing the CWD for formatting. You may also want to look to see if your project_directory is properly set at the time that you're trying to format.

A good debugging step might be to log (at level "info") what the CWD is when the formatting starts.

@wingyplus
Copy link
Contributor Author

I added the log in changes below:

diff --git a/apps/language_server/lib/language_server/providers/formatting.ex b/apps/language_server/lib/language_server/providers/formatting.ex
index 4534cf5..e861f37 100644
--- a/apps/language_server/lib/language_server/providers/formatting.ex
+++ b/apps/language_server/lib/language_server/providers/formatting.ex
@@ -1,12 +1,14 @@
 defmodule ElixirLS.LanguageServer.Providers.Formatting do
   import ElixirLS.LanguageServer.Protocol, only: [range: 4]
-  alias ElixirLS.LanguageServer.SourceFile
+  alias ElixirLS.LanguageServer.{JsonRpc, SourceFile}

   def supported? do
     function_exported?(Code, :format_string!, 2)
   end

   def format(source_file, uri, project_dir) do
+    JsonRpc.log_message(:info, "formatting file (#{uri}) on project directory (#{project_dir})")
+
     if can_format?(uri, project_dir) do
       file = SourceFile.path_from_uri(uri) |> Path.relative_to(project_dir)
       opts = formatter_opts(file)

Got the result when formatting start:

Sun May 10 16:07:22 2020:["<---", 3, "elixir-ls", {"response": {"method": "window/logMessage", "jsonrpc": "2.0", "params": {"message": "formatting file (file:///Users/thanabodee/src/github.
com/wingyplus/lsp_formatter/lib/lsp_formatter.ex) on project directory (/Users/thanabodee/src/github.com/wingyplus/lsp_formatter)", "type": 3}}}]

@wingyplus
Copy link
Contributor Author

wingyplus commented May 10, 2020

I investigate more on elixir side. I patch value from inspect Mix.Project.deps_paths() value to the exception below:

diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex
index b2393e686..12db1faa5 100644
--- a/lib/mix/lib/mix/tasks/format.ex
+++ b/lib/mix/lib/mix/tasks/format.ex
@@ -296,7 +296,7 @@ defp assert_valid_dep_and_fetch_path(dep, deps_paths) when is_atom(dep) do

       :error ->
         Mix.raise(
-          "Unknown dependency #{inspect(dep)} given to :import_deps in the formatter configuration. " <>
+          "Unknown dependency #{inspect(dep)} in the #{inspect(deps_paths)} given to :import_deps in the formatter configuration. " <>
             "The dependency is not listed in your mix.exs for environment #{inspect(Mix.env())}"
         )
     end

After call formatting. The result is deps_paths() returning empty map:

Sun May 10 17:51:45 2020:["<---", 3, "elixir-ls", {"response": {"id": 2, "jsonrpc": "2.0", "error": {"code": -32000, "message": "an exception was raised:\n    ** (Mix.Error) Unknown depende
ncy :ecto_sql in the %{} given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :test\n        (mix 1.11.0-dev) lib/mix.ex:435: M
ix.raise/1\n        (mix 1.11.0-dev) lib/mix/tasks/format.ex:246: anonymous fn/3 in Mix.Tasks.Format.eval_deps_opts/2\n        (elixir 1.11.0-dev) lib/enum.ex:2115: Enum.\"-reduce/3-lists^f
oldl/2-0-\"/3\n        (mix 1.11.0-dev) lib/mix/tasks/format.ex:245: Mix.Tasks.Format.eval_deps_opts/2\n        (mix 1.11.0-dev) lib/mix/tasks/format.ex:201: anonymous fn/5 in Mix.Tasks.For
mat.eval_deps_and_subdirectories/4\n        (mix 1.11.0-dev) lib/mix/tasks/format.ex:276: anonymous fn/2 in Mix.Tasks.Format.eval_subs_opts/3\n        (elixir 1.11.0-dev) lib/enum.ex:1097:
anonymous fn/3 in Enum.flat_map_reduce/3\n        (elixir 1.11.0-dev) lib/enum.ex:3690: Enumerable.List.reduce/3"}}, "request": {"id": 2, "jsonrpc": "2.0", "method": "textDocument/formattin
g", "params": {"options": {"insertSpaces": true, "tabSize": 2}, "textDocument": {"uri": "file:///Users/thanabodee/src/github.com/wingyplus/lsp_formatter/lib/lsp_formatter.ex"}}}}]

@wingyplus
Copy link
Contributor Author

I think Build can cause make deps cache disappear by this line. After I comment it. Formatting works after I save file but not works before save the file. Not sure why.

@axelson
Copy link
Member

axelson commented May 10, 2020

I'm more interested in File.cwd() than the project directory. But I think you might be on to something with Mix.Project.deps_paths() returning an empty map. But in my testing it doesn't appear that Mix.Project.clear_deps_cache() is causing that directly. What does Mix.Project.deps_path() (note no "s" at the end) return? Maybe that is somehow being set incorrectly?

@axelson
Copy link
Member

axelson commented May 10, 2020

Do you have instructions for a neovim setup that I could follow?

@wingyplus
Copy link
Contributor Author

wingyplus commented May 11, 2020

@axelson you needs to install vim-lsp and vim-lsp-settings. And setting ~/.config/nvim/init.vim with this setting (https://gist.github.com/wingyplus/18d85bf3080f974d6a5f73417aa40a58). For lsp log, you can see it at $HOME/vim-lsp.log or change this line for you want to change the location.

NOTE

@wingyplus
Copy link
Contributor Author

Add Mix.Project.deps_path() into raise exception message:

diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex
index b2393e686..b82a68334 100644
--- a/lib/mix/lib/mix/tasks/format.ex
+++ b/lib/mix/lib/mix/tasks/format.ex
@@ -297,7 +297,7 @@ defmodule Mix.Tasks.Format do
       :error ->
         Mix.raise(
           "Unknown dependency #{inspect(dep)} given to :import_deps in the formatter configuration. " <>
-            "The dependency is not listed in your mix.exs for environment #{inspect(Mix.env())}"
+            "The dependency is not listed in your mix.exs for environment #{inspect(Mix.env())}: #{inspect(Mix.Project.deps_path())}"
         )
     end
   end

format raise an exception with deps_path /Users/thanabodee/src/github.com/wingyplus/lsp_formatter/deps.

@wingyplus
Copy link
Contributor Author

wingyplus commented Jul 10, 2020

I would like to update this issue on neovim + vim-lsp that it's not gonna happens all of time that I called LspDocumentFormat(Sync) after upgrading elixir-ls to v0.5.0, which available on latest vim-lsp-settings plugin. It's just happens during elixir-ls is building deps and project source codes. And sometimes after that (I'm not sure why).

cc @axelson

@axelson
Copy link
Member

axelson commented Jul 10, 2020

@wingyplus are you getting an error similar to #252 ?

@wingyplus
Copy link
Contributor Author

@axelson I found it sometimes while adding a new dependency into mix.exs. But not often.

@wingyplus
Copy link
Contributor Author

wingyplus commented Jul 16, 2020

@axelson I just try to fix by caching formatter opts after build is done. It's seems the formatting issue is getting more stable. The initial branch for discussing is here https://github.com/wingyplus/elixir-ls/tree/experiment-formatter. Good to see your feedback.

@wingyplus
Copy link
Contributor Author

I starting to understand this issue. The Mix.Tasks.Format.formatter_opts_for_file call the Mix.Tasks.Format.eval_deps_opts and then calling Mix.Project.deps_paths() which calling Mix.Dep.cached under the hood. The currently on master branch, we delete deps and then fetch deps when deps has change compare to previous deps (refer to this line). But delete deps step was an outside if clause that compare deps. So that means it can be make elixir-lsp crash because deps cached already disappear. (NOTE the crash fix in #319). I think we can reduce this errors by moving this line to inside if clause (this line).

@axelson @lukaszsamson What do you think?

wingyplus pushed a commit to wingyplus/elixir-ls that referenced this issue Aug 15, 2020
Remove deps cached every build can cause deps cached disappear in :ets.
Fixes by move calling Mix.Project.clear_deps_cache/0 only when deps in
mix.exs was change.

Fixes elixir-lsp#235
@wingyplus
Copy link
Contributor Author

wingyplus commented Aug 15, 2020

I wrote a patch to show what am I thinking. Suggestions are welcome. ^_^

wingyplus pushed a commit to wingyplus/elixir-ls that referenced this issue Aug 23, 2020
Remove deps cached every build can cause deps cached disappear in :ets.
Fixes by move calling Mix.Project.clear_deps_cache/0 only when deps in
mix.exs was change.

Fixes elixir-lsp#235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants