Make formatter plugins which target .ex/.exs run alongside (not "instead of") standard formatting and other plugins which target sigils.

[evnp]

Expected behavior

• When a mix format plugin is used which specifies .ex/.exs file extensions, plugin formatting is run after standard mix format formatting for those files.
• When mix format plugin A is used which specifies .ex/.exs file extensions, alongside plugin B which specifies sigils within those files,
  • formatting from A is applied to the whole file
  • formatting from both A and B are applied to sigils specified by B (in order of plugins list in .formatter.exs)

Observed behavior

• When a mix format plugin is used which targets .ex/.exs file extensions, standard mix format formatting is not applied to those files.
• When mix format plugin A is used which targets .ex/.exs file extensions, alongside plugin B which targets sigils within those files,
  • formatting from A is applied to the whole file
  • formatting from B is not applied at all (to sigils within .ex/.exs files)

---

Working with some mix format plugins, I noticed that a plugin targeting .ex files would cause a second plugin targeting ~H"""...""" sigils to not format them at all. After looking into format.ex, it seems like this is because of the current implementation of find_formatter_for_file which has a cond with three branches:

    cond do
      plugins = find_plugins_for_extension(formatter_opts, ext) ->
        fn input ->
          Enum.reduce(plugins, input, fn plugin, input ->
            plugin.format(input, [extension: ext, file: file] ++ formatter_opts)
          end)
        end

      ext in ~w(.ex .exs) ->
        &elixir_format(&1, [file: file] ++ formatter_opts)

      true ->
        & &1
    end

It looks like sigil formatting happens during elixir_format, which iiuc won't run at all if a plugin is found for an .ex or .exs file matching the first case of this cond. Because of this, it seems that sigil-targeting plugins won't run at all for files where another plugin was found targeting .ex or .exs. I think it also means standard elixir formatting won't run on .ex/.exs files if a plugin targets them. These interpretations seem to be verified by the three tests cases in 12a14ef which fail prior to the fix.

Perhaps this is all intentional behavior? I found it a bit surprising, and didn't see mention of it at https://mix.hexdocs.pm/main/Mix.Tasks.Format.html#plugins, but happy to simply be informed here as well!

Using this command to run the three test cases:

bin/elixirc lib/mix/lib/mix/tasks/format.ex -o lib/elixir/ebin && LINE=607 bin/elixir lib/mix/test/mix/tasks/format_test.exs; LINE=634 bin/elixir lib/mix/test/mix/tasks/format_test.exs; LINE=664 bin/elixir lib/mix/test/mix/tasks/format_test.exs

Now, I can't help but add something you've likely heard many times before: Elixir has been (and continues to be) a joy to work with, thank you for all your labors making it the wonderful set of tools it is today.

[josevalim]

This is definitely a breaking change. What we could try to do is to either have an option that allows you to call the original formatter or pass a function with the original formatter that you could call.

[josevalim]

It is probably better as an option because of you have multiple .ex plugins, we only need to format it once. But then there is the question if we run it before or after.

[novaugust]

fwiw, styler does the equivalent of mix formatas part of turning ast back into text code, and even relies on the formatter to clean up any mess styler made along the way.
https://github.com/adobe/elixir-styler/blob/4674cdb3728cb83e93ec185b59e014609db5f059/lib/styler.ex#L92-L102

so anything non-optional would be redundant cpu in styler's case.

perhaps the plugins themselves can flag something in their features callback specifying whether they want the formatter to still run after they do (or not?). i think after makes more sense, as plugins expect to receive unformatted code and finish with formatted code. but that would just introduce the problem of multiple .ex plugins arguing over whether formatter should be run or not - probably if any want it, it's worth doing after running all?

you could get the same behaviour by having it be user configured in .formatter.exs, but i'd guess that in practice the user is only going to do that when a plugin's README tells them to, at which point the plugin could've just told the formatter itself.

OR if the status quo is preferable, maybe the Formatter ### Plugins docs just need updating to surface how plugin authors can call code similar to styler's above to run the formatter themselves when their plugin runs on .ex/.exs

[josevalim]

Thank you @novaugust. Given you can call the formatter anyway, I believe we can close this and revisit if that approach has issues!

[evnp]

Thanks for the eyes and discussion here. That this would be a breaking change makes sense, I guess hyrums law if nothing else would tell us as much.

I'm happy to follow styler here (thanks for the example @novaugust!) and can definitely work with the status quo if that seems like the best way forward. My biggest concern is clarity; when I looked at my .formatter.exs and the docs, I came away with the mistaken assumption that plugins wouldn't affect standard formatting, and spent time figuring out why they seemed to be preventing it. The mistake is on me, but I do think the docs at least could make this clearer. Happy to propose something if that would be useful.

The question of running formatting before or after is a great one. In my ideal world you (the end-user of .formatter.exs) would actually be able to configure standard formatting to run anywhere within the order of plugins, start, middle, or end. This would also make things read quite clearly in .formatter.exs:

# run standard mix format first:
plugins: [:elixir_format, ExtensionExDoubleNewlinePlugin, SigilWPlugin],

# run standard mix format in middle:
plugins: [ExtensionExDoubleNewlinePlugin, :elixir_format, SigilWPlugin],

# run standard mix format last:
plugins: [ExtensionExDoubleNewlinePlugin, SigilWPlugin, :elixir_format],

I'll admit that adding an optional atom to the plugins list is weird and likely unpalatable. It just seems to me like the only elegant way to achieve the above through configuration. It unfortunately makes the code changes more convoluted as well. I had written up a version last night to check feasibility, just hadn't gotten to new test cases yet, here it is for posterity: evnp#1

With the status quo, I guess any plugin which might be used to format .ex/.exs will probably want to add its own Inspect.Algebra.format (or Code.format_string!) call under an option. This is fine for plugins you control, but for plugins that don't implement such an option, it will be a limiting factor when using them to format ex./.exs
(eg. https://filter-formatter.hexdocs.pm/FilterFormatter.html)

[evnp]

I suppose the way around this last point as an end user is to define a simple plugin to replace the normal elixir formatting, and put it wherever you want in the plugins list. This basically has the same end result as the :elixir_format suggestion above, just a little less convenient to set up.

defmodule ElixirFormatter do
  @behaviour Mix.Tasks.Format

  def features(_opts) do
    [sigils: [], extensions: [".ex", ".exs"]]
  end

  def format(contents, opts) do
    formatted = Code.format_string!(contents, opts)
    IO.iodata_to_binary([formatted, ?\n])
  end
end

I'm wondering if Code.format_string!(contents, opts) is sufficient, or if a full conversion through ast/algebra per the styler example is necessary (presumably they also need the AST for other reasons):

expand code snippet

defmodule ElixirFormatter do
  @behaviour Mix.Tasks.Format

  def features(_opts) do
    [sigils: [], extensions: [".ex", ".exs"]]
  end

  def literal_encoder(literal, meta), do: {:ok, {:__block__, meta, [literal]}}

  def format(contents, opts) do
    file = opts[:file]
    {line_length, opts} = Keyword.pop(opts, :line_length, 98)

    {ast, comments} =
      contents
      |> Code.string_to_quoted_with_comments!(
        literal_encoder: &__MODULE__.literal_encoder/2,
        token_metadata: true,
        unescape: false,
        file: file
      )
    opts = [{:comments, comments}, {:escape, false} | opts]

    formatted =
      ast
      |> Code.quoted_to_algebra(opts)
      |> Inspect.Algebra.format(line_length)

    IO.iodata_to_binary([formatted, ?\n])
  end
end

[novaugust]

Code.format_string! is indeed what you want when you've already got a string. its implementation is essentially what the styler code above does (likely i ripped its internals to build the styler equivalent)