Default handling of name conflicts when generating classes

Default handling of name conflicts when generating classes

Whenever there is an identifier conflict when generating classes a new
name is generated by appending the field number. This is done
repeatedly until there is no conflict.

R=sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/protoc-plugin/commit/bdba84f

[Siggi Cherem (dart-lang), 2013-10-24 17:04:21]

lgtm, with some suggestions below.

A couple general questions about breaking changes:
 - if we add a member to GeneratedMessage in the future, we would be making a
breaking change on the generated Dart code. I assume that will not break the
actual encoded messages, correct?
 - if we remove a member from GeneratedMessage, should we remove it from
_methodNames or will we keep it to avoid making a breaking change?

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
File lib/message_generator.dart (right):

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
lib/message_generator.dart:66: "extensionsAreInitialized", "mergeFromMessage",
"mergeUnknownFields"]);
I'm slightly worried about how we maintain this list.

Two ideas: 
- maybe we can compute this list using mirrors? (loading GeneratedMessage and
Object via mirrors and adding all their members?)
- keep the list as is, but use mirrors in a test to ensure that this list
includes everything that could possibly conflict.

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
lib/message_generator.dart:223: identifier += field.number.toString();
should we also add an _ here?

Not sure how important this is, but maybe adding a _ will make it easier to
understand what the names mean:

hash_code = 1
hash_code_1 = 4
hash_code_14 = 15

hashCode14 is probably mapping hash_code_1, correct?

I propose to use:
hash_code = 1  --> hashCode1
hash_code_1 = 4 -> hashCode1_4
hash_code_14 = 15 -> hashCode14

[Søren Gjesse, 2013-10-25 09:02:20]

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
File lib/message_generator.dart (right):

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
lib/message_generator.dart:66: "extensionsAreInitialized", "mergeFromMessage",
"mergeUnknownFields"]);
On 2013/10/24 17:04:21, Siggi Cherem (dart-lang) wrote:
> I'm slightly worried about how we maintain this list.
> 
> Two ideas: 
> - maybe we can compute this list using mirrors? (loading GeneratedMessage and
> Object via mirrors and adding all their members?)
> - keep the list as is, but use mirrors in a test to ensure that this list
> includes everything that could possibly conflict.

Mirror currently does not work well with dart2dart which we use for generating
the actual protoc plugin. Kept the list, and added a test which use mirrors to
verify its content.

https://chromiumcodereview.appspot.com/40343003/diff/1/lib/message_generator....
lib/message_generator.dart:223: identifier += field.number.toString();
On 2013/10/24 17:04:21, Siggi Cherem (dart-lang) wrote:
> should we also add an _ here?
> 
> Not sure how important this is, but maybe adding a _ will make it easier to
> understand what the names mean:
> 
> hash_code = 1
> hash_code_1 = 4
> hash_code_14 = 15
> 
> hashCode14 is probably mapping hash_code_1, correct?
> 
> I propose to use:
> hash_code = 1  --> hashCode1
> hash_code_1 = 4 -> hashCode1_4
> hash_code_14 = 15 -> hashCode14

Added the underscore always - this also makes the changed names stand out from
the ones generated directly. The ones generated directly never has an _, e.g.:

hash_code = 1  --> hashCode_1
hash_code_1 = 4 -> hashCode1  // No longer conflicts with the one above.
hash_code_14 = 15 -> hashCode14  // Again no conflict - no rename.

[Søren Gjesse, 2013-10-25 09:03:06]

Committed patchset #2 manually as rbdba84f (presubmit successful).