Add --color_warnings to make warnings/errors like clang

This adds new (WARN|KATI_WARN|ERROR)_LOC log macro variants that take a
location as the first argument, and will prefix that location
information to the warning/error lines.

When --color_warnings is enabled, it reformats them to have a standard
warning:/error: infix, and adds colors in order to match the
warnings/errors produced by clang.

16 files changed, 139 insertions, 53 deletions

diff --git a/dep.cc b/dep.cc
index 4aebae0..75dff76 100644
--- a/dep.cc
+++ b/dep.cc
@@ -156,16 +156,18 @@ struct RuleMerger {
     if (rules.empty()) {
       is_double_colon = r->is_double_colon;
     } else if (is_double_colon != r->is_double_colon) {
-      ERROR("%s:%d: *** target file `%s' has both : and :: entries.",
-            LOCF(r->loc), output.c_str());
+      ERROR_LOC(r->loc, "*** target file `%s' has both : and :: entries.",
+                output.c_str());
     }
 
     if (primary_rule && !r->cmds.empty() &&
         !IsSuffixRule(output) && !r->is_double_colon) {
-      WARN("%s:%d: warning: overriding commands for target `%s'",
-           LOCF(r->cmd_loc()), output.c_str());
-      WARN("%s:%d: warning: ignoring old commands for target `%s'",
-           LOCF(primary_rule->cmd_loc()), output.c_str());
+      WARN_LOC(r->cmd_loc(),
+               "warning: overriding commands for target `%s'",
+               output.c_str());
+      WARN_LOC(primary_rule->cmd_loc(),
+               "warning: ignoring old commands for target `%s'",
+               output.c_str());
       primary_rule = r;
     }
     if (!primary_rule && !r->cmds.empty()) {
@@ -274,8 +276,7 @@ class DepBuilder {
       if (targets.empty()) {
         suffix_rules_.clear();
       } else {
-        WARN("%s:%d: kati doesn't support .SUFFIXES with prerequisites",
-             LOCF(loc));
+        WARN_LOC(loc, "kati doesn't support .SUFFIXES with prerequisites");
       }
     }
 
@@ -296,7 +297,7 @@ class DepBuilder {
     };
     for (const char** p = kUnsupportedBuiltinTargets; *p; p++) {
       if (GetRuleInputs(Intern(*p), &targets, &loc)) {
-        WARN("%s:%d: kati doesn't support %s", LOCF(loc), *p);
+        WARN_LOC(loc, "kati doesn't support %s", *p);
       }
     }
   }
diff --git a/eval.cc b/eval.cc
index 4b56f1f..7a08be7 100644
--- a/eval.cc
+++ b/eval.cc
@@ -383,7 +383,7 @@ string Evaluator::GetShellAndFlag() {
 }
 
 void Evaluator::Error(const string& msg) {
-  ERROR("%s:%d: %s", LOCF(loc_), msg.c_str());
+  ERROR_LOC(loc_, "%s", msg.c_str());
 }
 
 unordered_set<Symbol> Evaluator::used_undefined_vars_;
diff --git a/expr.cc b/expr.cc
index 5e7afe6..30a25b5 100644
--- a/expr.cc
+++ b/expr.cc
@@ -317,9 +317,9 @@ void ParseFunc(const Loc& loc,
     f->AddArg(v);
     i += n;
     if (i == s.size()) {
-      ERROR("%s:%d: *** unterminated call to function '%s': "
-            "missing '%c'.",
-            LOCF(loc), f->name(), terms[0]);
+      ERROR_LOC(loc, "*** unterminated call to function '%s': "
+                "missing '%c'.",
+                f->name(), terms[0]);
     }
     nargs++;
     if (s[i] == terms[0]) {
@@ -332,8 +332,8 @@ void ParseFunc(const Loc& loc,
   }
 
   if (nargs <= f->min_arity()) {
-    ERROR("%s:%d: *** insufficient number of arguments (%d) to function `%s'.",
-          LOCF(loc), nargs - 1, f->name());
+    ERROR_LOC(loc, "*** insufficient number of arguments (%d) to function `%s'.",
+              nargs - 1, f->name());
   }
 
   *index_out = i;
@@ -365,8 +365,8 @@ Value* ParseDollar(const Loc& loc, StringPiece s, size_t* index_out) {
         if (g_flags.enable_kati_warnings) {
           size_t found = sym.str().find_first_of(" ({");
           if (found != string::npos) {
-            KATI_WARN("%s:%d: *warning*: variable lookup with '%c': %.*s",
-                      loc, sym.str()[found], SPF(s));
+            KATI_WARN_LOC(loc, "*warning*: variable lookup with '%c': %.*s",
+                          sym.str()[found], SPF(s));
           }
         }
         Value* r = new SymRef(sym);
@@ -386,8 +386,8 @@ Value* ParseDollar(const Loc& loc, StringPiece s, size_t* index_out) {
           ParseFunc(loc, func, s, i+1, terms, index_out);
           return func;
         } else {
-          KATI_WARN("%s:%d: *warning*: unknown make function '%.*s': %.*s",
-                    loc, SPF(lit->val()), SPF(s));
+          KATI_WARN_LOC(loc, "*warning*: unknown make function '%.*s': %.*s",
+                        SPF(lit->val()), SPF(s));
         }
       }
 
@@ -428,12 +428,12 @@ Value* ParseDollar(const Loc& loc, StringPiece s, size_t* index_out) {
     // for detail.
     size_t found = s.find(cp);
     if (found != string::npos) {
-      KATI_WARN("%s:%d: *warning*: unmatched parentheses: %.*s",
-                loc, SPF(s));
+      KATI_WARN_LOC(loc, "*warning*: unmatched parentheses: %.*s",
+                    SPF(s));
       *index_out = s.size();
       return new SymRef(Intern(s.substr(2, found-2)));
     }
-    ERROR("%s:%d: *** unterminated variable reference.", LOCF(loc));
+    ERROR_LOC(loc, "*** unterminated variable reference.");
   }
 }
 
diff --git a/flags.cc b/flags.cc
index 3ad8e3c..5c14af7 100644
--- a/flags.cc
+++ b/flags.cc
@@ -97,6 +97,8 @@ void Flags::Parse(int argc, char** argv) {
       detect_android_echo = true;
     } else if (!strcmp(arg, "--detect_depfiles")) {
       detect_depfiles = true;
+    } else if (!strcmp(arg, "--color_warnings")) {
+      color_warnings = true;
     } else if (ParseCommandLineOptionWithArg(
         "-j", argv, &i, &num_jobs_str)) {
       num_jobs = strtol(num_jobs_str, NULL, 10);
diff --git a/flags.h b/flags.h
index d22d0cd..cd4090a 100644
--- a/flags.h
+++ b/flags.h
@@ -39,6 +39,7 @@ struct Flags {
   bool regen_debug;
   bool regen_ignoring_kati_binary;
   bool use_find_emulator;
+  bool color_warnings;
   const char* goma_dir;
   const char* ignore_dirty_pattern;
   const char* no_ignore_dirty_pattern;
diff --git a/func.cc b/func.cc
index 716cb2b..2031319 100644
--- a/func.cc
+++ b/func.cc
@@ -466,8 +466,8 @@ void EvalFunc(const vector<Value*>& args, Evaluator* ev, string*) {
   string* text = new string;
   args[0]->Eval(ev, text);
   if (ev->avoid_io()) {
-    KATI_WARN("%s:%d: *warning*: $(eval) in a recipe is not recommended: %s",
-              LOCF(ev->loc()), text->c_str());
+    KATI_WARN_LOC(ev->loc(), "*warning*: $(eval) in a recipe is not recommended: %s",
+                  text->c_str());
   }
   vector<Stmt*> stmts;
   Parse(*text, ev->loc(), &stmts);
@@ -555,9 +555,9 @@ void ShellFunc(const vector<Value*>& args, Evaluator* ev, string* s) {
   string cmd = args[0]->Eval(ev);
   if (ev->avoid_io() && !HasNoIoInShellScript(cmd)) {
     if (ev->eval_depth() > 1) {
-      ERROR("%s:%d: kati doesn't support passing results of $(shell) "
-            "to other make constructs: %s",
-            LOCF(ev->loc()), cmd.c_str());
+      ERROR_LOC(ev->loc(), "kati doesn't support passing results of $(shell) "
+                "to other make constructs: %s",
+                cmd.c_str());
     }
     StripShellComment(&cmd);
     *s += "$(";
@@ -595,8 +595,8 @@ void CallFunc(const vector<Value*>& args, Evaluator* ev, string* s) {
   const StringPiece func_name = TrimSpace(func_name_buf);
   Var* func = ev->LookupVar(Intern(func_name));
   if (!func->IsDefined()) {
-    KATI_WARN("%s:%d: *warning*: undefined user function: %s",
-              ev->loc(), func_name.as_string().c_str());
+    KATI_WARN_LOC(ev->loc(), "*warning*: undefined user function: %s",
+                  func_name.as_string().c_str());
   }
   vector<unique_ptr<SimpleVar>> av;
   for (size_t i = 1; i < args.size(); i++) {
@@ -676,8 +676,7 @@ void WarningFunc(const vector<Value*>& args, Evaluator* ev, string*) {
         StringPrintf("echo -e \"%s:%d: %s\" 2>&1", LOCF(ev->loc()), EchoEscape(a).c_str()));
     return;
   }
-  printf("%s:%d: %s\n", LOCF(ev->loc()), a.c_str());
-  fflush(stdout);
+  WARN_LOC(ev->loc(), "%s", a.c_str());
 }
 
 void ErrorFunc(const vector<Value*>& args, Evaluator* ev, string*) {
diff --git a/log.cc b/log.cc
index fb9fcd4..b7141f6 100644
--- a/log.cc
+++ b/log.cc
@@ -16,5 +16,46 @@
 
 #include "log.h"
 
+#include "flags.h"
+#include "strutil.h"
+
+#define BOLD "\033[1m"
+#define RESET "\033[0m"
+#define MAGENTA "\033[35m"
+#define RED "\033[31m"
+
+void ColorErrorLog(const char* file, int line, const char* msg) {
+  if (file == nullptr) {
+    ERROR("%s", msg);
+    return;
+  }
+
+  if (g_flags.color_warnings) {
+    StringPiece filtered = TrimPrefix(msg, "*** ");
+
+    ERROR(BOLD "%s:%d: " RED "error: " RESET BOLD "%s" RESET,
+          file, line, filtered.as_string().c_str());
+  } else {
+    ERROR("%s:%d: %s", file, line, msg);
+  }
+}
+
+void ColorWarnLog(const char* file, int line, const char* msg) {
+  if (file == nullptr) {
+    fprintf(stderr, "%s\n", msg);
+    return;
+  }
+
+  if (g_flags.color_warnings) {
+    StringPiece filtered = TrimPrefix(msg, "*warning*: ");
+    filtered = TrimPrefix(filtered, "warning: ");
+
+    fprintf(stderr, BOLD "%s:%d: " MAGENTA "warning: " RESET BOLD "%s" RESET "\n",
+            file, line, filtered.as_string().c_str());
+  } else {
+    fprintf(stderr, "%s:%d: %s\n", file, line, msg);
+  }
+}
+
 bool g_log_no_exit;
 string* g_last_error;
diff --git a/log.h b/log.h
index 11cf0e5..58c733a 100644
--- a/log.h
+++ b/log.h
@@ -21,6 +21,7 @@
 #include <string.h>
 
 #include "flags.h"
+#include "log.h"
 #include "stringprintf.h"
 
 using namespace std;
@@ -73,4 +74,22 @@ extern string* g_last_error;
 
 #define CHECK(c) if (!(c)) ERROR("%s:%d: %s", __FILE__, __LINE__, #c)
 
+// Set of logging functions that will automatically colorize lines that have
+// location information when --color_warnings is set.
+void ColorWarnLog(const char* file, int line, const char *msg);
+void ColorErrorLog(const char* file, int line, const char *msg);
+
+#define WARN_LOC(loc, ...) do {                                 \
+    ColorWarnLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+  } while (0)
+
+#define KATI_WARN_LOC(loc, ...) do {                                  \
+    if (g_flags.enable_kati_warnings)                             \
+      ColorWarnLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+  } while(0)
+
+#define ERROR_LOC(loc, ...) do {                                 \
+    ColorErrorLog(LOCF(loc), StringPrintf(__VA_ARGS__).c_str()); \
+  } while (0)
+
 #endif  // LOG_H_
diff --git a/main.cc b/main.cc
index 5afe2b1..a4df3c0 100644
--- a/main.cc
+++ b/main.cc
@@ -173,8 +173,8 @@ static int Run(const vector<Symbol>& targets,
   }
 
   for (ParseErrorStmt* err : GetParseErrors()) {
-    WARN("%s:%d: warning for parse error in an unevaluated line: %s",
-         LOCF(err->loc()), err->msg.c_str());
+    WARN_LOC(err->loc(), "warning for parse error in an unevaluated line: %s",
+             err->msg.c_str());
   }
 
   vector<DepNode*> nodes;
diff --git a/parser.cc b/parser.cc
index 61d1ac5..3822dd8 100644
--- a/parser.cc
+++ b/parser.cc
@@ -92,10 +92,10 @@ class Parser {
     }
 
     if (!if_stack_.empty())
-      ERROR("%s:%d: *** missing `endif'.", loc_.filename, loc_.lineno + 1);
+      ERROR_LOC(Loc(loc_.filename, loc_.lineno + 1), "*** missing `endif'.");
     if (!define_name_.empty())
-      ERROR("%s:%d: *** missing `endef', unterminated `define'.",
-            loc_.filename, define_start_line_);
+      ERROR_LOC(Loc(loc_.filename, define_start_line_),
+                "*** missing `endef', unterminated `define'.");
   }
 
   static void Init() {
@@ -304,7 +304,7 @@ class Parser {
     StringPiece rest = TrimRightSpace(RemoveComment(TrimLeftSpace(
         line.substr(sizeof("endef")))));
     if (!rest.empty()) {
-      WARN("%s:%d: extraneous text after `endef' directive", LOCF(loc_));
+      WARN_LOC(loc_, "extraneous text after `endef' directive");
     }
 
     AssignStmt* stmt = new AssignStmt();
@@ -374,7 +374,7 @@ class Parser {
       }
     }
     if (!s.empty()) {
-      WARN("%s:%d: extraneous text after `ifeq' directive", LOCF(loc_));
+      WARN_LOC(loc_, "extraneous text after `ifeq' directive");
       return true;
     }
     return true;
@@ -411,7 +411,7 @@ class Parser {
 
     num_if_nest_ = st->num_nest + 1;
     if (!HandleDirective(next_if, else_if_directives_)) {
-      WARN("%s:%d: extraneous text after `else' directive", LOCF(loc_));
+      WARN_LOC(loc_, "extraneous text after `else' directive");
     }
     num_if_nest_ = 0;
   }
diff --git a/rule.cc b/rule.cc
index 6942fb5..717d0f0 100644
--- a/rule.cc
+++ b/rule.cc
@@ -58,7 +58,7 @@ void ParseRule(Loc& loc, StringPiece line, char term,
                Rule** out_rule, RuleVarAssignment* rule_var) {
   size_t index = line.find(':');
   if (index == string::npos) {
-    ERROR("%s:%d: *** missing separator.", LOCF(loc));
+    ERROR_LOC(loc, "*** missing separator.");
   }
 
   StringPiece first = line.substr(0, index);
@@ -71,8 +71,7 @@ void ParseRule(Loc& loc, StringPiece line, char term,
       !outputs.empty() && IsPatternRule(outputs[0].str()));
   for (size_t i = 1; i < outputs.size(); i++) {
     if (IsPatternRule(outputs[i].str()) != is_first_pattern) {
-      ERROR("%s:%d: *** mixed implicit and normal rules: deprecated syntax",
-            LOCF(loc));
+      ERROR_LOC(loc, "*** mixed implicit and normal rules: deprecated syntax");
     }
   }
 
@@ -94,8 +93,8 @@ void ParseRule(Loc& loc, StringPiece line, char term,
     // target specific variable).
     // See https://github.com/google/kati/issues/83
     if (term_index == 0) {
-      KATI_WARN("%s:%d: defining a target which starts with `=', "
-                "which is not probably what you meant", LOCF(loc));
+      KATI_WARN_LOC(loc, "defining a target which starts with `=', "
+                    "which is not probably what you meant");
       buf = line.as_string();
       if (term)
         buf += term;
@@ -136,8 +135,7 @@ void ParseRule(Loc& loc, StringPiece line, char term,
   }
 
   if (is_first_pattern) {
-    ERROR("%s:%d: *** mixed implicit and normal rules: deprecated syntax",
-          LOCF(loc));
+    ERROR_LOC(loc, "*** mixed implicit and normal rules: deprecated syntax");
   }
 
   StringPiece second = rest.substr(0, index);
@@ -147,8 +145,8 @@ void ParseRule(Loc& loc, StringPiece line, char term,
     tok = TrimLeadingCurdir(tok);
     for (Symbol output : rule->outputs) {
       if (!Pattern(tok).Match(output.str())) {
-        WARN("%s:%d: target `%s' doesn't match the target pattern",
-             LOCF(loc), output.c_str());
+        WARN_LOC(loc, "target `%s' doesn't match the target pattern",
+                 output.c_str());
       }
     }
 
@@ -156,13 +154,13 @@ void ParseRule(Loc& loc, StringPiece line, char term,
   }
 
   if (rule->output_patterns.empty()) {
-    ERROR("%s:%d: *** missing target pattern.", LOCF(loc));
+    ERROR_LOC(loc, "*** missing target pattern.");
   }
   if (rule->output_patterns.size() > 1) {
-    ERROR("%s:%d: *** multiple target patterns.", LOCF(loc));
+    ERROR_LOC(loc, "*** multiple target patterns.");
   }
   if (!IsPatternRule(rule->output_patterns[0].str())) {
-    ERROR("%s:%d: *** target pattern contains no '%%'.", LOCF(loc));
+    ERROR_LOC(loc, "*** target pattern contains no '%%'.");
   }
   ParseInputs(rule, third);
 }
diff --git a/rule.h b/rule.h
index 84412c1..48394bb 100644
--- a/rule.h
+++ b/rule.h
@@ -49,7 +49,7 @@ class Rule {
 
  private:
   void Error(const string& msg) {
-    ERROR("%s:%d: %s", loc.filename, loc.lineno, msg.c_str());
+    ERROR_LOC(loc, "%s", msg.c_str());
   }
 };
 
diff --git a/runtest.rb b/runtest.rb
index fc594d5..190c11f 100755
--- a/runtest.rb
+++ b/runtest.rb
@@ -170,7 +170,7 @@ def normalize_kati_log(output)
   output.gsub!(/\/bin\/sh: ([^:]*): command not found/,
                "\\1: Command not found")
   output.gsub!(/.*: warning for parse error in an unevaluated line: .*\n/, '')
-  output.gsub!(/^FindEmulator: /, '')
+  output.gsub!(/^([^ ]+: )?FindEmulator: /, '')
   output.gsub!(/^\/bin\/sh: line 0: /, '')
   output.gsub!(/ (\.\/+)+kati\.\S+/, '') # kati log files in find_command.mk
   output.gsub!(/ (\.\/+)+test\S+.json/, '') # json files in find_command.mk
diff --git a/strutil.cc b/strutil.cc
index b094198..632d151 100644
--- a/strutil.cc
+++ b/strutil.cc
@@ -174,6 +174,13 @@ bool HasWord(StringPiece str, StringPiece w) {
   return true;
 }
 
+StringPiece TrimPrefix(StringPiece str, StringPiece prefix) {
+  ssize_t size_diff = str.size() - prefix.size();
+  if (size_diff < 0 || str.substr(0, prefix.size()) != prefix)
+    return str;
+  return str.substr(prefix.size());
+}
+
 StringPiece TrimSuffix(StringPiece str, StringPiece suffix) {
   ssize_t size_diff = str.size() - suffix.size();
   if (size_diff < 0 || str.substr(size_diff) != suffix)
diff --git a/strutil.h b/strutil.h
index d61160d..ff3d6a1 100644
--- a/strutil.h
+++ b/strutil.h
@@ -89,6 +89,8 @@ bool HasSuffix(StringPiece str, StringPiece suffix);
 
 bool HasWord(StringPiece str, StringPiece w);
 
+StringPiece TrimPrefix(StringPiece str, StringPiece suffix);
+
 StringPiece TrimSuffix(StringPiece str, StringPiece suffix);
 
 class Pattern {
diff --git a/strutil_test.cc b/strutil_test.cc
index 7a5a47d..a26cf4d 100644
--- a/strutil_test.cc
+++ b/strutil_test.cc
@@ -56,6 +56,20 @@ void TestHasSuffix() {
   assert(!HasSuffix("bar", "bbar"));
 }
 
+void TestTrimPrefix() {
+  ASSERT_EQ(TrimPrefix("foo", "foo"), "");
+  ASSERT_EQ(TrimPrefix("foo", "fo"), "o");
+  ASSERT_EQ(TrimPrefix("foo", ""), "foo");
+  ASSERT_EQ(TrimPrefix("foo", "fooo"), "foo");
+}
+
+void TestTrimSuffix() {
+  ASSERT_EQ(TrimSuffix("bar", "bar"), "");
+  ASSERT_EQ(TrimSuffix("bar", "ar"), "b");
+  ASSERT_EQ(TrimSuffix("bar", ""), "bar");
+  ASSERT_EQ(TrimSuffix("bar", "bbar"), "bar");
+}
+
 string SubstPattern(StringPiece str, StringPiece pat, StringPiece subst) {
   string r;
   Pattern(pat).AppendSubst(str, subst, &r);
@@ -190,6 +204,8 @@ int main() {
   TestWordScanner();
   TestHasPrefix();
   TestHasSuffix();
+  TestTrimPrefix();
+  TestTrimSuffix();
   TestSubstPattern();
   TestNoLineBreak();
   TestHasWord();