Qqwy / elixir-type_check

TypeCheck: Fast and flexible runtime type-checking for your Elixir projects.
MIT License
521 stars 25 forks source link

`%{String.t() => any}` syntax raises compile error #152

Closed marcandre closed 2 years ago

marcandre commented 2 years ago

I'm just starting to try type_check, and my very first attempt is failing miserably.

I'm probably missing something, sorry if that's the case.

defmodule EmptyMix do
  defstruct [:name]
  use TypeCheck, enable_runtime_checks: Mix.env() != :prod
  @type! t :: %__MODULE__{name: String.t()}

  @spec! example(%{String.t() => any} | EmptyMix.t()) :: EmptyMix.t()
  def example(%__MODULE__{} = struct), do: struct
end

I'm getting:

** (UndefinedFunctionError) function String.t/0 is undefined or private

It seems to be due to the second usage.

If I change the second use to ...%{binary() => any} ..., I get:

** (UndefinedFunctionError) function EmptyMix.t/0 is undefined (function not available)

What am I missing?

If I then change these EmptyMix.t() to t(), then I get:

** (CompileError) lib/type_check/spec.ex: invalid quoted expression: #TypeCheck.Type< binary() >

Please make sure your quoted expressions are made of valid AST nodes. If you would like to introduce a value into the AST, such as a four-element tuple or a map, make sure to call Macro.escape/1 before
Qqwy commented 2 years ago

Hey there!

No worries, what goes wrong here is a usage error, but a very subtle one. It may be a good indication that we need to improve the error messages for this particular problem.

The problem is the part %{String.t() => any()}. This is not actually a valid type. You have the choice between:

  1. %{required(String.t()) => any()}
  2. %{optional(String.t()) => any()}
  3. %{"some_particular_constant_value" => any()}.

It seems that currently TypeCheck tries to interpret String.t() as a literal constant value and therefore compiles it as if it were a normal function call. (And later on the compiler then will complain that this function does not exist). Switching String.t() out for binary() similarly results in it trying to use the type 'binary()' as a literal value, which leads to the "invalid quoted expression" error.

Let me see whether we can do something to produce more readable error messages when this mistake is made. I'm not sure this is possible in the general sense, but at least for some common cases it might be possible to add a "did you mean" section to the raised exception message before reraising.


The other problem you get, about EmptyMix.t() not being found is something that I consider a real bug in the library. Substituting it for t() is the way to circumvent this problem for now, but this should be handled by the library itself in my opinion. I'll make a new issue for this bug.

marcandre commented 2 years ago

Thanks for the reply :-)

The problem is the part %{String.t() => any()}. This is not actually a valid type.

Oh, it is not? I read the docs as it being equivalent to the require() form... I'll gladly make a PR to improve the official doc if needed. Or did you mean that it's not a valid type for type_check but valid in Elixir?

[...] I'll make a new issue for this bug.

Thanks.

Qqwy commented 2 years ago

Interesting! You're right!

Looking in the documentation, it seems like this was added in Elixir v1.11. Whether it was already allowed beforehand (and only missing in the docs) or not allowed at all I do not know.

But this changes things: The library should definitely allow this syntax and transform it to the required(type) form automatically :+1: !

marcandre commented 2 years ago

Ah, ok. There seems to be some level of support already though.

Qqwy commented 2 years ago

There seems to be some level of support already though.

That is a references to a test file in which the creation of types based on 'real' Elixir typespecs is tested. This is a test case where this variant of the syntax is tested, but it is constructed as a normal Elixir typespec. It does not go through the normal parsing step that happens when you call @type!, @spec! or TypeCheck.conforms.

Qqwy commented 2 years ago

The fix is part of the now-published release v0.12.4.