Merge pull request #577 from jskeet/csharp-fixup

Fix-ups suggested in PR #560

csharp/src/ProtocolBuffers.Test/CodedInputStreamTest.cs

@@ -229,7 +229,7 @@ namespace Google.Protobuf
         [Test]
         public void ReadWholeMessage_VaryingBlockSizes()
         {
-            TestAllTypes message = GeneratedMessageTest.GetSampleMessage();
+            TestAllTypes message = SampleMessages.CreateFullTestAllTypes();
 
             byte[] rawBytes = message.ToByteArray();
             Assert.AreEqual(rawBytes.Length, message.CalculateSize());

csharp/src/ProtocolBuffers.Test/CodedOutputStreamTest.cs

@@ -193,7 +193,7 @@ namespace Google.Protobuf
         [Test]
         public void WriteWholeMessage_VaryingBlockSizes()
         {
-            TestAllTypes message = GeneratedMessageTest.GetSampleMessage();
+            TestAllTypes message = SampleMessages.CreateFullTestAllTypes();
 
             byte[] rawBytes = message.ToByteArray();
 

csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs

@@ -42,61 +42,6 @@ namespace Google.Protobuf
     /// </summary>
     public class GeneratedMessageTest
     {
-        /// <summary>
-        /// Returns a sample TestAllTypes with all fields populated
-        /// </summary>
-        public static TestAllTypes GetSampleMessage()
-        {
-            return new TestAllTypes
-            {
-                SingleBool = true,
-                SingleBytes = ByteString.CopyFrom(1, 2, 3, 4),
-                SingleDouble = 23.5,
-                SingleFixed32 = 23,
-                SingleFixed64 = 1234567890123,
-                SingleFloat = 12.25f,
-                SingleForeignEnum = ForeignEnum.FOREIGN_BAR,
-                SingleForeignMessage = new ForeignMessage { C = 10 },
-                SingleImportEnum = ImportEnum.IMPORT_BAZ,
-                SingleImportMessage = new ImportMessage { D = 20 },
-                SingleInt32 = 100,
-                SingleInt64 = 3210987654321,
-                SingleNestedEnum = TestAllTypes.Types.NestedEnum.FOO,
-                SingleNestedMessage = new TestAllTypes.Types.NestedMessage { Bb = 35 },
-                SinglePublicImportMessage = new PublicImportMessage { E = 54 },
-                SingleSfixed32 = -123,
-                SingleSfixed64 = -12345678901234,
-                SingleSint32 = -456,
-                SingleSint64 = -12345678901235,
-                SingleString = "test",
-                SingleUint32 = uint.MaxValue,
-                SingleUint64 = ulong.MaxValue,
-                RepeatedBool = { true, false },
-                RepeatedBytes = { ByteString.CopyFrom(1, 2, 3, 4), ByteString.CopyFrom(5, 6), ByteString.CopyFrom(new byte[1000]) },
-                RepeatedDouble = { -12.25, 23.5 },
-                RepeatedFixed32 = { uint.MaxValue, 23 },
-                RepeatedFixed64 = { ulong.MaxValue, 1234567890123 },
-                RepeatedFloat = { 100f, 12.25f },
-                RepeatedForeignEnum = { ForeignEnum.FOREIGN_FOO, ForeignEnum.FOREIGN_BAR },
-                RepeatedForeignMessage = { new ForeignMessage(), new ForeignMessage { C = 10 } },
-                RepeatedImportEnum = { ImportEnum.IMPORT_BAZ, ImportEnum.IMPORT_ENUM_UNSPECIFIED },
-                RepeatedImportMessage = { new ImportMessage { D = 20 }, new ImportMessage { D = 25 } },
-                RepeatedInt32 = { 100, 200 },
-                RepeatedInt64 = { 3210987654321, long.MaxValue },
-                RepeatedNestedEnum = { TestAllTypes.Types.NestedEnum.FOO, TestAllTypes.Types.NestedEnum.NEG },
-                RepeatedNestedMessage = { new TestAllTypes.Types.NestedMessage { Bb = 35 }, new TestAllTypes.Types.NestedMessage { Bb = 10 } },
-                RepeatedPublicImportMessage = { new PublicImportMessage { E = 54 }, new PublicImportMessage { E = -1 } },
-                RepeatedSfixed32 = { -123, 123 },
-                RepeatedSfixed64 = { -12345678901234, 12345678901234 },
-                RepeatedSint32 = { -456, 100 },
-                RepeatedSint64 = { -12345678901235, 123 },
-                RepeatedString = { "foo", "bar" },
-                RepeatedUint32 = { uint.MaxValue, uint.MinValue },
-                RepeatedUint64 = { ulong.MaxValue, uint.MinValue },
-                OneofString = "Oneof string"                
-            };
-        }
-
         [Test]
         public void EmptyMessageFieldDistinctFromMissingMessageField()
         {

csharp/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj

@@ -81,6 +81,7 @@
     <Compile Include="Collections\MapFieldTest.cs" />
     <Compile Include="Collections\RepeatedFieldTest.cs" />
     <Compile Include="SampleEnum.cs" />
+    <Compile Include="SampleMessages.cs" />
     <Compile Include="TestProtos\MapUnittestProto3.cs" />
     <Compile Include="TestProtos\UnittestImportProto3.cs" />
     <Compile Include="TestProtos\UnittestImportPublicProto3.cs" />

csharp/src/ProtocolBuffers.Test/SampleEnum.cs

@@ -39,4 +39,4 @@ namespace Google.Protobuf
         None = 0,
         PositiveValue = 3
     }
-}
\ No newline at end of file
+}

csharp/src/ProtocolBuffers.Test/SampleMessages.cs

+#region Copyright notice and license
+// Protocol Buffers - Google's data interchange format
+// Copyright 2015 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#endregion
+
+using System;
+using Google.Protobuf.TestProtos;
+
+namespace Google.Protobuf
+{
+    /// <summary>
+    /// Helper methods to create sample instances of types generated from unit test messages.
+    /// </summary>
+    public class SampleMessages
+    {
+        /// <summary>
+        /// Creates a new sample TestAllTypes message with all fields populated.
+        /// The "oneof" field is populated with the string property (OneofString).
+        /// </summary>
+        public static TestAllTypes CreateFullTestAllTypes()
+        {
+            return new TestAllTypes
+            {
+                SingleBool = true,
+                SingleBytes = ByteString.CopyFrom(1, 2, 3, 4),
+                SingleDouble = 23.5,
+                SingleFixed32 = 23,
+                SingleFixed64 = 1234567890123,
+                SingleFloat = 12.25f,
+                SingleForeignEnum = ForeignEnum.FOREIGN_BAR,
+                SingleForeignMessage = new ForeignMessage { C = 10 },
+                SingleImportEnum = ImportEnum.IMPORT_BAZ,
+                SingleImportMessage = new ImportMessage { D = 20 },
+                SingleInt32 = 100,
+                SingleInt64 = 3210987654321,
+                SingleNestedEnum = TestAllTypes.Types.NestedEnum.FOO,
+                SingleNestedMessage = new TestAllTypes.Types.NestedMessage { Bb = 35 },
+                SinglePublicImportMessage = new PublicImportMessage { E = 54 },
+                SingleSfixed32 = -123,
+                SingleSfixed64 = -12345678901234,
+                SingleSint32 = -456,
+                SingleSint64 = -12345678901235,
+                SingleString = "test",
+                SingleUint32 = UInt32.MaxValue,
+                SingleUint64 = UInt64.MaxValue,
+                RepeatedBool = { true, false },
+                RepeatedBytes = { ByteString.CopyFrom(1, 2, 3, 4), ByteString.CopyFrom(5, 6), ByteString.CopyFrom(new byte[1000]) },
+                RepeatedDouble = { -12.25, 23.5 },
+                RepeatedFixed32 = { UInt32.MaxValue, 23 },
+                RepeatedFixed64 = { UInt64.MaxValue, 1234567890123 },
+                RepeatedFloat = { 100f, 12.25f },
+                RepeatedForeignEnum = { ForeignEnum.FOREIGN_FOO, ForeignEnum.FOREIGN_BAR },
+                RepeatedForeignMessage = { new ForeignMessage(), new ForeignMessage { C = 10 } },
+                RepeatedImportEnum = { ImportEnum.IMPORT_BAZ, ImportEnum.IMPORT_ENUM_UNSPECIFIED },
+                RepeatedImportMessage = { new ImportMessage { D = 20 }, new ImportMessage { D = 25 } },
+                RepeatedInt32 = { 100, 200 },
+                RepeatedInt64 = { 3210987654321, Int64.MaxValue },
+                RepeatedNestedEnum = { TestAllTypes.Types.NestedEnum.FOO, TestAllTypes.Types.NestedEnum.NEG },
+                RepeatedNestedMessage = { new TestAllTypes.Types.NestedMessage { Bb = 35 }, new TestAllTypes.Types.NestedMessage { Bb = 10 } },
+                RepeatedPublicImportMessage = { new PublicImportMessage { E = 54 }, new PublicImportMessage { E = -1 } },
+                RepeatedSfixed32 = { -123, 123 },
+                RepeatedSfixed64 = { -12345678901234, 12345678901234 },
+                RepeatedSint32 = { -456, 100 },
+                RepeatedSint64 = { -12345678901235, 123 },
+                RepeatedString = { "foo", "bar" },
+                RepeatedUint32 = { UInt32.MaxValue, UInt32.MinValue },
+                RepeatedUint64 = { UInt64.MaxValue, UInt32.MinValue },
+                OneofString = "Oneof string"                
+            };
+        }
+    }
+}

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

@@ -391,47 +391,7 @@ FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor,
   }
 }
 
-bool HasRequiredFields(const Descriptor* descriptor, std::set<const Descriptor*>* already_seen) {
-  if (already_seen->find(descriptor) != already_seen->end()) {
-    // The type is already in cache.  This means that either:
-    // a. The type has no required fields.
-    // b. We are in the midst of checking if the type has required fields,
-    //    somewhere up the stack.  In this case, we know that if the type
-    //    has any required fields, they'll be found when we return to it,
-    //    and the whole call to HasRequiredFields() will return true.
-    //    Therefore, we don't have to check if this type has required fields
-    //    here.
-    return false;
-  }
-  already_seen->insert(descriptor);
-
-  // If the type has extensions, an extension with message type could contain
-  // required fields, so we have to be conservative and assume such an
-  // extension exists.
-  if (descriptor->extension_count() > 0) {
-    return true;
-  }
-
-  for (int i = 0; i < descriptor->field_count(); i++) {
-    const FieldDescriptor* field = descriptor->field(i);
-    if (field->is_required()) {
-      return true;
-    }
-    if (GetCSharpType(field->type()) == CSHARPTYPE_MESSAGE) {
-      if (HasRequiredFields(field->message_type(), already_seen)) {
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
-bool HasRequiredFields(const Descriptor* descriptor) {
-  std::set<const Descriptor*> already_seen;
-  return HasRequiredFields(descriptor, &already_seen);
-}
-
-}  // namespace java
+}  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf
 }  // namespace google

src/google/protobuf/compiler/csharp/csharp_helpers.h

@@ -101,12 +101,6 @@ uint FixedMakeTag(const FieldDescriptor* descriptor);
 
 FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor, int fieldOrdinal);
 
-bool HasRequiredFields(const Descriptor* descriptor);
-
-inline bool SupportFieldPresence(const FileDescriptor* file) {
-  return file->syntax() != FileDescriptor::SYNTAX_PROTO3;
-}
-
 }  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf