diff --git a/.gitignore b/.gitignore index e8343ae..1feaa86 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ erl_crash.dump struct_cop-*.tar .elixir_ls +.DS_Store diff --git a/lib/struct_cop.test.exs b/lib/struct_cop.test.exs index 534ed26..b293e44 100644 --- a/lib/struct_cop.test.exs +++ b/lib/struct_cop.test.exs @@ -3,12 +3,12 @@ defmodule StructCopTest do describe "cast!/2" do test "returns struct" do - assert %TestStruct{a: 4} = %TestStruct{} |> StructCop.cast!(a: 4) + assert %TestStruct{a: 4} = TestStruct.new!() |> StructCop.cast!(a: 4) end test "raises ArgumentError for invalid attrs" do assert_raise ArgumentError, fn -> - %TestStruct{} + TestStruct.new!() |> StructCop.cast!(a: 5) end end @@ -28,16 +28,16 @@ defmodule StructCopTest do describe "cast/2" do test "returns {:ok, struct}" do - assert {:ok, %TestStruct{a: 4}} = %TestStruct{} |> StructCop.cast(a: 4) + assert {:ok, %TestStruct{a: 4}} = TestStruct.new!() |> StructCop.cast(a: 4) end test "handles struct as attrs" do - assert {:ok, %TestStruct{a: 4}} = %TestStruct{} |> StructCop.cast(%TestStruct{a: 4}) + assert {:ok, %TestStruct{a: 4}} = TestStruct.new!() |> StructCop.cast(TestStruct.new!(a: 4)) end test "returns {:error, %Ecto.Changeset{}} for invalid attrs" do assert {:error, %Ecto.Changeset{valid?: false, errors: errors}} = - %TestStruct{} |> StructCop.cast(a: 3) + TestStruct.new!() |> StructCop.cast(a: 3) assert :a in Keyword.keys(errors) end @@ -56,21 +56,21 @@ defmodule StructCopTest do describe "cast/2 for compound fileds" do test "casts array field" do assert {:ok, %TestStruct{array: [1, 2, 3]}} = - %TestStruct{} |> StructCop.cast(array: [1, 2, 3]) + TestStruct.new!() |> StructCop.cast(array: [1, 2, 3]) end test "casts map field" do assert {:ok, %TestStruct{map: %{abc: 1, cde: 2}}} = - %TestStruct{} |> StructCop.cast(map: %{abc: 1, cde: 2}) + TestStruct.new!() |> StructCop.cast(map: %{abc: 1, cde: 2}) end end test "cast/2 for embeds_one with nested StructCop" do assert {:ok, %TestStruct{nested: %TestStruct.Nested{a: 1, b: 2}}} = - %TestStruct{} |> StructCop.cast(nested: %{a: 1, b: 2}) + TestStruct.new!() |> StructCop.cast(nested: %{a: 1, b: 2}) assert {:error, %Ecto.Changeset{valid?: false, changes: %{nested: nested_changeset}}} = - %TestStruct{} |> StructCop.cast(nested: %{b: 2}) + TestStruct.new!() |> StructCop.cast(nested: %{b: 2}) assert %Ecto.Changeset{valid?: false, errors: errors} = nested_changeset @@ -79,10 +79,10 @@ defmodule StructCopTest do test "cast/2 for embeds_many with nested StructCop" do assert {:ok, %TestStruct{nesteds: [%TestStruct.Nested{a: 1, b: 2}]}} = - %TestStruct{} |> StructCop.cast(nesteds: [%{a: 1, b: 2}]) + TestStruct.new!() |> StructCop.cast(nesteds: [%{a: 1, b: 2}]) assert {:error, %Ecto.Changeset{valid?: false, changes: %{nesteds: [nested_changeset]}}} = - %TestStruct{} |> StructCop.cast(nesteds: [%{b: 2}]) + TestStruct.new!() |> StructCop.cast(nesteds: [%{b: 2}]) assert %Ecto.Changeset{valid?: false, errors: errors} = nested_changeset @@ -93,18 +93,19 @@ defmodule StructCopTest do assert {:ok, %TestStruct{ nesteds: [ - %TestStruct.Nested{a: 1, b: 2, deeply_nesteds: [%TestStruct.DeeplyNested{a: 1}]} + %TestStruct.Nested{a: 1, b: 2, deeply_nesteds: [%TestStruct.MyStruct{a: 1}]} ] }} = - %TestStruct{} |> StructCop.cast(nesteds: [%{a: 1, b: 2, deeply_nesteds: [%{a: 1}]}]) + TestStruct.new!() + |> StructCop.cast(nesteds: [%{a: 1, b: 2, deeply_nesteds: [%{a: 1}]}]) end test "cast/2 for embeds_one with nested embedded_schema" do assert {:ok, %TestStruct{nested_ecto: %TestStruct.EctoSchema{a: 1}}} = - %TestStruct{} |> StructCop.cast(nested_ecto: %{a: 1}) + TestStruct.new!() |> StructCop.cast(nested_ecto: %{a: 1}) assert {:error, %Ecto.Changeset{valid?: false, changes: %{nested_ecto: nested_changeset}}} = - %TestStruct{} |> StructCop.cast(nested_ecto: %{b: 2}) + TestStruct.new!() |> StructCop.cast(nested_ecto: %{b: 2}) assert %Ecto.Changeset{valid?: false, errors: errors} = nested_changeset diff --git a/lib/struct_cop/changeset.test.exs b/lib/struct_cop/changeset.test.exs index 83f71c0..ecb2028 100644 --- a/lib/struct_cop/changeset.test.exs +++ b/lib/struct_cop/changeset.test.exs @@ -6,14 +6,14 @@ defmodule StructCop.ChangesetTest do describe "cast_all/2" do test "works with %Ecto.Changeset{}" do assert %Ecto.Changeset{changes: %{b: 4, a: 4}} = - %TestStruct{} + TestStruct.new!() |> cast(%{a: 4}, [:a]) |> Subject.cast_all(b: 4) end test "fallbacks to &cast_all/2 for embeds without changeset" do assert %Ecto.Changeset{changes: %{inline: %Ecto.Changeset{changes: %{c: 10}}}} = - %TestStruct{} + TestStruct.new!() |> Subject.cast_all(inline: %{c: 10}) end end diff --git a/lib/struct_cop/error_message.test.exs b/lib/struct_cop/error_message.test.exs index 6b5437f..00dafcc 100644 --- a/lib/struct_cop/error_message.test.exs +++ b/lib/struct_cop/error_message.test.exs @@ -6,7 +6,7 @@ defmodule StructCop.ErrorMessageTest do def changeset do import Ecto.Changeset - %TestStruct{} + TestStruct.new!() |> cast(%{a: 4}, [:a]) |> validate_number(:a, greater_than: 5) |> validate_number(:a, less_than: 3) diff --git a/lib/struct_cop/macro.ex b/lib/struct_cop/macro.ex index ef0119a..7a36c00 100644 --- a/lib/struct_cop/macro.ex +++ b/lib/struct_cop/macro.ex @@ -25,6 +25,7 @@ defmodule StructCop.Macro do end def __struct_cop__, do: true + def __struct_cop_enforce_constructor__, do: true defoverridable changeset: 2 defoverridable validate: 1 diff --git a/lib/struct_cop/tracer.ex b/lib/struct_cop/tracer.ex new file mode 100644 index 0000000..359a1c8 --- /dev/null +++ b/lib/struct_cop/tracer.ex @@ -0,0 +1,28 @@ +defmodule StructCop.Tracer do + def trace({:struct_expansion, _meta, _module, _keys}, %Macro.Env{context: :match}), do: :ok + + def trace({:struct_expansion, _meta, module, _keys}, env) do + if function_exported?(module, :__struct_cop__, 0) && + module.__struct_cop_enforce_constructor__() do + raise CompileError, + file: env.file, + line: env.line, + description: """ + + #{struct_label(module)} enforces constructors usage (new/1 & new!/1), + ordinary struct creation is not allowed: + + * #{struct_label(module)} + * #{struct_label(module, "struct | arg: 123")} + """ + else + :ok + end + end + + def trace(_, _), do: :ok + + defp struct_label(module, content \\ nil) do + "%#{inspect(module)}{#{content}}" + end +end diff --git a/lib/struct_cop/tracer.test.exs b/lib/struct_cop/tracer.test.exs new file mode 100644 index 0000000..56f1148 --- /dev/null +++ b/lib/struct_cop/tracer.test.exs @@ -0,0 +1,90 @@ +defmodule StructCop.TracerTest do + use ExUnit.Case, async: true + + alias TestStruct.MyStruct + alias TestStruct.Ordinary + + setup do + Code.put_compiler_option(:tracers, [StructCop.Tracer]) + + :ok + end + + def compile(ast), do: Code.compile_quoted(ast) + + test "raises CompileError for %MyStruct{}" do + assert_raise CompileError, fn -> + quote do + %MyStruct{} + end + |> compile() + end + + assert_raise CompileError, fn -> + quote do + %MyStruct{} == %MyStruct{} + end + |> compile() + end + + assert_raise CompileError, fn -> + quote do + struct = MyStruct.new!() + %MyStruct{struct | a: 2} + end + |> compile() + end + + assert_raise CompileError, fn -> + quote do + defmodule MyMacroMod do + defmacro __using__(_) do + quote do + def call do + %MyStruct{} + end + end + end + end + + defmodule MyClientMod do + use MyMacroMod + end + end + |> compile() + end + end + + test "raises CompileError for struct!(MyStruct, attrs)" do + # TODO + end + + test "doesn't raise CompileError for ordinary structs" do + quote do + %Ordinary{} + end + |> compile() + + assert true + end + + test "doesn't raise for LEFT side of pattern match" do + quote do + defmodule MyMod do + def call(%MyStruct{}) do + end + end + end + |> compile() + + quote do + struct = MyStruct.new!(a: 2) + + %MyStruct{a: a} = struct + %MyStruct{a: 2} = struct + end + |> compile() + + assert true + end +end diff --git a/mix.exs b/mix.exs index 68c492a..eb596bd 100644 --- a/mix.exs +++ b/mix.exs @@ -7,7 +7,7 @@ defmodule StructCop.MixProject do [ app: :struct_cop, version: @version, - elixir: "~> 1.8", + elixir: "~> 1.10", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, test_coverage: [tool: ExCoveralls], diff --git a/test/support/test_structs.ex b/test/support/test_structs.ex index 114a432..20ce1d7 100644 --- a/test/support/test_structs.ex +++ b/test/support/test_structs.ex @@ -14,18 +14,16 @@ defmodule TestStruct.EctoSchema do end end -defmodule TestStruct.DeeplyNested do +defmodule TestStruct.Ordinary do + defstruct [:a, :b] +end + +defmodule TestStruct.MyStruct do use StructCop contract do field :a, :integer - end - - def validate(changeset) do - import Ecto.Changeset - - changeset - |> validate_required([:a]) + field :b, :integer end end @@ -35,7 +33,7 @@ defmodule TestStruct.Nested do contract do field :a, :integer field :b, :integer - embeds_many :deeply_nesteds, TestStruct.DeeplyNested + embeds_many :deeply_nesteds, TestStruct.MyStruct end def validate(changeset) do