Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions lib/surface/api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,11 @@ defmodule Surface.API do
end

defp get_valid_opts(:prop, _type, _opts) do
[:required, :default, :values, :accumulate]
[:required, :default, :values, :values!, :accumulate]
end

defp get_valid_opts(:data, _type, _opts) do
[:default, :values]
[:default, :values, :values!]
end

defp get_valid_opts(:slot, _type, _opts) do
Expand Down Expand Up @@ -408,7 +408,7 @@ defmodule Surface.API do
{:error, "invalid value for option :required. Expected a boolean, got: #{inspect(value)}"}
end

defp validate_opt(:prop, _name, _type, opts, :default, _value, caller) do
defp validate_opt(:prop, name, _type, opts, :default, value, caller) do
if Keyword.get(opts, :required, false) do
IOHelper.warn(
"setting a default value on a required prop has no effect. Either set the default value or set the prop as required, but not both.",
Expand All @@ -417,6 +417,14 @@ defmodule Surface.API do
)
end

warn_on_invalid_default(:prop, name, value, opts, caller)

:ok
end

defp validate_opt(:data, name, _type, opts, :default, value, caller) do
warn_on_invalid_default(:data, name, value, opts, caller)

:ok
end

Expand Down Expand Up @@ -473,6 +481,46 @@ defmodule Surface.API do
:ok
end

defp warn_on_invalid_default(type, name, default, opts, caller) do
accumulate? = Keyword.get(opts, :accumulate, false)
values! = Keyword.get(opts, :values!)

cond do
accumulate? and not is_list(default) ->
IOHelper.warn(
"#{type} `#{name}` default value `#{inspect(default)}` must be a list when `accumulate: true`",
caller,
fn _ -> caller.line end
)

accumulate? and not is_nil(values!) and
not MapSet.subset?(MapSet.new(default), MapSet.new(values!)) ->
IOHelper.warn(
"""
#{type} `#{name}` default value `#{inspect(default)}` does not exist in `:values!`

Hint: Either choose an existing value or replace `:values!` with `:values` to skip validation.
""",
caller,
fn _ -> caller.line end
)

not accumulate? and not is_nil(values!) and not (default in values!) ->
IOHelper.warn(
"""
#{type} `#{name}` default value `#{inspect(default)}` does not exist in `:values!`

Hint: Either choose an existing value or replace `:values!` with `:values` to skip validation.
""",
caller,
fn _ -> caller.line end
)

true ->
:ok
end
end

defp unknown_options_message(valid_opts, unknown_options) do
{plural, unknown_items} =
case unknown_options do
Expand Down
4 changes: 2 additions & 2 deletions test/api_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ defmodule Surface.APITest do
code = "prop label, :string, a: 1"

message =
~r/unknown option :a. Available options: \[:required, :default, :values, :accumulate\]/
~r/unknown option :a. Available options: \[:required, :default, :values, :values!, :accumulate\]/

assert_raise(CompileError, message, fn ->
eval(code)
Expand Down Expand Up @@ -404,7 +404,7 @@ defmodule Surface.APITest do

test "validate unknown type options" do
code = "data label, :string, a: 1"
message = ~r/unknown option :a. Available options: \[:default, :values\]/
message = ~r/unknown option :a. Available options: \[:default, :values, :values!\]/

assert_raise(CompileError, message, fn ->
eval(code)
Expand Down
57 changes: 57 additions & 0 deletions test/properties_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -706,4 +706,61 @@ defmodule Surface.PropertiesSyncTest do
code.exs:7:\
"""
end

test "warn if given default value doesn't exist in values list" do
id = :erlang.unique_integer([:positive]) |> to_string()
module = "TestComponentWithDefaultValueThatDoesntExistInValues_#{id}"

code = """
defmodule #{module} do
use Surface.Component

prop type, :string, values!: ["small", "medium", "large"], default: "x-large"

data data_type, :string, values!: ["small", "medium", "large"], default: "x-large"

prop invalid_type, :integer, default: [], values!: [0, 1, 2]

prop valid_acc, :integer, default: [1], values!: [0, 1, 2], accumulate: true

prop invalid_acc1, :integer, default: [3], values!: [0, 1, 2], accumulate: true

prop invalid_acc2, :string, values!: [1, 2, 3], default: 3, accumulate: true

def render(assigns) do
~H""
end
end
"""

output =
capture_io(:standard_error, fn ->
{{:module, _, _, _}, _} =
Code.eval_string(code, [], %{__ENV__ | file: "code.exs", line: 1})
end)

assert output =~ ~S"""
prop `type` default value `"x-large"` does not exist in `:values!`
"""

assert output =~ ~S"""
data `data_type` default value `"x-large"` does not exist in `:values!`
"""

assert output =~ ~S"""
prop `invalid_type` default value `[]` does not exist in `:values!`
"""

refute output =~ ~S"""
prop `valid_acc`
"""

assert output =~ ~S"""
prop `invalid_acc1` default value `[3]` does not exist in `:values!`
"""

assert output =~ ~S"""
prop `invalid_acc2` default value `3` must be a list when `accumulate: true`
"""
end
end