JSON schema compiler can't handle strings as types

JSON schema compiler can't handle strings as types

JSON schema compiler now supports having strings as types.

Similar to http://crbug.com/122075

BUG=124771
TEST=cpp_type_generator_test.py


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134674

[cduvall, 2012-04-27 22:28:52]

Thanks for the review!

https://chromiumcodereview.appspot.com/10206034/diff/1/tools/json_schema_comp...
File tools/json_schema_compiler/compiler.py (right):

https://chromiumcodereview.appspot.com/10206034/diff/1/tools/json_schema_comp...
tools/json_schema_compiler/compiler.py:81: referenced_namespace.name)
This should be |name| because the files are saved in the form
fileName.h, not file_name.h.

[Yoyo Zhou, 2012-05-01 00:21:58]

Mostly LGTM.

http://codereview.chromium.org/10206034/diff/1/chrome/common/extensions/api/e...
File chrome/common/extensions/api/experimental.fontSettings.json (right):

http://codereview.chromium.org/10206034/diff/1/chrome/common/extensions/api/e...
chrome/common/extensions/api/experimental.fontSettings.json:170: "properties":
{}
Why are these added?

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cc_...
File tools/json_schema_compiler/cc_generator.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cc_...
tools/json_schema_compiler/cc_generator.py:670: return
(self._cpp_type_generator.GetReferencedProperty(prop).type_
nit: I'd put the dot on the first line

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
File tools/json_schema_compiler/compiler.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
tools/json_schema_compiler/compiler.py:81: referenced_namespace.name)
On 2012/04/27 22:28:52, cduvall wrote:
> This should be |name| because the files are saved in the form
> fileName.h, not file_name.h.

This is a tangent, but it's a bummer that there are no tests for this... I've
seen fileName in enough places to know that it's what the generated filenames
look like, but you might check with kalman to see if this was the intention.

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cpp...
File tools/json_schema_compiler/cpp_type_generator.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cpp...
tools/json_schema_compiler/cpp_type_generator.py:179: c.Append('typedef
std::string %s;' % type_)
I am not sure about whether this is advisable stylistically, having the typedef
repeated multiple times. As it's generated code, it's not as much of a problem.
I also don't know of a workaround, unfortunately.

[cduvall, 2012-05-01 00:31:51]

http://codereview.chromium.org/10206034/diff/1/chrome/common/extensions/api/e...
File chrome/common/extensions/api/experimental.fontSettings.json (right):

http://codereview.chromium.org/10206034/diff/1/chrome/common/extensions/api/e...
chrome/common/extensions/api/experimental.fontSettings.json:170: "properties":
{}
On 2012/05/01 00:21:58, Yoyo Zhou wrote:
> Why are these added?
I added these so the schema compiler doesn't complain about the object not
having any properties or functions. It wouldn't generate the file without them.

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cc_...
File tools/json_schema_compiler/cc_generator.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cc_...
tools/json_schema_compiler/cc_generator.py:670: return
(self._cpp_type_generator.GetReferencedProperty(prop).type_
On 2012/05/01 00:21:58, Yoyo Zhou wrote:
> nit: I'd put the dot on the first line

Done.

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
File tools/json_schema_compiler/compiler.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
tools/json_schema_compiler/compiler.py:81: referenced_namespace.name)
On 2012/05/01 00:21:58, Yoyo Zhou wrote:
> On 2012/04/27 22:28:52, cduvall wrote:
> > This should be |name| because the files are saved in the form
> > fileName.h, not file_name.h.
> 
> This is a tangent, but it's a bummer that there are no tests for this... I've
> seen fileName in enough places to know that it's what the generated filenames
> look like, but you might check with kalman to see if this was the intention.

Yeah, it is too bad that fileName has been used. It seems like file_name would
be better for style. I'll ask kalman about it and maybe address it in a separate
bug depending on what he says.

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cpp...
File tools/json_schema_compiler/cpp_type_generator.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/cpp...
tools/json_schema_compiler/cpp_type_generator.py:179: c.Append('typedef
std::string %s;' % type_)
On 2012/05/01 00:21:58, Yoyo Zhou wrote:
> I am not sure about whether this is advisable stylistically, having the
typedef
> repeated multiple times. As it's generated code, it's not as much of a
problem.
> I also don't know of a workaround, unfortunately.

Yeah, this was the best solution I could come up with.

[cduvall, 2012-05-01 00:48:53]

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
File tools/json_schema_compiler/compiler.py (right):

http://codereview.chromium.org/10206034/diff/1/tools/json_schema_compiler/com...
tools/json_schema_compiler/compiler.py:81: referenced_namespace.name)
On 2012/05/01 00:31:51, cduvall wrote:
> On 2012/05/01 00:21:58, Yoyo Zhou wrote:
> > On 2012/04/27 22:28:52, cduvall wrote:
> > > This should be |name| because the files are saved in the form
> > > fileName.h, not file_name.h.
> > 
> > This is a tangent, but it's a bummer that there are no tests for this...
I've
> > seen fileName in enough places to know that it's what the generated
filenames
> > look like, but you might check with kalman to see if this was the intention.
> 
> Yeah, it is too bad that fileName has been used. It seems like file_name would
> be better for style. I'll ask kalman about it and maybe address it in a
separate
> bug depending on what he says.

Just talked to kalman, he wants the files to be named like "file_name". I'm
going to get this one committed first and then change this back after I finish
the bug for changing the filenames. Here's the bug: http://crbug.com/125669

[commit-bot: I haz the power, 2012-05-01 00:49:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cduvall@chromium.org/10206034/5003

[commit-bot: I haz the power, 2012-05-01 03:58:08]

Change committed as 134674