stavro / arc_ecto

An integration with Arc and Ecto.
255 stars 149 forks source link

Add a type for Dialyzer #79

Closed ejpcmac closed 5 years ago

ejpcmac commented 7 years ago

I’ve added an Arc.Ecto.t type, which can be useful when you want declare a @type for your Ecto schemas containing an Arc-managed file.

For instance:

defmodule MyApp.Something do
  use Ecto.Schema
  use Arc.Ecto.Schema

  @type t :: %__MODULE__{
    name: String.t | nil,
    some_file: Arc.Ecto.t | nil, # The type is useful here
  }

  schema "somethings" do
    field :name, :string
    field :some_file, MyApp.MyUploader.Type

    timestamps()
  end
end
nkezhaya commented 6 years ago

@stavro Any reason not to merge this?

ghost commented 6 years ago

Hello, sorry if it's a noob question. When using arc.ecto I'm getting this error in Dialyzer

The pattern #{'__meta__':=_} can never match the type #{'__struct__':='Elixir.Ecto.Changeset', 'action':='delete' | 'ignore' | 'insert' | 'nil' | 'replace' | 'update', 'changes':=#{atom()=>_}, 'constraints':=[#{'constraint':=binary(), 'field':=atom(), 'match':='exact' | 'suffix', 'message':={_,_}, 'type':='unique'}], 'data':='nil' | map(), 'empty_values':=_, 'errors':=[{atom(),{_,_}}], 'filters':=#{atom()=>_}, 'params':='nil' | #{binary()=>_}, 'prepare':=[fun((map()) -> map())], 'repo':=atom(), 'repo_opts':=[{atom(),_}], 'required':=[atom()], 'types':='nil' | #{atom()=>atom() | {'array',_} | {'embed',map()} | {'in',_} | {'map',_}}, 'valid?':=boolean(), 'validations':=[{atom(),_}]}

Would this type help?

ejpcmac commented 6 years ago

@pedro-gaspar Without the code triggering this Dialyzer warning, it is difficult to say anything.

ghost commented 6 years ago

Hey @ejpcmac, sorry for not being more detailed.

  def changeset(conference, attrs) do
    conference
    |> cast(attrs, [
      :name,
      :start_at,
      :end_at,
      :country,
      :url,
      :youtube,
      :twitter,
      :topic_id,
      :source_url
    ])
    |> cast_attachments(attrs, [:image]) # The dialyzer warning is here
    |> assoc_constraint(:topic)
    |> validate_required([:name])
  end

Although I get the warning it is saving attachments.

ejpcmac commented 6 years ago

@pedro-gaspar Looking at the .dialyzer_ignore from a project where I use arc_ecto, I have an error line similar to yours. If I remember correctly (it’s been more than a year!), the purpose of this pull request was to avoid a Dialyzer error, which could be this very one.

@stavro Any reason not to merge this? Dialyzer types are useful to those who use it. With editors integrations like the one in VSCode via Elixir LS, more and more people are using Dialyzer, so more and more users will get annoying warnings.