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

Mix Formatter not compiling plugins #11915

Closed
TheFirstAvenger opened this issue Jun 10, 2022 · 5 comments
Closed

Mix Formatter not compiling plugins #11915

TheFirstAvenger opened this issue Jun 10, 2022 · 5 comments
Labels
Milestone

Comments

@TheFirstAvenger
Copy link
Contributor

TheFirstAvenger commented Jun 10, 2022

Elixir and Erlang/OTP versions

Erlang/OTP 24 [erts-12.2.1] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1]
Elixir 1.13.4 (compiled with Erlang/OTP 23)

Operating system

Github Actions CI

Current behavior

We recently had an issue where we had configured the HTMLFormatter as a plugin in .formatter.exs and it was failing silently in CI.

There are two underlying issues here:

  1. When a plugin defined in .formatter.exs is not found, mix format outputs Skipping formatter plugin Phoenix.LiveView.HTMLFormatter because module cannot be found but does not return an error code. This is a problem in CI where the output isn't generally reviewed, only error returns.

  2. mix format does not compile the application being formatted or its dependencies. This was ok before plugins, but when a plugin is involved, especially if that plugin is not yet compiled, this is a problem.

The combination of these two potentially leaves users in the state of having a Github Actions CI job that performs a checkout and then a mix format, which runs without compiling, and then "silently" failed to use the desired plugin.

Expected behavior

  1. mix format should fail if a plugin is not found
  2. mix format should compile the app when a plugin is defined in .formatter.exs (maybe only when it is not found?)
@josevalim
Copy link
Member

We can do option 2 if a plugin is missing but I am not quite sure if we should do option 1 because for some workflows (like editors) formatting something with a warning may be better than not formatting anything at all? But hopefully 2 will address 1?

@TheFirstAvenger
Copy link
Contributor Author

My thoughts on the editor workflows is that personally, I would rather see an error in the editor that formatting failed due to a plugin not being found and be able to fix the issue than to have it silently not run some of the formatting I was expecting and not know why it wasn't formatting correctly.

Maybe if we can't find the plugin, we could run the formatter with what we do have, but still raise an error, that way we get the benefits of both?

@v0idpwn
Copy link
Contributor

v0idpwn commented Jun 11, 2022

Had problems with (2) in the past, +1 for ensuring plugin is compiled when formatting

@josevalim josevalim added this to the v1.14 milestone Jun 19, 2022
@marcandre
Copy link
Contributor

I'd be inclined to also do 1), i.e. and raise an error and refuse to run, especially until 2) is resolved.

@johannesE
Copy link

To anyone reading this and looking for a quick fix. Make sure that mix compile has been run.

If you are using a ci, here is an example that uses https://github.com/erlef/setup-beam:

      - name: Compile app (required for the formatter plugin)
        run: mix compile
        
      - name: Format with mix format
        run: mix format
        
      - name: Commit mix format code changes
        uses: EndBug/add-and-commit@v9
        with:
          author_name: "${{ github.event.pusher.name }}"
          author_email: "${{ github.event.pusher.email }}"
          message: 'Fix formatting of elixir code with mix format'
          add: 'lib'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants