Skip to content

Module.get_attribute/3 returns nil instead of the default value #13558

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

Closed
DaTrader opened this issue May 14, 2024 · 2 comments
Closed

Module.get_attribute/3 returns nil instead of the default value #13558

DaTrader opened this issue May 14, 2024 · 2 comments

Comments

@DaTrader
Copy link

Elixir and Erlang/OTP versions

Erlang/OTP 25 [erts-13.2.2.9] [source] [64-bit] [smp:24:24] [ds:24:24:10] [async-threads:1] [jit:ns]

Elixir 1.15.7 (compiled with Erlang/OTP 25)

Operating system

Linux Mint

Current behavior

On compilation the Module.get_attribute/3 function returns nil instead of the provided default value in certain circumstances. The bug was introduced in 1.15.0 and is still present in 1.16.2.

Output example on Elixir 1.14.5 and earlier (the correct behavior):

$ asdf global elixir 1.14.5-otp-25
$ mix compile --force
==> protocol_ex
Compiling 2 files (.ex)
Generated protocol_ex app
==> attr_bug
Compiling 2 files (.ex)
module: JsonDecodable.Customer
This should never be nil: %{}
module: JsonDecodable.Customer
This should never be nil: %{id: "id"}
module: JsonDecodable.Customer
This should never be nil: %{id: "id", name: "first_name"}
Generated attr_bug app

Output example on Elixir 1.15+ (the incorrect behavior):

$ asdf global elixir 1.15.7-otp-25
$ mix compile --force
==> protocol_ex
Compiling 2 files (.ex)
Generated protocol_ex app
==> attr_bug
Compiling 2 files (.ex)
module: JsonDecodable.Customer
This should never be nil: nil

== Compilation error in file lib/attr_bug.ex ==
** (BadMapError) expected a map, got: nil
    lib/attr_bug.ex:12: (module)
    (protocol_ex 0.4.4) lib/protocol_ex.ex:267: ProtocolEx.defimplEx_do/6
    lib/attr_bug.ex:11: (file)

To reproduce the bug, create a new Elixir project with the following files (unfortunately, Elixir complains if trying to make a single file demo app):

mix.exs:

defmodule AttrBug.MixProject do
  use Mix.Project

  def project do
    [
      app: :attr_bug,
      version: "0.1.0",
      elixir: "~> 1.14",
      start_permanent: Mix.env() == :prod,
      deps: deps()
    ]
  end

  # Run "mix help compile.app" to learn about applications.
  def application do
    [
      extra_applications: [:logger]
    ]
  end

  # Run "mix help deps" to learn about dependencies.
  defp deps do
    [
      { :protocol_ex, "~> 0.4.0"}
    ]
  end
end

lib/jsoned.ex:

import ProtocolEx

defprotocol_ex JsonDecodable do
  def decode( json_decodable)
end

defprotocol_ex JsonDecodable.For do
  def decodable_impl( for) do
    raise Protocol.UndefinedError, protocol: __MODULE__, value: for
  end

  def new_for!( for, args) do
    for.new!( args)
  end
end

defmodule Jsoned.Decoder do
  defmacro __using__( opts) do
    for = opts[ :for]
    impl = __CALLER__.module

    quote do
      def decode( { for, json}) when is_map( json) or is_list( json) do
        # Jsoned.Decoder.decode( { for, json}, unquote( impl))
      end

      defoverridable decode: 1

      import ProtocolEx

      defimpl_ex JsonedDecoder, unquote( for), for: JsonDecodable.For do
        def decodable_impl( unquote( for)) do
          unquote( impl)
        end
      end
    end
  end

  defmacro defdecode( key, type, opts) do
    json_key = "#{ opts[ :key] || key}"

    quote do
      def decode_property( { unquote( json_key), value}) do
        with { :ok, value} <- Jsoned.Decoder.decode_value( unquote( type), value) do
          { :ok, { unquote( key), value}}
        end
      end
    end
  end

  def decode_value( _, nil), do: { :ok, nil}
  def decode_value( :string, "" <> data), do: { :ok, data}
end

defmodule Jsoned do
  defmacro __using__( opts) do
    quote do
      def __json_keys__() do
        Jsoned.json_keys( __MODULE__)
      end

      use Jsoned.Decoder, unquote( opts)
      import Jsoned.Decoder, only: [ defdecode: 3]

      Module.register_attribute( __MODULE__, :jsoned, persist: true)
    end
  end

  def json_keys( impl) when is_atom( impl) do
    if attrs = impl.__info__( :attributes)[ :jsoned] do
      List.first( attrs) || %{}
    end
  end

  defmacro defjsoned( opts \\ [], [ do: body]) do
    Keyword.validate!( opts, [ :for])
    opts = Keyword.put_new( opts, :for, __CALLER__.module)
    for = opts[ :for]

    unless for do
      raise ArgumentError, "defjsoned/2 expects a :for option when declared outside a module"
    end

    quote do
      import ProtocolEx

      defimpl_ex unquote( for), { unquote( for), data} when is_map( data) or is_list( data), for: JsonDecodable do
        use Jsoned, unquote( opts)

        unquote( body)
      end
    end
  end

  defmacro defprop( key, type, opts \\ []) when is_atom( key) and is_list( opts) do
    do_defprop( key, type, opts)
  end

  defp do_defprop( key, type, opts) do
    json_key = "#{ opts[ :key] || key}"

    quote do
      @jsoned __MODULE__
              |> IO.inspect( label: "module")
              |> Module.get_attribute( :jsoned, %{})
              |> IO.inspect( label: "This should never be nil")
              |> Map.put( unquote( key), unquote( json_key))

      defdecode( unquote( key), unquote( type), unquote( opts))
    end
  end
end

lib/attr_bug.ex:

defmodule Customer do
  defstruct [ :id, :name, :address]

  def new!( args) do
    struct!( __MODULE__, args)
  end
end

import Jsoned

defjsoned for: Customer do
  defprop :id, :string
  defprop :name, :string, key: "first_name"
  defprop :address, :string
end

Expected behavior

The Module.get_attribute/3 should continue to behave exactly as it used to prior to Elixir 1.15.0.

@josevalim
Copy link
Member

This seems to be a simple way to reproduce it:

iex(1)> defmodule Foo do
...(1)>       Module.register_attribute( __MODULE__, :jsoned, persist: true)
...(1)> Module.get_attribute(__MODULE__, :jsoned, %{})
...(1)> end

It seems persisting it is setting a default value. You can temporarily work around it by setting the default value right after persisting it.

@DaTrader
Copy link
Author

I see, you meant put_attribute/3 not get_attribute/3 :)

Thanks for the tip (it works)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants