Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type checking when defining a struct #3

Open
mkaszubowski opened this issue Jun 21, 2018 · 14 comments
Open

Type checking when defining a struct #3

mkaszubowski opened this issue Jun 21, 2018 · 14 comments
Labels
help wanted Extra attention is needed T:Feature Type: Feature

Comments

@mkaszubowski
Copy link

Hi,

I've seen that you have default type checking in the roadmap and I think it would be amazing to have that. May I ask if you have any ideas how to tackle the problem? I am particularly interested in this as I tried to accomplish this few months ago, but unfortunately I failed.

I really hope that we will be able to work something out together :)

@ejpcmac
Copy link
Owner

ejpcmac commented Jun 21, 2018

Hi @mkaszubowski! As stated on the roadmap, I don’t know if this is really possible and I have no idea how to do this. It is here because it would be great and it deserves some research.

I think the main problem is we are dealing with Dialyzer types, which indeed is an external tool.

Ideas

Using a reduced set of types

A bit like Ecto, which defines its types. This way we could check the types at compile time, but at the cost of a lost in expressiveness. I don’t think this is a good path to explore.

Calling Dialyzer

I don’t have checked yet if Dialyzer has an API. If yes we could call it from TypedStruct. However I don’t think it is a good idea because Dialyzer can be slow do build PLTs.

Using a hack

We could produce some code that Dialyzer could then check. Things like:

field :with_default, String.t(), default: 5

# Generates
@spec __with_default_default__ :: String.t()
def __with_default_default__, do: 5  # Would lead to a Dialyzer warning

I didn’t test at the moment if it works, but I assume the problem would be the strangeness of the warning: the generated code is not visible to the user.

@ejpcmac
Copy link
Owner

ejpcmac commented Jun 21, 2018

Just an update on this: in the current version, Dialyzer warns when creating a struct with the default value if it does not match. For instance:

defmodule Struct do
  use TypedStruct

  typedstruct do
    field :with_default, String.t(), default: 5
  end

  @spec new :: t()
  def new do
    %__MODULE__{}
  end
end

leads to this warning:

lib/struct.ex:8: Invalid type specification for function 'Elixir.Struct':new/0. The success typing is () -> #{'__struct__':='Elixir.Struct', 'with_default':=5}

@mkaszubowski
Copy link
Author

Sure, I know it is hard, that's why I wanted to discuss this :) I'll try to describe my approach here (as I've said, I'd really like to have something like this available, so I hope we can finally figure something out).

My idea was to "override" the defstruct from Kernel when creating a struct. So I would basically have something like:

defmodule TypedStruct do
  defmacro __using__(_opts) do
    quote do
      require TypedStruct

      import Kernel, except: [defstruct: 1]
      import TypedStruct
    end
  end

  # ...
end

Then, in the TypedStruct module, I would define my own defstruct macro (basically copied from the elixir source). The addition was that in the __struct__/2 function (which is invoked when creating the struct), I would add another step which was verification of types. For this steps, I used guards like is_binary(field), is_integer because they can work in the compilation time.

I know, a bit of a hack, but it kind of worked... But only for constants. So I would get an error when doing %MyStruct{x: 5} (where x should be a string, for example), but the following did not work:

x = 5
%MyStruct{x: 5}

because the value of x is not known during compilation time. That's where I got stuck and didn't know if it is possible to progress further.

Of course, there are still some problems with that approach, even if it would be possible to implement. Some more information can be found here: https://groups.google.com/forum/#!searchin/elixir-lang-core/structs%7Csort:relevance/elixir-lang-core/U_wdxEqWj_Y/82EaD4D7AAAJ

@hauleth
Copy link

hauleth commented Jun 22, 2018

Maybe just attach to Ecto.Changeset? AFAIK there are some ideas to split Ecto into separate libraries where one of them would provide something similar to this one together with changesets to build such structures.

@mkaszubowski
Copy link
Author

Yeah, but using Ecto.Changeset (or anything like that, basically) would require users to use a different API for creating/update structs instead of just using %MyStruct{...} syntax.

Ideally, this type checking would be done when creating the struct. So if I did %User{age: "not_integer"}, I'd get an error - the same way as I know get an error when the field is missing and it's included in @enforece_keys.

Important thing here is that I'm not talking about a strong type system - this check would not be performed during compilation, but in runtime.

@ejpcmac
Copy link
Owner

ejpcmac commented Jul 4, 2018

@mkaszubowski Sorry for my late reply, I’ve been quite busy on other subjects the past few days.

Then, in the TypedStruct module, I would define my own defstruct macro (basically copied from the elixir source).

I don’t think it is a good idea to replace the standard implementation with something else. It could lead to problems if something is changed upstream, and it adds confusion about what’s going on under the hood. Plus, I think it is not necessary: we could type-check the default value in __field__/4 using guards like you’ve done. Do you know if it is possible to 1-1 map dialyzer types and guards? Or maybe are you aware of some Dialyzer API that we could use to just type-check a given value against a type?

I know, a bit of a hack, but it kind of worked... But only for constants.

That’s not a problem for what we want to achieve: type-check the default value and return a useful warning if it does not match.

If you are talking about validating the struct, this is another story. In this very case, yes a changeset like @hauleth has mentioned can be a good idea, especially if Ecto is splitted in the future. Another idea I had and mentioned in the roadmap it guard generation. I do not intend to type-check when calling %Struct{} for the very reason mentioned by José Valim in the thread you were involved in: it adds overhead, plus it makes the standard API to behave in a maybe-not-expected way, which is a bad practice IMO.

With auto-generated guards, we could do something like:

# is_struct_user/1 is a guard generated by TypedStruct to enable *optional* type-checking.
def a_function(user) when is_struct_user(user), do: :something

@ejpcmac ejpcmac added help wanted Extra attention is needed T:Feature Type: Feature labels Oct 30, 2018
@imaxmelnyk
Copy link

I would just add new or create function with all the fields or a map with all the fields and throw an error (or whatever :ok, :error) if there something. Not sure what other, mentioned here, libraries actually do (maybe they do exactly the same). And then, maybe it's possible to deny standard struct creation and force users to use those defined methods.

@jg-made
Copy link

jg-made commented Dec 22, 2019

I don't have a solution for this library but if anyone else is looking for a way to achieve this today, here is how I am doing it.
I install Vex in my project and define a validation_helper.ex module like this:

defmodule MyApp.ValidationHelpers do
  def is_nil_or_int(x) do
    is_nil(x) or is_integer(x)
  end
end

and then I define my structs like this:

defmodule MyApp.Accounts.Account do
  use TypedStruct
  use Vex.Struct

  typedstruct do
    field :balance, integer()
  end

  validates(:balance,
    by: [function: &MyApp.ValidationHelpers.is_nil_or_int/1, message: "must be an integer or nil"]
  )
end

Maybe the solution is to consolidate :vex and :typed_struct libs?

@c-alpha
Copy link

c-alpha commented Jan 15, 2021

Just stumbled upon Qqwy/elixir-type_check:

Usage Example

We add use TypeCheck to a module and wherever we want to add runtime type-checks we replace the normal calls to @type and @spec with @type! and @spec! respectively.

Sounds both, simple and promising enough to give it a try?

@ejpcmac
Copy link
Owner

ejpcmac commented Mar 12, 2021

Hi @c-alpha, sorry for my late reply, I was on a long break after a year in a scientific station at the time you wrote this comment and I forgot to reply then.

I don’t do much Elixir these times, as I’m mainly doing sysadmin and embedded Rust on my spare time right now, so I won’t have a project to try elixir-type_check quickly. Have you tested it? Is it good? If yes, maybe someone can write a little TypedStruct.Plugin to integrate it, and if it’s interesting enough I could consider to bring it to typed_struct itself at some point.

@ejpcmac
Copy link
Owner

ejpcmac commented Aug 26, 2021

For info, Domo seems also very interesting for checking the types, given a @type definition like the one generated by typed_struct.

@Qqwy
Copy link

Qqwy commented May 30, 2022

I won’t have a project to try elixir-type_check quickly. Have you tested it? Is it good?

Hi! Author of TypeCheck here.
I'm obviously biased, but my hope is that it is at least decent 😇.

Multiple people have expressed interest in using TypeCheck and TypedStruct together, so I'd love to collaborate with TypedStruct to make a plugin.

However, that would require TypedStruct's plugin system to be somewhat enhanced: Currently field/4 is called after a field (to be precise: after all fields) have been compiled.
Also, there is no way to turn off the definition of the type by TypedStruct.

To make it work with TypeCheck, we'd need two changes:

  • To support the advanced features of TypeCheck, we'd need access to the the type definition AST for each of the fields before they are is compiled;
  • and turn off the @type creation of TypedStruct. (Or alternatively give TypedStruct, for each field, an altered version of the type AST back, which TypedStruct can then use to compile create the @type).

If you want, I can contribute a PR with these changes.

@ejpcmac
Copy link
Owner

ejpcmac commented May 31, 2022

Hello @Qqwy, glad to hear from you here 😃 I’m starting a long-term project written in Elixir, so I’ll even have an occasion to test TypeCheck myself at some point.

If you want, I can contribute a PR with these changes.

Please go ahead, I’ll review this happily!

How do you plan to handle the option to turn off the @type creation? I’d like this feature not to be exposed to the end user, but I’m completely OK for it to be provided by the plugin API.

@leggebroten
Copy link

leggebroten commented Dec 6, 2023

If anybody is reading this so long after the fact, you can use

  • typed_struct_ctor
    – Adds validating (new and from) constructor functions.

    Try the macro out in real time without having to install or write any of your own code.
    All you need is a running instance of Livebook

    Run in Livebook

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed T:Feature Type: Feature
Projects
Status: Backlog
Development

No branches or pull requests

8 participants