Fix various issues with `gn analyze`.

tools/gn/analyzer.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/analyzer.h"
 
 #include <algorithm>
 #include <iterator>
 #include <set>
 #include <vector>
@@ skipping 139 matching lines @@
   strings = GetStringVector(*dict, "files", &err);
   if (err.has_error())
     return err;
   for (auto s : strings) {
     if (!IsPathSourceAbsolute(s) && !IsPathAbsolute(s))
       return Err(Location(),
                  "\"" + s + "\" is not a source-absolute or absolute path.");
     inputs->source_vec.push_back(SourceFile(s));
   }
 
-  strings = GetStringVector(*dict, "compile_targets", &err);
+  strings = GetStringVector(*dict, "additional_compile_targets", &err);
   if (err.has_error())
     return err;
 
   inputs->compile_included_all = false;
   for (auto& s : strings) {
     if (s == "all") {
       inputs->compile_included_all = true;
     } else {
       inputs->compile_vec.push_back(
           AbsoluteOrSourceAbsoluteStringToLabel(default_toolchain, s, &err));
@@ skipping 15 matching lines @@
   for (auto& s : inputs->source_vec)
     inputs->source_files.insert(&s);
   for (auto& l : inputs->compile_vec)
     inputs->compile_labels.insert(l);
   for (auto& l : inputs->test_vec)
     inputs->test_labels.insert(l);
   return Err();
 }
 
 std::string OutputsToJSON(const Outputs& outputs,
-                          const Label& default_toolchain) {
+                          const Label& default_toolchain, Err *err) {
   std::string output;
   auto value = base::MakeUnique<base::DictionaryValue>();
 
   if (outputs.error.size()) {
     WriteString(*value, "error", outputs.error);
     WriteLabels(default_toolchain, *value, "invalid_targets",
                 outputs.invalid_labels);
   } else {
     WriteString(*value, "status", outputs.status);
     WriteLabels(default_toolchain, *value, "compile_targets",
                 outputs.compile_labels);
     WriteLabels(default_toolchain, *value, "test_targets", outputs.test_labels);
   }
 
-  base::JSONWriter::Write(*value.get(), &output);
+  if (base::JSONWriter::Write(*value.get(), &output))
+    *err = Err();

[brettw, 2016/09/15 23:22:45]

You don't need this, only write the error if it's an error.
so
  if (!...
    *err = Err(...);

[Dirk Pranke, 2016/09/16 00:11:11]

ok.

+  else
+    *err = Err(Location(), "Failed to marshal JSON value for output");
   return output;
 }
 
 }  // namespace
 
 Analyzer::Analyzer(const Builder& builder)
     : all_targets_(builder.GetAllResolvedTargets()),
       default_toolchain_(builder.loader()->GetDefaultToolchain()) {
   for (const auto* target : all_targets_) {
     labels_to_targets_[target->label()] = target;
     for (const auto& dep_pair : target->GetDeps(Target::DEPS_ALL))
       dep_map_.insert(std::make_pair(dep_pair.ptr, target));
   }
   for (const auto* target : all_targets_) {
     if (dep_map_.find(target) == dep_map_.end())
       roots_.insert(target);
   }
 }
 
 Analyzer::~Analyzer() {}
 
 std::string Analyzer::Analyze(const std::string& input, Err* err) const {
   Inputs inputs;
   Outputs outputs;
 
   Err local_err = JSONToInputs(default_toolchain_, input, &inputs);
   if (local_err.has_error()) {
     outputs.error = local_err.message();
-    if (err)
-      *err = Err();
-    return "";
+    return OutputsToJSON(outputs, default_toolchain_, err);
   }
 
   LabelSet invalid_labels;
   for (const auto& label : InvalidLabels(inputs.compile_labels))
     invalid_labels.insert(label);
   for (const auto& label : InvalidLabels(inputs.test_labels))
     invalid_labels.insert(label);
   if (!invalid_labels.empty()) {
     outputs.error = "Invalid targets";
     outputs.invalid_labels = invalid_labels;
-    if (err)
-      *err = Err();
-    return OutputsToJSON(outputs, default_toolchain_);
+    return OutputsToJSON(outputs, default_toolchain_, err);
   }
 
   TargetSet affected_targets = AllAffectedTargets(inputs.source_files);
   if (affected_targets.empty()) {
     outputs.status = "No dependency";
-    if (err)
-      *err = Err();
-    return OutputsToJSON(outputs, default_toolchain_);
+    return OutputsToJSON(outputs, default_toolchain_, err);
   }
 
   // TODO: We can do smarter things when we detect changes to build files.
   // For example, if all of the ninja files are unchanged, we know that we
   // can ignore changes to these files. Also, for most .gn files, we can
   // treat a change as simply affecting every target, config, or toolchain
   // defined in that file.
   if (AnyBuildFilesWereModified(inputs.source_files)) {
     outputs.status = "Found dependency (all)";
     outputs.compile_labels = inputs.compile_labels;
     outputs.test_labels = inputs.test_labels;
-    if (err)
-      *err = Err();
-    return OutputsToJSON(outputs, default_toolchain_);
+    return OutputsToJSON(outputs, default_toolchain_, err);
   }
 
   TargetSet compile_targets = TargetsFor(inputs.compile_labels);
   if (inputs.compile_included_all) {
     for (auto& root : roots_)
       compile_targets.insert(root);
   }
   TargetSet filtered_targets = Filter(compile_targets);
   outputs.compile_labels =
       LabelsFor(Intersect(filtered_targets, affected_targets));
 
   TargetSet test_targets = TargetsFor(inputs.test_labels);
   outputs.test_labels = LabelsFor(Intersect(test_targets, affected_targets));
 
   if (outputs.compile_labels.empty() && outputs.test_labels.empty())
     outputs.status = "No dependency";
   else
     outputs.status = "Found dependency";
-  *err = Err();
-  return OutputsToJSON(outputs, default_toolchain_);
+  return OutputsToJSON(outputs, default_toolchain_, err);
 }
 
 TargetSet Analyzer::AllAffectedTargets(
     const SourceFileSet& source_files) const {
   TargetSet direct_matches;
   for (const auto& source_file : source_files)
     AddTargetsDirectlyReferringToFileTo(source_file, &direct_matches);
   TargetSet all_matches;
   for (const auto& match : direct_matches)
     AddAllRefsTo(match, &all_matches);
@@ skipping 88 matching lines @@
 void Analyzer::AddAllRefsTo(const Target* target, TargetSet* results) const {
   if (results->find(target) != results->end())
     return;  // Already found this target.
   results->insert(target);
 
   auto dep_begin = dep_map_.lower_bound(target);
   auto dep_end = dep_map_.upper_bound(target);
   for (auto cur_dep = dep_begin; cur_dep != dep_end; cur_dep++)
     AddAllRefsTo(cur_dep->second, results);
 }