dialyzer -Wunmatched_returns warnings

[kostis]

If one runs:

dialyzer ebin -Wunmatched_returns

one gets various warnings which, although not errors, show places where return values are ignored which often signifies places where the intention of the programmer may not be the one that the code performs. In most cases, these warnings are very easy to fix.

For example, the first of these warnings reads:

mochiweb.erl:23: Expression produces a value of type 'ok' | {'error',_}, but this value is unmatched

and comes from the code:

%% @spec stop() -> ok
%% @doc Stop the MochiWeb server.
stop() ->
    Res = application:stop(mochiweb),
    application:stop(crypto),
    Res.

At least judging from the @spec, the intention here is that the mochiweb application should indeed successfully stop, but the code above does not ensure that. In fact, there is no guarantee that the function will return 'ok' as its spec says. Also, there may be an error when trying to stop the crypto application. To ensure these two, and shut off this warning, the following code is better:

%% @spec stop() -> ok
%% @doc Stop the MochiWeb server.
stop() ->
    ok = application:stop(mochiweb),
    ok = application:stop(crypto).

Of course, it might just be that the @spec is wrong here, and this function should actually return the result of the application:stop(mochiweb) call and ignore the return value of application:stop(crypto). In such a case, the following code is better:

%% @spec stop() -> ok | {error, term()}
%% @doc Stop the MochiWeb server.
stop() ->
    Res = application:stop(mochiweb),
    _ = application:stop(crypto),
    Res.

Anyway, you may want to review all these places and fix them appropriately.

I can help in this, if you want me to.

Kostis

[norton]

The following dialyzer errors are present with version 2.0.0

mochiglobal.erl:33: Expression produces a value of type {'error','badfile' | 'native_code' | 'nofile' | 'not_purged' | 'stickydirectory'} | {'module',atom()}, but this value is unmatched mochiweb.erl:23: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:99: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:103: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:106: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:129: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:132: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebhttp.erl:169: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiweb_request.erl:224: Expression produces a value of type 'ok' | {'mochiwebresponse',,,}, but this value is unmatched mochiweb_request.erl:402: Expression produces a value of type [any()], but this value is unmatched mochiwebrequest.erl:501: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebrequest.erl:504: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebrequest.erl:518: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiwebrequest.erl:530: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiweb_request.erl:615: Expression produces a value of type 'ok' | {'error',atom()}, but this value is unmatched mochiweb_socketserver.erl:120: Expression produces a value of type 'ok' | 'void' | {'error',}, but this value is unmatched mochiweb_socketserver.erl:122: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched mochiweb_socketserver.erl:123: Expression produces a value of type 'ok' | {'error',}, but this value is unmatched reloader.erl:62: Expression produces a value of type ['error' | 'gone' | 'reload' | 'reload_but_test_failed' | 'unmodified'], but this value is unmatched

[dreid]

@norton could you please post the full dialyzer command you used to get all those warnings? Currently make dialyze (which runs ./rebar dialyze) generates no output.

[norton]

I manually run dialyzer as part of project that uses mochiweb's json encoder/decoder. The recipe is contained in this Makefile:

https://github.com/hibari/hibari/blob/master/Makefile

The ignored dialyze warnings are manually kept inside the dialyze-ignore-warnings.txt file in the same directory as the above Makefile.

How to repeat:

$ wget -O - http://android.git.kernel.org/repo > ~/bin/repo
$ chmod a+x ~/bin/repo
$ mkdir workdir
$ cd workdir 
$ yes | repo init -q -u git://github.com/hibari/manifests.git -m hibari-default.xml 
$ repo sync -q
$ cd hibari
$ make

To build the plt (required once): $ make build-plt

To run dialyzer: $ make dialyze

To run dialyze and ignore specs: $ make dialyze-nospec

[kostis]

As I wrote in the mail that started this thread, one can get the unmatched return warnings by running

dialyzer ebin -Wunmatched_returns

If you want to stick to using 'rebar', perhaps the following might help. Add to your rebar.config file the line:

{dialyzer_opts, [{warnings,[unmatched_returns]}]}.

Then I think you can get them with 'rebar dialyze' too.

My original comment explains the rewrite that one needs to do to shut off some of them. Let me know if you want help in some other ones.

[dreid]

@kostis yeah thanks, for some reason I thought @norton's output included more than just unmatched_returns.

It'd also be helpful if you could over my branch which fixes the unmatched_returns. Comparison view is here: https://github.com/mochi/mochiweb/compare/master...dr-dialyzer-more-warnings

[kostis]

Allow me some comments. There was a change from:

  case Ssl of
    true ->
      application:start(crypto),
      application:start(public_key),
      application:start(ssl);
    false ->
      void
  end,

to

  _ = case Ssl of
            true ->
               ok = mochiweb:ensure_started(crypto),
               ok = mochiweb:ensure_started(public_key),
               ok = mochiweb:ensure_started(ssl);
            false ->
               void
       end,

this shuts off the warning alright, but why return something different than 'ok' in the false branch? If you return 'ok' there too, you actually do not need the underscore (dialyzer does not complain when only a non-structured value like 'ok' is unmatched). You can then write it as:

   case Ssl of
       true ->
          ok = mochiweb:ensure_started(crypto),
          ok = mochiweb:ensure_started(public_key),
          ok = mochiweb:ensure_started(ssl);
        false ->
           ok
    end,

Try to do a similar thing in:

   _ = case Expect of
             "100-continue" ->
                 start_raw_response({100, gb_trees:empty()});
            _Else ->
                 ok
         end,

either by modifying the return type of start_raw_response/1 or by writing the code as:

   case Expect of
       "100-continue" ->
           _ = start_raw_response({100, gb_trees:empty()}),
           ok;
        _Else ->
            ok
    end,

which is nicer in my opinion.

Finally, the following, in the same file as the one above:

  [erase(K) || K  - [?SAVE_QS,
                     ?SAVE_PATH,
                     ?SAVE_RECV,
                     ?SAVE_BODY,
                     ?SAVE_BODY_LENGTH,
                     ?SAVE_POST,
                     ?SAVE_COOKIE,
                     ?SAVE_FORCE_CLOSE]],

is more kosher written using lists:foreach/2, as in:

  L = [?SAVE_QS, ?SAVE_PATH, ?SAVE_RECV, ?SAVE_BODY,
       ?SAVE_BODY_LENGTH, ?SAVE_POST, ?SAVE_COOKIE,
       ?SAVE_FORCE_CLOSE],
  lists:foreach(fun (K) -> erase(K) end, L)

[dreid]

Thank you @kostis, at 2am I was incapable of coming up with better refactorings for those sections.

[dreid]

I've now pushed these fixes in 61a8c55854572fac59fce2fc9fbb7221af3b7b66

[norton]

Thanks. I'll test it out.