Default handling of name conflicts when generating classes

lib/message_generator.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of protoc;
 
 const String SP = ' ';
 
 class MessageGenerator implements ProtobufContainer {
   final String classname;
   final String fqname;
   final GenerationContext _context;
   final DescriptorProto _descriptor;
   final List<EnumGenerator> _enumGenerators = <EnumGenerator>[];
   final List<ProtobufField> _fieldList = <ProtobufField>[];
   final List<MessageGenerator> _messageGenerators = <MessageGenerator>[];
   final List<ExtensionGenerator> _extensionGenerators = <ExtensionGenerator>[];
+  final Set<String> _methodNames = new Set<String>();
 
   MessageGenerator(DescriptorProto descriptor, ProtobufContainer parent,
                    this._context)
       : _descriptor = descriptor,
         classname = (parent == null || parent.classname == '') ?
             descriptor.name : '${parent.classname}_${descriptor.name}',
         fqname = (parent == null || parent.fqname == null) ? descriptor.name :
             (parent.fqname == '.' ?
                 '.${descriptor.name}' : '${parent.fqname}.${descriptor.name}') {
     _context.register(this);
@@ skipping 15 matching lines @@
     _fieldList.clear();
     for (FieldDescriptorProto field in _descriptor.field) {
       _fieldList.add(new ProtobufField(field, this, _context));
     }
     for (MessageGenerator m in _messageGenerators) {
       m.initializeFields();
     }
   }
 
   void generate(IndentingWriter out) {
+    _methodNames.clear();
+    _methodNames.addAll(
+        ["hashCode", "noSuchMethod","runtimeType", "toString"]);
+    _methodNames.addAll(
+        ["fromBuffer", "fromJson", "hasRequiredFields", "isInitialized",
+         "clear", "getTagNumber", "check",
+         "writeToBuffer", "writeToCodedBufferWriter",
+         "mergeFromCodedBufferReader", "mergeFromBuffer",
+         "writeToJson", "mergeFromJson",
+         "addExtension", "getExtension", "setExtension",
+         "hasExtension", "clearExtension",
+         "getField", "setField", "hasField", "clearField",
+         "extensionsAreInitialized", "mergeFromMessage", "mergeUnknownFields"]);

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

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.

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

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.

+
     for (EnumGenerator e in _enumGenerators) {
       e.generate(out);
     }
 
     for (MessageGenerator m in _messageGenerators) {
       m.generate(out);
     }
 
     out.addBlock('class ${classname} extends GeneratedMessage${SP}{',
         '}', ()
@@ skipping 119 matching lines @@
           }
         }
       }
     }
     return false;
   }
 
   void generateFieldsAccessorsMutators(IndentingWriter out) {
     for (ProtobufField field in _fieldList) {
       out.println();
-      out.println('${field.typeString} get ${field.externalFieldName}'
+      String identifier = field.externalFieldName;
+      String hasIdentifier = "has" + field.titlecaseFieldName;
+      String clearIdentifier = "clear" + field.titlecaseFieldName;
+      if (field.single) {
+        while (_methodNames.contains(identifier) ||
+               _methodNames.contains(hasIdentifier) ||
+               _methodNames.contains(clearIdentifier)) {
+          identifier += field.number.toString();
+          hasIdentifier += field.number.toString();
+          clearIdentifier += field.number.toString();
+        }
+        _methodNames.add(identifier);
+        _methodNames.add(hasIdentifier);
+        _methodNames.add(clearIdentifier);
+      } else {
+        while (_methodNames.contains(identifier)) {
+          identifier += field.number.toString();

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

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]

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.

+        }
+        _methodNames.add(identifier);
+      }
+      out.println('${field.typeString} get ${identifier}'
           '${SP}=>${SP}getField(${field.number});');
       if (field.single) {
-        out.println('void set ${field.externalFieldName}'
+        out.println('void set ${identifier}'
             '(${field.typeString} v)${SP}'
             '{${SP}setField(${field.number},${SP}v);${SP}}');
-        out.println('bool has${field.titlecaseFieldName}()${SP}=>'
+        out.println('bool $hasIdentifier()${SP}=>'
             '${SP}hasField(${field.number});');
-        out.println('void clear${field.titlecaseFieldName}()${SP}=>'
+        out.println('void $clearIdentifier()${SP}=>'
             '${SP}clearField(${field.number});');
       }
     }
   }
 }