GN: Bug fixes in process_file_template

process_file_template would fail when the template argument was a string instead of a list, but the previous behavior (and the documentation) describes it as being able to take a string or a list as an argument.

It would also fail if the result was not an absolute file. This is required for most uses of sources expansions, but not for process_file_template which just returns the strings of whatever the results were. This adds "AsStrings" version of the sources expansions writer.

R=jamesr@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#289112}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289112 0039d316-1c4b-4281-b951-d872f2087c98

tools/gn/BUILD.gn

@@ -192,6 +192,7 @@ test("gn_unittests") {
     "function_get_label_info_unittest.cc",
     "function_get_path_info_unittest.cc",
     "function_get_target_outputs_unittest.cc",
+    "function_process_file_template_unittest.cc",
     "function_rebase_path_unittest.cc",
     "function_write_file_unittest.cc",
     "functions_target_unittest.cc",

tools/gn/function_process_file_template.cc

@@ -37,14 +37,8 @@ const char kProcessFileTemplate_Help[] =
     "  The template can be a string or a list. If it is a list, multiple\n"
     "  output strings are generated for each input.\n"
     "\n"
-    "  The following template substrings are used in the template arguments\n"
-    "  and are replaced with the corresponding part of the input file name:\n"
-    "\n"
-    "    {{source}}\n"
-    "        The entire source name.\n"
-    "\n"
-    "    {{source_name_part}}\n"
-    "        The source name with no path or extension.\n"
+    "  The template should contain source expansions to which each name in\n"
+    "  the source list is applied. See \"gn help source_expansion\".\n"
     "\n"
     "Example:\n"
     "\n"
@@ -78,19 +72,34 @@ Value RunProcessFileTemplate(Scope* scope,
                                   scope->GetSourceDir(), &input_files, err))
     return Value();
 
-  // Template.
+  std::vector<std::string> result_files;
   SubstitutionList subst;
-  if (!subst.Parse(args[1], err))
+
+  // Template.
+  const Value& template_arg = args[1];
+  if (template_arg.type() == Value::STRING) {
+    // Convert the string to a SubstitutionList with one pattern in it to
+    // simplify the code below.
+    std::vector<std::string> list;
+    list.push_back(template_arg.string_value());
+    if (!subst.Parse(list, template_arg.origin(), err))
+      return Value();
+  } else if (template_arg.type() == Value::LIST) {
+    if (!subst.Parse(template_arg, err))
+      return Value();
+  } else {
+    *err = Err(template_arg, "Not a string or a list.");
     return Value();
+  }
 
-  std::vector<SourceFile> result_files;
-  SubstitutionWriter::ApplyListToSources(
+  SubstitutionWriter::ApplyListToSourcesAsString(
       scope->settings(), subst, input_files, &result_files);
 
+  // Convert the list of strings to the return Value.
   Value ret(function, Value::LIST);
   ret.list_value().reserve(result_files.size());
   for (size_t i = 0; i < result_files.size(); i++)
-    ret.list_value().push_back(Value(function, result_files[i].value()));
+    ret.list_value().push_back(Value(function, result_files[i]));
 
   return ret;
 }

tools/gn/function_process_file_template_unittest.cc

+// 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/functions.h"
+#include "tools/gn/test_with_scope.h"
+
+TEST(FunctionProcessFileTemplates, SingleString) {
+  TestWithScope setup;
+
+  std::vector<Value> args;
+
+  Value sources(NULL, Value::LIST);
+  sources.list_value().push_back(Value(NULL, "//src/foo.txt"));
+  args.push_back(sources);
+
+  Value expansion(NULL, "1234{{source_name_part}}5678");
+  args.push_back(expansion);
+
+  Err err;
+  Value result = functions::RunProcessFileTemplate(
+      setup.scope(), NULL, args, &err);
+  EXPECT_FALSE(err.has_error());
+
+  ASSERT_TRUE(result.type() == Value::LIST);
+  ASSERT_EQ(1u, result.list_value().size());
+  ASSERT_TRUE(result.list_value()[0].type() == Value::STRING);
+  ASSERT_EQ("1234foo5678", result.list_value()[0].string_value());
+}
+
+TEST(FunctionProcessFileTemplates, MultipleStrings) {
+  TestWithScope setup;
+
+  std::vector<Value> args;
+
+  Value sources(NULL, Value::LIST);
+  sources.list_value().push_back(Value(NULL, "//src/one.txt"));
+  sources.list_value().push_back(Value(NULL, "//src/two.txt"));
+  args.push_back(sources);
+
+  Value expansions(NULL, Value::LIST);
+  expansions.list_value().push_back(
+      Value(NULL, "1234{{source_name_part}}5678"));
+  expansions.list_value().push_back(
+      Value(NULL, "ABCD{{source_file_part}}EFGH"));
+  args.push_back(expansions);
+
+  Err err;
+  Value result = functions::RunProcessFileTemplate(
+      setup.scope(), NULL, args, &err);
+  EXPECT_FALSE(err.has_error());
+
+  ASSERT_TRUE(result.type() == Value::LIST);
+  ASSERT_EQ(4u, result.list_value().size());
+  ASSERT_TRUE(result.list_value()[0].type() == Value::STRING);
+  ASSERT_TRUE(result.list_value()[1].type() == Value::STRING);
+  ASSERT_TRUE(result.list_value()[2].type() == Value::STRING);
+  ASSERT_TRUE(result.list_value()[3].type() == Value::STRING);
+  ASSERT_EQ("1234one5678", result.list_value()[0].string_value());
+  ASSERT_EQ("ABCDone.txtEFGH", result.list_value()[1].string_value());
+  ASSERT_EQ("1234two5678", result.list_value()[2].string_value());
+  ASSERT_EQ("ABCDtwo.txtEFGH", result.list_value()[3].string_value());
+}

tools/gn/gn.gyp

@@ -193,6 +193,7 @@
         'function_get_label_info_unittest.cc',
         'function_get_path_info_unittest.cc',
         'function_get_target_outputs_unittest.cc',
+        'function_process_file_template_unittest.cc',
         'function_rebase_path_unittest.cc',
         'function_write_file_unittest.cc',
         'functions_target_unittest.cc',

tools/gn/substitution_writer.cc

@@ -163,6 +163,20 @@ SourceFile SubstitutionWriter::ApplyPatternToSource(
       const Settings* settings,
       const SubstitutionPattern& pattern,
       const SourceFile& source) {
+  std::string result_value = ApplyPatternToSourceAsString(
+      settings, pattern, source);
+  CHECK(!result_value.empty() && result_value[0] == '/')
+      << "The result of the pattern \""
+      << pattern.AsString()
+      << "\" was not a path beginning in \"/\" or \"//\".";
+  return SourceFile(SourceFile::SWAP_IN, &result_value);
+}
+
+// static
+std::string SubstitutionWriter::ApplyPatternToSourceAsString(
+    const Settings* settings,
+    const SubstitutionPattern& pattern,
+    const SourceFile& source) {
   std::string result_value;
   for (size_t i = 0; i < pattern.ranges().size(); i++) {
     const SubstitutionPattern::Subrange& subrange = pattern.ranges()[i];
@@ -174,11 +188,7 @@ SourceFile SubstitutionWriter::ApplyPatternToSource(
                                 OUTPUT_ABSOLUTE, SourceDir()));
     }
   }
-  CHECK(!result_value.empty() && result_value[0] == '/')
-      << "The result of the pattern \""
-      << pattern.AsString()
-      << "\" was not a path beginning in \"/\" or \"//\".";
-  return SourceFile(SourceFile::SWAP_IN, &result_value);
+  return result_value;
 }
 
 // static
@@ -208,6 +218,18 @@ void SubstitutionWriter::ApplyListToSource(
   }
 }
 
+// static
+void SubstitutionWriter::ApplyListToSourceAsString(
+    const Settings* settings,
+    const SubstitutionList& list,
+    const SourceFile& source,
+    std::vector<std::string>* output) {
+  for (size_t i = 0; i < list.list().size(); i++) {
+    output->push_back(ApplyPatternToSourceAsString(
+        settings, list.list()[i], source));
+  }
+}
+
 // static
 void SubstitutionWriter::ApplyListToSourceAsOutputFile(
     const Settings* settings,
@@ -231,6 +253,17 @@ void SubstitutionWriter::ApplyListToSources(
     ApplyListToSource(settings, list, sources[i], output);
 }
 
+// static
+void SubstitutionWriter::ApplyListToSourcesAsString(
+    const Settings* settings,
+    const SubstitutionList& list,
+    const std::vector<SourceFile>& sources,
+    std::vector<std::string>* output) {
+  output->clear();
+  for (size_t i = 0; i < sources.size(); i++)
+    ApplyListToSourceAsString(settings, list, sources[i], output);
+}
+
 // static
 void SubstitutionWriter::ApplyListToSourcesAsOutputFile(
     const Settings* settings,

tools/gn/substitution_writer.h

@@ -47,11 +47,18 @@ class SubstitutionWriter {
       std::vector<OutputFile>* output);
 
   // Applies the substitution pattern to a source file, returning the result
-  // as either a SourceFile or OutputFile.
+  // as either a string, a SourceFile or an OutputFile. If the result is
+  // expected to be a SourceFile or an OutputFile, this will CHECK if the
+  // result isn't in the correct directory. The caller should validate this
+  // first (see for example IsFileInOuputDir).
   static SourceFile ApplyPatternToSource(
       const Settings* settings,
       const SubstitutionPattern& pattern,
       const SourceFile& source);
+  static std::string ApplyPatternToSourceAsString(
+      const Settings* settings,
+      const SubstitutionPattern& pattern,
+      const SourceFile& source);
   static OutputFile ApplyPatternToSourceAsOutputFile(
       const Settings* settings,
       const SubstitutionPattern& pattern,
@@ -66,6 +73,11 @@ class SubstitutionWriter {
       const SubstitutionList& list,
       const SourceFile& source,
       std::vector<SourceFile>* output);
+  static void ApplyListToSourceAsString(
+      const Settings* settings,
+      const SubstitutionList& list,
+      const SourceFile& source,
+      std::vector<std::string>* output);
   static void ApplyListToSourceAsOutputFile(
       const Settings* settings,
       const SubstitutionList& list,
@@ -79,6 +91,11 @@ class SubstitutionWriter {
       const SubstitutionList& list,
       const std::vector<SourceFile>& sources,
       std::vector<SourceFile>* output);
+  static void ApplyListToSourcesAsString(
+      const Settings* settings,
+      const SubstitutionList& list,
+      const std::vector<SourceFile>& sources,
+      std::vector<std::string>* output);
   static void ApplyListToSourcesAsOutputFile(
       const Settings* settings,
       const SubstitutionList& list,