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

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: 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/h_generator.py » ('j') | no next file with comments »
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 6cc8751eb5c158421bc5160cbfde8f749c9895ea..6c8a573916e12b176c62775d0f6b352570546db1 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -182,9 +182,13 @@ class _Generator(object):
classname = cpp_util.Classname(schema_util.StripNamespace(type_.name))
c = Code()
(c.Append('// static')
- .Append('bool %(namespace)s::Populate(')
- .Sblock(' const base::Value& value, %(name)s* out) {')
- )
+ .Append('bool %(namespace)s::Populate('))
+ if self._namespace.generate_error_messages:
+ c.Sblock(' const base::Value& value, %(name)s* out, '
+ 'std::string* error) {')
cduvall 2013/06/07 19:10:42 line this up with the "'" on the line above
Aaron Jacobs 2013/06/07 20:36:13 Done.
+ else:
+ c.Sblock(' 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 +202,21 @@ class _Generator(object):
.Append('return true;')
.Eblock('}')
)
+ if self._namespace.generate_error_messages:
+ self._GenerateError(c,
+ 'type-check',
+ '"unexpected type, got " << value.GetType()')
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)) {')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(c,
+ 'type-check',
+ '"expected dictionary, got " << value.GetType()',
+ 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 +267,15 @@ 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)) {')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(c,
+ 'get-from-dict',
+ '"key not found: %(key)s"',
+ True)
+ (c.Append(' return false;')
+ .Append('}')
.Concat(self._GeneratePopulatePropertyFromValue(
prop, value_var, dst, 'false'))
)
@@ -272,12 +292,19 @@ class _Generator(object):
def _GenerateTypeFromValue(self, cpp_namespace, type_):
classname = cpp_util.Classname(schema_util.StripNamespace(type_.name))
c = Code()
- (c.Append('// static')
- .Append('scoped_ptr<%s> %s::FromValue(const base::Value& value) {' % (
+ c.Append('// static')
+ if self._namespace.generate_error_messages:
+ c.Append('scoped_ptr<%s> %s::FromValue(const base::Value& value, '
+ 'std::string* error) {' % (classname, cpp_namespace))
+ else:
+ c.Append('scoped_ptr<%s> %s::FromValue(const base::Value& value) {' % (
classname, cpp_namespace))
- .Append(' scoped_ptr<%s> out(new %s());' % (classname, classname))
- .Append(' if (!Populate(value, out.get()))')
- .Append(' return scoped_ptr<%s>();' % classname)
+ c.Append(' scoped_ptr<%s> out(new %s());' % (classname, classname))
+ if self._namespace.generate_error_messages:
+ c.Append(' if (!Populate(value, out.get(), error))')
+ else:
+ c.Append(' if (!Populate(value, out.get()))')
+ (c.Append(' return scoped_ptr<%s>();' % classname)
.Append(' return out.Pass();')
.Append('}')
)
@@ -478,13 +505,20 @@ 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) {')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
cduvall 2013/06/07 19:10:42 move this up in the (
Aaron Jacobs 2013/06/07 20:36:13 Done.
+ 'params',
+ '"expected %(total)d arguments, got " << %(var)s.GetSize()',
+ True)
c.Append(' return scoped_ptr<Params>();')
+ c.Append('}')
c.Substitute({
'var': var,
'required': num_required,
@@ -499,12 +533,15 @@ class _Generator(object):
E.g for function "Bar", generate Bar::Params::Create()
"""
c = Code()
- (c.Append('// static')
- .Sblock('scoped_ptr<Params> '
+ c.Append('// static')
+ if self._namespace.generate_error_messages:
+ c.Sblock('scoped_ptr<Params> Params::Create('
+ 'const base::ListValue& args, std::string* error) {')
cduvall 2013/06/07 19:10:42 line up with (
Aaron Jacobs 2013/06/07 20:36:13 Done.
+ else:
+ c.Sblock('scoped_ptr<Params> '
'Params::Create(const base::ListValue& args) {')
- .Concat(self._GenerateParamsCheck(function, 'args'))
- .Append('scoped_ptr<Params> params(new Params());')
- )
+ (c.Concat(self._GenerateParamsCheck(function, 'args'))
+ .Append('scoped_ptr<Params> params(new Params());'))
for param in function.params:
c.Concat(self._InitializePropertyToDefault(param, 'params'))
@@ -525,10 +562,13 @@ class _Generator(object):
.Eblock('}')
)
if not param.optional:
- (c.Sblock('else {')
- .Append('return %s;' % failure_value)
- .Eblock('}')
- )
+ c.Sblock('else {')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(c,
+ 'params',
+ '"missing non-optional value: %(value_var)s"')
+ (c.Append('return %s;' % failure_value)
+ .Eblock('}'))
c.Substitute({'value_var': value_var, 'i': i})
(c.Append()
.Append('return params.Pass();')
@@ -573,34 +613,70 @@ 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')))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
cduvall 2013/06/07 19:10:42 put after (
Aaron Jacobs 2013/06/07 20:36:13 Done.
+ 'get-as-type',
+ '"expected %s, got " << %s->GetType()' %
+ (self._type_helper.GetCppType(type_), src_var), True)
+ (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)))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
cduvall 2013/06/07 19:10:42 put after ( if it fits (everywhere)
Aaron Jacobs 2013/06/07 20:36:13 Done.
+ 'get-as-type',
+ '"expected %s, got " << %s->GetType()' %
+ (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('scoped_ptr<%(cpp_type)s> temp(new %(cpp_type)s());')
- .Append('if (!%(cpp_type)s::Populate(*dictionary, temp.get()))')
- .Append(' return %(failure_value)s;')
+ .Append('if (!%(src_var)s->GetAsDictionary(&dictionary)) {'))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'get-as-dict',
+ '"expected dictionary, got " << %(src_var)s->GetType()', True)
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
+ .Append('scoped_ptr<%(cpp_type)s> temp(new %(cpp_type)s());'))
+ if self._namespace.generate_error_messages:
+ c.Append('if (!%(cpp_type)s::Populate('
+ '*dictionary, temp.get(), error)) {')
+ else:
+ c.Append('if (!%(cpp_type)s::Populate(*dictionary, temp.get())) {')
+ (c.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(' return %(failure_value)s;')
+ .Append('if (!%(src_var)s->GetAsDictionary(&dictionary)) {'))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'get-as-dict',
+ '"expected dictionary, got " << %(src_var)s->GetType()', True)
+ (c.Append(' return %(failure_value)s;')
+ .Append('}'))
+ if self._namespace.generate_error_messages:
+ c.Append('if (!%(cpp_type)s::Populate('
+ '*dictionary, &%(dst_var)s, error)) {')
+ else:
+ c.Append('if (!%(cpp_type)s::Populate(*dictionary, &%(dst_var)s)) {')
+ (c.Append(' return %(failure_value)s;')
+ .Append('}')
)
elif underlying_type.property_type == PropertyType.FUNCTION:
if is_ptr:
@@ -610,8 +686,14 @@ 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)) {'))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'get-as-list',
+ '"expected list, got " << %(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 +703,51 @@ 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;')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(c,
+ 'populate-array',
+ '"unable to populate array"',
+ True)
+ (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(' return %(failure_value)s;')
+ c.Append('scoped_ptr<%(cpp_type)s> temp(new %(cpp_type)s());')
+ if self._namespace.generate_error_messages:
+ c.Append('if (!%(cpp_type)s::Populate('
+ '*%(src_var)s, temp.get(), error))')
+ else:
+ c.Append('if (!%(cpp_type)s::Populate(*%(src_var)s, temp.get()))')
+ (c.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;')
- )
+ if self._namespace.generate_error_messages:
+ c.Append('if (!%(cpp_type)s::Populate('
+ '*%(src_var)s, &%(dst_var)s, error))')
+ else:
+ c.Append('if (!%(cpp_type)s::Populate(*%(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)) {')
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'type-check',
+ '"expected %(value_type)s, got " << %(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 +817,26 @@ 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)))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'get-as-string',
+ '"expected std::string, got " << %s->GetType()' % 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_))))
+ if self._namespace.generate_error_messages:
+ self._GenerateError(
+ c,
+ 'get-from-string',
+ '"got bad enum value from variable \'%s\'"' % dst_var, True)
+ (c.Append(' return %s;' % failure_value)
+ .Append('}')
)
return c
@@ -841,3 +954,21 @@ class _Generator(object):
prop.unix_name,
self._type_helper.GetEnumNoneValue(prop.type_)))
return c
+
+ def _GenerateError(self, c, location, body, indent = False):
+ """Generates an error message pertaining to population failure, and appends
+ it to |c|.
+
+ E.g...
+ (get-from-dict): 'key not found: content' at
+ gen/chrome/common/extensions/api/omnibox.cc:241
+ """
+ padding = '';
+ if indent:
+ padding = ' ';
+ (c.Append(padding + 'if (error) {')
+ .Append(padding + ' std::stringstream ss;')
+ .Append(padding + ' ss << "' + location + ': \'" << ' + body +
+ ' << "\' at " << __FILE__ << ":" << (__LINE__ - 3);')
+ .Append(padding + ' *error = ss.str();')
+ .Append(padding + '}'))
« no previous file with comments | « no previous file | tools/json_schema_compiler/h_generator.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698