Skip to content

Commit

Permalink
Fix broken CLI features and add CLI tests (#189)
Browse files Browse the repository at this point in the history
E.g. checking a directory was broken.

Since eunit captures stdout, it's quite hard to test the CLI from
within eunit. Instead, simple CLI tests are added in the Makefile.
  • Loading branch information
zuiderkwast authored Sep 18, 2019
1 parent 31cfcd4 commit eb41bac
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 17 deletions.
33 changes: 31 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,39 @@ clean:
rm -rf _build
rm -f gradualizer

.PHONY: tests
tests:
.PHONY: tests eunit cli-tests
tests: eunit cli-tests

eunit:
rebar3 eunit

cli-tests: escript
# CLI test cases
# 1. When checking a dir; printing filename is the default
./gradualizer test/dir \
|perl -ne 'm%^test/dir/test_in_dir.erl:% or die "CLI 1 ($$_)"'
# 2. --no-print-file with directory
./gradualizer --no-print-file test/dir \
|perl -ne '/^The/ or die "CLI 2 ($$_)"'
# 3. --print-module with directory
./gradualizer --print-module test/dir \
|perl -ne '/^test_in_dir:/ or die "CLI 3 ($$_)"'
# 4. --print-basename with directory
./gradualizer --print-basename test/dir \
|perl -ne '/^test_in_dir.erl:/ or die "CLI 4 ($$_)"'
# 5. Checking a single file; not printing filename is the default
./gradualizer test/dir/test_in_dir.erl \
|perl -ne '/^The/ or die "CLI 5 ($$_)"'
# 6. Brief formatting
./gradualizer --fmt-location brief --print-basename test/dir \
|perl -ne '/^test_in_dir.erl:6:12: The variable N is ex/ or die "CLI 6 ($$_)"'
# 7. Verbose formatting, without filename
./gradualizer --fmt-location verbose --no-print-file test/dir \
|perl -ne '/^The variable N on line 6 at column 12/ or die "CLI 7 ($$_)"'
# 8. No location, no filename
./gradualizer --fmt-location none --no-print-file test/dir/test_in_dir.erl \
|perl -ne '/^The variable N is expected/ or die "CLI 8 ($$_)"'

.PHONY: cover
cover:
rebar3 do eunit -c, cover
Expand Down
40 changes: 30 additions & 10 deletions src/gradualizer.erl
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
%%% - `stop_on_first_error': if `true' stop type checking at the first error,
%%% if `false' continue checking all functions in the given file and all files
%%% in the given directory.
%%% - `print_file': if `true' prefix error printouts with the file name the
%%% error is from.
%%% - `{print_file, true | false | module | basename}': if `true' prefix error
%%% printouts with the file name the error is from. Default `false'.
%%% - `crash_on_error': if `true' crash on the first produced error
%%% - `return_errors': if `true', turns off error printing and errors
%%% (in their internal format) are returned in a list instead of being
Expand Down Expand Up @@ -56,16 +56,29 @@ type_check_file(File, Opts) ->
".beam" ->
gradualizer_file_utils:get_forms_from_beam(File);
Ext ->
throw({unknown_file_extension, Ext})
case filelib:is_dir(File) of
true -> type_check_dir(File, Opts);
false -> throw({unknown_file_extension, Ext})
end
end,
case ParsedFile of
{ok, Forms} ->
Opts2 = proplists:expand([{print_file, [{print_file, File}]}], Opts),
Opts2 = add_filename_to_opts(File, Opts),
type_check_forms(File, Forms, Opts2);
Error ->
throw(Error)
end.

%% @doc Prepends `{filename, string()}', depending on whether and how the
%% filename should be printed according to the print_file option in Opts.
add_filename_to_opts(Filename, Opts) ->
case proplists:get_value(print_file, Opts, false) of
false -> Opts;
true -> [{filename, Filename} | Opts];
basename -> [{filename, filename:basename(Filename)} | Opts];
module -> [{filename, filename:rootname(
filename:basename(Filename))} | Opts]
end.

%% @doc Type check a module
-spec type_check_module(module()) -> ok | nok | [{file:filename(), any()}].
Expand All @@ -84,13 +97,11 @@ type_check_module(Module, Opts) when is_atom(Module) ->
end.

%% @doc Type check all source or beam files in a directory.
%% (Option `print_file' is implicitely true)
-spec type_check_dir(file:filename()) -> ok | nok | [{file:filename(), any()}].
type_check_dir(Dir) ->
type_check_dir(Dir, []).

%% @doc Type check all source or beam files in a directory.
%% (Option `print_file' is implicitely true)
-spec type_check_dir(file:filename(), options()) ->
ok | nok | [{file:filename(), any()}].
type_check_dir(Dir, Opts) ->
Expand All @@ -102,14 +113,12 @@ type_check_dir(Dir, Opts) ->
end.

%% @doc Type check a source or beam file
%% (Option `print_file' is implicitely true)
-spec type_check_files([file:filename()]) ->
ok | nok | [{file:filename(), any()}].
type_check_files(Files) ->
type_check_files(Files, []).

%% @doc Type check a source or beam
%% (Option `print_file' is implicitely true)
-spec type_check_files([file:filename()], options()) ->
ok | nok | [{file:filename(), any()}].
type_check_files(Files, Opts) ->
Expand All @@ -119,15 +128,15 @@ type_check_files(Files, Opts) ->
lists:foldl(
fun(File, Errors) when Errors =:= [];
not StopOnFirstError ->
type_check_file(File, [print_file|Opts]) ++ Errors;
type_check_file_or_dir(File, Opts) ++ Errors;
(_, Errors) ->
Errors
end, [], Files);
true ->
lists:foldl(
fun(File, Res) when Res =:= ok;
not StopOnFirstError ->
case type_check_file(File, [print_file|Opts]) of
case type_check_file_or_dir(File, Opts) of
ok -> Res;
nok -> nok
end;
Expand All @@ -136,6 +145,17 @@ type_check_files(Files, Opts) ->
end, ok, Files)
end.

-spec type_check_file_or_dir(file:filename(), options()) ->
ok | nok | [{file:filename(), any()}].
type_check_file_or_dir(File, Opts) ->
IsRegular = filelib:is_regular(File),
IsDir = filelib:is_dir(File),
if
IsDir -> type_check_dir(File, Opts);
IsRegular -> type_check_file(File, Opts);
true -> throw({file_not_found, File}) % TODO: better errors
end.

%% @doc Type check an abstract syntax tree of a module. This can be useful
%% for tools where the abstract forms are generated in memory.
%%
Expand Down
24 changes: 20 additions & 4 deletions src/gradualizer_cli.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,24 @@ handle_args(Args) ->
HasHelp -> print_usage(), ok;
HasVersion -> print_version(), ok;
Rest =:= [] -> erlang:error("No files specified to check (try --)");
true -> gradualizer:type_check_files(Rest, Opts)
true ->
Opts1 = add_default_print_file_to_opts(Rest, Opts),
gradualizer:type_check_files(Rest, Opts1)
end,
case Status of
ok -> halt(0);
nok -> halt(1)
end.

%% Adds print_file option if there are more than one file to check and
%% the print_file is not already specified.
-spec add_default_print_file_to_opts(FilesToCheck :: list(),
gradualizer:options()) ->
gradualizer:options().
add_default_print_file_to_opts(Files, Opts) ->
[print_file || not proplists:is_defined(print_file, Opts),
length(Files) > 1 orelse filelib:is_dir(hd(Files))] ++ Opts.

-spec get_ver(atom()) -> string().
get_ver(App) ->
{_, _, Ver} = lists:keyfind(App, 1, application:loaded_applications()),
Expand All @@ -48,10 +59,13 @@ print_usage() ->
io:format(" the code path; see erl -pa [string]~n"),
io:format(" -I Include path for Erlang source files; see -I in~n"),
io:format(" the manual page erlc(1)~n"),
io:format(" --print-file prefix error printouts with the file name the~n"),
io:format(" error is from~n"),
io:format(" --print-file prefix error printouts with the file name~n"),
io:format(" - the default when checking a directory or more~n"),
io:format(" than one file~n"),
io:format(" --print-basename prefix error printouts with the file basename~n"),
io:format(" --print-module prefix error printouts with the module~n"),
io:format(" --no-print-file inverse of --print-file~n"),
io:format(" - the default behaviour~n"),
io:format(" - the default when checking a single file~n"),
io:format(" --stop-on-first-error stop type checking at the first error~n"),
io:format(" --no-stop-on-first-error inverse of --stop-on-first-error~n"),
io:format(" - the default behaviour~n"),
Expand All @@ -78,6 +92,8 @@ parse_opts([A | Args], Opts) ->
"--path-add" -> handle_path_add(A, Args, Opts);
"-I" -> handle_include_path(Args, Opts);
"--print-file" -> parse_opts(Args, [print_file | Opts]);
"--print-module" -> parse_opts(Args, [{print_file, module} | Opts]);
"--print-basename" -> parse_opts(Args, [{print_file, basename} | Opts]);
"--no-print-file" -> parse_opts(Args, [{print_file, false} | Opts]);
"--stop-on-first-error" -> parse_opts(Args, [stop_on_first_error | Opts]);
"--no-stop-on-first-error" -> parse_opts(Args, [{stop_on_first_error, false} | Opts]);
Expand Down
2 changes: 1 addition & 1 deletion src/typechecker.erl
Original file line number Diff line number Diff line change
Expand Up @@ -4030,7 +4030,7 @@ print_errors(Errors, Opts) ->
ok.

print_error(Error, Opts) ->
File = proplists:get_value(print_file, Opts),
File = proplists:get_value(filename, Opts),
FmtLoc = proplists:get_value(fmt_location, Opts, verbose),
case File of
undefined -> ok;
Expand Down
6 changes: 6 additions & 0 deletions test/dir/test_in_dir.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-module(test_in_dir).

-export([fail/1]).

-spec fail(integer()) -> atom().
fail(N) -> N.

0 comments on commit eb41bac

Please sign in to comment.