From d585b3e9d28fd9ca59507e5f7b7c3c89da38ad9b Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Tue, 10 Dec 2024 15:30:53 +0200 Subject: [PATCH 1/5] Run `mix format` on generated files by tasks `phx.gen.*`. 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. I implemented it with this considerations: Formatting is [recommended](https://hexdocs.pm/mix/Mix.Tasks.Format.html#module-when-to-format-code) and [implemented in the very basis of elixir](https://hexdocs.pm/elixir/Code.html#format_string!/2). 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. --- 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.new`is 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 came up with simple disabling formatting via our new config `with_generator_env([format_extensions: []], function)` applied in `in_tmp_project` and `in_tmp_umbrella_project`. --- 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). --- installer/test/mix_helper.exs | 26 ++--- .../app_with_defaults_test.exs | 52 ++++++++- .../app_with_mssql_adapter_test.exs | 56 ++++++++-- .../app_with_mysql_adapter_test.exs | 67 ++++++++++-- .../app_with_sqlite3_adapter.exs | 48 +++++++- .../umbrella_app_with_defaults_test.exs | 51 ++++++++- lib/mix/phoenix.ex | 41 +++++++ lib/mix/tasks/phx.gen.context.ex | 39 +++++-- lib/mix/tasks/phx.gen.embedded.ex | 43 +++++--- lib/mix/tasks/phx.gen.html.ex | 35 ++++-- lib/mix/tasks/phx.gen.json.ex | 30 +++-- lib/mix/tasks/phx.gen.live.ex | 35 +++--- lib/mix/tasks/phx.gen.schema.ex | 103 ++++++++++++------ test/mix/tasks/phx.gen.html_test.exs | 65 +++++------ 14 files changed, 511 insertions(+), 180 deletions(-) diff --git a/installer/test/mix_helper.exs b/installer/test/mix_helper.exs index 1addd2ec1f..baa047b732 100644 --- a/installer/test/mix_helper.exs +++ b/installer/test/mix_helper.exs @@ -28,7 +28,6 @@ defmodule MixHelper do end def in_tmp_project(which, function) do - conf_before = Application.get_env(:phoenix, :generators) || [] base = Path.join([tmp_path(), random_string(10)]) path = Path.join([base, to_string(which)]) @@ -38,24 +37,14 @@ defmodule MixHelper do File.cd!(path, fn -> File.touch!("mix.exs") - - File.write!(".formatter.exs", """ - [ - import_deps: [:phoenix, :ecto, :ecto_sql], - inputs: ["*.exs"] - ] - """) - - function.() + with_generator_env([format_extensions: []], function) end) after File.rm_rf!(base) - Application.put_env(:phoenix, :generators, conf_before) end end def in_tmp_umbrella_project(which, function) do - conf_before = Application.get_env(:phoenix, :generators) || [] base = Path.join([tmp_path(), random_string(10)]) path = Path.join([base, to_string(which)]) @@ -72,9 +61,10 @@ defmodule MixHelper do File.write!(Path.join(config_path, file), "import Config\n") end - File.cd!(apps_path, function) + File.cd!(apps_path, fn -> + with_generator_env([format_extensions: []], function) + end) after - Application.put_env(:phoenix, :generators, conf_before) File.rm_rf!(base) end end @@ -131,15 +121,15 @@ defmodule MixHelper do end def with_generator_env(app_name \\ :phoenix, new_env, fun) do - config_before = Application.fetch_env(app_name, :generators) - Application.put_env(app_name, :generators, new_env) + config_before = Application.get_env(app_name, :generators) + Application.put_env(app_name, :generators, Keyword.merge(config_before || [], new_env)) try do fun.() after case config_before do - {:ok, config} -> Application.put_env(app_name, :generators, config) - :error -> Application.delete_env(app_name, :generators) + nil -> Application.delete_env(app_name, :generators) + config -> Application.put_env(app_name, :generators, config) end end end diff --git a/integration_test/test/code_generation/app_with_defaults_test.exs b/integration_test/test/code_generation/app_with_defaults_test.exs index 57d069f794..e7cc727858 100644 --- a/integration_test/test/code_generation/app_with_defaults_test.exs +++ b/integration_test/test/code_generation/app_with_defaults_test.exs @@ -27,7 +27,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -50,7 +60,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted order:integer:unique), app_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted order:integer:unique), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -74,7 +87,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -97,7 +120,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -121,7 +147,18 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--live"]) - mix_run!(~w(phx.gen.live Blog Post posts title:unique body:string p:boolean s:enum:a:b:c), app_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + public:boolean + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -147,7 +184,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--live"]) - mix_run!(~w(phx.gen.live Blog Post posts title body:string public:boolean status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts title body:string public:boolean status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ diff --git a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs index 96ca203de2..413120a36f 100644 --- a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs +++ b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs @@ -8,7 +8,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql"]) - mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -34,7 +37,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql"]) - mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -60,7 +66,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql", "--live"]) - mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -85,9 +94,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do describe "phx.gen.auth + pbkdf2 + existing context" do test "has no compilation or formatter warnings (--live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) - mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) + mix_run!( + ~w(phx.gen.html Accounts Group groups + name + details + other_very_important_attribute + one_more_attribute_with_long_name), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -100,7 +117,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), + app_root_path + ) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -109,7 +129,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do test "has no compilation or formatter warnings (--no-live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -124,7 +145,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), + app_root_path + ) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -134,7 +158,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do @tag database: :mssql test "has a passing test suite" do with_installer_tmp("app_with_defaults (--live)", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -149,7 +174,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), + app_root_path + ) drop_test_database(app_root_path) assert_tests_pass(app_root_path) @@ -159,7 +187,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do @tag database: :mssql test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -174,7 +203,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), + app_root_path + ) drop_test_database(app_root_path) assert_tests_pass(app_root_path) diff --git a/integration_test/test/code_generation/app_with_mysql_adapter_test.exs b/integration_test/test/code_generation/app_with_mysql_adapter_test.exs index 0886d421f9..d65c9da8e7 100644 --- a/integration_test/test/code_generation/app_with_mysql_adapter_test.exs +++ b/integration_test/test/code_generation/app_with_mysql_adapter_test.exs @@ -8,7 +8,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mysql_app", ["--database", "mysql"]) - mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mysql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -21,6 +31,9 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do """) end) + # TODO: Uncomment when fix unformatted code from `phx.new` generator. + # assert_no_compilation_warnings(app_root_path) + # assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -34,7 +47,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mysql_app", ["--database", "mysql"]) - mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mysql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -47,6 +70,9 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do """) end) + # TODO: Uncomment when fix unformatted code from `phx.new` generator. + # assert_no_compilation_warnings(app_root_path) + # assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -60,7 +86,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mysql_app", ["--database", "mysql", "--live"]) - mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_mysql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -76,6 +112,9 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do """) end) + # TODO: Uncomment when fix unformatted code from `phx.new` generator. + # assert_no_compilation_warnings(app_root_path) + # assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -85,7 +124,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do describe "phx.gen.auth + argon2" do test "has no compilation or formatter warnings (--live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --live), app_root_path) @@ -96,9 +136,13 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do test "has no compilation or formatter warnings (--no-live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), + app_root_path + ) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -108,7 +152,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do @tag database: :mysql test "has a passing test suite (--live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --live), app_root_path) @@ -120,9 +165,13 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do @tag database: :mysql test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) - mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), app_root_path) + mix_run!( + ~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), + app_root_path + ) drop_test_database(app_root_path) assert_tests_pass(app_root_path) diff --git a/integration_test/test/code_generation/app_with_sqlite3_adapter.exs b/integration_test/test/code_generation/app_with_sqlite3_adapter.exs index 4576a6cfc0..5306ac1594 100644 --- a/integration_test/test/code_generation/app_with_sqlite3_adapter.exs +++ b/integration_test/test/code_generation/app_with_sqlite3_adapter.exs @@ -8,7 +8,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_sqlite3_app", ["--database", "sqlite3"]) - mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_sqlite3_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -21,6 +31,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do """) end) + assert_no_compilation_warnings(app_root_path) + assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -34,7 +46,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_sqlite3_app", ["--database", "sqlite3"]) - mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_sqlite3_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -47,6 +69,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do """) end) + assert_no_compilation_warnings(app_root_path) + assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -60,7 +84,17 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_sqlite3_app", ["--database", "sqlite3", "--live"]) - mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts + title + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + app_root_path + ) modify_file(Path.join(app_root_path, "lib/default_sqlite3_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -76,6 +110,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do """) end) + assert_no_compilation_warnings(app_root_path) + assert_passes_formatter_check(app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) end) @@ -108,7 +144,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do @tag database: :sqlite3 test "has a passing test suite (--live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) mix_run!(~w(phx.gen.auth Accounts User users --live), app_root_path) @@ -119,7 +156,8 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) + {app_root_path, _} = + generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) mix_run!(~w(phx.gen.auth Accounts User users --no-live), app_root_path) diff --git a/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs b/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs index 9dc71ba64d..ddd3b44c27 100644 --- a/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs +++ b/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs @@ -29,7 +29,17 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -53,7 +63,10 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -78,7 +91,17 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -102,7 +125,10 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -127,7 +153,17 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella", "--live"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.live Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts + title:unique + body:string + preface + author_name + author_email + other_very_important_attribute + status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -154,7 +190,10 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella", "--live"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) + mix_run!( + ~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), + web_root_path + ) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ diff --git a/lib/mix/phoenix.ex b/lib/mix/phoenix.ex index 78181df0b1..6c7fad659d 100644 --- a/lib/mix/phoenix.ex +++ b/lib/mix/phoenix.ex @@ -65,6 +65,47 @@ defmodule Mix.Phoenix do defp to_app_source(app, source_dir) when is_atom(app), do: Application.app_dir(app, source_dir) + @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 + @doc """ Inflects path, scope, alias and more from the given name. diff --git a/lib/mix/tasks/phx.gen.context.ex b/lib/mix/tasks/phx.gen.context.ex index 1dfad60e39..6516ab8ff5 100644 --- a/lib/mix/tasks/phx.gen.context.ex +++ b/lib/mix/tasks/phx.gen.context.ex @@ -72,6 +72,10 @@ defmodule Mix.Tasks.Phx.Gen.Context do You can skip this prompt and automatically merge the new schema access functions and tests into the existing context using `--merge-with-existing-context`. To prevent changes to the existing context and exit the generator, use `--no-merge-with-existing-context`. + + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ use Mix.Task @@ -112,6 +116,7 @@ defmodule Mix.Tasks.Phx.Gen.Context do context |> copy_new_files(paths, binding) + |> format_files() |> print_shell_instructions() end @@ -149,17 +154,18 @@ defmodule Mix.Tasks.Phx.Gen.Context do end @doc false + def files_to_be_generated(%Context{generate?: false}), do: [] + def files_to_be_generated(%Context{schema: schema}) do - if schema.generate? do - Gen.Schema.files_to_be_generated(schema) - else - [] - end + Gen.Schema.files_to_be_generated(schema) end @doc false + def copy_new_files(%Context{generate?: false} = context, _, _), do: context + def copy_new_files(%Context{schema: schema} = context, paths, binding) do - if schema.generate?, do: Gen.Schema.copy_new_files(schema, paths, binding) + Gen.Schema.copy_new_files(schema, paths, binding) + inject_schema_access(context, paths, binding) inject_tests(context, paths, binding) inject_test_fixture(context, paths, binding) @@ -293,13 +299,24 @@ defmodule Mix.Tasks.Phx.Gen.Context do end end + defp format_files(%Context{} = context) do + files_to_format(context) |> Mix.Phoenix.maybe_format() + context + end + + @doc false + def files_to_format(%Context{generate?: false}), do: [] + + def files_to_format(%Context{schema: schema} = context) do + [context.file, context.test_file, context.test_fixtures_file] ++ + Gen.Schema.files_to_format(schema) + end + @doc false + def print_shell_instructions(%Context{generate?: false}), do: :ok + def print_shell_instructions(%Context{schema: schema}) do - if schema.generate? do - Gen.Schema.print_shell_instructions(schema) - else - :ok - end + Gen.Schema.print_shell_instructions(schema) end defp schema_access_template(%Context{schema: schema}) do diff --git a/lib/mix/tasks/phx.gen.embedded.ex b/lib/mix/tasks/phx.gen.embedded.ex index 4e82eec674..b38b6dcd21 100644 --- a/lib/mix/tasks/phx.gen.embedded.ex +++ b/lib/mix/tasks/phx.gen.embedded.ex @@ -26,9 +26,14 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect(attr)}`\n"} * `:datetime` - An alias for `:naive_datetime` + + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ + use Mix.Task alias Mix.Phoenix.Schema @@ -38,7 +43,9 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do @doc false def run(args) do if Mix.Project.umbrella?() do - Mix.raise "mix phx.gen.embedded must be invoked from within your *_web application root directory" + Mix.raise( + "mix phx.gen.embedded must be invoked from within your *_web application root directory" + ) end schema = build(args) @@ -47,13 +54,16 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do prompt_for_conflicts(schema) - copy_new_files(schema, paths, schema: schema) + schema + |> copy_new_files(paths, schema: schema) + |> format_files() end @doc false def build(args) do {schema_opts, parsed, _} = OptionParser.parse(args, switches: @switches) [schema_name | attrs] = validate_args!(parsed) + opts = schema_opts |> Keyword.put(:embedded, true) @@ -69,43 +79,50 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do if Schema.valid?(schema) do args else - raise_with_help "Expected the schema argument, #{inspect schema}, to be a valid module name" + raise_with_help( + "Expected the schema argument, #{inspect(schema)}, to be a valid module name" + ) end end + def validate_args!(_) do - raise_with_help "Invalid arguments" + raise_with_help("Invalid arguments") end @doc false - @spec raise_with_help(String.t) :: no_return() + @spec raise_with_help(String.t()) :: no_return() def raise_with_help(msg) do - Mix.raise """ + Mix.raise(""" #{msg} mix phx.gen.embedded expects a module name followed by any number of attributes: mix phx.gen.embedded Blog.Post title:string - """ + """) end - defp prompt_for_conflicts(schema) do schema |> files_to_be_generated() |> Mix.Phoenix.prompt_for_conflicts() end - @doc false - def files_to_be_generated(%Schema{} = schema) do + defp files_to_be_generated(%Schema{} = schema) do [{:eex, "embedded_schema.ex", schema.file}] end - @doc false - def copy_new_files(%Schema{} = schema, paths, binding) do + defp copy_new_files(%Schema{} = schema, paths, binding) do files = files_to_be_generated(schema) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.embedded", binding, files) schema end + + defp format_files(%Schema{} = schema) do + files_to_format(schema) |> Mix.Phoenix.maybe_format() + schema + end + + defp files_to_format(%Schema{} = schema), do: [schema.file] end diff --git a/lib/mix/tasks/phx.gen.html.ex b/lib/mix/tasks/phx.gen.html.ex index e9222dacc5..57410d1e24 100644 --- a/lib/mix/tasks/phx.gen.html.ex +++ b/lib/mix/tasks/phx.gen.html.ex @@ -88,7 +88,12 @@ defmodule Mix.Tasks.Phx.Gen.Html do You can also change the table name or configure the migrations to use binary ids for primary keys, see `mix phx.gen.schema` for more information. + + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ + use Mix.Task alias Mix.Phoenix.{Context, Schema} @@ -107,31 +112,24 @@ defmodule Mix.Tasks.Phx.Gen.Html do {context, schema} = Gen.Context.build(args) Gen.Context.prompt_for_code_injection(context) - binding = [context: context, schema: schema, inputs: inputs(schema)] + binding = [context: context, schema: schema] paths = Mix.Phoenix.generator_paths() prompt_for_conflicts(context) context |> copy_new_files(paths, binding) + |> format_files() |> print_shell_instructions() end defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - @doc false def files_to_be_generated(%Context{schema: schema, context_app: context_app}) do singular = schema.singular @@ -157,12 +155,25 @@ defmodule Mix.Tasks.Phx.Gen.Html do @doc false def copy_new_files(%Context{} = context, paths, binding) do + Gen.Context.copy_new_files(context, paths, binding) + + binding = Keyword.merge(binding, inputs: inputs(context.schema)) files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.html", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) + context end + 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 + @doc false def print_shell_instructions(%Context{schema: schema, context_app: ctx_app} = context) do if schema.web_namespace do @@ -185,7 +196,7 @@ defmodule Mix.Tasks.Phx.Gen.Html do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) end @doc false diff --git a/lib/mix/tasks/phx.gen.json.ex b/lib/mix/tasks/phx.gen.json.ex index 97177b4935..a31fa32662 100644 --- a/lib/mix/tasks/phx.gen.json.ex +++ b/lib/mix/tasks/phx.gen.json.ex @@ -85,6 +85,10 @@ defmodule Mix.Tasks.Phx.Gen.Json do You can also change the table name or configure the migrations to use binary ids for primary keys, see `mix phx.gen.schema` for more information. + + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ use Mix.Task @@ -116,24 +120,17 @@ defmodule Mix.Tasks.Phx.Gen.Json do context |> copy_new_files(paths, binding) + |> format_files() |> print_shell_instructions() end defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - @doc false def files_to_be_generated(%Context{schema: schema, context_app: context_app}) do singular = schema.singular @@ -154,13 +151,24 @@ defmodule Mix.Tasks.Phx.Gen.Json do @doc false def copy_new_files(%Context{} = context, paths, binding) do + Gen.Context.copy_new_files(context, paths, binding) + files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.json", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) context end + 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 + @doc false def print_shell_instructions(%Context{schema: schema, context_app: ctx_app} = context) do if schema.web_namespace do @@ -183,6 +191,6 @@ defmodule Mix.Tasks.Phx.Gen.Json do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) end end diff --git a/lib/mix/tasks/phx.gen.live.ex b/lib/mix/tasks/phx.gen.live.ex index 3aa01cdd02..27ef798fd9 100644 --- a/lib/mix/tasks/phx.gen.live.ex +++ b/lib/mix/tasks/phx.gen.live.ex @@ -99,7 +99,12 @@ defmodule Mix.Tasks.Phx.Gen.Live do You can also change the table name or configure the migrations to use binary ids for primary keys, see `mix help phx.gen.schema` for more information. + + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ + use Mix.Task alias Mix.Phoenix.{Context, Schema} @@ -118,7 +123,7 @@ defmodule Mix.Tasks.Phx.Gen.Live do {context, schema} = Gen.Context.build(args) Gen.Context.prompt_for_code_injection(context) - binding = [context: context, schema: schema, inputs: inputs(schema)] + binding = [context: context, schema: schema] paths = Mix.Phoenix.generator_paths() prompt_for_conflicts(context) @@ -126,24 +131,17 @@ defmodule Mix.Tasks.Phx.Gen.Live do context |> copy_new_files(binding, paths) |> maybe_inject_imports() + |> format_files() |> print_shell_instructions() end defp prompt_for_conflicts(context) do context |> files_to_be_generated() - |> Kernel.++(context_files(context)) + |> Kernel.++(Gen.Context.files_to_be_generated(context)) |> Mix.Phoenix.prompt_for_conflicts() end - defp context_files(%Context{generate?: true} = context) do - Gen.Context.files_to_be_generated(context) - end - - defp context_files(%Context{generate?: false}) do - [] - end - defp files_to_be_generated(%Context{schema: schema, context_app: context_app}) do web_prefix = Mix.Phoenix.web_path(context_app) test_prefix = Mix.Phoenix.web_test_path(context_app) @@ -163,18 +161,19 @@ defmodule Mix.Tasks.Phx.Gen.Live do end defp copy_new_files(%Context{} = context, binding, paths) do - files = files_to_be_generated(context) + Gen.Context.copy_new_files(context, paths, binding) binding = Keyword.merge(binding, + inputs: inputs(context.schema), assigns: %{ web_namespace: inspect(context.web_module), gettext: true } ) + files = files_to_be_generated(context) Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.live", binding, files) - if context.generate?, do: Gen.Context.copy_new_files(context, paths, binding) context end @@ -224,6 +223,16 @@ defmodule Mix.Tasks.Phx.Gen.Live do end end + 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 + @doc false def print_shell_instructions(%Context{schema: schema, context_app: ctx_app} = context) do prefix = Module.concat(context.web_module, schema.web_namespace) @@ -250,7 +259,7 @@ defmodule Mix.Tasks.Phx.Gen.Live do """) end - if context.generate?, do: Gen.Context.print_shell_instructions(context) + Gen.Context.print_shell_instructions(context) maybe_print_upgrade_info() end diff --git a/lib/mix/tasks/phx.gen.schema.ex b/lib/mix/tasks/phx.gen.schema.ex index 68f3c82ddf..cc830b3202 100644 --- a/lib/mix/tasks/phx.gen.schema.ex +++ b/lib/mix/tasks/phx.gen.schema.ex @@ -41,7 +41,7 @@ defmodule Mix.Tasks.Phx.Gen.Schema do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect(attr)}`\n"} * `:datetime` - An alias for `:naive_datetime` The generator also supports references, which we will properly @@ -107,7 +107,6 @@ defmodule Mix.Tasks.Phx.Gen.Schema do $ mix phx.gen.schema Blog.Post posts --migration-dir /path/to/directory - ## prefix By default migrations and schemas are generated without a prefix. @@ -147,19 +146,33 @@ defmodule Mix.Tasks.Phx.Gen.Schema do instead of a `NaiveDateTime`. This can also be set to `:utc_datetime_usec` for microsecond precision. + ## Format + #{Mix.Phoenix.override_format_instruction()} + """ + use Mix.Task alias Mix.Phoenix.Schema - @switches [migration: :boolean, binary_id: :boolean, table: :string, web: :string, - context_app: :string, prefix: :string, repo: :string, migration_dir: :string, - primary_key: :string] + @switches [ + migration: :boolean, + binary_id: :boolean, + table: :string, + web: :string, + context_app: :string, + prefix: :string, + repo: :string, + migration_dir: :string, + primary_key: :string + ] @doc false def run(args) do if Mix.Project.umbrella?() do - Mix.raise "mix phx.gen.schema must be invoked from within your *_web application root directory" + Mix.raise( + "mix phx.gen.schema must be invoked from within your *_web application root directory" + ) end schema = build(args, []) @@ -169,6 +182,7 @@ defmodule Mix.Tasks.Phx.Gen.Schema do schema |> copy_new_files(paths, schema: schema) + |> format_files() |> print_shell_instructions() end @@ -201,53 +215,68 @@ defmodule Mix.Tasks.Phx.Gen.Schema do end defp put_context_app(opts, nil), do: opts + defp put_context_app(opts, string) do Keyword.put(opts, :context_app, String.to_atom(string)) end @doc false + def files_to_be_generated(%Schema{generate?: false}), do: [] + def files_to_be_generated(%Schema{} = schema) do [{:eex, "schema.ex", schema.file}] end @doc false - def copy_new_files(%Schema{context_app: ctx_app, repo: repo, opts: opts} = schema, paths, binding) do - files = files_to_be_generated(schema) + def copy_new_files(%Schema{generate?: false} = schema, _, _), do: schema + + def copy_new_files(%Schema{} = schema, paths, binding) do + migration_file = + if schema.migration?, do: [{:eex, "migration.exs", migration_path(schema)}], else: [] + + files = files_to_be_generated(schema) ++ migration_file Mix.Phoenix.copy_from(paths, "priv/templates/phx.gen.schema", binding, files) - if schema.migration? do - migration_dir = - cond do - migration_dir = opts[:migration_dir] -> - migration_dir + schema + end - opts[:repo] -> - repo_name = repo |> Module.split() |> List.last() |> Macro.underscore() - Mix.Phoenix.context_app_path(ctx_app, "priv/#{repo_name}/migrations/") + defp migration_path(%Schema{context_app: ctx_app, repo: repo, opts: opts, table: table}) do + migration_dir = + cond do + migration_dir = opts[:migration_dir] -> + migration_dir - true -> - Mix.Phoenix.context_app_path(ctx_app, "priv/repo/migrations/") - end + opts[:repo] -> + repo_name = repo |> Module.split() |> List.last() |> Macro.underscore() + Mix.Phoenix.context_app_path(ctx_app, "priv/#{repo_name}/migrations/") - migration_path = Path.join(migration_dir, "#{timestamp()}_create_#{schema.table}.exs") + true -> + Mix.Phoenix.context_app_path(ctx_app, "priv/repo/migrations/") + end - Mix.Phoenix.copy_from paths, "priv/templates/phx.gen.schema", binding, [ - {:eex, "migration.exs", migration_path}, - ] - end + Path.join(migration_dir, "#{timestamp()}_create_#{table}.exs") + end + defp format_files(%Schema{} = schema) do + files_to_format(schema) |> Mix.Phoenix.maybe_format() schema end @doc false + def files_to_format(%Schema{generate?: false}), do: [] + def files_to_format(%Schema{} = schema), do: [schema.file, migration_path(schema)] + + @doc false + def print_shell_instructions(%Schema{generate?: false}), do: :ok + def print_shell_instructions(%Schema{} = schema) do if schema.migration? do - Mix.shell().info """ + Mix.shell().info(""" Remember to update your repository by running migrations: $ mix ecto.migrate - """ + """) end end @@ -255,21 +284,28 @@ defmodule Mix.Tasks.Phx.Gen.Schema do def validate_args!([schema, plural | _] = args, help) do cond do not Schema.valid?(schema) -> - help.raise_with_help "Expected the schema argument, #{inspect schema}, to be a valid module name" + help.raise_with_help( + "Expected the schema argument, #{inspect(schema)}, to be a valid module name" + ) + String.contains?(plural, ":") or plural != Phoenix.Naming.underscore(plural) -> - help.raise_with_help "Expected the plural argument, #{inspect plural}, to be all lowercase using snake_case convention" + help.raise_with_help( + "Expected the plural argument, #{inspect(plural)}, to be all lowercase using snake_case convention" + ) + true -> args end end + def validate_args!(_, help) do - help.raise_with_help "Invalid arguments" + help.raise_with_help("Invalid arguments") end @doc false - @spec raise_with_help(String.t) :: no_return() + @spec raise_with_help(String.t()) :: no_return() def raise_with_help(msg) do - Mix.raise """ + Mix.raise(""" #{msg} mix phx.gen.schema expects both a module name and @@ -277,13 +313,14 @@ defmodule Mix.Tasks.Phx.Gen.Schema do any number of attributes: mix phx.gen.schema Blog.Post blog_posts title:string - """ + """) end defp timestamp do {{y, m, d}, {hh, mm, ss}} = :calendar.universal_time() "#{y}#{pad(m)}#{pad(d)}#{pad(hh)}#{pad(mm)}#{pad(ss)}" end - defp pad(i) when i < 10, do: << ?0, ?0 + i >> + + defp pad(i) when i < 10, do: <> defp pad(i), do: to_string(i) end diff --git a/test/mix/tasks/phx.gen.html_test.exs b/test/mix/tasks/phx.gen.html_test.exs index 450e0599b7..0b4c4e1ead 100644 --- a/test/mix/tasks/phx.gen.html_test.exs +++ b/test/mix/tasks/phx.gen.html_test.exs @@ -331,6 +331,7 @@ defmodule Mix.Tasks.Phx.Gen.HtmlTest do test "with a matching plural and singular term", config do in_tmp_project(config.test, fn -> Gen.Html.run(~w(Tracker Series series value:integer)) + assert_file("lib/phoenix_web/controllers/series_controller.ex", fn file -> assert file =~ "render(conn, :index, series_collection: series)" end) @@ -399,58 +400,60 @@ defmodule Mix.Tasks.Phx.Gen.HtmlTest do describe "inside umbrella" do test "without context_app generators config uses web dir", config do in_tmp_umbrella_project(config.test, fn -> - Application.put_env(:phoenix, :generators, context_app: nil) - Gen.Html.run(~w(Accounts User users name:string)) + with_generator_env([context_app: nil], fn -> + Gen.Html.run(~w(Accounts User users name:string)) - assert_file("lib/phoenix/accounts.ex") - assert_file("lib/phoenix/accounts/user.ex") + assert_file("lib/phoenix/accounts.ex") + assert_file("lib/phoenix/accounts/user.ex") - assert_file("lib/phoenix_web/controllers/user_controller.ex", fn file -> - assert file =~ "defmodule PhoenixWeb.UserController" - assert file =~ "use PhoenixWeb, :controller" - end) + assert_file("lib/phoenix_web/controllers/user_controller.ex", fn file -> + assert file =~ "defmodule PhoenixWeb.UserController" + assert file =~ "use PhoenixWeb, :controller" + end) - assert_file("lib/phoenix_web/controllers/user_html.ex", fn file -> - assert file =~ "defmodule PhoenixWeb.UserHTML" - end) + assert_file("lib/phoenix_web/controllers/user_html.ex", fn file -> + assert file =~ "defmodule PhoenixWeb.UserHTML" + end) - assert_file("test/phoenix_web/controllers/user_controller_test.exs", fn file -> - assert file =~ "defmodule PhoenixWeb.UserControllerTest" + assert_file("test/phoenix_web/controllers/user_controller_test.exs", fn file -> + assert file =~ "defmodule PhoenixWeb.UserControllerTest" + end) end) end) end test "raises with false context_app", config do in_tmp_umbrella_project(config.test, fn -> - Application.put_env(:phoenix, :generators, context_app: false) - - assert_raise Mix.Error, ~r/no context_app configured/, fn -> - Gen.Html.run(~w(Accounts User users name:string)) - end + with_generator_env([context_app: false], fn -> + assert_raise Mix.Error, ~r/no context_app configured/, fn -> + Gen.Html.run(~w(Accounts User users name:string)) + end + end) end) end test "with context_app generators config does not use web dir", config do in_tmp_umbrella_project(config.test, fn -> File.mkdir!("another_app") - Application.put_env(:phoenix, :generators, context_app: {:another_app, "another_app"}) - Gen.Html.run(~w(Accounts User users name:string)) + with_generator_env([context_app: {:another_app, "another_app"}], fn -> + Gen.Html.run(~w(Accounts User users name:string)) - assert_file("another_app/lib/another_app/accounts.ex") - assert_file("another_app/lib/another_app/accounts/user.ex") + assert_file("another_app/lib/another_app/accounts.ex") + assert_file("another_app/lib/another_app/accounts/user.ex") - assert_file("lib/phoenix/controllers/user_controller.ex", fn file -> - assert file =~ "defmodule Phoenix.UserController" - assert file =~ "use Phoenix, :controller" - end) + assert_file("lib/phoenix/controllers/user_controller.ex", fn file -> + assert file =~ "defmodule Phoenix.UserController" + assert file =~ "use Phoenix, :controller" + end) - assert_file("lib/phoenix/controllers/user_html.ex", fn file -> - assert file =~ "defmodule Phoenix.UserHTML" - end) + assert_file("lib/phoenix/controllers/user_html.ex", fn file -> + assert file =~ "defmodule Phoenix.UserHTML" + end) - assert_file("test/phoenix/controllers/user_controller_test.exs", fn file -> - assert file =~ "defmodule Phoenix.UserControllerTest" + assert_file("test/phoenix/controllers/user_controller_test.exs", fn file -> + assert file =~ "defmodule Phoenix.UserControllerTest" + end) end) end) end From 7fd09e2ecc843301ec40f8748364ed7982accd99 Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Thu, 12 Dec 2024 03:10:08 +0200 Subject: [PATCH 2/5] Add .vscode settings folder into gitignore. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e46730c560..7fd3308754 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ erl_crash.dump phoenix-*.ez .DS_Store + +/.vscode/ From b8730ccf87ad45a7f931bacff08e54a6d10135e5 Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Thu, 12 Dec 2024 03:10:24 +0200 Subject: [PATCH 3/5] Revert auto-formatted code. --- .../app_with_defaults_test.exs | 15 ++---- .../app_with_mssql_adapter_test.exs | 39 ++++------------ .../app_with_mysql_adapter_test.exs | 22 +++------ .../app_with_sqlite3_adapter.exs | 6 +-- .../umbrella_app_with_defaults_test.exs | 15 ++---- lib/mix/tasks/phx.gen.embedded.ex | 19 ++++---- lib/mix/tasks/phx.gen.schema.ex | 46 ++++++------------- 7 files changed, 45 insertions(+), 117 deletions(-) diff --git a/integration_test/test/code_generation/app_with_defaults_test.exs b/integration_test/test/code_generation/app_with_defaults_test.exs index e7cc727858..cf8dd08605 100644 --- a/integration_test/test/code_generation/app_with_defaults_test.exs +++ b/integration_test/test/code_generation/app_with_defaults_test.exs @@ -60,10 +60,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!( - ~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted order:integer:unique), - app_root_path - ) + mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted order:integer:unique), app_root_path) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -120,10 +117,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog") - mix_run!( - ~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), - app_root_path - ) + mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -184,10 +178,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do with_installer_tmp("app_with_defaults", fn tmp_dir -> {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--live"]) - mix_run!( - ~w(phx.gen.live Blog Post posts title body:string public:boolean status:enum:unpublished:published:deleted), - app_root_path - ) + mix_run!(~w(phx.gen.live Blog Post posts title body:string public:boolean status:enum:unpublished:published:deleted), app_root_path) modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file -> inject_before_final_end(file, """ diff --git a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs index 413120a36f..824934af83 100644 --- a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs +++ b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs @@ -8,10 +8,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql"]) - mix_run!( - ~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), - app_root_path - ) + mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -37,10 +34,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql"]) - mix_run!( - ~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), - app_root_path - ) + mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -66,10 +60,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_mssql_app", ["--database", "mssql", "--live"]) - mix_run!( - ~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), - app_root_path - ) + mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), app_root_path) modify_file(Path.join(app_root_path, "lib/default_mssql_app_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -94,8 +85,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do describe "phx.gen.auth + pbkdf2 + existing context" do test "has no compilation or formatter warnings (--live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!( ~w(phx.gen.html Accounts Group groups @@ -117,10 +107,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), app_root_path) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -129,8 +116,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do test "has no compilation or formatter warnings (--no-live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -145,10 +131,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), app_root_path) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -158,8 +141,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do @tag database: :mssql test "has a passing test suite" do with_installer_tmp("app_with_defaults (--live)", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -174,10 +156,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --live), app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) diff --git a/integration_test/test/code_generation/app_with_mysql_adapter_test.exs b/integration_test/test/code_generation/app_with_mysql_adapter_test.exs index d65c9da8e7..936efe9262 100644 --- a/integration_test/test/code_generation/app_with_mysql_adapter_test.exs +++ b/integration_test/test/code_generation/app_with_mysql_adapter_test.exs @@ -124,8 +124,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do describe "phx.gen.auth + argon2" do test "has no compilation or formatter warnings (--live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --live), app_root_path) @@ -136,13 +135,9 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do test "has no compilation or formatter warnings (--no-live)" do with_installer_tmp("new with defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mysql", "--binary-id"]) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), app_root_path) assert_no_compilation_warnings(app_root_path) assert_passes_formatter_check(app_root_path) @@ -152,8 +147,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do @tag database: :mysql test "has a passing test suite (--live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --live), app_root_path) @@ -165,13 +159,9 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMySqlAdapterTest do @tag database: :mysql test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "mysql", "--binary-id"]) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib argon2 --no-live), app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) diff --git a/integration_test/test/code_generation/app_with_sqlite3_adapter.exs b/integration_test/test/code_generation/app_with_sqlite3_adapter.exs index 5306ac1594..b0871603b1 100644 --- a/integration_test/test/code_generation/app_with_sqlite3_adapter.exs +++ b/integration_test/test/code_generation/app_with_sqlite3_adapter.exs @@ -144,8 +144,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do @tag database: :sqlite3 test "has a passing test suite (--live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) mix_run!(~w(phx.gen.auth Accounts User users --live), app_root_path) @@ -156,8 +155,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithSQLite3AdapterTest do test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "default_app", ["--database", "sqlite3"]) mix_run!(~w(phx.gen.auth Accounts User users --no-live), app_root_path) diff --git a/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs b/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs index ddd3b44c27..0fdd0b09a1 100644 --- a/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs +++ b/integration_test/test/code_generation/umbrella_app_with_defaults_test.exs @@ -63,10 +63,7 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!( - ~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), - web_root_path - ) + mix_run!(~w(phx.gen.html Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -125,10 +122,7 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!( - ~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), - web_root_path - ) + mix_run!(~w(phx.gen.json Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ @@ -190,10 +184,7 @@ defmodule Phoenix.Integration.CodeGeneration.UmbrellaAppWithDefaultsTest do {app_root_path, _} = generate_phoenix_app(tmp_dir, "rainy_day", ["--umbrella", "--live"]) web_root_path = Path.join(app_root_path, "apps/rainy_day_web") - mix_run!( - ~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), - web_root_path - ) + mix_run!(~w(phx.gen.live Blog Post posts title body:string status:enum:unpublished:published:deleted), web_root_path) modify_file(Path.join(web_root_path, "lib/rainy_day_web/router.ex"), fn file -> inject_before_final_end(file, """ diff --git a/lib/mix/tasks/phx.gen.embedded.ex b/lib/mix/tasks/phx.gen.embedded.ex index b38b6dcd21..ead9db4aab 100644 --- a/lib/mix/tasks/phx.gen.embedded.ex +++ b/lib/mix/tasks/phx.gen.embedded.ex @@ -26,7 +26,7 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect(attr)}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} * `:datetime` - An alias for `:naive_datetime` ## Format @@ -43,9 +43,7 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do @doc false def run(args) do if Mix.Project.umbrella?() do - Mix.raise( - "mix phx.gen.embedded must be invoked from within your *_web application root directory" - ) + Mix.raise "mix phx.gen.embedded must be invoked from within your *_web application root directory" end schema = build(args) @@ -79,29 +77,28 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do if Schema.valid?(schema) do args else - raise_with_help( - "Expected the schema argument, #{inspect(schema)}, to be a valid module name" - ) + raise_with_help "Expected the schema argument, #{inspect schema}, to be a valid module name" end end def validate_args!(_) do - raise_with_help("Invalid arguments") + raise_with_help "Invalid arguments" end @doc false - @spec raise_with_help(String.t()) :: no_return() + @spec raise_with_help(String.t) :: no_return() def raise_with_help(msg) do - Mix.raise(""" + Mix.raise """ #{msg} mix phx.gen.embedded expects a module name followed by any number of attributes: mix phx.gen.embedded Blog.Post title:string - """) + """ end + defp prompt_for_conflicts(schema) do schema |> files_to_be_generated() diff --git a/lib/mix/tasks/phx.gen.schema.ex b/lib/mix/tasks/phx.gen.schema.ex index cc830b3202..869fe80f8b 100644 --- a/lib/mix/tasks/phx.gen.schema.ex +++ b/lib/mix/tasks/phx.gen.schema.ex @@ -41,7 +41,7 @@ defmodule Mix.Tasks.Phx.Gen.Schema do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect(attr)}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} * `:datetime` - An alias for `:naive_datetime` The generator also supports references, which we will properly @@ -155,24 +155,14 @@ defmodule Mix.Tasks.Phx.Gen.Schema do alias Mix.Phoenix.Schema - @switches [ - migration: :boolean, - binary_id: :boolean, - table: :string, - web: :string, - context_app: :string, - prefix: :string, - repo: :string, - migration_dir: :string, - primary_key: :string - ] + @switches [migration: :boolean, binary_id: :boolean, table: :string, web: :string, + context_app: :string, prefix: :string, repo: :string, migration_dir: :string, + primary_key: :string] @doc false def run(args) do if Mix.Project.umbrella?() do - Mix.raise( - "mix phx.gen.schema must be invoked from within your *_web application root directory" - ) + Mix.raise "mix phx.gen.schema must be invoked from within your *_web application root directory" end schema = build(args, []) @@ -215,7 +205,6 @@ defmodule Mix.Tasks.Phx.Gen.Schema do end defp put_context_app(opts, nil), do: opts - defp put_context_app(opts, string) do Keyword.put(opts, :context_app, String.to_atom(string)) end @@ -271,12 +260,12 @@ defmodule Mix.Tasks.Phx.Gen.Schema do def print_shell_instructions(%Schema{} = schema) do if schema.migration? do - Mix.shell().info(""" + Mix.shell().info """ Remember to update your repository by running migrations: $ mix ecto.migrate - """) + """ end end @@ -284,28 +273,22 @@ defmodule Mix.Tasks.Phx.Gen.Schema do def validate_args!([schema, plural | _] = args, help) do cond do not Schema.valid?(schema) -> - help.raise_with_help( - "Expected the schema argument, #{inspect(schema)}, to be a valid module name" - ) - + help.raise_with_help "Expected the schema argument, #{inspect schema}, to be a valid module name" String.contains?(plural, ":") or plural != Phoenix.Naming.underscore(plural) -> - help.raise_with_help( - "Expected the plural argument, #{inspect(plural)}, to be all lowercase using snake_case convention" - ) - + help.raise_with_help "Expected the plural argument, #{inspect plural}, to be all lowercase using snake_case convention" true -> args end end def validate_args!(_, help) do - help.raise_with_help("Invalid arguments") + help.raise_with_help "Invalid arguments" end @doc false - @spec raise_with_help(String.t()) :: no_return() + @spec raise_with_help(String.t) :: no_return() def raise_with_help(msg) do - Mix.raise(""" + Mix.raise """ #{msg} mix phx.gen.schema expects both a module name and @@ -313,14 +296,13 @@ defmodule Mix.Tasks.Phx.Gen.Schema do any number of attributes: mix phx.gen.schema Blog.Post blog_posts title:string - """) + """ end defp timestamp do {{y, m, d}, {hh, mm, ss}} = :calendar.universal_time() "#{y}#{pad(m)}#{pad(d)}#{pad(hh)}#{pad(mm)}#{pad(ss)}" end - - defp pad(i) when i < 10, do: <> + defp pad(i) when i < 10, do: << ?0, ?0 + i >> defp pad(i), do: to_string(i) end From a6819bae4b3d9cd37c349e16fdbbcb018e294c9e Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Thu, 12 Dec 2024 15:47:59 +0200 Subject: [PATCH 4/5] Address review comments. - Revert specific gitignore change. - Delete console notification about formatting files. --- .gitignore | 2 -- lib/mix/phoenix.ex | 18 +++++++----------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 7fd3308754..e46730c560 100644 --- a/.gitignore +++ b/.gitignore @@ -21,5 +21,3 @@ erl_crash.dump phoenix-*.ez .DS_Store - -/.vscode/ diff --git a/lib/mix/phoenix.ex b/lib/mix/phoenix.ex index 6c7fad659d..b4504bbc6d 100644 --- a/lib/mix/phoenix.ex +++ b/lib/mix/phoenix.ex @@ -68,21 +68,17 @@ defmodule Mix.Phoenix do @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. + By default files with extensions `#{inspect(@default_format_extensions)}` are formatted. + It can be adjusted with generators config `format_extensions`, and turned off with value `[]`. 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)) + format_extensions = + Application.get_env(otp_app(), :generators, []) + |> Keyword.get(:format_extensions, @default_format_extensions) - if files != [] do - Mix.Task.run("format", files) - if !config, do: Mix.shell().info(override_format_instruction()) - end + files = Enum.filter(files, &String.ends_with?(&1, format_extensions)) + if files != [], do: Mix.Task.run("format", files) end @doc """ From 2e43a4397ab7d9516af18afed4bc611310357fda Mon Sep 17 00:00:00 2001 From: Pavel Shpak Date: Thu, 12 Dec 2024 16:32:10 +0200 Subject: [PATCH 5/5] Correct typos on reverting auto-formatting. --- .../test/code_generation/app_with_mssql_adapter_test.exs | 8 ++------ lib/mix/phoenix.ex | 2 +- lib/mix/tasks/phx.gen.embedded.ex | 2 +- lib/mix/tasks/phx.gen.schema.ex | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs index 824934af83..0b9903b748 100644 --- a/integration_test/test/code_generation/app_with_mssql_adapter_test.exs +++ b/integration_test/test/code_generation/app_with_mssql_adapter_test.exs @@ -166,8 +166,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do @tag database: :mssql test "has a passing test suite (--no-live)" do with_installer_tmp("app_with_defaults", fn tmp_dir -> - {app_root_path, _} = - generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) + {app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--database", "mssql", "--live"]) mix_run!(~w(phx.gen.html Accounts Group groups name), app_root_path) @@ -182,10 +181,7 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithMSSQLAdapterTest do """) end) - mix_run!( - ~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), - app_root_path - ) + mix_run!(~w(phx.gen.auth Accounts User users --hashing-lib pbkdf2 --merge-with-existing-context --no-live), app_root_path) drop_test_database(app_root_path) assert_tests_pass(app_root_path) diff --git a/lib/mix/phoenix.ex b/lib/mix/phoenix.ex index b4504bbc6d..24b5f1d793 100644 --- a/lib/mix/phoenix.ex +++ b/lib/mix/phoenix.ex @@ -82,7 +82,7 @@ defmodule Mix.Phoenix do end @doc """ - Override format instruction for console and docs. + Format docs with override instruction. """ def override_format_instruction do """ diff --git a/lib/mix/tasks/phx.gen.embedded.ex b/lib/mix/tasks/phx.gen.embedded.ex index ead9db4aab..edb9c644ce 100644 --- a/lib/mix/tasks/phx.gen.embedded.ex +++ b/lib/mix/tasks/phx.gen.embedded.ex @@ -26,7 +26,7 @@ defmodule Mix.Tasks.Phx.Gen.Embedded do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} * `:datetime` - An alias for `:naive_datetime` ## Format diff --git a/lib/mix/tasks/phx.gen.schema.ex b/lib/mix/tasks/phx.gen.schema.ex index 869fe80f8b..35b03efd4a 100644 --- a/lib/mix/tasks/phx.gen.schema.ex +++ b/lib/mix/tasks/phx.gen.schema.ex @@ -41,7 +41,7 @@ defmodule Mix.Tasks.Phx.Gen.Schema do The following types are supported: - #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} + #{for attr <- Mix.Phoenix.Schema.valid_types(), do: " * `#{inspect attr}`\n"} * `:datetime` - An alias for `:naive_datetime` The generator also supports references, which we will properly