More tests, more fixes.

csharp/ProtocolBuffers.Test/CodedInputStreamTest.cs

@@ -17,6 +17,7 @@ using System;
 using System.IO;
 using Google.ProtocolBuffers.TestProtos;
 using NUnit.Framework;
+using System.Diagnostics;
 
 namespace Google.ProtocolBuffers {
   [TestFixture]
@@ -190,7 +191,6 @@ namespace Google.ProtocolBuffers {
 
       byte[] rawBytes = message.ToByteArray();
       Assert.AreEqual(rawBytes.Length, message.SerializedSize);
-
       TestAllTypes message2 = TestAllTypes.ParseFrom(rawBytes);
       TestUtil.AssertAllFieldsSet(message2);
 

csharp/ProtocolBuffers.Test/DynamicMessageTest.cs

@@ -9,13 +9,12 @@ namespace Google.ProtocolBuffers {
   public class DynamicMessageTest {
 
     private ReflectionTester reflectionTester;
-
     private ReflectionTester extensionsReflectionTester;
 
     [SetUp]
     public void SetUp() {
-      reflectionTester = new ReflectionTester(TestAllTypes.Descriptor, null);
-      extensionsReflectionTester = new ReflectionTester(TestAllExtensions.Descriptor, TestUtil.CreateExtensionRegistry());
+      reflectionTester = ReflectionTester.CreateTestAllTypesInstance();
+      extensionsReflectionTester = ReflectionTester.CreateTestAllExtensionsInstance();
     }
 
     [Test]

csharp/ProtocolBuffers.Test/GeneratedMessageTest.cs

+using System;
+using System.Collections.Generic;
+using System.Text;
+using NUnit.Framework;
+using Google.ProtocolBuffers.TestProtos;
+
+namespace Google.ProtocolBuffers {
+  [TestFixture]
+  public class GeneratedMessageTest {
+    ReflectionTester reflectionTester;
+    ReflectionTester extensionsReflectionTester;
+    
+    [SetUp]
+    public void SetUp() {
+      reflectionTester = ReflectionTester.CreateTestAllTypesInstance();
+      extensionsReflectionTester = ReflectionTester.CreateTestAllExtensionsInstance();
+    }
+    
+    [Test]
+    public void DefaultInstance() {
+      Assert.AreSame(TestAllTypes.DefaultInstance, TestAllTypes.DefaultInstance.DefaultInstanceForType);
+      Assert.AreSame(TestAllTypes.DefaultInstance, TestAllTypes.CreateBuilder().DefaultInstanceForType);
+    }
+
+    [Test]
+    public void Accessors() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+      TestUtil.SetAllFields(builder);
+      TestAllTypes message = builder.Build();
+      TestUtil.AssertAllFieldsSet(message);
+    }
+    
+    [Test]
+    public void RepeatedSetters() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+      TestUtil.SetAllFields(builder);
+      TestUtil.ModifyRepeatedFields(builder);
+      TestAllTypes message = builder.Build();
+      TestUtil.AssertRepeatedFieldsModified(message);
+    }
+
+    [Test]
+    public void RepeatedAppend() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+
+      builder.AddRangeRepeatedInt32(new int[]{1, 2, 3, 4});
+      builder.AddRangeRepeatedForeignEnum((new ForeignEnum[] { ForeignEnum.FOREIGN_BAZ }));
+
+      ForeignMessage foreignMessage = ForeignMessage.CreateBuilder().SetC(12).Build();
+      builder.AddRangeRepeatedForeignMessage(new ForeignMessage[] {foreignMessage});
+
+      TestAllTypes message = builder.Build();
+      TestUtil.AssertEqual(message.RepeatedInt32List, new int[]{1, 2, 3, 4});
+      TestUtil.AssertEqual(message.RepeatedForeignEnumList, new ForeignEnum[] {ForeignEnum.FOREIGN_BAZ});
+      Assert.AreEqual(1, message.RepeatedForeignMessageCount);
+      Assert.AreEqual(12, message.GetRepeatedForeignMessage(0).C);
+    }
+
+    [Test]
+    public void SettingForeignMessageUsingBuilder() {
+      TestAllTypes message = TestAllTypes.CreateBuilder()
+          // Pass builder for foreign message instance.
+          .SetOptionalForeignMessage(ForeignMessage.CreateBuilder().SetC(123))
+          .Build();
+      TestAllTypes expectedMessage = TestAllTypes.CreateBuilder()
+          // Create expected version passing foreign message instance explicitly.
+          .SetOptionalForeignMessage(ForeignMessage.CreateBuilder().SetC(123).Build())
+          .Build();
+      Assert.AreEqual(expectedMessage, message);
+    }
+
+    [Test]
+    public void SettingRepeatedForeignMessageUsingBuilder() {
+      TestAllTypes message = TestAllTypes.CreateBuilder()
+          // Pass builder for foreign message instance.
+          .AddRepeatedForeignMessage(ForeignMessage.CreateBuilder().SetC(456))
+          .Build();
+      TestAllTypes expectedMessage = TestAllTypes.CreateBuilder()
+          // Create expected version passing foreign message instance explicitly.
+          .AddRepeatedForeignMessage(ForeignMessage.CreateBuilder().SetC(456).Build())
+          .Build();
+      Assert.AreEqual(expectedMessage, message);
+    }
+
+    
+    [Test]
+    public void Defaults() {
+      TestUtil.AssertClear(TestAllTypes.DefaultInstance);
+      TestUtil.AssertClear(TestAllTypes.CreateBuilder().Build());
+
+      Assert.AreEqual("\u1234", TestExtremeDefaultValues.DefaultInstance.Utf8String);
+    }
+
+    [Test]
+    public void ReflectionGetters() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+      TestUtil.SetAllFields(builder);
+      TestAllTypes message = builder.Build();
+      reflectionTester.AssertAllFieldsSetViaReflection(message);
+    }
+
+    [Test]
+    public void ReflectionSetters() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+      reflectionTester.SetAllFieldsViaReflection(builder);
+      TestAllTypes message = builder.Build();
+      TestUtil.AssertAllFieldsSet(message);
+    }
+
+    [Test]
+    public void ReflectionRepeatedSetters() {
+      TestAllTypes.Builder builder = TestAllTypes.CreateBuilder();
+      reflectionTester.SetAllFieldsViaReflection(builder);
+      reflectionTester.ModifyRepeatedFieldsViaReflection(builder);
+      TestAllTypes message = builder.Build();
+      TestUtil.AssertRepeatedFieldsModified(message);
+    }
+
+    [Test]
+    public void ReflectionDefaults() {
+      reflectionTester.AssertClearViaReflection(TestAllTypes.DefaultInstance);
+      reflectionTester.AssertClearViaReflection(TestAllTypes.CreateBuilder().Build());
+    }
+    // =================================================================
+    // Extensions.
+
+    [Test]
+    public void ExtensionAccessors() {
+      TestAllExtensions.Builder builder = TestAllExtensions.CreateBuilder();
+      TestUtil.SetAllExtensions(builder);
+      TestAllExtensions message = builder.Build();
+      TestUtil.AssertAllExtensionsSet(message);
+    }
+
+    [Test]
+    public void ExtensionRepeatedSetters() {
+      TestAllExtensions.Builder builder = TestAllExtensions.CreateBuilder();
+      TestUtil.SetAllExtensions(builder);
+      TestUtil.ModifyRepeatedExtensions(builder);
+      TestAllExtensions message = builder.Build();
+      TestUtil.AssertRepeatedExtensionsModified(message);
+    }
+
+    [Test]
+    public void ExtensionDefaults() {
+      TestUtil.AssertExtensionsClear(TestAllExtensions.DefaultInstance);
+      TestUtil.AssertExtensionsClear(TestAllExtensions.CreateBuilder().Build());
+    }
+
+    [Test]
+    public void ExtensionReflectionGetters() {
+      TestAllExtensions.Builder builder = TestAllExtensions.CreateBuilder();
+      TestUtil.SetAllExtensions(builder);
+      TestAllExtensions message = builder.Build();
+      extensionsReflectionTester.AssertAllFieldsSetViaReflection(message);
+    }
+
+    [Test]
+    public void ExtensionReflectionSetters() {
+      TestAllExtensions.Builder builder = TestAllExtensions.CreateBuilder();
+      extensionsReflectionTester.SetAllFieldsViaReflection(builder);
+      TestAllExtensions message = builder.Build();
+      TestUtil.AssertAllExtensionsSet(message);
+    }
+
+    [Test]
+    public void ExtensionReflectionRepeatedSetters() {
+      TestAllExtensions.Builder builder = TestAllExtensions.CreateBuilder();
+      extensionsReflectionTester.SetAllFieldsViaReflection(builder);
+      extensionsReflectionTester.ModifyRepeatedFieldsViaReflection(builder);
+      TestAllExtensions message = builder.Build();
+      TestUtil.AssertRepeatedExtensionsModified(message);
+    }
+
+    [Test]
+    public void ExtensionReflectionDefaults() {
+      extensionsReflectionTester.AssertClearViaReflection(TestAllExtensions.DefaultInstance);
+      extensionsReflectionTester.AssertClearViaReflection(TestAllExtensions.CreateBuilder().Build());
+    }
+
+    public void testClearExtension() {
+      // clearExtension() is not actually used in TestUtil, so try it manually.
+      Assert.IsFalse(TestAllExtensions.CreateBuilder()
+          .SetExtension(UnitTestProtoFile.OptionalInt32Extension, 1)
+          .ClearExtension(UnitTestProtoFile.OptionalInt32Extension)
+          .HasExtension(UnitTestProtoFile.OptionalInt32Extension));
+      Assert.AreEqual(0,
+        TestAllExtensions.CreateBuilder()
+          .AddExtension(UnitTestProtoFile.RepeatedInt32Extension, 1)
+          .ClearExtension(UnitTestProtoFile.RepeatedInt32Extension)
+          .GetExtensionCount(UnitTestProtoFile.RepeatedInt32Extension));
+    }
+
+    // =================================================================
+    // multiple_files_test
+    /* FIXME: Find this proto!
+    public void MultipleFilesOption() {
+      // We mostly just want to check that things compile.
+      MessageWithNoOuter message = MessageWithNoOuter.CreateBuilder()
+          .setNested(MessageWithNoOuter.NestedMessage.CreateBuilder().setI(1))
+          .addForeign(TestAllTypes.CreateBuilder().setOptionalInt32(1))
+          .setNestedEnum(MessageWithNoOuter.NestedEnum.BAZ)
+          .setForeignEnum(EnumWithNoOuter.BAR)
+          .Build();
+      Assert.AreEqual(message, MessageWithNoOuter.parseFrom(message.toByteString()));
+
+      Assert.AreEqual(MultipleFilesTestProto.getDescriptor(),
+                   MessageWithNoOuter.getDescriptor().getFile());
+
+      Descriptors.FieldDescriptor field =
+        MessageWithNoOuter.getDescriptor().findFieldByName("foreign_enum");
+      Assert.AreEqual(EnumWithNoOuter.BAR.getValueDescriptor(),
+                   message.getField(field));
+
+      Assert.AreEqual(MultipleFilesTestProto.getDescriptor(),
+                   ServiceWithNoOuter.getDescriptor().getFile());
+
+      assertFalse(
+        TestAllExtensions.getDefaultInstance().hasExtension(
+          MultipleFilesTestProto.extensionWithOuter));
+    }
+    */
+  }
+}

csharp/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj

@@ -50,6 +50,7 @@
     <Compile Include="CodedOutputStreamTest.cs" />
     <Compile Include="DescriptorsTest.cs" />
     <Compile Include="DynamicMessageTest.cs" />
+    <Compile Include="GeneratedMessageTest.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="ReflectionTester.cs" />
     <Compile Include="TestProtos\UnitTestEmbedOptimizeForProtoFile.cs" />

csharp/ProtocolBuffers.Test/ReflectionTester.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Text;
 using Google.ProtocolBuffers.Descriptors;
 using NUnit.Framework;
+using Google.ProtocolBuffers.TestProtos;
 
 namespace Google.ProtocolBuffers {
   /// <summary>
@@ -52,7 +53,7 @@ namespace Google.ProtocolBuffers {
     /// All of the TestAllExtensions extensions must be registered in the registry.
     /// TODO(jonskeet): Enforce all of these with two factory methods.
     /// </summary>
-    public ReflectionTester(MessageDescriptor baseDescriptor,
+    private ReflectionTester(MessageDescriptor baseDescriptor,
                             ExtensionRegistry extensionRegistry) {
       this.baseDescriptor = baseDescriptor;
       this.extensionRegistry = extensionRegistry;
@@ -129,9 +130,24 @@ namespace Google.ProtocolBuffers {
       Assert.IsNotNull(importBaz     );
     }
 
-    /**
-     * Shorthand to get a FieldDescriptor for a field of unittest::TestAllTypes.
-     */
+    /// <summary>
+    /// Creates an instance for the TestAllTypes message, with no extension registry.
+    /// </summary>
+    public static ReflectionTester CreateTestAllTypesInstance() {
+      return new ReflectionTester(TestAllTypes.Descriptor, null);
+    }
+
+    /// <summary>
+    /// Creates an instance for the TestAllExtensions message, with an
+    /// extension registry from TestUtil.CreateExtensionRegistry.
+    /// </summary>
+    public static ReflectionTester CreateTestAllExtensionsInstance() {      
+      return new ReflectionTester(TestAllExtensions.Descriptor, TestUtil.CreateExtensionRegistry());
+    }
+
+    /// <summary>
+    /// Shorthand to get a FieldDescriptor for a field of unittest::TestAllTypes.
+    /// </summary>
     private FieldDescriptor f(String name) {
       FieldDescriptor result;
       if (extensionRegistry == null) {
@@ -143,11 +159,10 @@ namespace Google.ProtocolBuffers {
       return result;
     }
 
-    /**
-     * Calls {@code parent.CreateBuilderForField()} or uses the
-     * {@code ExtensionRegistry} to find an appropriateIBuilder, depending
-     * on what type is being tested.
-     */
+    /// <summary>
+    /// Calls parent.CreateBuilderForField() or uses the extension registry
+    /// to find an appropriate builder, depending on what type is being tested.
+    /// </summary>
     private IBuilder CreateBuilderForField(IBuilder parent, FieldDescriptor field) {
       if (extensionRegistry == null) {
         return parent.CreateBuilderForField(field);
@@ -159,13 +174,11 @@ namespace Google.ProtocolBuffers {
       }
     }
 
-    // -------------------------------------------------------------------
-
-    /**
-     * Set every field of {@code message} to the values expected by
-     * {@code assertAllFieldsSet()}, using the {@link MessageIBuilder}
-     * reflection interface.
-     */
+    /// <summary>
+    /// Sets every field of the message to the values expected by
+    /// AssertAllFieldsSet, using the reflection interface.
+    /// </summary>
+    /// <param name="message"></param>
     internal void SetAllFieldsViaReflection(IBuilder message) {
       message[f("optional_int32"   )] = 101 ;
       message[f("optional_int64"   )] = 102L;

csharp/ProtocolBuffers.Test/TestProtos/UnitTestProtoFile.cs

@@ -4841,7 +4841,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         result.repeatedNestedEnum_.Add(value);
         return this;
       }
-      public Builder AddAllRepeatedNestedEnum(scg::IEnumerable<self::TestAllTypes.Types.NestedEnum> values) {
+      public Builder AddRangeRepeatedNestedEnum(scg::IEnumerable<self::TestAllTypes.Types.NestedEnum> values) {
         if (result.repeatedNestedEnum_.Count == 0) {
           result.repeatedNestedEnum_ = new scg::List<self::TestAllTypes.Types.NestedEnum>();
         }
@@ -4874,7 +4874,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         result.repeatedForeignEnum_.Add(value);
         return this;
       }
-      public Builder AddAllRepeatedForeignEnum(scg::IEnumerable<self::ForeignEnum> values) {
+      public Builder AddRangeRepeatedForeignEnum(scg::IEnumerable<self::ForeignEnum> values) {
         if (result.repeatedForeignEnum_.Count == 0) {
           result.repeatedForeignEnum_ = new scg::List<self::ForeignEnum>();
         }
@@ -4907,7 +4907,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         result.repeatedImportEnum_.Add(value);
         return this;
       }
-      public Builder AddAllRepeatedImportEnum(scg::IEnumerable<self::ImportEnum> values) {
+      public Builder AddRangeRepeatedImportEnum(scg::IEnumerable<self::ImportEnum> values) {
         if (result.repeatedImportEnum_.Count == 0) {
           result.repeatedImportEnum_ = new scg::List<self::ImportEnum>();
         }
@@ -11640,7 +11640,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         result.repeatedEnumField_.Add(value);
         return this;
       }
-      public Builder AddAllRepeatedEnumField(scg::IEnumerable<self::ForeignEnum> values) {
+      public Builder AddRangeRepeatedEnumField(scg::IEnumerable<self::ForeignEnum> values) {
         if (result.repeatedEnumField_.Count == 0) {
           result.repeatedEnumField_ = new scg::List<self::ForeignEnum>();
         }

csharp/ProtocolBuffers/ExtendableBuilder.cs

@@ -52,7 +52,7 @@ namespace Google.ProtocolBuffers {
     /// Sets the value of one element of a repeated extension.
     /// </summary>
     public ExtendableBuilder<TMessage, TBuilder> SetExtension<TExtension>(
-        GeneratedExtensionBase<TMessage, IList<TExtension>> extension, int index, Type value) {
+        GeneratedExtensionBase<TMessage, IList<TExtension>> extension, int index, TExtension value) {
       ExtendableMessage<TMessage, TBuilder> message = MessageBeingBuilt;
       message.VerifyExtensionContainingType(extension);
       message.Extensions[extension.Descriptor, index] = extension.SingularToReflectionType(value);
@@ -118,7 +118,6 @@ namespace Google.ProtocolBuffers {
         } else {
           base[field] = value;
         }
-        InternalFieldAccessors[field].SetValue(this, value);
       }
     }
 

src/google/protobuf/compiler/csharp/csharp_enum_field.cc

@@ -198,7 +198,7 @@ GenerateBuilderMembers(io::Printer* printer) const {
     "  result.$name$_.Add(value);\r\n"
     "  return this;\r\n"
     "}\r\n"
-    "public Builder AddAll$capitalized_name$(scg::IEnumerable<$type$> values) {\r\n"
+    "public Builder AddRange$capitalized_name$(scg::IEnumerable<$type$> values) {\r\n"
     "  if (result.$name$_.Count == 0) {\r\n"
     "    result.$name$_ = new scg::List<$type$>();\r\n"
     "  }\r\n"

src/google/protobuf/compiler/csharp/csharp_extension.cc

@@ -57,7 +57,7 @@ void ExtensionGenerator::Generate(io::Printer* printer) {
   if (descriptor_->is_repeated()) {
     printer->Print(vars,
       "public static readonly\r\n"
-      "  pb::GeneratedExtensionBase<$containing_type$, scg::IList<$type$>> name =\r\n"
+      "  pb::GeneratedExtensionBase<$containing_type$, scg::IList<$type$>> $name$ =\r\n"
       "      pb::GeneratedRepeatExtension<$containing_type$, $type$>.CreateInstance(Descriptor.Extensions[$index$]);\r\n");
   } else {
     printer->Print(vars,

src/google/protobuf/compiler/csharp/csharp_file.cc

@@ -165,11 +165,9 @@ void FileGenerator::Generate(io::Printer* printer) {
   // Extensions must be generated in the outer class since they are values,
   // not classes.
   printer->Print("#region Extensions\r\n");
-  printer->Print("/*");
   for (int i = 0; i < file_->extension_count(); i++) {
     ExtensionGenerator(file_->extension(i)).Generate(printer);
   }
-  printer->Print("*/\r\n");
   printer->Print("#endregion\r\n\r\n");
 
   printer->Print("#region Static variables\r\n");

src/google/protobuf/compiler/csharp/csharp_message.cc

@@ -435,7 +435,7 @@ void MessageGenerator::GenerateBuilder(io::Printer* printer) {
 
   if (descriptor_->extension_range_count() > 0) {
     printer->Print(
-      "$access$ sealed partial class Builder : pb::GeneratedBuilder<$classname$, $classname$.Builder>.ExtendableBuilder {\r\n",
+      "$access$ sealed partial class Builder : pb::ExtendableBuilder<$classname$, $classname$.Builder> {\r\n",
       "classname", ClassName(descriptor_),
       "access", ClassAccessLevel(descriptor_->file()));
   } else {
@@ -522,6 +522,10 @@ void MessageGenerator::GenerateCommonBuilderMethods(io::Printer* printer) {
   //TODO(jonskeet): Work out what this is really for...
   if (descriptor_->file()->options().optimize_for() == FileOptions::SPEED) {
     printer->Print(
+      "protected override IBuilder MergeFromImpl(CodedInputStream data, ExtensionRegistry extensionRegistry) {\r\n"
+      "  return MergeFrom(data, extensionRegistry);\r\n"
+      "}\r\n"
+      "\r\n"
       "public override IBuilder MergeFrom(pb::IMessage other) {\r\n"
       "  if (other is $classname$) {\r\n"
       "    return MergeFrom(($classname$) other);\r\n"