Support converting referenced enum array into string.

Support converting referenced enum array into string.

This patch in JSON Schema Compiler first generates a vector with
strings of the enum array using the correct ToString() function
of both inline and referenced enums. Then the result is created
from the vector of strings.

R=kalman@chromium.org
BUG=371808, 368368
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271244

[wjywbs, 2014-05-09 14:13:20]

Please take a look. This supports arrays with referencd enums in both current
and external files. This will also fix adding ExtensionInfo.availableLaunchTypes
in management API. I put the diff in the bug. Thanks.

https://codereview.chromium.org/276603003/diff/1/tools/json_schema_compiler/u...
File tools/json_schema_compiler/util.h (left):

https://codereview.chromium.org/276603003/diff/1/tools/json_schema_compiler/u...
tools/json_schema_compiler/util.h:175: 
sorry for "reverting" your changes... but ToString is not always in TParent.

[not at google - send to devlin, 2014-05-12 21:46:13]

started looking at this but it's too much for me today. i'll have a look
tomorrow.

https://codereview.chromium.org/276603003/diff/1/tools/json_schema_compiler/u...
File tools/json_schema_compiler/util.h (left):

https://codereview.chromium.org/276603003/diff/1/tools/json_schema_compiler/u...
tools/json_schema_compiler/util.h:175: 
On 2014/05/09 14:13:21, wjywbs wrote:
> sorry for "reverting" your changes... but ToString is not always in TParent.

Yes. I should have reverted this earlier :)

https://codereview.chromium.org/276603003/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:370:
'value->SetWithoutPathExpansion("' + prop.name + '", %s);',
use string formatting not +ing complex strings together.

To escape the %s use %%s.

https://codereview.chromium.org/276603003/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:504: .Append('for (typename
std::vector<%s>::const_iterator it = %sbegin();'
line length

[wjywbs, 2014-05-13 03:32:53]

I fixed the code style. PTAL, thanks.

https://codereview.chromium.org/276603003/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:370:
'value->SetWithoutPathExpansion("' + prop.name + '", %s);',
On 2014/05/12 21:46:13, kalman wrote:
> use string formatting not +ing complex strings together.
> 
> To escape the %s use %%s.

Thanks. I don't know %%s before.

[not at google - send to devlin, 2014-05-14 22:52:44]

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:490: underlying_item_type =
self._type_helper.FollowRef(
just |item_type| will be fine.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:496: vardot = '(%s).' % var
how about just:
vardot = '(%s)%s' % (var, '->' if is_ptr else '.')

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:502: enum_list_var = '%s_list' %
prop_name
try to scope variable declarations inside braces. will require slight
restructuring.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:509: maybe_namespace))
the reason why this logic was implemented in C++ before, with templates, was to
avoid duplicating code; while templates generate a separate implementation per
type, they at least only do it once. this will generate identical code each time
an array of these need to be serialised.

It might make sense to have an exception here, since it'll be so rare, and it's
pretty complex. but note that.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:513: var = enum_list_var
needs comments around here explaining what is going on

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:515: c.Append(code %
self._CreateValueFromTypeInternal(type_, var, is_ptr))
why "internal"? this separation seems mostly arbitrary. could you call the
function what it actually does, and add a comment down there?

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/test/arrays_unittest.cc (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/test/arrays_unittest.cc:97:
EXPECT_EQ(enum_array_reference.types.at(0), ENUMERATION_ONE);
- assert correct size
- use [0] not .at(0)

you can also be a bit tricky, like:

TheEnumType expected_types[] = {
  ENUMERATION_ONE, ENUMERATION_TWO, ENUMERATION_THREE
};
EXPECT_EQ(
    std::vector<TheEnumType>(expected_types,
                             expected_types + arraysize(expected_types),
    enum_array_reference.types);

modulo whatever "git cl format" decides is the right way to indent that.

this comment applies to all the tests.

[wjywbs, 2014-05-16 00:47:37]

I updated the code and test. PTAL. Thanks.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:502: enum_list_var = '%s_list' %
prop_name
On 2014/05/14 22:52:45, kalman wrote:
> try to scope variable declarations inside braces. will require slight
> restructuring.

Do you mean something like this:
enum_array_dict = {
  'vardot': '(%s)%s' % (var, '->' if is_ptr else '.'),
  'maybe_namespace': '',
  'enum_list_var': '%s_list' % prop_name
}

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
File tools/json_schema_compiler/test/arrays_unittest.cc (right):

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/test/arrays_unittest.cc:138: EXPECT_EQ(3u,
enum_array_mixed.inline_enums.size());
This is the result of git cl format.

[not at google - send to devlin, 2014-05-16 17:13:47]

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:502: enum_list_var = '%s_list' %
prop_name
On 2014/05/16 00:47:37, wjywbs wrote:
> On 2014/05/14 22:52:45, kalman wrote:
> > try to scope variable declarations inside braces. will require slight
> > restructuring.
> 
> Do you mean something like this:
> enum_array_dict = {
>   'vardot': '(%s)%s' % (var, '->' if is_ptr else '.'),
>   'maybe_namespace': '',
>   'enum_list_var': '%s_list' % prop_name
> }

I meant the C++ generated code. The python is fine.

[not at google - send to devlin, 2014-05-16 17:36:33]

lgtm

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:502: enum_list_var = '%s_list' %
prop_name
On 2014/05/16 17:13:47, kalman wrote:
> On 2014/05/16 00:47:37, wjywbs wrote:
> > On 2014/05/14 22:52:45, kalman wrote:
> > > try to scope variable declarations inside braces. will require slight
> > > restructuring.
> > 
> > Do you mean something like this:
> > enum_array_dict = {
> >   'vardot': '(%s)%s' % (var, '->' if is_ptr else '.'),
> >   'maybe_namespace': '',
> >   'enum_list_var': '%s_list' % prop_name
> > }
> 
> I meant the C++ generated code. The python is fine.

Because i'm worried that somewhere up in the stack the identifier will conflict
and confuse things. Inside braces it will work.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:509: maybe_namespace))
On 2014/05/14 22:52:45, kalman wrote:
> the reason why this logic was implemented in C++ before, with templates, was
to
> avoid duplicating code; while templates generate a separate implementation per
> type, they at least only do it once. this will generate identical code each
time
> an array of these need to be serialised.
> 
> It might make sense to have an exception here, since it'll be so rare, and
it's
> pretty complex. but note that.

When I said "but note that" I mean add a comment for it.

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:513: # optional enum arrays, so
|is_ptr| is set to false and uses the
s/so //

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:522: """Generates the statement to
create a base::Value given a type.
document arguments

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
File tools/json_schema_compiler/test/arrays_unittest.cc (right):

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/test/arrays_unittest.cc:138: EXPECT_EQ(3u,
enum_array_mixed.inline_enums.size());
On 2014/05/16 00:47:37, wjywbs wrote:
> This is the result of git cl format.

ouch.

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/test/arrays_unittest.cc:139:
EnumArrayMixed::Inline_enumsType expected_inline_types[] = {
... but you don't need to assert size if you're doing the full vector
comparison. same below.

I was saying you need to assert size if you're checking individual elements, but
a neat replacement for all of that is constructing a vector and comparing it.
vector knows to check size in == obviously.

[wjywbs, 2014-05-16 19:12:23]

PTAL, thanks.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:502: enum_list_var = '%s_list' %
prop_name
On 2014/05/16 17:36:34, kalman wrote:
> Because i'm worried that somewhere up in the stack the identifier will
conflict
> and confuse things. Inside braces it will work.

Done, but this makes the python code here more complex.

https://codereview.chromium.org/276603003/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:509: maybe_namespace))
On 2014/05/16 17:36:34, kalman wrote:
> When I said "but note that" I mean add a comment for it.

I added the comment above in the paragraph of "Enums are treated specially...".

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:513: # optional enum arrays, so
|is_ptr| is set to false and uses the
On 2014/05/16 17:36:34, kalman wrote:
> s/so //

Done.

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:522: """Generates the statement to
create a base::Value given a type.
On 2014/05/16 17:36:34, kalman wrote:
> document arguments

Done.

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
File tools/json_schema_compiler/test/arrays_unittest.cc (right):

https://codereview.chromium.org/276603003/diff/60001/tools/json_schema_compil...
tools/json_schema_compiler/test/arrays_unittest.cc:139:
EnumArrayMixed::Inline_enumsType expected_inline_types[] = {
Done.

https://codereview.chromium.org/276603003/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/276603003/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:519: .Eblock('}'))
Because the variable is scoped, the code generated by
_GenerateCreateValueFromType also needs to be scoped inside to access the
variable. I posted the generated code in the bug.

[not at google - send to devlin, 2014-05-16 19:51:47]

lgtm

[wjywbs, 2014-05-16 19:52:53]

The CQ bit was checked by wjywbs@gmail.com

[commit-bot: I haz the power, 2014-05-16 19:53:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wjywbs@gmail.com/276603003/100001

[wjywbs, 2014-05-16 20:52:12]

The CQ bit was unchecked by wjywbs@gmail.com

[wjywbs, 2014-05-16 21:01:23]

I removed the unused "typename" in the for loop to fix win compile errors, and
rebased. PTAL, thanks.

[not at google - send to devlin, 2014-05-16 21:02:10]

lgtm

[wjywbs, 2014-05-16 21:03:03]

The CQ bit was checked by wjywbs@gmail.com

[commit-bot: I haz the power, 2014-05-16 21:03:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wjywbs@gmail.com/276603003/120001

[commit-bot: I haz the power, 2014-05-17 17:32:10]

Change committed as 271244