Format: .formatter.exs with import_deps results in errors
sascha-wolf opened this issue ยท 9 comments
This issue is closely related to elixir-lang/elixir#7327.
Current behaviour
Let's assume I'm in a project with a .formatter.exs
file looking like this:
[
import_deps: [:some_dependency_with_formatting_rules],
inputs: [
# Some files here ...
],
locals_without_parens: [
# Some configurations ...
]
]
Now I'm working on a file in lib/my_cool_module.ex
and then execute elixir-format
. This results in the following error:
** (Mix) Unknown dependency :some_dependency_with_formatting_rules given to :import_deps in the formatter configuration. The dependency is not listed in your mix.exs for environment :dev
The dependency is available in dev
and included in the root mix.exs
. The issue stems from the fact that mix format
tries find the dependencies in the current folder - which is the buffer's default-directory
- instead of looking for them in the .formatter.exs
folder. Due to that it's unable to find the dependencies and raises this error.
Possible fix
Due to mix format
looking for a .formatter.exs
file in the current folder by default, it might make sense to allow the user to control the execution directory for elixir-format
, this would also make a --dot-formatter
configuration obsolete.
In addition to that a sensible default for this folder could be the projectile root folder, as this usually contains the .formatter.exs
.
Alternative fix
This could be fixed on the mix level.
I've actually submitted elixir-lang/elixir#7328 a while ago, which would have fixed this issue. Sadly it got rejected due to having to cd into the .formatter.exs
folder when loading the dependencies.
Instead it was decided that mix format
should raise a more "descriptive" error, which you can see above.
Workaround
As a workaround I point my elixir-format-elixir-path
to a bash script with the following content:
if git rev-parse --is-inside-work-tree 1>/dev/null 2>/dev/null; then
cd "$(git rev-parse --absolute-git-dir | xargs dirname)"
fi
/path/to/my/elixir $@
This script checks if I'm in a git repository and then cd
s into the root folder of the repository. It basically is a hacked version of the suggested fix above.
Another workaround is to implement your suggested fix using advices:
(defun set-default-directory-to-mix-project-root (original-fun &rest args)
(if-let* ((mix-project-root (and (projectile-project-p)
(projectile-locate-dominating-file buffer-file-name
".formatter.exs"))))
(let ((default-directory mix-project-root))
(apply original-fun args))
(apply original-fun args)))
(advice-add 'elixir-format :around #'set-default-directory-to-mix-project-root)
It doesn't requies a wrapper script and seems to work with umbrella projects too.
@michaelvobrien opened a PR to the repo where the format code came from ( michaelvobrien/mix-format.el@ecde90b ) which goes in this direction too.
It seems like we may need to run through a few potential loops:
- Go up until we find a .formatter.exs file, run from that directory if found
- Go up until we find a mix.exs file, run from that directory if found
- Just format
What do you all think?
I like the idea. I'm not sure if step 1 is necessary though. Is it possible that a .formatter.exs
file for a mix project is located not in the directory which also contains the mix.exs
file for the project?
@smaximov it is possible yes.
An umbrella project with .formatter.exs
defined at the project directory, but with another mix.exs
defined an the app. Whether that's correct is another question!
The formatter could then set an import_deps
from a dependency that is only defined in the mix.exs
that is in the app, leading to that current issue.
# Case 1
/apps/my_app/.mix
/apps/my_app/.formatter.exs
# Case 2
/apps/my_app/.mix.exs
/.formatter.exs
# Case 3
/.mix.exs
/.formatter.exs
Note from https://hexdocs.pm/mix/Mix.Tasks.Format.html#module-formatting-options:
:import_deps (a list of dependencies as atoms) - specifies a list of dependencies whose formatter configuration will be imported. When specified, the formatter should run in the same directory as the mix.exs file that defines those dependencies.
So I think that in @Trevoke proposal, step 1 and 2 should be swapped (and possibly merged), but when running from the dominating mix.exs
, we should also look for the first dominating .formatter.exs
as well.
Usually in umbrella applications the sub-apps point to the _build
folder in the project root. In fact they get created that way by mix.
So executing mix format
still would happen in the directory root.
So executing mix format still would happen in the directory root.
I believe it should work only if the umbrella root has a .formatter.exs
file with :subdirectories
pointing to the umbrella sub-applications (e.g. [subdirectories: ["apps/*"]]
), which totally makes sense for umbrella projects, though it's not created by default (I missed this setting and found out about it only after I re-read the docs after reading your comment).
though it's not created by default
Disregard that, mix new
generates correct .formatter.exs
files for regular and umbrella projects. It's the Phoenix generator that doesn't generate .formatter.exs
at the moment.
Just to add to the discussion...
Another way to detect if we are in an umbrella is to run what exunit.el does (this is very elegant IMHO):
Code by @ananthakumaran
(defvar-local exunit-project-root nil)
(make-variable-buffer-local 'exunit-project-root)
(defvar-local exunit-umbrella-project-root nil)
(make-variable-buffer-local 'exunit-umbrella-project-root)
(defun exunit-project-root ()
"Return the current project root.
This value is cached in a buffer local to avoid filesytem access
on every call."
(or
exunit-project-root
(let ((root (locate-dominating-file default-directory "mix.exs")))
(unless root
(error "Couldn't locate project root folder. Make sure the current file is inside a project"))
(setq exunit-project-root (expand-file-name root)))))
(defun exunit-umbrella-project-root ()
"Return the current umbrella root.
This value is cached in a buffer local to avoid filesytem access
on every call."
(or
exunit-umbrella-project-root
(let ((root (locate-dominating-file default-directory "apps")))
(unless root
(error "Couldn't locate umbrella root folder. Make sure the current file is inside a umbrella project"))
(setq exunit-umbrella-project-root (expand-file-name root)))))
Of course that to use mix format properly we would need to "try" finding "apps" first and then, if not found, we would fall back to current project root.
How did you add this bash script. Is elixir-format-elixir-path
something you put in config.el
? Can't seem to find docs for this.
I got no match doing this:
M-x customize-option
Customize-variable: elixir-format-mix-path