From 495c05d83f01ddcf87ab3fece027045053a35ea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 8 Jul 2014 18:54:40 +0200 Subject: [PATCH] Make elixir_errors API more strict and consider :file in meta --- lib/elixir/lib/kernel/typespec.ex | 2 +- lib/elixir/lib/module.ex | 6 +- lib/elixir/src/elixir_def.erl | 19 +++-- lib/elixir/src/elixir_errors.erl | 128 +++++++++++++++++------------- lib/elixir/src/elixir_locals.erl | 2 +- lib/elixir/src/elixir_module.erl | 6 +- 6 files changed, 95 insertions(+), 68 deletions(-) diff --git a/lib/elixir/lib/kernel/typespec.ex b/lib/elixir/lib/kernel/typespec.ex index a4117b320a..91211fe15d 100644 --- a/lib/elixir/lib/kernel/typespec.ex +++ b/lib/elixir/lib/kernel/typespec.ex @@ -553,7 +553,7 @@ defmodule Kernel.Typespec do if not export and doc do :elixir_errors.warn(caller.line, caller.file, "type #{name}/#{arity} is private, " <> - "@typedoc's are always discarded for private types\n") + "@typedoc's are always discarded for private types") end {{kind, {name, arity}, type}, caller.line, export, doc} diff --git a/lib/elixir/lib/module.ex b/lib/elixir/lib/module.ex index ec3ca42b19..dff51730ef 100644 --- a/lib/elixir/lib/module.ex +++ b/lib/elixir/lib/module.ex @@ -804,7 +804,7 @@ defmodule Module do [] is_list(warn) -> :elixir_errors.warn warn_info(warn), "undefined module attribute @#{key}, " <> - "please remove access to @#{key} or explicitly set it to nil before access\n" + "please remove access to @#{key} or explicitly set it to nil before access" nil true -> nil @@ -911,7 +911,9 @@ defmodule Module do :ok -> :ok {:error, :private_doc} -> - :elixir_errors.warn line, env.file, "function #{name}/#{arity} is private, @doc's are always discarded for private functions\n" + :elixir_errors.warn line, env.file, + "function #{name}/#{arity} is private, " <> + "@doc's are always discarded for private functions" end delete_attribute(module, :doc) diff --git a/lib/elixir/src/elixir_def.erl b/lib/elixir/src/elixir_def.erl index 03989204af..19b7968349 100644 --- a/lib/elixir/src/elixir_def.erl +++ b/lib/elixir/src/elixir_def.erl @@ -55,7 +55,7 @@ store_definition(Line, Kind, CheckClauses, Call, Body, Pos) -> {Name, Args} = case NameAndArgs of {N, _, A} when is_atom(N), is_atom(A) -> {N, []}; {N, _, A} when is_atom(N), is_list(A) -> {N, A}; - _ -> elixir_errors:form_error(Line, ?m(E, file), ?MODULE, {invalid_def, Kind, NameAndArgs}) + _ -> elixir_errors:form_error([{line, Line}], ?m(E, file), ?MODULE, {invalid_def, Kind, NameAndArgs}) end, %% Now that we have verified the call format, @@ -190,7 +190,7 @@ translate_definition(Kind, Line, Module, Name, Args, Guards, Body, E) when is_in translate_clause(nil, _Line, _Kind, _Args, [], _Body, _S) -> {[], false}; translate_clause(nil, Line, Kind, _Args, _Guards, _Body, #elixir_scope{file=File}) -> - elixir_errors:form_error(Line, File, ?MODULE, {missing_do, Kind}); + elixir_errors:form_error([{line, Line}], File, ?MODULE, {missing_do, Kind}); translate_clause(_, Line, Kind, Args, Guards, Body, S) -> {TClause, TS} = elixir_clauses:clause(Line, fun elixir_translator:translate_args/2, Args, Body, Guards, true, S), @@ -347,7 +347,7 @@ store_each(Check, Kind, File, Location, Table, CTable, Defaults, {function, Line check_valid_kind(_Line, _File, _Name, _Arity, Kind, Kind) -> []; check_valid_kind(Line, File, Name, Arity, Kind, StoredKind) -> - elixir_errors:form_error(Line, File, ?MODULE, + elixir_errors:form_error([{line, Line}], File, ?MODULE, {changed_kind, {Name, Arity, StoredKind, Kind}}). check_valid_clause(Line, File, Name, Arity, Kind, Table, StoredLine, StoredFile) -> @@ -362,7 +362,8 @@ check_valid_clause(Line, File, Name, Arity, Kind, Table, StoredLine, StoredFile) % Any clause after clause with defaults (body less does not count) check_valid_defaults(Line, File, Name, Arity, Kind, _, StoredDefaults, true) when StoredDefaults > 0 -> - elixir_errors:form_error(Line, File, ?MODULE, {clauses_with_defaults, {Kind, Name, Arity}}); + elixir_errors:form_error([{line, Line}], File, ?MODULE, + {clauses_with_defaults, {Kind, Name, Arity}}); % Clause without defaults check_valid_defaults(_Line, _File, _Name, _Arity, _Kind, 0, _, _) -> []; % Clause with defaults after clause without defaults @@ -370,12 +371,13 @@ check_valid_defaults(Line, File, Name, Arity, Kind, _, 0, _) -> elixir_errors:handle_file_warning(File, {Line, ?MODULE, {out_of_order_defaults, {Kind, Name, Arity}}}); % Clause with defaults after clause with defaults check_valid_defaults(Line, File, Name, Arity, Kind, _, _, _) -> - elixir_errors:form_error(Line, File, ?MODULE, {clauses_with_defaults, {Kind, Name, Arity}}). + elixir_errors:form_error([{line, Line}], File, ?MODULE, + {clauses_with_defaults, {Kind, Name, Arity}}). check_previous_defaults(Table, Line, Name, Arity, Kind, Defaults, E) -> Matches = ets:match(Table, {{Name, '$2'}, '$1', '_', '_', '_', '_', {'$3', '_', '_'}}), [ begin - elixir_errors:form_error(Line, ?m(E, file), ?MODULE, + elixir_errors:form_error([{line, Line}], ?m(E, file), ?MODULE, {defs_with_defaults, Name, {Kind, Arity}, {K, A}}) end || [K, A, D] <- Matches, A /= Arity, D /= 0, defaults_conflict(A, D, Arity, Defaults)]. @@ -385,7 +387,8 @@ defaults_conflict(A, D, Arity, Defaults) -> check_args_for_bodyless_clause(Line, Args, E) -> [ begin - elixir_errors:form_error(Line, ?m(E, file), ?MODULE, invalid_args_for_bodyless_clause) + elixir_errors:form_error([{line, Line}], ?m(E, file), ?MODULE, + invalid_args_for_bodyless_clause) end || Arg <- Args, invalid_arg(Arg) ]. invalid_arg({Name, _, Kind}) when is_atom(Name), is_atom(Kind) -> @@ -396,7 +399,7 @@ invalid_arg(_) -> true. assert_no_aliases_name(Line, '__aliases__', [Atom], #{file := File}) when is_atom(Atom) -> - elixir_errors:form_error(Line, File, ?MODULE, {no_alias, Atom}); + elixir_errors:form_error([{line, Line}], File, ?MODULE, {no_alias, Atom}); assert_no_aliases_name(_Meta, _Aliases, _Args, _S) -> ok. diff --git a/lib/elixir/src/elixir_errors.erl b/lib/elixir/src/elixir_errors.erl index 9a6da9d8da..5235b5c4ef 100644 --- a/lib/elixir/src/elixir_errors.erl +++ b/lib/elixir/src/elixir_errors.erl @@ -2,37 +2,37 @@ % This is not exposed in the Elixir language. -module(elixir_errors). -export([compile_error/3, compile_error/4, - form_error/4, parse_error/4, warn/2, warn/3, + form_error/4, form_warn/4, parse_error/4, warn/2, warn/3, handle_file_warning/2, handle_file_warning/3, handle_file_error/2]). -include("elixir.hrl"). --type line_or_meta() :: integer() | list(). +-spec warn(non_neg_integer(), binary(), iolist()) -> ok. -warn(Warning) -> - CompilerPid = get(elixir_compiler_pid), - if - CompilerPid =/= undefined -> - elixir_code_server:cast({register_warning, CompilerPid}); - true -> false - end, - io:put_chars(standard_error, Warning), - ok. +warn(Line, File, Warning) when is_integer(Line), is_binary(File) -> + warn(file_format(Line, File), Warning). + +-spec warn(iolist(), iolist()) -> ok. warn(Caller, Warning) -> - warn([Caller, "warning: ", Warning]). + do_warn([Caller, "warning: ", Warning, $\n]). -warn(Line, File, Warning) when is_integer(Line) -> - warn(file_format(Line, File, "warning: " ++ Warning)). +%% General forms handling. -%% Raised during expansion/translation/compilation. - --spec form_error(line_or_meta(), binary(), module(), any()) -> no_return(). +-spec form_error(list(), binary(), module(), any()) -> no_return(). form_error(Meta, File, Module, Desc) -> compile_error(Meta, File, format_error(Module, Desc)). --spec compile_error(line_or_meta(), binary(), iolist()) -> no_return(). --spec compile_error(line_or_meta(), binary(), iolist(), list()) -> no_return(). +-spec form_warn(list(), binary(), module(), any()) -> ok. + +form_warn(Meta, File, Module, Desc) when is_list(Meta) -> + {MetaLine, MetaFile} = meta_location(Meta, File), + warn(MetaLine, MetaFile, format_error(Module, Desc)). + +%% Compilation error. + +-spec compile_error(list(), binary(), iolist()) -> no_return(). +-spec compile_error(list(), binary(), iolist(), list()) -> no_return(). compile_error(Meta, File, Message) when is_list(Message) -> raise(Meta, File, 'Elixir.CompileError', elixir_utils:characters_to_binary(Message)). @@ -40,23 +40,23 @@ compile_error(Meta, File, Message) when is_list(Message) -> compile_error(Meta, File, Format, Args) when is_list(Format) -> compile_error(Meta, File, io_lib:format(Format, Args)). -%% Raised on tokenizing/parsing +%% Tokenization parsing/errors. --spec parse_error(line_or_meta(), binary(), binary(), binary()) -> no_return(). +-spec parse_error(non_neg_integer(), binary(), binary(), binary()) -> no_return(). -parse_error(Meta, File, Error, <<>>) -> +parse_error(Line, File, Error, <<>>) -> Message = case Error of <<"syntax error before: ">> -> <<"syntax error: expression is incomplete">>; _ -> Error end, - raise(Meta, File, 'Elixir.TokenMissingError', Message); + do_raise(Line, File, 'Elixir.TokenMissingError', Message); %% Show a nicer message for missing end tokens -parse_error(Meta, File, <<"syntax error before: ">>, <<"'end'">>) -> - raise(Meta, File, 'Elixir.SyntaxError', <<"unexpected token: end">>); +parse_error(Line, File, <<"syntax error before: ">>, <<"'end'">>) -> + do_raise(Line, File, 'Elixir.SyntaxError', <<"unexpected token: end">>); %% Binaries are wrapped in [<<...>>], so we need to unwrap them -parse_error(Meta, File, Error, <<"[", _/binary>> = Full) when is_binary(Error) -> +parse_error(Line, File, Error, <<"[", _/binary>> = Full) when is_binary(Error) -> Rest = case binary:split(Full, <<"<<">>) of [Lead, Token] -> @@ -67,14 +67,14 @@ parse_error(Meta, File, Error, <<"[", _/binary>> = Full) when is_binary(Error) - [_] -> <<$">> end, - raise(Meta, File, 'Elixir.SyntaxError', <>); + do_raise(Line, File, 'Elixir.SyntaxError', <>); %% Everything else is fine as is -parse_error(Meta, File, Error, Token) when is_binary(Error), is_binary(Token) -> - Message = <>, - raise(Meta, File, 'Elixir.SyntaxError', Message). +parse_error(Line, File, Error, Token) when is_binary(Error), is_binary(Token) -> + Message = <>, + do_raise(Line, File, 'Elixir.SyntaxError', Message). -%% Handle warnings and errors (called during module compilation) +%% Handle warnings and errors from Erlang land (called during module compilation) %% Ignore on bootstrap handle_file_warning(true, _File, {_Line, sys_core_fold, nomatch_guard}) -> ok; @@ -157,39 +157,27 @@ handle_file_error(File, {Line,erl_lint,{unsafe_var,Var,{In,_Where}}}) -> _ -> In end, Message = io_lib:format("cannot define variable ~ts inside ~ts", [format_var(Var), Translated]), - raise(Line, File, 'Elixir.CompileError', iolist_to_binary(Message)); + do_raise(Line, File, 'Elixir.CompileError', elixir_utils:characters_to_binary(Message)); handle_file_error(File, {Line,erl_lint,{spec_fun_undefined,{M,F,A}}}) -> Message = io_lib:format("spec for undefined function ~ts.~ts/~B", [elixir_aliases:inspect(M), F, A]), - raise(Line, File, 'Elixir.CompileError', iolist_to_binary(Message)); + do_raise(Line, File, 'Elixir.CompileError', elixir_utils:characters_to_binary(Message)); handle_file_error(File, {Line,Module,Desc}) -> - form_error(Line, File, Module, Desc). + Message = format_error(Module, Desc), + do_raise(Line, File, 'Elixir.CompileError', elixir_utils:characters_to_binary(Message)). %% Helpers raise(Meta, File, Kind, Message) when is_list(Meta) -> - raise(?line(Meta), File, Kind, Message); + {MetaLine, MetaFile} = meta_location(Meta, File), + do_raise(MetaLine, MetaFile, Kind, Message). -raise(none, File, Kind, Message) -> - raise(0, File, Kind, Message); +file_format(0, File) -> + io_lib:format("~ts: ", [elixir_utils:relative_to_cwd(File)]); -raise(Line, File, Kind, Message) when is_integer(Line), is_binary(File) -> - %% Populate the stacktrace so we can raise it - try - throw(ok) - catch - ok -> ok - end, - Stacktrace = erlang:get_stacktrace(), - Exception = Kind:exception([{description, Message}, {file, File}, {line, Line}]), - erlang:raise(error, Exception, tl(Stacktrace)). - -file_format(0, File, Message) when is_binary(File) -> - io_lib:format("~ts: ~ts~n", [elixir_utils:relative_to_cwd(File), Message]); - -file_format(Line, File, Message) when is_binary(File) -> - io_lib:format("~ts:~w: ~ts~n", [elixir_utils:relative_to_cwd(File), Line, Message]). +file_format(Line, File) -> + io_lib:format("~ts:~w: ", [elixir_utils:relative_to_cwd(File), Line]). format_var(Var) -> list_to_atom(lists:takewhile(fun(X) -> X /= $@ end, atom_to_list(Var))). @@ -220,3 +208,37 @@ translate_comp_op('=<') -> '<='; translate_comp_op('=:=') -> '==='; translate_comp_op('=/=') -> '!=='; translate_comp_op(Other) -> Other. + +meta_location(Meta, File) -> + case lists:keyfind(file, 1, Meta) of + {file, MetaFile} when is_binary(MetaFile) -> + case lists:keyfind(keep, 1, Meta) of + {keep, MetaLine} when is_integer(MetaLine) -> ok; + _ -> MetaLine = 0 + end, + {MetaLine, MetaFile}; + _ -> + {?line(Meta), File} + end. + +do_warn(Warning) -> + CompilerPid = get(elixir_compiler_pid), + if + CompilerPid =/= undefined -> + elixir_code_server:cast({register_warning, CompilerPid}); + true -> false + end, + io:put_chars(standard_error, Warning), + ok. + +do_raise(none, File, Kind, Message) -> + do_raise(0, File, Kind, Message); +do_raise(Line, File, Kind, Message) when is_integer(Line), is_binary(File), is_binary(Message) -> + try + throw(ok) + catch + ok -> ok + end, + Stacktrace = erlang:get_stacktrace(), + Exception = Kind:exception([{description, Message}, {file, File}, {line, Line}]), + erlang:raise(error, Exception, tl(Stacktrace)). \ No newline at end of file diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl index e2bf04d06c..98979866d0 100644 --- a/lib/elixir/src/elixir_locals.erl +++ b/lib/elixir/src/elixir_locals.erl @@ -133,7 +133,7 @@ ensure_no_import_conflict(_Line, _File, 'Elixir.Kernel', _All) -> ensure_no_import_conflict(Line, File, Module, All) -> if_tracker(Module, ok, fun(Pid) -> _ = [ begin - elixir_errors:form_error(Line, File, ?MODULE, {function_conflict, Error}) + elixir_errors:form_error([{line, Line}], File, ?MODULE, {function_conflict, Error}) end || Error <- ?tracker:collect_imports_conflicts(Pid, All) ], ok end). diff --git a/lib/elixir/src/elixir_module.erl b/lib/elixir/src/elixir_module.erl index d67c2582c4..e0eb4c8762 100644 --- a/lib/elixir/src/elixir_module.erl +++ b/lib/elixir/src/elixir_module.erl @@ -42,7 +42,7 @@ compile(Module, Block, Vars, #{line := Line} = Env) when is_atom(Module) -> end; compile(Module, _Block, _Vars, #{line := Line, file := File}) -> - elixir_errors:form_error(Line, File, ?MODULE, {invalid_module, Module}). + elixir_errors:form_error([{line, Line}], File, ?MODULE, {invalid_module, Module}). do_compile(Line, Module, Block, Vars, E) -> File = ?m(E, file), @@ -102,7 +102,7 @@ build(Line, File, Module, Docs, Lexical) -> true -> [{OldFile, OldLine}] = ets:lookup_element(OldTable, ?location_attr, 2), Error = {module_in_definition, Module, OldFile, OldLine}, - elixir_errors:form_error(Line, File, ?MODULE, Error); + elixir_errors:form_error([{line, Line}], File, ?MODULE, Error); false -> [] end, @@ -385,7 +385,7 @@ add_info_function(Line, File, Module, All, Def, Defmacro) -> Pair = {'__info__', 1}, case lists:member(Pair, All) of true -> - elixir_errors:form_error(Line, File, ?MODULE, {internal_function_overridden, Pair}); + elixir_errors:form_error([{line, Line}], File, ?MODULE, {internal_function_overridden, Pair}); false -> Spec = {attribute, Line, spec, {Pair,