Refactor GN source expansions.

This is in preparation for using {{patterns}} in toolchain definitions. It makes them
more generic and usable for this, without actually implementing any new features or
changing behavior. The only external change should be that the $in variable is used for
actions instead of making a redundant new variable (used to be called $source).

It removes the FileTemplate class and explodes it into several files: the type list and
helpers (substitution_type), a single pattern (substitution_pattern), a list of patterns
(substitution_list), and a class of helper functions for processing templates in various
ways (substitution_writer).

Previously, the things needing substitutions (args, outputs, depfile) were stored as
strings and parsed in to FileTemplates as needed. This new method stores the
SubstitutionList/Pattern directly on the target. This allows it to issue parse errors
(previously such errors were ignored since errors weren't reportable when writing Ninja
files) and cleans up a lot of the code that uses it.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/429423002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287847 0039d316-1c4b-4281-b951-d872f2087c98

base/BUILD.gn

@@ -171,7 +171,6 @@ component("base") {
     "environment.cc",
     "environment.h",
     "event_recorder.h",
-    "event_recorder_stubs.cc",
     "event_recorder_win.cc",
     "file_descriptor_posix.h",
     "file_util.cc",

tools/gn/BUILD.gn

@@ -44,8 +44,6 @@ static_library("gn_lib") {
     "err.h",
     "escape.cc",
     "escape.h",
-    "file_template.cc",
-    "file_template.h",
     "filesystem_utils.cc",
     "filesystem_utils.h",
     "functions.cc",
@@ -132,6 +130,14 @@ static_library("gn_lib") {
     "standard_out.h",
     "string_utils.cc",
     "string_utils.h",
+    "substitution_list.cc",
+    "substitution_list.h",
+    "substitution_pattern.cc",
+    "substitution_pattern.h",
+    "substitution_type.cc",
+    "substitution_type.h",
+    "substitution_writer.cc",
+    "substitution_writer.h",
     "target.cc",
     "target.h",
     "target_generator.cc",
@@ -175,12 +181,12 @@ executable("gn") {
 
 test("gn_unittests") {
   sources = [
+    "action_target_generator_unittest.cc",
     "builder_unittest.cc",
     "c_include_iterator_unittest.cc",
     "config_values_extractors_unittest.cc",
     "escape_unittest.cc",
     "filesystem_utils_unittest.cc",
-    "file_template_unittest.cc",
     "function_foreach_unittest.cc",
     "function_get_label_info_unittest.cc",
     "function_get_path_info_unittest.cc",
@@ -208,6 +214,8 @@ test("gn_unittests") {
     "scope_unittest.cc",
     "source_dir_unittest.cc",
     "string_utils_unittest.cc",
+    "substitution_pattern_unittest.cc",
+    "substitution_writer_unittest.cc",
     "target_generator_unittest.cc",
     "target_unittest.cc",
     "template_unittest.cc",

tools/gn/action_target_generator.cc

@@ -13,21 +13,6 @@
 #include "tools/gn/value_extractors.h"
 #include "tools/gn/variables.h"
 
-namespace {
-
-// Returns true if the list of files looks like it might have a {{ }} pattern
-// in it. Used for error checking.
-bool StringListHasPattern(const std::vector<std::string>& files) {
-  for (size_t i = 0; i < files.size(); i++) {
-    if (files[i].find("{{") != std::string::npos &&
-        files[i].find("}}") != std::string::npos)
-      return true;
-  }
-  return false;
-}
-
-}  // namespace
-
 ActionTargetGenerator::ActionTargetGenerator(
     Target* target,
     Scope* scope,
@@ -67,7 +52,7 @@ void ActionTargetGenerator::DoRun() {
   if (err_->has_error())
     return;
 
-  FillOutputs();
+  FillOutputs(output_type_ == Target::ACTION_FOREACH);
   if (err_->has_error())
     return;
 
@@ -108,32 +93,35 @@ void ActionTargetGenerator::FillScriptArgs() {
   if (!value)
     return;
 
-  if (!ExtractListOfStringValues(*value, &target_->action_values().args(),
-                                 err_))
+  if (!target_->action_values().args().Parse(*value, err_))
     return;
 }
 
 void ActionTargetGenerator::FillDepfile() {
   const Value* value = scope_->GetValue(variables::kDepfile, true);
   if (!value)
     return;
-  target_->action_values().set_depfile(
-      scope_->settings()->build_settings()->build_dir().ResolveRelativeFile(
-          value->string_value()));
+
+  SubstitutionPattern depfile;
+  if (!depfile.Parse(*value, err_))
+    return;
+  if (!EnsureSubstitutionIsInOutputDir(depfile, *value))
+    return;
+
+  target_->action_values().set_depfile(depfile);
 }
 
 void ActionTargetGenerator::CheckOutputs() {
-  const std::vector<std::string>& outputs = target_->action_values().outputs();
-  if (outputs.empty()) {
+  const SubstitutionList& outputs = target_->action_values().outputs();
+  if (outputs.list().empty()) {
     *err_ = Err(function_call_, "Action has no outputs.",
         "If you have no outputs, the build system can not tell when your\n"
         "script needs to be run.");
     return;
   }
 
   if (output_type_ == Target::ACTION) {
-    // Make sure the outputs for an action have no patterns in them.
-    if (StringListHasPattern(outputs)) {
+    if (!outputs.required_types().empty()) {
       *err_ = Err(function_call_, "Action has patterns in the output.",
           "An action target should have the outputs completely specified. If\n"
           "you want to provide a mapping from source to output, use an\n"
@@ -142,7 +130,7 @@ void ActionTargetGenerator::CheckOutputs() {
     }
   } else if (output_type_ == Target::ACTION_FOREACH) {
     // A foreach target should always have a pattern in the outputs.
-    if (!StringListHasPattern(outputs)) {
+    if (outputs.required_types().empty()) {
       *err_ = Err(function_call_,
           "action_foreach should have a pattern in the output.",
           "An action_foreach target should have a source expansion pattern in\n"

tools/gn/action_target_generator_unittest.cc

@@ -0,0 +1,42 @@
+// Copyright 2014 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 "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/scheduler.h"
+#include "tools/gn/test_with_scope.h"
+
+// Tests that actions can't have output substitutions.
+TEST(ActionTargetGenerator, ActionOutputSubstitutions) {
+  Scheduler scheduler;
+  TestWithScope setup;
+  Scope::ItemVector items_;
+  setup.scope()->set_item_collector(&items_);
+
+  // First test one with no substitutions, this should be valid.
+  TestParseInput input_good(
+      "action(\"foo\") {\n"
+      "  script = \"//foo.py\"\n"
+      "  sources = [ \"//bar.txt\" ]\n"
+      "  outputs = [ \"//out/Debug/one.txt\" ]\n"
+      "}");
+  ASSERT_FALSE(input_good.has_error());
+
+  // This should run fine.
+  Err err;
+  input_good.parsed()->Execute(setup.scope(), &err);
+  ASSERT_FALSE(err.has_error()) << err.message();
+
+  // Same thing with a pattern in the output should fail.
+  TestParseInput input_bad(
+      "action(\"foo\") {\n"
+      "  script = \"//foo.py\"\n"
+      "  sources = [ \"//bar.txt\" ]\n"
+      "  outputs = [ \"//out/Debug/{{source_name_part}}.txt\" ]\n"
+      "}");
+  ASSERT_FALSE(input_bad.has_error());
+
+  // This should run fine.
+  input_bad.parsed()->Execute(setup.scope(), &err);
+  ASSERT_TRUE(err.has_error());
+}

tools/gn/action_values.h

@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "tools/gn/source_file.h"
+#include "tools/gn/substitution_list.h"
 
 // Holds the values (outputs, args, script name, etc.) for either an action or
 // an action_foreach target.
@@ -23,24 +24,24 @@ class ActionValues {
   void set_script(const SourceFile& s) { script_ = s; }
 
   // Arguments to the script.
-  std::vector<std::string>& args() { return args_; }
-  const std::vector<std::string>& args() const { return args_; }
+  SubstitutionList& args() { return args_; }
+  const SubstitutionList& args() const { return args_; }
 
   // Files created by the script. These are strings rather than SourceFiles
   // since they will often contain {{source expansions}}.
-  std::vector<std::string>& outputs() { return outputs_; }
-  const std::vector<std::string>& outputs() const { return outputs_; }
+  SubstitutionList& outputs() { return outputs_; }
+  const SubstitutionList& outputs() const { return outputs_; }
 
   // Depfile generated by the script.
-  const SourceFile& depfile() const { return depfile_; }
-  bool has_depfile() const { return !depfile_.is_null(); }
-  void set_depfile(const SourceFile& depfile) { depfile_ = depfile; }
+  const SubstitutionPattern& depfile() const { return depfile_; }
+  bool has_depfile() const { return !depfile_.ranges().empty(); }
+  void set_depfile(const SubstitutionPattern& depfile) { depfile_ = depfile; }
 
  private:
   SourceFile script_;
-  std::vector<std::string> args_;
-  std::vector<std::string> outputs_;
-  SourceFile depfile_;
+  SubstitutionList args_;
+  SubstitutionList outputs_;
+  SubstitutionPattern depfile_;
 
   DISALLOW_COPY_AND_ASSIGN(ActionValues);
 };

tools/gn/command_desc.cc

@@ -10,12 +10,12 @@
 #include "tools/gn/commands.h"
 #include "tools/gn/config.h"
 #include "tools/gn/config_values_extractors.h"
-#include "tools/gn/file_template.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/item.h"
 #include "tools/gn/label.h"
 #include "tools/gn/setup.h"
 #include "tools/gn/standard_out.h"
+#include "tools/gn/substitution_writer.h"
 #include "tools/gn/target.h"
 
 namespace commands {
@@ -282,25 +282,6 @@ void PrintFileList(const Target::FileList& files,
     OutputString(indent + sorted[i].value() + "\n");
 }
 
-// This sorts the list.
-void PrintStringList(const std::vector<std::string>& strings,
-                     const std::string& header,
-                     bool indent_extra,
-                     bool display_header) {
-  if (strings.empty())
-    return;
-
-  if (display_header)
-    OutputString("\n" + header + ":\n");
-
-  std::string indent = indent_extra ? "    " : "  ";
-
-  std::vector<std::string> sorted = strings;
-  std::sort(sorted.begin(), sorted.end());
-  for (size_t i = 0; i < sorted.size(); i++)
-    OutputString(indent + sorted[i] + "\n");
-}
-
 void PrintSources(const Target* target, bool display_header) {
   PrintFileList(target->sources(), "sources", false, display_header);
 }
@@ -310,28 +291,35 @@ void PrintInputs(const Target* target, bool display_header) {
 }
 
 void PrintOutputs(const Target* target, bool display_header) {
-  if (target->output_type() == Target::ACTION) {
-    // Just display the outputs directly.
-    PrintStringList(target->action_values().outputs(), "outputs", false,
-                    display_header);
-  } else if (target->output_type() == Target::ACTION_FOREACH ||
-             target->output_type() == Target::COPY_FILES) {
-    // Display both the output pattern and resolved list.
-    if (display_header)
-      OutputString("\noutputs:\n");
-
-    // Display the pattern.
-    OutputString("  Output pattern:\n");
-    PrintStringList(target->action_values().outputs(), "", true, false);
+  if (display_header)
+    OutputString("\noutputs:\n");
 
-    // Now display what that resolves to given the sources.
-    OutputString("\n  Resolved output file list:\n");
+  if (target->output_type() == Target::ACTION) {
+    // Action, print out outputs, don't apply sources to it.
+    for (size_t i = 0; i < target->action_values().outputs().list().size();
+         i++) {
+      OutputString("  " +
+                   target->action_values().outputs().list()[i].AsString() +
+                   "\n");
+    }
+  } else {
+    const SubstitutionList& outputs = target->action_values().outputs();
+    if (!outputs.required_types().empty()) {
+      // Display the pattern and resolved pattern separately, since there are
+      // subtitutions used.
+      OutputString("  Output pattern:\n");
+      for (size_t i = 0; i < outputs.list().size(); i++)
+        OutputString("    " + outputs.list()[i].AsString() + "\n");
+
+      // Now display what that resolves to given the sources.
+      OutputString("\n  Resolved output file list:\n");
+    }
 
-    std::vector<std::string> output_strings;
-    FileTemplate file_template = FileTemplate::GetForTargetOutputs(target);
-    for (size_t i = 0; i < target->sources().size(); i++)
-      file_template.Apply(target->sources()[i], &output_strings);
-    PrintStringList(output_strings, "", true, false);
+    // Resolved output list.
+    std::vector<SourceFile> output_files;
+    SubstitutionWriter::ApplyListToSources(target->settings(), outputs,
+                                           target->sources(), &output_files);
+    PrintFileList(output_files, "", true, false);
   }
 }
 
@@ -344,16 +332,18 @@ void PrintScript(const Target* target, bool display_header) {
 void PrintArgs(const Target* target, bool display_header) {
   if (display_header)
     OutputString("\nargs:\n");
-  for (size_t i = 0; i < target->action_values().args().size(); i++)
-    OutputString("  " + target->action_values().args()[i] + "\n");
+  for (size_t i = 0; i < target->action_values().args().list().size(); i++) {
+    OutputString("  " +
+                 target->action_values().args().list()[i].AsString() + "\n");
+  }
 }
 
 void PrintDepfile(const Target* target, bool display_header) {
-  if (target->action_values().depfile().value().empty())
+  if (target->action_values().depfile().empty())
     return;
   if (display_header)
     OutputString("\ndepfile:\n");
-  OutputString("  " + target->action_values().depfile().value() + "\n");
+  OutputString("  " + target->action_values().depfile().AsString() + "\n");
 }
 
 // Attribute the origin for attributing from where a target came from. Does

tools/gn/command_help.cc

@@ -8,12 +8,12 @@
 #include "tools/gn/args.h"
 #include "tools/gn/commands.h"
 #include "tools/gn/err.h"
-#include "tools/gn/file_template.h"
 #include "tools/gn/functions.h"
 #include "tools/gn/input_conversion.h"
 #include "tools/gn/pattern.h"
 #include "tools/gn/setup.h"
 #include "tools/gn/standard_out.h"
+#include "tools/gn/substitution_writer.h"
 #include "tools/gn/variables.h"
 
 namespace commands {

tools/gn/copy_target_generator.cc

@@ -26,7 +26,7 @@ void CopyTargetGenerator::DoRun() {
   FillSources();
   if (err_->has_error())
     return;
-  FillOutputs();
+  FillOutputs(true);
   if (err_->has_error())
     return;
 
@@ -35,7 +35,7 @@ void CopyTargetGenerator::DoRun() {
         "You have to specify at least one file to copy in the \"sources\".");
     return;
   }
-  if (target_->action_values().outputs().size() != 1) {
+  if (target_->action_values().outputs().list().size() != 1) {
     *err_ = Err(function_call_, "Copy command must have exactly one output.",
         "You must specify exactly one value in the \"outputs\" array for the "
         "destination of the copy\n(see \"gn help copy\"). If there are "

tools/gn/escape.cc

@@ -46,8 +46,6 @@ void EscapeStringToString_Ninja(const base::StringPiece& str,
                                 bool* needed_quoting) {
   for (size_t i = 0; i < str.size(); i++)
     NinjaEscapeChar(str[i], dest);
-  if (needed_quoting)
-    *needed_quoting = false;
 }
 
 // Escape for CommandLineToArgvW and additionally escape Ninja characters.