Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(54)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: tools/json_schema_compiler/cc_generator.py

Issue 16462004: Add optional schema compiler error messages Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Kalman's requests Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tools/json_schema_compiler/code.py » ('j') | tools/json_schema_compiler/h_generator.py » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/json_schema_compiler/cc_generator.py
diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py
index 772ed0792b1baf241da346aa9c246926d46933fe..b64e0592540bbbac407935cb1aa3ab3cebc610f1 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -31,6 +31,8 @@ class _Generator(object):
self._type_helper.GetCppNamespaceName(self._namespace))
self._util_cc_helper = (
util_cc_helper.UtilCCHelper(self._type_helper))
+ self._generate_error_messages = namespace.compiler_options.get(
+ 'generate_error_messages', False)
def Generate(self):
"""Generates a Code object with the .cc for a single namespace.
@@ -183,8 +185,9 @@ class _Generator(object):
c = Code()
(c.Append('// static')
.Append('bool %(namespace)s::Populate(')
- .Sblock(' const base::Value& value, %(name)s* out) {')
- )
+ .Sblock(' %s) {' % self._GenerateParams(
+ ['const base::Value& value', '%(name)s* out'])))
+
if type_.property_type == PropertyType.CHOICES:
for choice in type_.choices:
value_type = cpp_util.GetValueType(self._type_helper.FollowRef(choice))
@@ -198,11 +201,19 @@ class _Generator(object):
.Append('return true;')
.Eblock('}')
)
+ c.Concat(self._GenerateError(
+ '"unexpected type, got " + '
+ 'json_schema_compiler::util::ValueTypeToString(value.GetType())'))
not at google - send to devlin 2013/06/14 21:48:39 can you use the same style here (and everywhere),
Aaron Jacobs 2013/06/15 00:47:18 Done.
c.Append('return false;')
elif type_.property_type == PropertyType.OBJECT:
- (c.Append('if (!value.IsType(base::Value::TYPE_DICTIONARY))')
- .Append(' return false;')
- )
+ c.Append('if (!value.IsType(base::Value::TYPE_DICTIONARY)) {')
+ c.Concat(self._GenerateError(
+ '"expected dictionary, got " + '
+ 'json_schema_compiler::util::ValueTypeToString(value.GetType())',
not at google - send to devlin 2013/06/14 21:48:39 and also there are wrapper for all these in util_c
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ True))
+ (c.Append(' return false;')
+ .Append('}'))
+
if type_.properties or type_.additional_properties is not None:
c.Append('const base::DictionaryValue* dict = '
'static_cast<const base::DictionaryValue*>(&value);')
@@ -253,9 +264,11 @@ class _Generator(object):
self._type_helper.GetEnumNoneValue(prop.type_)))
c.Eblock('}')
else:
- (c.Append(
- 'if (!%(src)s->GetWithoutPathExpansion("%(key)s", &%(value_var)s))')
- .Append(' return false;')
+ c.Append(
+ 'if (!%(src)s->GetWithoutPathExpansion("%(key)s", &%(value_var)s)) {')
+ c.Concat(self._GenerateError('"%%(key)s is required"', True))
not at google - send to devlin 2013/06/14 21:48:39 let's standardise on something. single quotes - I
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ (c.Append(' return false;')
+ .Append('}')
.Concat(self._GeneratePopulatePropertyFromValue(
prop, value_var, dst, 'false'))
)
@@ -273,10 +286,11 @@ class _Generator(object):
classname = cpp_util.Classname(schema_util.StripNamespace(type_.name))
c = Code()
(c.Append('// static')
- .Append('scoped_ptr<%s> %s::FromValue(const base::Value& value) {' % (
- classname, cpp_namespace))
+ .Append('scoped_ptr<%s> %s::FromValue(%s) {' % (classname,
+ cpp_namespace, self._GenerateParams(['const base::Value& value'])))
.Append(' scoped_ptr<%s> out(new %s());' % (classname, classname))
- .Append(' if (!Populate(value, out.get()))')
+ .Append(' if (!Populate(%s))' % self._GenerateArgs(
+ ['value', 'out.get()']))
.Append(' return scoped_ptr<%s>();' % classname)
.Append(' return out.Pass();')
.Append('}')
@@ -478,13 +492,17 @@ class _Generator(object):
if not param.optional:
num_required += 1
if num_required == len(function.params):
- c.Append('if (%(var)s.GetSize() != %(total)d)')
+ c.Append('if (%(var)s.GetSize() != %(total)d) {')
elif not num_required:
- c.Append('if (%(var)s.GetSize() > %(total)d)')
+ c.Append('if (%(var)s.GetSize() > %(total)d) {')
else:
c.Append('if (%(var)s.GetSize() < %(required)d'
- ' || %(var)s.GetSize() > %(total)d)')
+ ' || %(var)s.GetSize() > %(total)d) {')
+ c.Concat(self._GenerateError(
+ '"expected %%(total)d arguments, got " + %%(var)s.GetSize()',
+ True))
c.Append(' return scoped_ptr<Params>();')
+ c.Append('}')
c.Substitute({
'var': var,
'required': num_required,
@@ -500,11 +518,10 @@ class _Generator(object):
"""
c = Code()
(c.Append('// static')
- .Sblock('scoped_ptr<Params> '
- 'Params::Create(const base::ListValue& args) {')
+ .Sblock('scoped_ptr<Params> Params::Create(%s) {' % self._GenerateParams(
+ ['const base::ListValue& args']))
.Concat(self._GenerateParamsCheck(function, 'args'))
- .Append('scoped_ptr<Params> params(new Params());')
- )
+ .Append('scoped_ptr<Params> params(new Params());'))
for param in function.params:
c.Concat(self._InitializePropertyToDefault(param, 'params'))
@@ -525,10 +542,10 @@ class _Generator(object):
.Eblock('}')
)
if not param.optional:
- (c.Sblock('else {')
- .Append('return %s;' % failure_value)
- .Eblock('}')
- )
+ c.Sblock('else {')
+ c.Concat(self._GenerateError('"%%(value_var)s is required"'))
not at google - send to devlin 2013/06/14 21:48:39 ditto, single quotes around the key in here
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ (c.Append('return %s;' % failure_value)
+ .Eblock('}'))
c.Substitute({'value_var': value_var, 'i': i})
(c.Append()
.Append('return params.Pass();')
@@ -573,34 +590,56 @@ class _Generator(object):
if underlying_type.property_type.is_fundamental:
if is_ptr:
(c.Append('%(cpp_type)s temp;')
- .Append('if (!%s)' % cpp_util.GetAsFundamentalValue(
- self._type_helper.FollowRef(type_), src_var, '&temp'))
- .Append(' return %(failure_value)s;')
+ .Append('if (!%s) {' % cpp_util.GetAsFundamentalValue(
+ self._type_helper.FollowRef(type_), src_var, '&temp')))
+ c.Concat(self._GenerateError('"\'%s\': expected %s, got " + '
not at google - send to devlin 2013/06/14 21:48:39 ditto, and the inside-parens thing all through her
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ 'json_schema_compiler::util::ValueTypeToString(%s->GetType())' %
+ (src_var, self._type_helper.GetCppType(type_), src_var), True))
not at google - send to devlin 2013/06/14 21:48:39 src_var isn't right, that's a c++ variable name. w
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
.Append('%(dst_var)s.reset(new %(cpp_type)s(temp));')
)
else:
- (c.Append('if (!%s)' % cpp_util.GetAsFundamentalValue(
+ (c.Append('if (!%s) {' % cpp_util.GetAsFundamentalValue(
self._type_helper.FollowRef(type_),
src_var,
- '&%s' % dst_var))
- .Append(' return %(failure_value)s;')
+ '&%s' % dst_var)))
+ c.Concat(self._GenerateError('"\'%s\': expected %s, got " + '
+ 'json_schema_compiler::util::ValueTypeToString(%s->GetType())' %
+ (src_var, self._type_helper.GetCppType(type_), src_var), True))
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
)
elif underlying_type.property_type == PropertyType.OBJECT:
if is_ptr:
(c.Append('const base::DictionaryValue* dictionary = NULL;')
- .Append('if (!%(src_var)s->GetAsDictionary(&dictionary))')
- .Append(' return %(failure_value)s;')
+ .Append('if (!%(src_var)s->GetAsDictionary(&dictionary)) {'))
+ c.Concat(self._GenerateError(
+ '"\'%%(src_var)s\': expected dictionary, got " + '
+ 'json_schema_compiler::util::ValueTypeToString('
+ '%%(src_var)s->GetType())', True))
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
.Append('scoped_ptr<%(cpp_type)s> temp(new %(cpp_type)s());')
- .Append('if (!%(cpp_type)s::Populate(*dictionary, temp.get()))')
+ .Append('if (!%%(cpp_type)s::Populate(%s)) {' % self._GenerateArgs(
+ ['*dictionary', 'temp.get()']))
.Append(' return %(failure_value)s;')
+ .Append('}')
.Append('%(dst_var)s = temp.Pass();')
)
else:
(c.Append('const base::DictionaryValue* dictionary = NULL;')
- .Append('if (!%(src_var)s->GetAsDictionary(&dictionary))')
- .Append(' return %(failure_value)s;')
- .Append('if (!%(cpp_type)s::Populate(*dictionary, &%(dst_var)s))')
+ .Append('if (!%(src_var)s->GetAsDictionary(&dictionary)) {'))
+ c.Concat(self._GenerateError(
+ '"\'%%(src_var)s\': expected dictionary, got " + '
+ 'json_schema_compiler::util::ValueTypeToString('
+ '%%(src_var)s->GetType())', True))
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
+ .Append('if (!%%(cpp_type)s::Populate(%s)) {' % self._GenerateArgs(
+ ['*dictionary', '&%(dst_var)s']))
.Append(' return %(failure_value)s;')
+ .Append('}')
)
elif underlying_type.property_type == PropertyType.FUNCTION:
if is_ptr:
@@ -610,8 +649,13 @@ class _Generator(object):
elif underlying_type.property_type == PropertyType.ARRAY:
# util_cc_helper deals with optional and required arrays
(c.Append('const base::ListValue* list = NULL;')
- .Append('if (!%(src_var)s->GetAsList(&list))')
- .Append(' return %(failure_value)s;'))
+ .Append('if (!%(src_var)s->GetAsList(&list)) {'))
+ c.Concat(self._GenerateError(
+ '"\'%%(src_var)s\': expected list, got " + '
+ 'json_schema_compiler::util::ValueTypeToString('
+ '%%(src_var)s->GetType())', True))
+ (c.Append(' return %(failure_value)s;')
+ .Append('}'))
item_type = underlying_type.item_type
if item_type.property_type == PropertyType.ENUM:
c.Concat(self._GenerateListValueToEnumArrayConversion(
@@ -621,32 +665,40 @@ class _Generator(object):
failure_value,
is_ptr=is_ptr))
else:
- (c.Append('if (!%s)' % self._util_cc_helper.PopulateArrayFromList(
+ c.Append('if (!%s) {' % self._util_cc_helper.PopulateArrayFromList(
underlying_type,
'list',
dst_var,
is_ptr))
- .Append(' return %(failure_value)s;')
+ c.Concat(self._GenerateError('"unable to populate array"', True))
not at google - send to devlin 2013/06/14 21:48:39 key can be known here, right?
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
)
elif underlying_type.property_type == PropertyType.CHOICES:
if is_ptr:
(c.Append('scoped_ptr<%(cpp_type)s> temp(new %(cpp_type)s());')
- .Append('if (!%(cpp_type)s::Populate(*%(src_var)s, temp.get()))')
+ .Append('if (!%%(cpp_type)s::Populate(%s))' % self._GenerateArgs(
+ ['*%(src_var)s', 'temp.get()']))
.Append(' return %(failure_value)s;')
.Append('%(dst_var)s = temp.Pass();')
)
else:
- (c.Append('if (!%(cpp_type)s::Populate(*%(src_var)s, &%(dst_var)s))')
- .Append(' return %(failure_value)s;')
- )
+ c.Append('if (!%%(cpp_type)s::Populate(%s))' % self._GenerateArgs(
+ ['*%(src_var)s', '&%(dst_var)s']))
+ c.Append(' return %(failure_value)s;')
elif underlying_type.property_type == PropertyType.ENUM:
c.Concat(self._GenerateStringToEnumConversion(type_,
src_var,
dst_var,
failure_value))
elif underlying_type.property_type == PropertyType.BINARY:
- (c.Append('if (!%(src_var)s->IsType(%(value_type)s))')
- .Append(' return %(failure_value)s;')
+ c.Append('if (!%(src_var)s->IsType(%(value_type)s)) {')
+ c.Concat(self._GenerateError(
+ '"\'%%(src_var)s\': expected %(value_type)s, got " + '
+ 'json_schema_compiler::util::ValueTypeToString('
+ '%(src_var)s->GetType())', True))
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
.Append('const base::BinaryValue* binary_value =')
.Append(' static_cast<const base::BinaryValue*>(%(src_var)s);')
)
@@ -716,14 +768,22 @@ class _Generator(object):
c = Code()
enum_as_string = '%s_as_string' % type_.unix_name
(c.Append('std::string %s;' % enum_as_string)
- .Append('if (!%s->GetAsString(&%s))' % (src_var, enum_as_string))
- .Append(' return %s;' % failure_value)
+ .Append('if (!%s->GetAsString(&%s)) {' % (src_var, enum_as_string)))
+ c.Concat(self._GenerateError(
+ '"\'%s\': expected std::string, got " + '
not at google - send to devlin 2013/06/14 21:48:39 just "string" not "std::string"
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ 'json_schema_compiler::util::ValueTypeToString('
+ '%s->GetType())' % (src_var, src_var), True))
+ (c.Append(' return %s;' % failure_value)
+ .Append('}')
.Append('%s = Parse%s(%s);' % (dst_var,
self._type_helper.GetCppType(type_),
enum_as_string))
- .Append('if (%s == %s)' % (dst_var,
- self._type_helper.GetEnumNoneValue(type_)))
- .Append(' return %s;' % failure_value)
+ .Append('if (%s == %s) {' % (dst_var,
+ self._type_helper.GetEnumNoneValue(type_))))
+ c.Concat(self._GenerateError(
+ '"got bad enum value from variable \'%s\'"' % dst_var, True))
+ (c.Append(' return %s;' % failure_value)
+ .Append('}')
)
return c
@@ -841,3 +901,32 @@ class _Generator(object):
prop.unix_name,
self._type_helper.GetEnumNoneValue(prop.type_)))
return c
+
+ def _GenerateError(self, body, indent = False):
+ """Generates an error message pertaining to population failure.
+
+ E.g 'expected bool, got int'
+ """
+ c = Code()
+ if not self._generate_error_messages:
+ return c
+ padding = '';
+ if indent:
+ padding = ' ';
+ (c.Append(padding + 'if (error)')
+ .Append(padding + ' *error = ' + body + ';'))
not at google - send to devlin 2013/06/14 21:48:39 why do you need this padding logic? the caller can
Aaron Jacobs 2013/06/15 00:47:18 Done.
+ return c
+
+ def _GenerateParams(self, params):
+ """Builds the parameter list for a function, given an array of parameters.
+ """
+ if self._generate_error_messages:
+ params.append('std::string* error')
+ return ', '.join(params)
+
+ def _GenerateArgs(self, args):
+ """Builds the argument list for a function, given an array of arguments.
+ """
+ if self._generate_error_messages:
+ args.append('error')
+ return ', '.join(args)
« no previous file with comments | « no previous file | tools/json_schema_compiler/code.py » ('j') | tools/json_schema_compiler/h_generator.py » ('J')

Powered by Google App Engine
This is Rietveld 408576698