GN: Handle appending non-lists to the sources variable.

Previously this would assert because the thing that filtered lists assumed the thing being appended was a list.

BUG=
R=scottmg@chromium.org

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

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

6 files changed, 101 insertions, 4 deletions

diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn
index 7867a48..f772aa1 100644
--- a/tools/gn/BUILD.gn
+++ b/tools/gn/BUILD.gn
@@ -196,6 +196,7 @@ test("gn_unittests") {
     "ninja_copy_target_writer_unittest.cc",
     "ninja_helper_unittest.cc",
     "ninja_script_target_writer_unittest.cc",
+    "operators_unittest.cc",
     "parser_unittest.cc",
     "path_output_unittest.cc",
     "pattern_unittest.cc",
diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp
index 8deb840..2b79a00 100644
--- a/tools/gn/gn.gyp
+++ b/tools/gn/gn.gyp
@@ -185,6 +185,7 @@
         'ninja_helper_unittest.cc',
         'ninja_copy_target_writer_unittest.cc',
         'ninja_script_target_writer_unittest.cc',
+        'operators_unittest.cc',
         'parser_unittest.cc',
         'path_output_unittest.cc',
         'pattern_unittest.cc',
diff --git a/tools/gn/operators.cc b/tools/gn/operators.cc
index 450e7d1..03075eb 100644
--- a/tools/gn/operators.cc
+++ b/tools/gn/operators.cc
@@ -23,17 +23,20 @@ void AppendFilteredSourcesToValue(const Scope* scope,
                                   Value* dest) {
   const PatternList* filter = scope->GetSourcesAssignmentFilter();
 
-  const std::vector<Value>& source_list = source.list_value();
-
   if (source.type() == Value::STRING) {
     if (!filter || filter->is_empty() ||
         !filter->MatchesValue(source))
       dest->list_value().push_back(source);
     return;
   }
+  if (source.type() != Value::LIST) {
+    // Any non-list and non-string being added to a list can just get appended,
+    // we're not going to filter it.
+    dest->list_value().push_back(source);
+    return;
+  }
 
-  // Otherwise source is a list.
-  DCHECK(source.type() == Value::LIST);
+  const std::vector<Value>& source_list = source.list_value();
   if (!filter || filter->is_empty()) {
     // No filter, append everything.
     for (size_t i = 0; i < source_list.size(); i++)
diff --git a/tools/gn/operators_unittest.cc b/tools/gn/operators_unittest.cc
new file mode 100644
index 0000000..16d626f
--- /dev/null
+++ b/tools/gn/operators_unittest.cc
@@ -0,0 +1,86 @@
+// Copyright (c) 2013 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/operators.h"
+#include "tools/gn/parse_tree.h"
+#include "tools/gn/pattern.h"
+#include "tools/gn/test_with_scope.h"
+
+namespace {
+
+bool IsValueIntegerEqualing(const Value& v, int64 i) {
+  if (v.type() != Value::INTEGER)
+    return false;
+  return v.int_value() == i;
+}
+
+bool IsValueStringEqualing(const Value& v, const char* s) {
+  if (v.type() != Value::STRING)
+    return false;
+  return v.string_value() == s;
+}
+
+}  // namespace
+
+TEST(Operators, SourcesAppend) {
+  Err err;
+  TestWithScope setup;
+
+  // Set up "sources" with an empty list.
+  const char sources[] = "sources";
+  setup.scope()->SetValue(sources, Value(NULL, Value::LIST), NULL);
+
+  // Set up the operator.
+  BinaryOpNode node;
+  const char token_value[] = "+=";
+  Token op(Location(), Token::PLUS_EQUALS, token_value);
+  node.set_op(op);
+
+  // Append to the sources variable.
+  Token identifier_token(Location(), Token::IDENTIFIER, sources);
+  node.set_left(scoped_ptr<ParseNode>(new IdentifierNode(identifier_token)));
+
+  // Set up the filter on the scope to remove everything ending with "rm"
+  scoped_ptr<PatternList> pattern_list(new PatternList);
+  pattern_list->Append(Pattern("*rm"));
+  setup.scope()->set_sources_assignment_filter(pattern_list.Pass());
+
+  // Append an integer.
+  const char integer_value[] = "5";
+  Token integer(Location(), Token::INTEGER, integer_value);
+  node.set_right(scoped_ptr<ParseNode>(new LiteralNode(integer)));
+  node.Execute(setup.scope(), &err);
+  EXPECT_FALSE(err.has_error());
+
+  // Append a string that doesn't match the pattern, it should get appended.
+  const char string_1_value[] = "\"good\"";
+  Token string_1(Location(), Token::STRING, string_1_value);
+  node.set_right(scoped_ptr<ParseNode>(new LiteralNode(string_1)));
+  node.Execute(setup.scope(), &err);
+  EXPECT_FALSE(err.has_error());
+
+  // Append a string that does match the pattern, it should be a no-op.
+  const char string_2_value[] = "\"foo-rm\"";
+  Token string_2(Location(), Token::STRING, string_2_value);
+  node.set_right(scoped_ptr<ParseNode>(new LiteralNode(string_2)));
+  node.Execute(setup.scope(), &err);
+  EXPECT_FALSE(err.has_error());
+
+  // Append a list with the two strings from above.
+  ListNode list;
+  list.append_item(scoped_ptr<ParseNode>(new LiteralNode(string_1)));
+  list.append_item(scoped_ptr<ParseNode>(new LiteralNode(string_2)));
+  ExecuteBinaryOperator(setup.scope(), &node, node.left(), &list, &err);
+  EXPECT_FALSE(err.has_error());
+
+  // The sources variable in the scope should now have: [ 5, "good", "good" ]
+  const Value* value = setup.scope()->GetValue(sources);
+  ASSERT_TRUE(value);
+  ASSERT_EQ(Value::LIST, value->type());
+  ASSERT_EQ(3u, value->list_value().size());
+  EXPECT_TRUE(IsValueIntegerEqualing(value->list_value()[0], 5));
+  EXPECT_TRUE(IsValueStringEqualing(value->list_value()[1], "good"));
+  EXPECT_TRUE(IsValueStringEqualing(value->list_value()[2], "good"));
+}
diff --git a/tools/gn/pattern.cc b/tools/gn/pattern.cc
index 7a198e9..8ee30a2 100644
--- a/tools/gn/pattern.cc
+++ b/tools/gn/pattern.cc
@@ -188,6 +188,10 @@ PatternList::PatternList() {
 PatternList::~PatternList() {
 }
 
+void PatternList::Append(const Pattern& pattern) {
+  patterns_.push_back(pattern);
+}
+
 void PatternList::SetFromValue(const Value& v, Err* err) {
   patterns_.clear();
 
diff --git a/tools/gn/pattern.h b/tools/gn/pattern.h
index a7769fa..24dffd2 100644
--- a/tools/gn/pattern.h
+++ b/tools/gn/pattern.h
@@ -74,6 +74,8 @@ class PatternList {
 
   bool is_empty() const { return patterns_.empty(); }
 
+  void Append(const Pattern& pattern);
+
   // Initializes the pattern list from a give list of pattern strings. Sets
   // |*err| on failure.
   void SetFromValue(const Value& v, Err* err);