From e024778599c71559f97003014ae8457a19e01256 Mon Sep 17 00:00:00 2001 From: Kostis Sagonas Date: Sun, 10 Oct 2010 14:11:13 -0600 Subject: [PATCH] Dialyzer related cleanups --- src/getopt.erl | 4 ++-- src/mustache.erl | 4 ++-- src/rebar_cleaner.erl | 3 +-- src/rebar_core.erl | 6 +++--- src/rebar_deps.erl | 25 ++++++++++++---------- src/rebar_erlc_compiler.erl | 4 ++-- src/rebar_eunit.erl | 35 ++++++++++++++++++------------- src/rebar_otp_app.erl | 5 ++--- src/rebar_protobuffs_compiler.erl | 2 +- src/rebar_reltool.erl | 2 +- src/rebar_templater.erl | 8 ++++--- src/rebar_xref.erl | 24 ++++++++++----------- 12 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/getopt.erl b/src/getopt.erl index 454a96e..bb7fae2 100644 --- a/src/getopt.erl +++ b/src/getopt.erl @@ -285,7 +285,7 @@ arg_spec_type(Type) when is_atom(Type) -> %% @doc Convert an argument string to its corresponding data type. --spec to_type(atom(), string()) -> arg_value(). +-spec to_type(arg_type(), string()) -> arg_value(). to_type(binary, Arg) -> list_to_binary(Arg); to_type(atom, Arg) -> @@ -327,7 +327,7 @@ is_arg_false(Arg) -> (Arg =:= "0"). --spec is_valid_arg(arg_spec() | arg_type(), string()) -> boolean(). +-spec is_valid_arg(arg_spec(), nonempty_string()) -> boolean(). is_valid_arg({Type, _DefaultArg}, Arg) -> is_valid_arg(Type, Arg); is_valid_arg(boolean, Arg) -> diff --git a/src/mustache.erl b/src/mustache.erl index eca17cd..a713bd8 100644 --- a/src/mustache.erl +++ b/src/mustache.erl @@ -47,7 +47,7 @@ compile(Mod) -> compile(Mod, File) -> code:purge(Mod), - code:load_file(Mod), + {module, _} = code:load_file(Mod), {ok, TemplateBin} = file:read_file(File), Template = re:replace(TemplateBin, "\"", "\\\\\"", [global, {return,list}]), State = #mstate{mod = Mod}, @@ -216,4 +216,4 @@ escape([X | Rest], Acc) -> start([T]) -> Out = render(list_to_atom(T)), io:format(Out ++ "~n", []). - \ No newline at end of file + diff --git a/src/rebar_cleaner.erl b/src/rebar_cleaner.erl index bdeefe5..2ff828e 100644 --- a/src/rebar_cleaner.erl +++ b/src/rebar_cleaner.erl @@ -36,5 +36,4 @@ clean(Config, _AppFile) -> %% Get a list of files to delete from config and remove them FilesToClean = rebar_config:get(Config, clean_files, []), - [rebar_file_utils:rm_rf(F) || F <- FilesToClean], - ok. + lists:foreach(fun (F) -> rebar_file_utils:rm_rf(F) end, FilesToClean). diff --git a/src/rebar_core.erl b/src/rebar_core.erl index bbc104b..bf37c87 100644 --- a/src/rebar_core.erl +++ b/src/rebar_core.erl @@ -63,7 +63,7 @@ run(RawArgs) -> Commands = parse_args(RawArgs), %% Make sure crypto is running - crypto:start(), + ok = crypto:start(), %% Initialize logging system rebar_log:init(), @@ -286,10 +286,10 @@ process_commands([]) -> end; process_commands([Command | Rest]) -> %% Reset skip dirs - [erlang:erase({skip_dir, D}) || D <- skip_dirs()], + lists:foreach(fun (D) -> erlang:erase({skip_dir, D}) end, skip_dirs()), Operations = erlang:get(operations), - process_dir(rebar_utils:get_cwd(), rebar_config:new(), Command, sets:new()), + _ = process_dir(rebar_utils:get_cwd(), rebar_config:new(), Command, sets:new()), case erlang:get(operations) of Operations -> %% This command didn't do anything diff --git a/src/rebar_deps.erl b/src/rebar_deps.erl index a79e485..ef1ee22 100644 --- a/src/rebar_deps.erl +++ b/src/rebar_deps.erl @@ -68,7 +68,9 @@ preprocess(Config, _) -> %% WILL run (and we want it to) for transitivity purposes. case rebar_config:get_global(skip_deps, false) of "true" -> - [rebar_core:skip_dir(D#dep.dir) || D <- AvailableDeps]; + lists:foreach(fun (#dep{dir = Dir}) -> + rebar_core:skip_dir(Dir) + end, AvailableDeps); _ -> ok end, @@ -97,9 +99,10 @@ compile(Config, AppFile) -> %% No missing deps ok; {_, MissingDeps} -> - [?CONSOLE("Dependency not available: ~p-~s (~p)\n", - [D#dep.app, D#dep.vsn_regex, D#dep.source]) || - D <- MissingDeps], + lists:foreach(fun (#dep{app=App, vsn_regex=Vsn, source=Src}) -> + ?CONSOLE("Dependency not available: ~p-~s (~p)\n", + [App, Vsn, Src]) + end, MissingDeps), ?FAIL end. @@ -121,8 +124,8 @@ compile(Config, AppFile) -> DepsDir = get_deps_dir(), Deps = rebar_config:get_local(Config, deps, []), {AvailableDeps, _} = find_deps(Deps), - [delete_dep(D) || D <- AvailableDeps, - lists:prefix(DepsDir, D#dep.dir) == true], + _ = [delete_dep(D) || D <- AvailableDeps, + lists:prefix(DepsDir, D#dep.dir) == true], ok. @@ -149,9 +152,9 @@ update_deps_code_path([]) -> update_deps_code_path([Dep | Rest]) -> case is_app_available(Dep#dep.app, Dep#dep.vsn_regex, Dep#dep.dir) of {true, _} -> - code:add_patha(filename:join(Dep#dep.dir, "ebin")); + true = code:add_patha(filename:join(Dep#dep.dir, "ebin")); false -> - ok + true end, update_deps_code_path(Rest). @@ -246,9 +249,9 @@ use_source(Dep, Count) -> %% Already downloaded -- verify the versioning matches up with our regex case is_app_available(Dep#dep.app, Dep#dep.vsn_regex, Dep#dep.dir) of {true, _} -> - %% Available version matches up -- we're good to go; add the - %% app dir to our code path - code:add_patha(filename:join(Dep#dep.dir, "ebin")), + %% Available version matches up -- we're good to go; + %% add the app dir to our code path + true = code:add_patha(filename:join(Dep#dep.dir, "ebin")), Dep; false -> %% The app that was downloaded doesn't match up (or had diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index a9c652f..dc80e4f 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -147,7 +147,7 @@ include_path(Source, Config) -> ErlOpts = rebar_config:get(Config, erl_opts, []), ["include", filename:dirname(Source)] ++ proplists:get_all_values(i, ErlOpts). --spec inspect(Source::string(), IncludePath::[string()]) -> {string(), [string()]}. +-spec inspect(Source::string(), IncludePath::[string(),...]) -> {string(), [string()]}. inspect(Source, IncludePath) -> ModuleDefault = filename:basename(Source, ".erl"), case epp:open(Source, IncludePath) of @@ -281,7 +281,7 @@ gather_src([], Srcs) -> gather_src([Dir|Rest], Srcs) -> gather_src(Rest, Srcs ++ rebar_utils:find_files(Dir, ".*\\.erl\$")). --spec src_dirs(SrcDirs::[string()]) -> [string()]. +-spec src_dirs(SrcDirs::[string()]) -> [string(),...]. src_dirs([]) -> ["src"]; src_dirs(SrcDirs) -> diff --git a/src/rebar_eunit.erl b/src/rebar_eunit.erl index d5b69d7..9f2baf1 100644 --- a/src/rebar_eunit.erl +++ b/src/rebar_eunit.erl @@ -123,7 +123,7 @@ eunit(Config, AppFile) -> end, %% Restore code path - code:set_path(CodePath), + true = code:set_path(CodePath), ok. clean(_Config, _File) -> @@ -148,12 +148,12 @@ perform_eunit(Config, Modules) -> %% Move down into ?EUNIT_DIR while we run tests so any generated files %% are created there (versus in the source dir) Cwd = rebar_utils:get_cwd(), - file:set_cwd(?EUNIT_DIR), + ok = file:set_cwd(?EUNIT_DIR), EunitResult = perform_eunit(EunitOpts, Modules, Suite), %% Return to original working dir - file:set_cwd(Cwd), + ok = file:set_cwd(Cwd), EunitResult. @@ -225,7 +225,9 @@ cover_analyze(Config, Modules, SrcModules) -> cover_write_index(lists:sort(Coverage), SrcModules), %% Write coverage details for each file - [{ok, _} = cover:analyze_to_file(M, cover_file(M), [html]) || {M, _, _} <- Coverage], + lists:foreach(fun({M, _, _}) -> + {ok, _} = cover:analyze_to_file(M, cover_file(M), [html]) + end, Coverage), Index = filename:join([rebar_utils:get_cwd(), ?EUNIT_DIR, "index.html"]), ?CONSOLE("Cover analysis: ~s\n", [Index]), @@ -255,11 +257,11 @@ cover_init(true, BeamFiles) -> _ -> %% At least one module compiled successfully - %% It's not an error for cover compilation to fail partially, but we do want - %% to warn about them - [?CONSOLE("Cover compilation warning for ~p: ~p", [Beam, Desc]) || {Beam, {error, Desc}} <- Compiled] - end, - ok; + %% It's not an error for cover compilation to fail partially, + %% but we do want to warn about them + _ = [?CONSOLE("Cover compilation warning for ~p: ~p", [Beam, Desc]) || {Beam, {error, Desc}} <- Compiled], + ok + end; cover_init(Config, BeamFiles) -> cover_init(rebar_config:get(Config, cover_enabled, false), BeamFiles). @@ -313,7 +315,7 @@ cover_write_index(Coverage, SrcModules) -> cover_write_index_section(F, "Source", SrcCoverage), cover_write_index_section(F, "Test", TestCoverage), ok = file:write(F, ""), - file:close(F). + ok = file:close(F). cover_write_index_section(_F, _SectionName, []) -> ok; @@ -329,9 +331,10 @@ cover_write_index_section(F, SectionName, Coverage) -> ok = file:write(F, ?FMT("

Total: ~s

\n", [TotalCoverage])), ok = file:write(F, "\n"), - [ok = file:write(F, ?FMT("\n", - [Module, Module, percentage(Cov, NotCov)])) || - {Module, Cov, NotCov} <- Coverage], + lists:foreach(fun({Module, Cov, NotCov}) -> + ok = file:write(F, ?FMT("\n", + [Module, Module, percentage(Cov, NotCov)])) + end, Coverage), ok = file:write(F, "
ModuleCoverage %
~s~s
~s~s
\n"). cover_print_coverage(Coverage) -> @@ -352,8 +355,10 @@ cover_print_coverage(Coverage) -> %% Print the output the console ?CONSOLE("~nCode Coverage:~n", []), - [?CONSOLE("~*s : ~3s~n", - [Width, Mod, percentage(C, N)]) || {Mod, C, N} <- Coverage], + lists:foreach(fun({Mod, C, N}) -> + ?CONSOLE("~*s : ~3s~n", + [Width, Mod, percentage(C, N)]) + end, Coverage), ?CONSOLE("~n~*s : ~s~n", [Width, "Total", TotalCoverage]). cover_file(Module) -> diff --git a/src/rebar_otp_app.erl b/src/rebar_otp_app.erl index 4cac897..0c30062 100644 --- a/src/rebar_otp_app.erl +++ b/src/rebar_otp_app.erl @@ -68,8 +68,7 @@ clean(_Config, File) -> %% If the app file is a .app.src, delete the generated .app file case rebar_app_utils:is_app_src(File) of true -> - file:delete(rebar_app_utils:app_src_to_app(File)), - ok; + file:delete(rebar_app_utils:app_src_to_app(File)); false -> ok end. @@ -94,7 +93,7 @@ preprocess(AppSrcFile) -> ok = file:write_file(AppFile, Spec), %% Make certain that the ebin/ directory is available on the code path - code:add_path(filename:absname(filename:dirname(AppFile))), + true = code:add_path(filename:absname(filename:dirname(AppFile))), AppFile; diff --git a/src/rebar_protobuffs_compiler.erl b/src/rebar_protobuffs_compiler.erl index 83e0bd1..6118562 100644 --- a/src/rebar_protobuffs_compiler.erl +++ b/src/rebar_protobuffs_compiler.erl @@ -110,7 +110,7 @@ compile_each([{Proto, Beam, Hrl} | Rest]) -> %% into the ebin/ and include/ directories respectively %% TODO: Protobuffs really needs to be better about this...sigh. [] = os:cmd(?FMT("mv ~s ebin", [Beam])), - filelib:ensure_dir(filename:join("include", Hrl)), + ok = filelib:ensure_dir(filename:join("include", Hrl)), [] = os:cmd(?FMT("mv ~s include", [Hrl])), ok; Other -> diff --git a/src/rebar_reltool.erl b/src/rebar_reltool.erl index 23b4146..d01dbd0 100644 --- a/src/rebar_reltool.erl +++ b/src/rebar_reltool.erl @@ -267,7 +267,7 @@ execute_overlay([], _Vars, _BaseDir, _TargetDir) -> ok; execute_overlay([{mkdir, Out} | Rest], Vars, BaseDir, TargetDir) -> OutFile = render(filename:join([TargetDir, Out, "dummy"]), Vars), - filelib:ensure_dir(OutFile), + ok = filelib:ensure_dir(OutFile), ?DEBUG("Created dir ~s\n", [filename:dirname(OutFile)]), execute_overlay(Rest, Vars, BaseDir, TargetDir); execute_overlay([{copy, In} | Rest], _Vars, BaseDir, TargetDir) -> diff --git a/src/rebar_templater.erl b/src/rebar_templater.erl index e27c203..56ea1e4 100644 --- a/src/rebar_templater.erl +++ b/src/rebar_templater.erl @@ -58,8 +58,10 @@ %% Build a list of available templates AvailTemplates = find_disk_templates() ++ find_escript_templates(), ?CONSOLE("Available templates:\n", []), - [?CONSOLE("\t* ~s: ~s (~p)\n", [filename:basename(F, ".template"), F, Type]) || - {Type, F} <- AvailTemplates], + _ = [begin + BaseName = filename:basename(F, ".template"), + ?CONSOLE("\t* ~s: ~s (~p)\n", [BaseName, F, Type]) + end || {Type, F} <- AvailTemplates], ok. @@ -244,7 +246,7 @@ write_file(Output, Data, Force) -> %% otherwise just process the next template if Force =:= "1"; FileExists =:= false -> - filelib:ensure_dir(Output), + ok = filelib:ensure_dir(Output), if {Force, FileExists} =:= {"1", true} -> ?CONSOLE("Writing ~s (forcibly overwriting)~n", diff --git a/src/rebar_xref.erl b/src/rebar_xref.erl index 1f67a37..19700d3 100644 --- a/src/rebar_xref.erl +++ b/src/rebar_xref.erl @@ -50,7 +50,7 @@ xref(Config, _) -> %% Save the code path prior to doing anything OrigPath = code:get_path(), - code:add_path(filename:join(rebar_utils:get_cwd(), "ebin")), + true = code:add_path(filename:join(rebar_utils:get_cwd(), "ebin")), %% Get list of xref checks we want to run XrefChecks = rebar_config:get(Config, xref_checks, [exports_not_used, @@ -73,7 +73,7 @@ xref(Config, _) -> end, %% Restore the original code path - code:set_path(OrigPath), + true = code:set_path(OrigPath), ok. @@ -93,9 +93,10 @@ check_undefined_function_calls(_Config) -> {ok, UndefinedCalls0} = xref:analyze(xref, undefined_function_calls), UndefinedCalls = [{find_mfa_source(Caller), format_fa(Caller), format_mfa(Target)} || {Caller, Target} <- UndefinedCalls0], - [?CONSOLE("~s:~w: Warning ~s calls undefined function ~s\n", - [Source, Line, FunStr, Target]) || - {{Source, Line}, FunStr, Target} <- UndefinedCalls], + lists:foreach(fun({{Source, Line}, FunStr, Target}) -> + ?CONSOLE("~s:~w: Warning ~s calls undefined function ~s\n", + [Source, Line, FunStr, Target]) + end, UndefinedCalls), ok. @@ -114,19 +115,18 @@ filter_away_ignored(UnusedExports) -> %% any functions marked to ignore. We then use this list to mask any functions %% marked as unused exports by xref F = fun(Mod) -> - Attrs = ks(attributes, Mod:module_info()), - Ignore = ks(ignore_xref, Attrs), - Callbacks = [B:behaviour_info(callbacks) || B <- ks(behaviour, Attrs)], + Attrs = kf(attributes, Mod:module_info()), + Ignore = kf(ignore_xref, Attrs), + Callbacks = [B:behaviour_info(callbacks) || B <- kf(behaviour, Attrs)], [{Mod, F, A} || {F, A} <- Ignore ++ lists:flatten(Callbacks)] end, AttrIgnore = lists:flatten(lists:map(F, lists:usort([M || {M, _, _} <- UnusedExports]))), [X || X <- UnusedExports, not(lists:member(X, AttrIgnore))]. - -ks(Key, List) -> - case lists:keysearch(Key, 1, List) of - {value, {Key, Value}} -> +kf(Key, List) -> + case lists:keyfind(Key, 1, List) of + {Key, Value} -> Value; false -> []