Include column in warnings emitted by compiler (#11339)

We also extend Mix.Tasks.Compiler.Diagnostic
to include {line, column} as possible position.
We also explicitly document the behaviour of
line=0, which is equivalent to unknown line.
Elixir was already setting the line to zero in
multiple occasions prior to this patch, so this
patch makes it official and we stop returning `nil`
for said cases.

Author: josevalim

lib/eex/lib/eex/compiler.ex

@@ -72,7 +72,7 @@ defmodule EEx.Compiler do
       message =
         "the contents of this expression won't be output unless the EEx block starts with \"<%=\""
 
-      :elixir_errors.erl_warn(start_line, state.file, message)
+      :elixir_errors.erl_warn({start_line, start_column}, state.file, message)
     end
 
     {rest, line, contents} =
@@ -117,7 +117,7 @@ defmodule EEx.Compiler do
       "unexpected beginning of EEx tag \"<%#{modifier}\" on \"<%#{modifier}#{chars}%>\", " <>
         "please remove \"#{modifier}\" accordingly"
 
-    :elixir_errors.erl_warn(line, state.file, message)
+    :elixir_errors.erl_warn({line, column}, state.file, message)
     generate_buffer([{:middle_expr, line, column, '', chars} | t], buffer, scope, state)
     # TODO: Make this an error on Elixir v2.0 since it accidentally worked previously.
     # raise EEx.SyntaxError, message: message, file: state.file, line: line
@@ -155,7 +155,7 @@ defmodule EEx.Compiler do
       "unexpected beginning of EEx tag \"<%#{modifier}\" on end of " <>
         "expression \"<%#{modifier}#{chars}%>\", please remove \"#{modifier}\" accordingly"
 
-    :elixir_errors.erl_warn(line, state.file, message)
+    :elixir_errors.erl_warn({line, column}, state.file, message)
     generate_buffer([{:end_expr, line, column, '', chars} | t], buffer, scope, state)
     # TODO: Make this an error on Elixir v2.0 since it accidentally worked previously.
     # raise EEx.SyntaxError, message: message, file: state.file, line: line, column: column

lib/eex/lib/eex/tokenizer.ex

@@ -72,8 +72,8 @@ defmodule EEx.Tokenizer do
         {key, expr} =
           case :elixir_tokenizer.tokenize(expr, 1, file: "eex", check_terminators: false) do
             {:ok, _line, _column, warnings, tokens} ->
-              Enum.each(Enum.reverse(warnings), fn {line, file, msg} ->
-                :elixir_errors.erl_warn(line, file, msg)
+              Enum.each(Enum.reverse(warnings), fn {location, file, msg} ->
+                :elixir_errors.erl_warn(location, file, msg)
               end)
 
               token_key(tokens, expr)

lib/elixir/lib/io.ex

@@ -305,7 +305,7 @@ defmodule IO do
   @spec warn(chardata | String.Chars.t(), Exception.stacktrace()) :: :ok
   def warn(message, []) do
     message = [to_chardata(message), ?\n]
-    :elixir_errors.io_warn(0, nil, message, message)
+    :elixir_errors.log_and_print_warning(0, nil, message, message)
   end
 
   def warn(message, [{_, _, _, opts} | _] = stacktrace) do
@@ -314,7 +314,7 @@ defmodule IO do
     line = opts[:line]
     file = opts[:file]
 
-    :elixir_errors.io_warn(
+    :elixir_errors.log_and_print_warning(
       line || 0,
       file && List.to_string(file),
       message,

lib/elixir/lib/kernel/parallel_compiler.ex

@@ -3,6 +3,12 @@ defmodule Kernel.ParallelCompiler do
   A module responsible for compiling and requiring files in parallel.
   """
 
+  @typedoc "The line. 0 indicates no line."
+  @type line() :: non_neg_integer()
+  @type location() :: line() | {line(), column :: non_neg_integer}
+  @type warning() :: {file :: Path.t(), location(), message :: String.t()}
+  @type error() :: {file :: Path.t(), line(), message :: String.t()}
+
   @doc """
   Starts a task for parallel compilation.
 
@@ -91,6 +97,7 @@ defmodule Kernel.ParallelCompiler do
 
   """
   @doc since: "1.6.0"
+  @spec compile([Path.t()], keyword()) :: {:ok, [atom], [warning]} | {:error, [error], [warning]}
   def compile(files, options \\ []) when is_list(options) do
     spawn_workers(files, :compile, options)
   end
@@ -101,6 +108,8 @@ defmodule Kernel.ParallelCompiler do
   See `compile/2` for more information.
   """
   @doc since: "1.6.0"
+  @spec compile_to_path([Path.t()], Path.t(), keyword()) ::
+          {:ok, [atom], [warning]} | {:error, [error], [warning]}
   def compile_to_path(files, path, options \\ []) when is_binary(path) and is_list(options) do
     spawn_workers(files, {:compile, path}, options)
   end
@@ -126,10 +135,21 @@ defmodule Kernel.ParallelCompiler do
 
   """
   @doc since: "1.6.0"
+  @spec compile_to_path([Path.t()], keyword()) ::
+          {:ok, [atom], [warning]} | {:error, [error], [warning]}
   def require(files, options \\ []) when is_list(options) do
     spawn_workers(files, :require, options)
   end
 
+  @doc """
+  Prints a warning returned by the compiler.
+  """
+  @doc since: "1.13.0"
+  @spec print_warning(warning) :: :ok
+  def print_warning({file, location, warning}) do
+    :elixir_errors.print_warning(location, file, warning)
+  end
+
   @doc false
   @deprecated "Use Kernel.ParallelCompiler.compile/2 instead"
   def files(files, options \\ []) when is_list(options) do
@@ -557,10 +577,10 @@ defmodule Kernel.ParallelCompiler do
         state = %{state | timer_ref: timer_ref}
         spawn_workers(queue, spawned, waiting, files, result, warnings, state)
 
-      {:warning, file, line, message} ->
+      {:warning, file, location, message} ->
         file = file && Path.absname(file)
         message = :unicode.characters_to_binary(message)
-        warning = {file, line, message}
+        warning = {file, location, message}
         wait_for_messages(queue, spawned, waiting, files, result, [warning | warnings], state)
 
       {:file_ok, child_pid, ref, file, lexical} ->
@@ -729,7 +749,7 @@ defmodule Kernel.ParallelCompiler do
     line = get_line(file, reason, stack)
     file = Path.absname(file)
     message = :unicode.characters_to_binary(Kernel.CLI.format_error(kind, reason, stack))
-    {file, line, message}
+    {file, line || 0, message}
   end
 
   defp get_line(_file, %{line: line}, _stack) when is_integer(line) and line > 0 do

lib/elixir/src/elixir_errors.erl

@@ -4,21 +4,26 @@
 %% Note that this is also called by the Erlang backend, so we also support
 %% the line number to be none (as it may happen in some erlang errors).
 -module(elixir_errors).
--export([compile_error/3, compile_error/4, warning_prefix/0,
-         form_error/4, form_warn/4, parse_error/4, erl_warn/3, io_warn/4]).
+-export([compile_error/3, compile_error/4,  form_error/4, parse_error/4]).
+-export([warning_prefix/0, erl_warn/3, print_warning/3, log_and_print_warning/4, form_warn/4]).
 -include("elixir.hrl").
+-type location() :: non_neg_integer() | {non_neg_integer(), non_neg_integer()}.
 
 %% Low-level warning, should be used only from Erlang passes.
--spec erl_warn(non_neg_integer() | none, unicode:chardata(), unicode:chardata()) -> ok.
+-spec erl_warn(location() | none, unicode:chardata(), unicode:chardata()) -> ok.
 erl_warn(none, File, Warning) ->
   erl_warn(0, File, Warning);
-erl_warn(Line, File, Warning) when is_integer(Line), is_binary(File) ->
-  io_warn(Line, File, Warning, [Warning, "\n  ", file_format(Line, File), $\n]).
+erl_warn(Location, File, Warning) when is_binary(File) ->
+  send_warning(Location, File, Warning),
+  print_warning(Location, File, Warning).
 
-%% Low-level warning, all other warnings are built on top of it.
--spec io_warn(non_neg_integer(), unicode:chardata() | nil, unicode:chardata(), unicode:chardata()) -> ok.
-io_warn(Line, File, LogMessage, PrintMessage) when is_integer(Line), is_binary(File) or (File == nil) ->
-  send_warning(Line, File, LogMessage),
+-spec print_warning(location(), unicode:chardata(), unicode:chardata()) -> ok.
+print_warning(Location, File, Warning) ->
+  print_warning([Warning, "\n  ", file_format(Location, File), $\n]).
+
+-spec log_and_print_warning(location(), unicode:chardata() | nil, unicode:chardata(), unicode:chardata()) -> ok.
+log_and_print_warning(Location, File, LogMessage, PrintMessage) when is_binary(File) or (File == nil) ->
+  send_warning(Location, File, LogMessage),
   print_warning(PrintMessage).
 
 -spec warning_prefix() -> binary().
@@ -59,7 +64,7 @@ do_form_warn(Meta, GivenFile, E, Warning) ->
         file_format(Line, File)
     end,
 
-  io_warn(Line, File, Warning, [Warning, "\n  ", Location, $\n]).
+  log_and_print_warning(Line, File, Warning, [Warning, "\n  ", Location, $\n]).
 
 %% Compilation error.
 
@@ -176,9 +181,12 @@ send_warning(Line, File, Message) ->
   end,
   ok.
 
+file_format({0, _Column}, File) ->
+  io_lib:format("~ts", [elixir_utils:relative_to_cwd(File)]);
+file_format({Line, Column}, File) ->
+  io_lib:format("~ts:~w:~w", [elixir_utils:relative_to_cwd(File), Line, Column]);
 file_format(0, File) ->
   io_lib:format("~ts", [elixir_utils:relative_to_cwd(File)]);
-
 file_format(Line, File) ->
   io_lib:format("~ts:~w", [elixir_utils:relative_to_cwd(File), Line]).
 

lib/elixir/src/elixir_parser.yrl

@@ -652,7 +652,7 @@ Erlang code.
 %% The following directive is needed for (significantly) faster
 %% compilation of the generated .erl file by the HiPE compiler
 -compile([{hipe, [{regalloc, linear_scan}]}]).
--compile({inline, meta_from_token/1, meta_from_location/1, line_from_location/1, is_eol/1}).
+-compile({inline, meta_from_token/1, meta_from_location/1, is_eol/1}).
 -import(lists, [reverse/1, reverse/2]).
 
 meta_from_token(Token) ->
@@ -664,8 +664,6 @@ meta_from_location({Line, Column, _}) ->
     false -> [{line, Line}]
   end.
 
-line_from_location({Line, _Column, _}) -> Line.
-
 do_end_meta(Do, End) ->
   case ?token_metadata() of
     true ->
@@ -728,13 +726,14 @@ build_op(AST, {_Kind, Location, '//'}, Right) ->
       return_error(Location, "the range step operator (//) must immediately follow the range definition operator (..), for example: 1..9//2. If you wanted to define a default argument, use (\\\\) instead. Syntax error before: ", "'//'")
   end;
 
-build_op({UOp, _, [Left]}, {_Kind, Location, 'in'}, Right) when ?rearrange_uop(UOp) ->
+build_op({UOp, _, [Left]}, {_Kind, {Line, Column, _} = Location, 'in'}, Right) when ?rearrange_uop(UOp) ->
   %% TODO: Remove "not left in right" rearrangement on v2.0
-  elixir_errors:erl_warn(line_from_location(Location), ?file(),
+  elixir_errors:erl_warn({Line, Column}, ?file(),
     "\"not expr1 in expr2\" is deprecated. "
     "Instead use \"expr1 not in expr2\" if you require Elixir v1.5+, "
     "or \"not(expr1 in expr2)\" if you have to support earlier Elixir versions"),
-  {UOp, meta_from_location(Location), [{'in', meta_from_location(Location), [Left, Right]}]};
+  Meta = meta_from_location(Location),
+  {UOp, Meta, [{'in', Meta, [Left, Right]}]};
 
 build_op(Left, {_Kind, Location, 'not in'}, Right) ->
   Meta = meta_from_location(Location),
@@ -1168,16 +1167,16 @@ error_invalid_kw_identifier({_, Location, KW}) ->
   return_error(Location, "syntax error before: ", "'" ++ atom_to_list(KW) ++ ":'").
 
 %% TODO: Make this an error on v2.0
-warn_empty_paren({_, {Line, _, _}}) ->
-  elixir_errors:erl_warn(Line, ?file(),
+warn_empty_paren({_, {Line, Column, _}}) ->
+  elixir_errors:erl_warn({Line, Column}, ?file(),
     "invalid expression (). "
     "If you want to invoke or define a function, make sure there are "
     "no spaces between the function name and its arguments. If you wanted "
     "to pass an empty block or code, pass a value instead, such as a nil or an atom").
 
 %% TODO: Make this an error on v2.0
-warn_pipe({arrow_op, {Line, _, _}, Op}, {_, [_ | _], [_ | _]}) ->
-  elixir_errors:erl_warn(Line, ?file(),
+warn_pipe({arrow_op, {Line, Column, _}, Op}, {_, [_ | _], [_ | _]}) ->
+  elixir_errors:erl_warn({Line, Column}, ?file(),
     io_lib:format(
       "parentheses are required when piping into a function call. For example:\n\n"
       "    foo 1 ~ts bar 2 ~ts baz 3\n\n"
@@ -1190,7 +1189,7 @@ warn_pipe({arrow_op, {Line, _, _}, Op}, {_, [_ | _], [_ | _]}) ->
 warn_pipe(_Token, _) ->
   ok.
 
-warn_empty_stab_clause({stab_op, {Line, _, _}, '->'}) ->
-  elixir_errors:erl_warn(Line, ?file(),
+warn_empty_stab_clause({stab_op, {Line, Column, _}, '->'}) ->
+  elixir_errors:erl_warn({Line, Column}, ?file(),
     "an expression is always required on the right side of ->. "
     "Please provide a value after ->").