Fix Alisdair's review, add more types and docs

src/rebar_app_discover.erl

@@ -13,7 +13,9 @@
 -include("rebar.hrl").
 -include_lib("providers/include/providers.hrl").
 
-%% @doc from the base directory, 
+%% @doc from the base directory, find all the applications
+%% at the top level and their dependencies based on the configuration
+%% and profile information.
 -spec do(rebar_state:t(), [file:filename()]) -> rebar_state:t().
 do(State, LibDirs) ->
     BaseDir = rebar_state:dir(State),
@@ -158,7 +160,7 @@ project_app_config(AppInfo, State) ->
     Opts = maybe_reset_hooks(Dir, rebar_state:opts(State), State),
     {C, rebar_state:opts(State, Opts)}.
 
-%% @doc Here we check if the app is at the root of the project.
+%% @private Check if the app is at the root of the project.
 %% If it is, then drop the hooks from the config so they aren't run twice
 -spec maybe_reset_hooks(file:filename(), Opts, rebar_state:t()) -> Opts when
       Opts :: rebar_dict().

src/rebar_config.erl

@@ -125,9 +125,9 @@ write_lock_file(LockFile, Locks) ->
                                            format_attrs(Attrs)]))
     end.
 
-%% @private Attributes have a special formatting to ensure there's only one per
-%% line in terms of pkg_hash, so we disturb source diffing as little
-%% as possible.
+%% @private Because attributes for packages are fairly large, there is the need
+%% for a special formatting to ensure there's only one entry per lock file
+%% line and that diffs are generally stable.
 -spec format_attrs([term()]) -> iodata().
 format_attrs([]) -> [];
 format_attrs([{pkg_hash, Vals}|T]) ->

src/rebar_digraph.erl

@@ -1,3 +1,5 @@
+%%% @doc build a digraph of applications in order to figure out dependency
+%%% and compile order.
 -module(rebar_digraph).
 
 -export([compile_order/1
@@ -7,7 +9,9 @@
 
 -include("rebar.hrl").
 
-%% Sort apps with topological sort to get proper build order
+%% @doc Sort apps with topological sort to get proper build order
+-spec compile_order([rebar_app_info:t()]) ->
+    {ok, [rebar_app_info:t()]} | {error, no_sort | {cycles, [[binary(),...]]}}.
 compile_order(Apps) ->
     Graph = digraph:new(),
     lists:foreach(fun(App) ->
@@ -33,6 +37,11 @@ compile_order(Apps) ->
     true = digraph:delete(Graph),
     Order.
 
+%% @private Add a package and its dependencies to an existing digraph
+-spec add(digraph:graph(), {PkgName, [Dep]}) -> ok when
+      PkgName :: binary(),
+      Dep :: {Name, term()} | Name,
+      Name :: atom() | iodata().
 add(Graph, {PkgName, Deps}) ->
     case digraph:vertex(Graph, PkgName) of
         false ->
@@ -57,6 +66,8 @@ add(Graph, {PkgName, Deps}) ->
                           digraph:add_edge(Graph, V, V3)
                   end, Deps).
 
+%% @doc based on a list of vertices and edges, build a digraph.
+-spec restore_graph({[digraph:vertex()], [digraph:edge()]}) -> digraph:graph().
 restore_graph({Vs, Es}) ->
     Graph = digraph:new(),
     lists:foreach(fun({V, LastUpdated}) ->
@@ -67,6 +78,8 @@ restore_graph({Vs, Es}) ->
                   end, Es),
     Graph.
 
+%% @doc convert a given exception's payload into an io description.
+-spec format_error(any()) -> iolist().
 format_error(no_solution) ->
     io_lib:format("No solution for packages found.", []).
 
@@ -74,22 +87,27 @@ format_error(no_solution) ->
 %% Internal Functions
 %%====================================================================
 
+%% @doc alias for `digraph_utils:subgraph/2'.
 subgraph(Graph, Vertices) ->
     digraph_utils:subgraph(Graph, Vertices).
 
+%% @private from a list of app names, fetch the proper app info records
+%% for them.
 -spec names_to_apps([atom()], [rebar_app_info:t()]) -> [rebar_app_info:t()].
 names_to_apps(Names, Apps) ->
     [element(2, App) || App <- [find_app_by_name(Name, Apps) || Name <- Names], App =/= error].
 
+%% @private fetch the proper app info record for a given app name.
 -spec find_app_by_name(atom(), [rebar_app_info:t()]) -> {ok, rebar_app_info:t()} | error.
 find_app_by_name(Name, Apps) ->
     ec_lists:find(fun(App) ->
                           rebar_app_info:name(App) =:= Name
                   end, Apps).
 
-%% The union of all entries in the applications list for an app and
+%% @private The union of all entries in the applications list for an app and
 %% the deps listed in its rebar.config is all deps that may be needed
 %% for building the app.
+-spec all_apps_deps(rebar_app_info:t()) -> [binary()].
 all_apps_deps(App) ->
     Applications = lists:usort([atom_to_binary(X, utf8) || X <- rebar_app_info:applications(App)]),
     Deps = lists:usort(lists:map(fun({Name, _}) -> Name; (Name) -> Name end, rebar_app_info:deps(App))),

src/rebar_dir.erl

@@ -1,3 +1,4 @@
+%%% @doc utility functions for directory and path handling of all kind.
 -module(rebar_dir).
 
 -export([base_dir/1,
@@ -29,10 +30,14 @@
 
 -include("rebar.hrl").
 
+%% @doc returns the directory root for build artifacts
+%% for the current profile, such as `_build/default/'.
 -spec base_dir(rebar_state:t()) -> file:filename_all().
 base_dir(State) ->
     profile_dir(rebar_state:opts(State), rebar_state:current_profiles(State)).
 
+%% @doc returns the directory root for build artifacts for a given set
+%% of profiles.
 -spec profile_dir(rebar_dict(), [atom()]) -> file:filename_all().
 profile_dir(Opts, Profiles) ->
     {BaseDir, ProfilesStrings} = case [ec_cnv:to_list(P) || P <- Profiles] of
@@ -46,25 +51,36 @@ profile_dir(Opts, Profiles) ->
     ProfilesDir = string:join(ProfilesStrings, "+"),
     filename:join(BaseDir, ProfilesDir).
 
+%% @doc returns the directory where dependencies should be placed
+%% given the current profile.
 -spec deps_dir(rebar_state:t()) -> file:filename_all().
 deps_dir(State) ->
     filename:join(base_dir(State), rebar_state:get(State, deps_dir, ?DEFAULT_DEPS_DIR)).
 
+%% @doc returns the directory where a dependency should be placed
+%% given the current profile, based on its app name. Expects to be passed
+%% the result of `deps_dir/1' as a first argument.
 -spec deps_dir(file:filename_all(), file:filename_all()) -> file:filename_all().
 deps_dir(DepsDir, App) ->
     filename:join(DepsDir, App).
 
+%% @doc returns the absolute path for the project root (by default,
+%% the current working directory for the currently running escript).
 root_dir(State) ->
     filename:absname(rebar_state:get(State, root_dir, ?DEFAULT_ROOT_DIR)).
 
+%% @doc returns the expected location of the `_checkouts' directory.
 -spec checkouts_dir(rebar_state:t()) -> file:filename_all().
 checkouts_dir(State) ->
     filename:join(root_dir(State), rebar_state:get(State, checkouts_dir, ?DEFAULT_CHECKOUTS_DIR)).
 
+%% @doc returns the expected location of a given app in the checkouts
+%% directory for the project.
 -spec checkouts_dir(rebar_state:t(), file:filename_all()) -> file:filename_all().
 checkouts_dir(State, App) ->
     filename:join(checkouts_dir(State), App).
 
+%% @doc Returns the directory where plugins are located.
 -spec plugins_dir(rebar_state:t()) -> file:filename_all().
 plugins_dir(State) ->
     case lists:member(global, rebar_state:current_profiles(State)) of
@@ -74,33 +90,50 @@ plugins_dir(State) ->
             filename:join(base_dir(State), rebar_state:get(State, plugins_dir, ?DEFAULT_PLUGINS_DIR))
     end.
 
+%% @doc returns the list of relative path where the project applications can
+%% be located.
 -spec lib_dirs(rebar_state:t()) -> file:filename_all().
 lib_dirs(State) ->
     rebar_state:get(State, project_app_dirs, ?DEFAULT_PROJECT_APP_DIRS).
 
+%% @doc returns the user's home directory.
+-spec home_dir() -> file:filename_all().
 home_dir() ->
     {ok, [[Home]]} = init:get_argument(home),
     Home.
 
+%% @doc returns the directory where the global configuration files for rebar3
+%% may be stored.
+-spec global_config_dir(rebar_state:t()) -> file:filename_all().
 global_config_dir(State) ->
     Home = home_dir(),
     rebar_state:get(State, global_rebar_dir, filename:join([Home, ".config", "rebar3"])).
 
+%% @doc returns the path of the global rebar.config file
+-spec global_config(rebar_state:t()) -> file:filename_all().
 global_config(State) ->
     filename:join(global_config_dir(State), "rebar.config").
 
+%% @doc returns the default path of the global rebar.config file
+-spec global_config() -> file:filename_all().
 global_config() ->
     Home = home_dir(),
     filename:join([Home, ".config", "rebar3", "rebar.config"]).
 
+%% @doc returns the location for the global cache directory
 -spec global_cache_dir(rebar_dict()) -> file:filename_all().
 global_cache_dir(Opts) ->
     Home = home_dir(),
     rebar_opts:get(Opts, global_rebar_dir, filename:join([Home, ".cache", "rebar3"])).
 
+%% @doc appends the cache directory to the path passed to this function.
+-spec local_cache_dir(file:filename_all()) -> file:filename_all().
 local_cache_dir(Dir) ->
     filename:join(Dir, ".rebar3").
 
+%% @doc returns the current working directory, with some specific
+%% conversions and handling done to be cross-platform compatible.
+-spec get_cwd() -> file:filename_all().
 get_cwd() ->
     {ok, Dir} = file:get_cwd(),
     %% On windows cwd may return capital letter for drive,
@@ -109,9 +142,14 @@ get_cwd() ->
     %% cwd as soon as it possible.
     filename:join([Dir]).
 
+%% @doc returns the file location for the global template
+%% configuration variables file.
+-spec template_globals(rebar_state:t()) -> file:filename_all().
 template_globals(State) ->
     filename:join([global_config_dir(State), "templates", "globals"]).
 
+%% @doc returns the location for the global template directory
+-spec template_dir(rebar_state:t()) -> file:filename_all().
 template_dir(State) ->
     filename:join([global_config_dir(State), "templates"]).
 
@@ -129,6 +167,8 @@ processing_base_dir(State, Dir) ->
     AbsDir = filename:absname(Dir),
     AbsDir =:= rebar_state:get(State, base_dir).
 
+%% @doc make a path absolute
+-spec make_absolute_path(file:filename()) -> file:filename().
 make_absolute_path(Path) ->
     case filename:pathtype(Path) of
         absolute ->
@@ -142,11 +182,16 @@ make_absolute_path(Path) ->
             filename:join([Dir, Path])
     end.
 
+%% @doc normalizing a path removes all of the `..' and the
+%% `.' segments it may contain.
+-spec make_normalized_path(file:filename()) -> file:filename().
 make_normalized_path(Path) ->
     AbsPath = make_absolute_path(Path),
     Components = filename:split(AbsPath),
     make_normalized_path(Components, []).
 
+%% @private drops path fragments for normalization
+-spec make_normalized_path([string()], [string()]) -> file:filename().
 make_normalized_path([], NormalizedPath) ->
     filename:join(lists:reverse(NormalizedPath));
 make_normalized_path([H|T], NormalizedPath) ->
@@ -181,37 +226,42 @@ do_make_relative_path(Source, Target) ->
     Base = lists:duplicate(max(length(Target) - 1, 0), ".."),
     filename:join(Base ++ Source).
 
-%%%-----------------------------------------------------------------
-%%% 'src_dirs' and 'extra_src_dirs' can be configured with options
+%%% @doc
+%%% `src_dirs' and `extra_src_dirs' can be configured with options
 %%% like this:
-%%%
+%%% ```
 %%% {src_dirs,[{"foo",[{recursive,false}]}]}
 %%% {extra_src_dirs,[{"bar",[recursive]}]} (equivalent to {recursive,true})
-%%%
-%%% src_dirs/1,2 and extra_src_dirs/1,2 return only the list of
-%%% directories for the 'src_dirs' and 'extra_src_dirs' options
-%%% respectively, while src_dirs_opts/2 return the options list for
-%%% the given directory, no matter if it is configured as 'src_dirs' or
-%%% 'extra_src_dirs'.
-%%%
+%%% '''
+%%% `src_dirs/1,2' and `extra_src_dirs/1,2' return only the list of
+%%% directories for the `src_dirs' and `extra_src_dirs' options
+%%% respectively, while `src_dirs_opts/2' returns the options list for
+%%% the given directory, no matter if it is configured as `src_dirs' or
+%%% `extra_src_dirs'.
 -spec src_dirs(rebar_dict()) -> list(file:filename_all()).
 src_dirs(Opts) -> src_dirs(Opts, []).
 
+%% @doc same as `src_dirs/1', but allows to pass in a list of default options.
 -spec src_dirs(rebar_dict(), list(file:filename_all())) -> list(file:filename_all()).
 src_dirs(Opts, Default) ->
     src_dirs(src_dirs, Opts, Default).
 
+%% @doc same as `src_dirs/1', but for the `extra_src_dirs' options
 -spec extra_src_dirs(rebar_dict()) -> list(file:filename_all()).
 extra_src_dirs(Opts) -> extra_src_dirs(Opts, []).
 
+%% @doc same as `src_dirs/2', but for the `extra_src_dirs' options
 -spec extra_src_dirs(rebar_dict(), list(file:filename_all())) -> list(file:filename_all()).
 extra_src_dirs(Opts, Default) ->
     src_dirs(extra_src_dirs, Opts, Default).
 
+%% @private agnostic version of src_dirs and extra_src_dirs.
 src_dirs(Type, Opts, Default) ->
     lists:usort([case D0 of {D,_} -> D; _ -> D0 end ||
                     D0 <- raw_src_dirs(Type,Opts,Default)]).
 
+%% @private extracts the un-formatted src_dirs or extra_src_dirs
+%% options as configured.
 raw_src_dirs(Type, Opts, Default) ->
     ErlOpts = rebar_opts:erl_opts(Opts),
     Vs = proplists:get_all_values(Type, ErlOpts),
@@ -220,19 +270,23 @@ raw_src_dirs(Type, Opts, Default) ->
         Dirs -> Dirs
     end.
 
+%% @doc returns all the source directories (`src_dirs' and
+%% `extra_src_dirs').
 -spec all_src_dirs(rebar_dict()) -> list(file:filename_all()).
 all_src_dirs(Opts) -> all_src_dirs(Opts, [], []).
 
+%% @doc returns all the source directories (`src_dirs' and
+%% `extra_src_dirs') while being able to configure defaults for both.
 -spec all_src_dirs(rebar_dict(), list(file:filename_all()), list(file:filename_all())) ->
     list(file:filename_all()).
 all_src_dirs(Opts, SrcDefault, ExtraDefault) ->
     lists:usort(src_dirs(Opts, SrcDefault) ++ extra_src_dirs(Opts, ExtraDefault)).
 
-%%%-----------------------------------------------------------------
+%%% @doc
 %%% Return the list of options for the given src directory
 %%% If the same option is given multiple times for a directory in the
-%%% config, the priority order is: first occurence of 'src_dirs'
-%%% followed by first occurence of 'extra_src_dirs'.
+%%% config, the priority order is: first occurence of `src_dirs'
+%%% followed by first occurence of `extra_src_dirs'.
 -spec src_dir_opts(rebar_dict(), file:filename_all()) -> [{atom(),term()}].
 src_dir_opts(Opts, Dir) ->
     RawSrcDirs = raw_src_dirs(src_dirs, Opts, []),
@@ -241,7 +295,7 @@ src_dir_opts(Opts, Dir) ->
                       D==Dir],
     lists:ukeysort(1,proplists:unfold(lists:append(AllOpts))).
 
-%%%-----------------------------------------------------------------
+%%% @doc
 %%% Return the value of the 'recursive' option for the given directory.
 %%% If not given, the value of 'recursive' in the 'erlc_compiler'
 %%% options is used, and finally the default is 'true'.
@@ -254,16 +308,17 @@ recursive(Opts, Dir) ->
     R = proplists:get_value(recursive, DirOpts, Default),
     R.
 
-%% given a path if that path is an ancestor of an app dir return the path relative to that
-%% apps outdir. if the path is not an ancestor to any app dirs but is an ancestor of the
-%% project root return the path relative to the project base_dir. if it is not an ancestor
+%% @doc given a path if that path is an ancestor of an app dir, return the path relative to that
+%% apps outdir. If the path is not an ancestor to any app dirs but is an ancestor of the
+%% project root, return the path relative to the project base_dir. If it is not an ancestor
 %% of either return it unmodified
 -spec retarget_path(rebar_state:t(), string()) -> string().
-
 retarget_path(State, Path) ->
     ProjectApps = rebar_state:project_apps(State),
     retarget_path(State, Path, ProjectApps).
 
+%% @private worker for retarget_path/2
+%% @end
 %% not relative to any apps in project, check to see it's relative to
 %% project root
 retarget_path(State, Path, []) ->

src/rebar_dist_utils.erl

@@ -7,6 +7,9 @@
 %%%%%%%%%%%%%%%%%%
 %%% PUBLIC API %%%
 %%%%%%%%%%%%%%%%%%
+
+%% @doc allows to pick whether to use a short or long name, and
+%% starts the distributed mode for it.
 -spec either(Name::atom(), SName::atom(), Opts::[{setcookie,term()}]) -> atom().
 either(undefined, undefined, _) ->
     'nonode@nohost';
@@ -19,12 +22,18 @@ either(undefined, SName, Opts) ->
 either(_, _, _) ->
     ?ABORT("Cannot have both short and long node names defined", []).
 
+%% @doc starts a node with a short name.
+-spec short(SName::atom(), Opts::[{setcookie,term()}]) -> term().
 short(Name, Opts) ->
     start(Name, shortnames, Opts).
 
+%% @doc starts a node with a long name.
+-spec long(Name::atom(), Opts::[{setcookie,term()}]) -> term().
 long(Name, Opts) ->
     start(Name, longnames, Opts).
 
+%% @doc utility function to extract all distribution options
+%% from a rebar3 state tuple.
 -spec find_options(rebar_state:t()) -> {Long, Short, Opts} when
       Long :: atom(),
       Short :: atom(),