Make the JSON Schema compiler handle enums declared in other namespaces.

BUG=477457
R=rdevlin.cronin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#326703}

5 files changed, 84 insertions, 50 deletions

diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py
index 73413ef..6faac39 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -363,11 +363,16 @@ class _Generator(object):
     for prop in type_.properties.values():
       prop_var = 'this->%s' % prop.unix_name
       if prop.optional:
-        # Optional enum values are generated with a NONE enum value.
         underlying_type = self._type_helper.FollowRef(prop.type_)
         if underlying_type.property_type == PropertyType.ENUM:
-          c.Sblock('if (%s != %s) {' %
+          # Optional enum values are generated with a NONE enum value,
+          # potentially from another namespace.
+          maybe_namespace = ''
+          if underlying_type.namespace != self._namespace:
+            maybe_namespace = '%s::' % underlying_type.namespace.unix_name
+          c.Sblock('if (%s != %s%s) {' %
               (prop_var,
+               maybe_namespace,
                self._type_helper.GetEnumNoneValue(prop.type_)))
         else:
           c.Sblock('if (%s.get()) {' % prop_var)
diff --git a/tools/json_schema_compiler/h_generator.py b/tools/json_schema_compiler/h_generator.py
index 566df07..facd5e4 100644
--- a/tools/json_schema_compiler/h_generator.py
+++ b/tools/json_schema_compiler/h_generator.py
@@ -41,6 +41,12 @@ class _Generator(object):
     output_file = os.path.splitext(self._namespace.source_file)[0] + '.h'
     ifndef_name = cpp_util.GenerateIfndefName(output_file)
 
+    # Hack: tabs and windows have circular references, so only generate hard
+    # references for them (i.e. anything that can't be forward declared). In
+    # other cases, generate soft dependencies so that they can include
+    # non-optional types from other namespaces.
+    include_soft = self._namespace.name not in ('tabs', 'windows')
+
     (c.Append('#ifndef %s' % ifndef_name)
       .Append('#define %s' % ifndef_name)
       .Append()
@@ -53,15 +59,14 @@ class _Generator(object):
       .Append('#include "base/memory/linked_ptr.h"')
       .Append('#include "base/memory/scoped_ptr.h"')
       .Append('#include "base/values.h"')
-      .Cblock(self._type_helper.GenerateIncludes())
+      .Cblock(self._type_helper.GenerateIncludes(include_soft=include_soft))
       .Append()
     )
 
-    # TODO(calamity): These forward declarations should be #includes to allow
-    # $ref types from other files to be used as required params. This requires
-    # some detangling of windows and tabs which will currently lead to circular
-    # #includes.
-    c.Cblock(self._type_helper.GenerateForwardDeclarations())
+    # Hack: we're not generating soft includes for tabs and windows, so we need
+    # to generate forward declarations for them.
+    if not include_soft:
+      c.Cblock(self._type_helper.GenerateForwardDeclarations())
 
     cpp_namespace = cpp_util.GetCppNamespace(
         self._namespace.environment.namespace_pattern,
diff --git a/tools/json_schema_compiler/test/crossref.json b/tools/json_schema_compiler/test/crossref.json
index a1e994b..ca67a46 100644
--- a/tools/json_schema_compiler/test/crossref.json
+++ b/tools/json_schema_compiler/test/crossref.json
@@ -9,7 +9,17 @@
         "type": "object",
         "properties": {
           "testType": {
-            "$ref": "simple_api.TestType",
+            "$ref": "simple_api.TestType"
+          },
+          "testEnumRequired": {
+            "$ref": "simple_api.TestEnum"
+          },
+          "testEnumOptional": {
+            "$ref": "simple_api.TestEnum",
+            "optional": true
+          },
+          "testEnumOptionalExtra": {
+            "$ref": "simple_api.TestEnum",
             "optional": true
           }
         }
diff --git a/tools/json_schema_compiler/test/crossref_unittest.cc b/tools/json_schema_compiler/test/crossref_unittest.cc
index 7d8879d..cc20675 100644
--- a/tools/json_schema_compiler/test/crossref_unittest.cc
+++ b/tools/json_schema_compiler/test/crossref_unittest.cc
@@ -7,63 +7,72 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 
-using namespace test::api::crossref;
+using namespace test::api;
 
 namespace {
 
-static scoped_ptr<base::DictionaryValue> CreateTestTypeDictionary() {
+scoped_ptr<base::DictionaryValue> CreateTestTypeValue() {
   base::DictionaryValue* value(new base::DictionaryValue());
-  value->SetWithoutPathExpansion("number", new base::FundamentalValue(1.1));
-  value->SetWithoutPathExpansion("integer", new base::FundamentalValue(4));
-  value->SetWithoutPathExpansion("string", new base::StringValue("bling"));
-  value->SetWithoutPathExpansion("boolean", new base::FundamentalValue(true));
+  value->Set("number", new base::FundamentalValue(1.1));
+  value->Set("integer", new base::FundamentalValue(4));
+  value->Set("string", new base::StringValue("bling"));
+  value->Set("boolean", new base::FundamentalValue(true));
   return scoped_ptr<base::DictionaryValue>(value);
 }
 
 }  // namespace
 
-TEST(JsonSchemaCompilerCrossrefTest, CrossrefTypePopulate) {
-  scoped_ptr<CrossrefType> crossref_type(new CrossrefType());
-  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue());
-  value->Set("testType", CreateTestTypeDictionary().release());
-  EXPECT_TRUE(CrossrefType::Populate(*value, crossref_type.get()));
-  EXPECT_TRUE(crossref_type->test_type.get());
-  EXPECT_TRUE(CreateTestTypeDictionary()->Equals(
-        crossref_type->test_type->ToValue().get()));
-  EXPECT_TRUE(value->Equals(crossref_type->ToValue().get()));
+TEST(JsonSchemaCompilerCrossrefTest, CrossrefTypePopulateAndToValue) {
+  base::DictionaryValue crossref_orig;
+  crossref_orig.Set("testType", CreateTestTypeValue().release());
+  crossref_orig.Set("testEnumRequired", new base::StringValue("one"));
+  crossref_orig.Set("testEnumOptional", new base::StringValue("two"));
+
+  // Test Populate of the value --> compiled type.
+  crossref::CrossrefType crossref_type;
+  ASSERT_TRUE(crossref::CrossrefType::Populate(crossref_orig, &crossref_type));
+  EXPECT_EQ(1.1, crossref_type.test_type.number);
+  EXPECT_EQ(4, crossref_type.test_type.integer);
+  EXPECT_EQ("bling", crossref_type.test_type.string);
+  EXPECT_EQ(true, crossref_type.test_type.boolean);
+  EXPECT_EQ(simple_api::TEST_ENUM_ONE, crossref_type.test_enum_required);
+  EXPECT_EQ(simple_api::TEST_ENUM_TWO, crossref_type.test_enum_optional);
+  EXPECT_EQ(simple_api::TEST_ENUM_NONE, crossref_type.test_enum_optional_extra);
+
+  // Test ToValue of the compiled type --> value.
+  scoped_ptr<base::DictionaryValue> crossref_value = crossref_type.ToValue();
+  ASSERT_TRUE(crossref_value);
+  EXPECT_TRUE(crossref_orig.Equals(crossref_value.get()));
 }
 
 TEST(JsonSchemaCompilerCrossrefTest, TestTypeOptionalParamCreate) {
   scoped_ptr<base::ListValue> params_value(new base::ListValue());
-  params_value->Append(CreateTestTypeDictionary().release());
-  scoped_ptr<TestTypeOptionalParam::Params> params(
-      TestTypeOptionalParam::Params::Create(*params_value));
+  params_value->Append(CreateTestTypeValue().release());
+  scoped_ptr<crossref::TestTypeOptionalParam::Params> params(
+      crossref::TestTypeOptionalParam::Params::Create(*params_value));
   EXPECT_TRUE(params.get());
   EXPECT_TRUE(params->test_type.get());
   EXPECT_TRUE(
-      CreateTestTypeDictionary()->Equals(params->test_type->ToValue().get()));
+      CreateTestTypeValue()->Equals(params->test_type->ToValue().get()));
 }
 
 TEST(JsonSchemaCompilerCrossrefTest, TestTypeOptionalParamFail) {
   scoped_ptr<base::ListValue> params_value(new base::ListValue());
-  scoped_ptr<base::DictionaryValue> test_type_value =
-      CreateTestTypeDictionary();
+  scoped_ptr<base::DictionaryValue> test_type_value = CreateTestTypeValue();
   test_type_value->RemoveWithoutPathExpansion("number", NULL);
   params_value->Append(test_type_value.release());
-  scoped_ptr<TestTypeOptionalParam::Params> params(
-      TestTypeOptionalParam::Params::Create(*params_value));
+  scoped_ptr<crossref::TestTypeOptionalParam::Params> params(
+      crossref::TestTypeOptionalParam::Params::Create(*params_value));
   EXPECT_FALSE(params.get());
 }
 
 TEST(JsonSchemaCompilerCrossrefTest, GetTestType) {
-  scoped_ptr<base::DictionaryValue> value = CreateTestTypeDictionary();
-  scoped_ptr<test::api::simple_api::TestType> test_type(
-      new test::api::simple_api::TestType());
-  EXPECT_TRUE(
-      test::api::simple_api::TestType::Populate(*value, test_type.get()));
+  scoped_ptr<base::DictionaryValue> value = CreateTestTypeValue();
+  scoped_ptr<simple_api::TestType> test_type(new simple_api::TestType());
+  EXPECT_TRUE(simple_api::TestType::Populate(*value, test_type.get()));
 
   scoped_ptr<base::ListValue> results =
-      GetTestType::Results::Create(*test_type);
+      crossref::GetTestType::Results::Create(*test_type);
   base::DictionaryValue* result_dict = NULL;
   results->GetDictionary(0, &result_dict);
   EXPECT_TRUE(value->Equals(result_dict));
@@ -74,16 +83,16 @@ TEST(JsonSchemaCompilerCrossrefTest, TestTypeInObjectParamsCreate) {
     scoped_ptr<base::ListValue> params_value(new base::ListValue());
     scoped_ptr<base::DictionaryValue> param_object_value(
         new base::DictionaryValue());
-    param_object_value->Set("testType", CreateTestTypeDictionary().release());
+    param_object_value->Set("testType", CreateTestTypeValue().release());
     param_object_value->Set("boolean", new base::FundamentalValue(true));
     params_value->Append(param_object_value.release());
-    scoped_ptr<TestTypeInObject::Params> params(
-        TestTypeInObject::Params::Create(*params_value));
+    scoped_ptr<crossref::TestTypeInObject::Params> params(
+        crossref::TestTypeInObject::Params::Create(*params_value));
     EXPECT_TRUE(params.get());
     EXPECT_TRUE(params->param_object.test_type.get());
     EXPECT_TRUE(params->param_object.boolean);
-    EXPECT_TRUE(CreateTestTypeDictionary()->Equals(
-          params->param_object.test_type->ToValue().get()));
+    EXPECT_TRUE(CreateTestTypeValue()->Equals(
+        params->param_object.test_type->ToValue().get()));
   }
   {
     scoped_ptr<base::ListValue> params_value(new base::ListValue());
@@ -91,8 +100,8 @@ TEST(JsonSchemaCompilerCrossrefTest, TestTypeInObjectParamsCreate) {
         new base::DictionaryValue());
     param_object_value->Set("boolean", new base::FundamentalValue(true));
     params_value->Append(param_object_value.release());
-    scoped_ptr<TestTypeInObject::Params> params(
-        TestTypeInObject::Params::Create(*params_value));
+    scoped_ptr<crossref::TestTypeInObject::Params> params(
+        crossref::TestTypeInObject::Params::Create(*params_value));
     EXPECT_TRUE(params.get());
     EXPECT_FALSE(params->param_object.test_type.get());
     EXPECT_TRUE(params->param_object.boolean);
@@ -104,18 +113,18 @@ TEST(JsonSchemaCompilerCrossrefTest, TestTypeInObjectParamsCreate) {
     param_object_value->Set("testType", new base::StringValue("invalid"));
     param_object_value->Set("boolean", new base::FundamentalValue(true));
     params_value->Append(param_object_value.release());
-    scoped_ptr<TestTypeInObject::Params> params(
-        TestTypeInObject::Params::Create(*params_value));
+    scoped_ptr<crossref::TestTypeInObject::Params> params(
+        crossref::TestTypeInObject::Params::Create(*params_value));
     EXPECT_FALSE(params.get());
   }
   {
     scoped_ptr<base::ListValue> params_value(new base::ListValue());
     scoped_ptr<base::DictionaryValue> param_object_value(
         new base::DictionaryValue());
-    param_object_value->Set("testType", CreateTestTypeDictionary().release());
+    param_object_value->Set("testType", CreateTestTypeValue().release());
     params_value->Append(param_object_value.release());
-    scoped_ptr<TestTypeInObject::Params> params(
-        TestTypeInObject::Params::Create(*params_value));
+    scoped_ptr<crossref::TestTypeInObject::Params> params(
+        crossref::TestTypeInObject::Params::Create(*params_value));
     EXPECT_FALSE(params.get());
   }
 }
diff --git a/tools/json_schema_compiler/test/simple_api.json b/tools/json_schema_compiler/test/simple_api.json
index 9ab5403..7831187 100644
--- a/tools/json_schema_compiler/test/simple_api.json
+++ b/tools/json_schema_compiler/test/simple_api.json
@@ -24,6 +24,11 @@
             "description": "Some integer."
           }
         }
+      },
+      {
+        "id": "TestEnum",
+        "type": "string",
+        "enum": ["one", "two", "three"]
       }
     ],
     "functions": [