Run mix format on generated files by tasks phx.gen.*

[ShPakvel]

Issue

There is assert_passes_formatter_check(app_root_path) in integrated_test/ tests. Which was pseudo-truthy, because it passes only with specific short input for phx.gen.* tasks. Simply adding several more attributes for a phx.gen.* task run, we witnessing mentioned assertion to fail.

User-developer can pass any number of attributes or long names for modules. It is hard to achieve correctly formatted code in generated files. And even harder to supported it during changes.

This why I propose to apply mix format for phx.gen.* tasks.

Solution

I implemented it with this considerations:

Formatting is recommended and implemented in the very basis of elixir. But there are still projects which do not use it. And we need to respect that decision and provide mechanism to configure or even turn off formatting during files generation.

Based on this, following conditions are reasonable for me:

• By default format is turned on for files with extensions [".ex", ".exs", ".heex"].
• Run format once for all files related to generation and print notice in console about it, with instruction how to configure or turn it off.
• Format all files for just generated content: new and modified. If developers use formatting, then it is reasonable to do. If they do not use formatting, then they will turned it off in general.

Applied to live, html, json, context, schema, embedded. Can be applied to others later, if this will be approved.
@josevalim, @chrismccord, I believe this is pretty useful feature, that bring flexibility on future generators updates and decrease manual work for adjusting formatting complexity, simplifying code as well.

Notes about implementation.

UPD: Auto-formatted code is reverted.
Because code formatting on Phoenix project is not fully applied and protected by CI, some files were auto-formatted on save. I expect it is ok based on response to this issue.

For quick introduction, here is the feature in essence.

Gathering files with generated content done this way (example from phx.gen.html):

  defp format_files(%Context{} = context) do
    files_to_format(context) |> Mix.Phoenix.maybe_format()
    context
  end

  defp files_to_format(%Context{} = context) do
    (files_to_be_generated(context) |> Enum.map(fn {_, _, file} -> file end)) ++
      Gen.Context.files_to_format(context)
  end

It is invoked after copy new files and modify existing.

    |> copy_new_files(paths, binding)
+   |> format_files()
    |> print_shell_instructions()

And format executed with this code:

  @default_format_extensions [".ex", ".exs", ".heex"]
  @doc """
  Conditionally run `mix format` for generated files.
  By default, unless `format_extensions` is set in generators config,
  files with extensions `#{inspect(@default_format_extensions)}` are formatted and
  override format instruction is printed into console.
  `format_extensions` can be set to `[]` to turn off formatting.
  If no files pass condition, formatting is not performed.
  """
  def maybe_format(files) when is_list(files) do
    config = Application.get_env(otp_app(), :generators, []) |> Keyword.get(:format_extensions)
    format_extensions = config || @default_format_extensions
    files = Enum.filter(files, &String.ends_with?(&1, format_extensions))

    if files != [] do
      Mix.Task.run("format", files)
      if !config, do: Mix.shell().info(override_format_instruction())
    end
  end

  @doc """
  Override format instruction for console and docs.
  """
  def override_format_instruction do
    """

    Files with generated content (new and modified) are formatted with `mix format`.

    By default files with extensions `#{inspect(@default_format_extensions)}` are formatted.
    List of extensions can be changed via generators config:

        config :your_app, :generators,
          format_extensions: [".ex", ".exs"]

    Formatting can be turned off by setting empty list:

        config :your_app, :generators,
          format_extensions: []
    """
  end

For integration_test/ I added extra attributes in mix_run! for phx.gen.* tasks, to truthfully test formatting. There is issue for mysql case (some files from phx.newis not formatted). So for mysql I left TODO for future separate investigation, as it is not related to phx.gen.* files formatting.

For test/mix/tasks/ there is no deps [:phoenix, :ecto, :ecto_sql] we can format code according to. So for now I disabled formatting via new config with_generator_env([format_extensions: []], function) applied in in_tmp_project and in_tmp_umbrella_project. The with_generator_env function itself has minor update as well, to merge passing generators config instead of fully replacing it.
It is pretty simple and generally applied change I came up with as first idea. Let me know if you want to resolve it in some better way.

---

Small improvements that were done in the same gen files:

• Simplify conditions for context and schema files by relocating logic from live, html, json into context and schema itself, where it belongs.
• Invoke context files copy before parent generator files, to skip extra binding creation. It is not only skipping unused data passing in arguments, but also improve work with code (no need to double check if it is used in context or not).

[josevalim]

Hi @ShPakvel! Thank you for the PR. As I mentioned in the other issue, please don’t format the files. It makes our work in reviewing the PRs much harder. Generally speaking, a PR should only do one thing. :)

[ShPakvel]

@josevalim Auto-formatting is reverted. I disabled formatting locally for phoenix project. Auto-formatting won't happen again from my side.

[ShPakvel]

CI failed one job with random reason. Not related to changes. The same CI passed on referenced branch in my fork.

[ShPakvel]

Addressed review comments. ✅