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

Dialyzer warning about nil missing from spec #8717

Closed
JanEricNitschke opened this issue Aug 13, 2024 · 8 comments
Closed

Dialyzer warning about nil missing from spec #8717

JanEricNitschke opened this issue Aug 13, 2024 · 8 comments
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@JanEricNitschke
Copy link

Hi,

i have these functions

  @spec ai_random_move(board_t()) :: integer()
  def ai_random_move(board) do
    Enum.random(empty_spots(board))
  end

  @spec try_win_move(board_t(), String.t()) :: integer() | nil
  def try_win_move(board, player) do
    Enum.find(empty_spots(board), &winner?(make_move(board, &1, player), player))
  end

  @spec ai_win_move(board_t(), String.t()) :: integer()
  def ai_win_move(board, player) do
    try_win_move(board, player) || ai_random_move(board)
  end

and i am getting a warning from dialyzer

lib/tictactoe_elixir.ex:154:missing_range
The type specification is missing types returned by function.

Function:
TictactoeElixir.ai_win_move/2

Type specification return types:
integer()

Missing from spec:
nil

This does not seem correct to me, as the nil can never actually be returned due to the ||. Is this a shortcoming of dialyzer or am i doing something wrong here?

@JanEricNitschke JanEricNitschke added the bug Issue is reported as a bug label Aug 13, 2024
@IngelaAndin IngelaAndin added the team:VM Assigned to OTP team VM label Aug 14, 2024
@garazdawi
Copy link
Contributor

Hello! We only have a very basic understanding of Elixir, so it would help a lot if you could translate the example to Erlang.

@jhogberg
Copy link
Contributor

Thanks for your report! This is not a warning that is enabled by default, but one that you specifically have to opt into because it may give false positives like this one. I'm inclined to believe that this is such a false positive, but if you can reduce this to an Erlang example I'll happily take a look to be sure.

@JanEricNitschke
Copy link
Author

Hi,

unfortunately i am not sure what the simplest erlang translation is that also exhibits the issue.

What this boils down to is that i have an expression (integer | nil) || integer.

Where || is the short circuiting or in elixir which treats nil as falsey. So this should only every return an integer but it is still being flagged as potentially returning nil.

@jhogberg
Copy link
Contributor

What this boils down to is that i have an expression (integer | nil) || integer.

Where || is the short circuiting or in elixir which treats nil as falsey. So this should only every return an integer but it is still being flagged as potentially returning nil.

Yes, however, || may be expressed (as Erlang code) in a manner that makes dialyzer miss this fact. I'd need to have a look at the expansion to Erlang code to know for certain whether this is a false positive or a bug.

@JanEricNitschke
Copy link
Author

Yeah, makes sense. Ill try to look around if i can find what erlang code this is transpiled to.

@JanEricNitschke
Copy link
Author

JanEricNitschke commented Aug 14, 2024

Ok so i now reduced it to this standalone elixir code:

defmodule Stuff do
  @spec f1(boolean()) :: integer() | nil
  def f1(arg) do
    cond do
      arg -> 3
      true -> nil
    end
  end

  @spec f2() :: 5
  def f2() do
    3 + 2
  end

  @spec f3() :: integer()
  def f3() do
    f1(true) || f2()
  end
end

Decompiled it to erlang with this: https://github.com/michalmuskala/decompile

and got this:

-file("lib/text.ex", 1).

-module('Elixir.Stuff').

-compile([no_auto_import]).

-spec f3() -> integer().

-spec f2() -> 5.

-spec f1(boolean()) -> integer() | nil.

-export(['__info__'/1,f1/1,f2/0,f3/0]).

-spec '__info__'(attributes | compile | functions | macros | md5 |
                 module | deprecated | struct) ->
                    any().

'__info__'(module) ->
    'Elixir.Stuff';
'__info__'(functions) ->
    [{f1, 1}, {f2, 0}, {f3, 0}];
'__info__'(macros) ->
    [];
'__info__'(struct) ->
    nil;
'__info__'(Key = attributes) ->
    erlang:get_module_info('Elixir.Stuff', Key);
'__info__'(Key = compile) ->
    erlang:get_module_info('Elixir.Stuff', Key);
'__info__'(Key = md5) ->
    erlang:get_module_info('Elixir.Stuff', Key);
'__info__'(deprecated) ->
    [].

f1(_arg@1) ->
    case _arg@1 of
        _@1
            when
                _@1 /= nil
                andalso
                _@1 /= false ->
            3;
        _ ->
            case true of
                true ->
                    nil
            end
    end.

f2() ->
    3 + 2.

f3() ->
    case f1(true) of
        _@1
            when
                _@1 =:= false
                orelse
                _@1 =:= nil ->
            f2();
        _@2 ->
            _@2
    end.

Running this through dialyzer -Wunmatched_returns -Werror_handling -Wunderspecs -Wextra_return -Wmissing_return --src Elixir.Stuff.erl

gives:

text.ex:7:2: The success typing for 'Elixir.Stuff':f3/0 implies that the function might also return
          'nil' but the specification return is
          integer()

@jhogberg
Copy link
Contributor

Thanks! This is a shortcoming in dialyzer that cannot be fixed in the general case, if you don't want warnings that may be false positives, stop providing -Wunderspecs -Wextra_return -Wmissing_return.

@JanEricNitschke
Copy link
Author

Ok, thanks a lot for the information. I think i will keep these arguments but just suppress them in the cases where i can verify that they are false positives.

Just wanted to verify that these are indeed false positives and that i didnt miss anything and also that they arent easy to address on your side.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

4 participants