apache / couchdb

Seamless multi-master syncing database with an intuitive HTTP/JSON API, designed for reliability
https://couchdb.apache.org/
Apache License 2.0
6.28k stars 1.03k forks source link

Add type specs to clouseau_rpc #5309

Closed iilyak closed 1 month ago

iilyak commented 1 month ago

Overview

This PR serves as a code documentation since we don't run type checks during CI. It doesn't modify existing code just adding type specs.

Testing recommendations

Since we don't run dyalizer during CI and given the fact that it fails on main. There is no good way to test correctness of the spec other than comparing the output of dilayzer on main and on the current branch.

on main branch

couchdb on *main* 
❯ make
...
couchdb on *main*
❯ make dialyze apps=dreyfus
==> dreyfus (build-plt)
WARN:  ''build-plt'' command does not apply to directory /Users/iilyak/Code/couchdb/rel
WARN:  ''build-plt'' command does not apply to directory /Users/iilyak/Code/couchdb
==> dreyfus (dialyze)
src/dreyfus_bookmark.erl:61:29: Record construction
          #shard{name :: '_', ref :: '_', opts :: '_'} violates the declared type of field opts ::
          'undefined' | [any()]
src/dreyfus_httpd.erl:59:21: The pattern
          {'ok', Bookmark0, TotalHits, Hits0} can never match the type
          {'error', _} | {'ok', _, _, _, _, _}
src/dreyfus_index.erl:110:1: The inferred return type of init/1
         ('ok') has nothing in common with
          'ignore' |
          {'ok', _} |
          {'stop', _} |
          {'ok', _,
           'hibernate' | 'infinity' |
           non_neg_integer() |
           {'continue', _}}, which is the expected return type for the callback of the gen_server behaviour
src/dreyfus_index.erl:152:36: The created fun has no local return
src/dreyfus_index.erl:231:36: The created fun has no local return
src/dreyfus_index_updater.erl:23:1: Function update/2 has no local return
ERROR: dialyze failed while processing /Users/iilyak/Code/couchdb/src/dreyfus: rebar_abort
make: *** [dialyze] Error 1

on add-specs-to-clouseau_rpc branch

couchdb on *add-specs-to-clouseau_rpc* 
❯ make
...
couchdb on *add-specs-to-clouseau_rpc*
❯ make dialyze apps=dreyfus
==> dreyfus (build-plt)
WARN:  ''build-plt'' command does not apply to directory /Users/iilyak/Code/couchdb/rel
WARN:  ''build-plt'' command does not apply to directory /Users/iilyak/Code/couchdb
==> dreyfus (dialyze)
src/dreyfus_bookmark.erl:61:29: Record construction
          #shard{name :: '_', ref :: '_', opts :: '_'} violates the declared type of field opts ::
          'undefined' | [any()]
src/dreyfus_httpd.erl:59:21: The pattern
          {'ok', Bookmark0, TotalHits, Hits0} can never match the type
          {'error', _} | {'ok', _, _, _, _, _}
src/dreyfus_index.erl:110:1: The inferred return type of init/1
         ('ok') has nothing in common with
          'ignore' |
          {'ok', _} |
          {'stop', _} |
          {'ok', _,
           'hibernate' | 'infinity' |
           non_neg_integer() |
           {'continue', _}}, which is the expected return type for the callback of the gen_server behaviour
src/dreyfus_index.erl:152:36: The created fun has no local return
src/dreyfus_index.erl:231:36: The created fun has no local return
src/dreyfus_index.erl:297:17: The variable Error can never match since previous clauses completely covered the type
          {'ok', non_neg_integer()}
src/dreyfus_index_updater.erl:23:1: Function update/2 has no local return
ERROR: dialyze failed while processing /Users/iilyak/Code/couchdb/src/dreyfus: rebar_abort
make: *** [dialyze] Error 1

Related Issues or Pull Requests

Checklist

iilyak commented 1 month ago

Very nice, @iilyak !

I noticed it has one extra error now after the types were added.

src/dreyfus_index.erl:297:17: The variable Error can never match since previous clauses completely covered the type
          {'ok', non_neg_integer()}

I wonder why that is? It would be nice if adding more types would reduce the discrepancies and show less errors not more.

This has been fixed by changing return type.

-          {'ok', non_neg_integer()}
+          {'ok', non_neg_integer()} | error()
nickva commented 1 month ago

This has been fixed by changing return type.

-          {'ok', non_neg_integer()}
+          {'ok', non_neg_integer()} | error()

Ah, very nice!