fix: adding error for generated read writer for abstract class (#2191)

* fix adding error for gernated read writer for abstract class

weaver can not initialize class abstract class so cant not create a reader
this gives a helpful error telling the server to make a custom reader

* adding tests for error message

* fixing typo

* renaming

* fixing expected error messages

Assets/Mirror/Editor/Weaver/Readers.cs

@@ -80,6 +80,11 @@ public static MethodReference GetReadFunc(TypeReference variableReference, int r
                 Weaver.Error($"Cannot generate reader for interface {variableReference.Name}. Use a supported type or provide a custom reader", variableReference);
                 return null;
             }
+            if (variableDefinition.IsAbstract)
+            {
+                Weaver.Error($"Cannot generate reader for abstract class {variableReference.Name}. Use a supported type or provide a custom reader", variableReference);
+                return null;
+            }
 
             if (variableDefinition.IsEnum)
             {

Assets/Mirror/Editor/Weaver/Writers.cs

@@ -114,6 +114,11 @@ static MethodDefinition GenerateWriter(TypeReference variableReference, int recu
                 Weaver.Error($"Cannot generate writer for interface {variableReference.Name}. Use a supported type or provide a custom writer", variableReference);
                 return null;
             }
+            if (variableDefinition.IsAbstract)
+            {
+                Weaver.Error($"Cannot generate writer for abstract class {variableReference.Name}. Use a supported type or provide a custom writer", variableReference);
+                return null;
+            }
 
             // generate writer for class/struct
 

Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj

@@ -1,4 +1,4 @@
-<Project>
+<Project>
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
     <RootNamespace>_WeaverTests2.csproj</RootNamespace>
@@ -62,6 +62,7 @@
     <None Remove="WeaverTests.cs.meta" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="WeaverGeneratedReaderWriterTests~\CanUseCustomReadWriteForAbstractClass.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CanUseCustomReadWriteForInterfaces.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CanUseCustomReadWriteForTypesFromDifferentAssemblies.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForArraySegment.cs" />
@@ -73,10 +74,10 @@
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForEnums.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForInheritedFromScriptableObject.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForList.cs" />
+    <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStructArraySegment.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStructFromDifferentAssemblies.cs" />
+    <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStructList.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStructs.cs" />
-    <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStuctArraySegment.cs" />
-    <Compile Include="WeaverGeneratedReaderWriterTests~\CreatesForStuctList.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\ExcludesNonSerializedFields.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorForClassWithNoValidConstructor.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorForInvalidArraySegmentType.cs" />
@@ -84,6 +85,7 @@
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorForInvalidListType.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorForJaggedArray.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorForMultidimensionalArray.cs" />
+    <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorWhenUsingAbstractClass.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorWhenUsingInterface.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorWhenUsingMonoBehaviour.cs" />
     <Compile Include="WeaverGeneratedReaderWriterTests~\GivesErrorWhenUsingObject.cs" />

Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs

@@ -147,6 +147,22 @@ public void CanUseCustomReadWriteForInterfaces()
             IsSuccess();
         }
 
+        [Test]
+        public void GivesErrorWhenUsingAbstractClass()
+        {
+            HasError("Cannot generate writer for abstract class DataBase. Use a supported type or provide a custom writer",
+                "GeneratedReaderWriter.GivesErrorWhenUsingAbstractClass.DataBase");
+            // TODO change weaver to run checks for write/read at the same time
+            //HasError("Cannot generate reader for abstract class DataBase. Use a supported type or provide a custom reader",
+            //    "GeneratedReaderWriter.GivesErrorWhenUsingAbstractClass.DataBase");
+        }
+
+        [Test]
+        public void CanUseCustomReadWriteForAbstractClass()
+        {
+            IsSuccess();
+        }
+
         [Test]
         public void CreatesForEnums()
         {

Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/CanUseCustomReadWriteForAbstractClass.cs

@@ -0,0 +1,62 @@
+using Mirror;
+
+namespace GeneratedReaderWriter.CanUseCustomReadWriteForAbstractClass
+{
+    public class CanUseCustomReadWriteForAbstractClass : NetworkBehaviour
+    {
+        [ClientRpc]
+        public void RpcDoSomething(DataBase data)
+        {
+            // empty
+        }
+    }
+
+    public abstract class DataBase
+    {
+        public int someField;
+        public abstract int id { get; }
+    }
+
+    public class SomeData : DataBase
+    {
+        public float anotherField;
+        public override int id => 1;
+    }
+
+    public static class DataReadWrite
+    {
+        public static void WriteData(this NetworkWriter writer, DataBase data)
+        {
+            writer.WriteInt32(data.id);
+            // write extra stuff depending on id here
+            writer.WriteInt32(data.someField);
+
+            if (data.id == 1)
+            {
+                SomeData someData = (SomeData)data;
+                writer.WriteSingle(someData.anotherField);
+            }
+        }
+
+        public static DataBase ReadData(this NetworkReader reader)
+        {
+            int id = reader.ReadInt32();
+
+            int someField = reader.ReadInt32();
+            DataBase data = null;
+            if (data.id == 1)
+            {
+                SomeData someData = new SomeData()
+                {
+                    someField = someField
+                };
+                // read extra stuff depending on id here
+
+                someData.anotherField = reader.ReadSingle();
+
+                data = someData;
+            }
+            return data;
+        }
+    }
+}

Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/GivesErrorWhenUsingAbstractClass.cs

@@ -0,0 +1,19 @@
+using Mirror;
+
+namespace GeneratedReaderWriter.GivesErrorWhenUsingAbstractClass
+{
+    public class GivesErrorWhenUsingAbstractClass : NetworkBehaviour
+    {
+        [ClientRpc]
+        public void RpcDoSomething(DataBase data)
+        {
+            // empty
+        }
+    }
+
+    public abstract class DataBase
+    {
+        public int someField;
+        public abstract int id { get; }
+    }
+}

Assets/Mirror/Tests/Editor/Weaver/WeaverNetworkBehaviourTests.cs

@@ -95,8 +95,11 @@ public void NetworkBehaviourTargetRpcParamRef()
         [Test]
         public void NetworkBehaviourTargetRpcParamAbstract()
         {
-            HasError("AbstractClass can't be deserialized because it has no default constructor",
+            HasError("Cannot generate writer for abstract class AbstractClass. Use a supported type or provide a custom writer",
                 "WeaverNetworkBehaviourTests.NetworkBehaviourTargetRpcParamAbstract.NetworkBehaviourTargetRpcParamAbstract/AbstractClass");
+            // TODO change weaver to run checks for write/read at the same time
+            //HasError("AbstractClass can't be deserialized because it has no default constructor",
+            //    "WeaverNetworkBehaviourTests.NetworkBehaviourTargetRpcParamAbstract.NetworkBehaviourTargetRpcParamAbstract/AbstractClass");
         }
 
         [Test]
@@ -177,8 +180,11 @@ public void NetworkBehaviourClientRpcParamRef()
         [Test]
         public void NetworkBehaviourClientRpcParamAbstract()
         {
-            HasError("AbstractClass can't be deserialized because it has no default constructor",
+            HasError("Cannot generate writer for abstract class AbstractClass. Use a supported type or provide a custom writer",
                 "WeaverNetworkBehaviourTests.NetworkBehaviourClientRpcParamAbstract.NetworkBehaviourClientRpcParamAbstract/AbstractClass");
+            // TODO change weaver to run checks for write/read at the same time
+            //HasError("AbstractClass can't be deserialized because it has no default constructor",
+            //    "WeaverNetworkBehaviourTests.NetworkBehaviourClientRpcParamAbstract.NetworkBehaviourClientRpcParamAbstract/AbstractClass");
         }
 
         [Test]
@@ -239,8 +245,11 @@ public void NetworkBehaviourCmdParamRef()
         [Test]
         public void NetworkBehaviourCmdParamAbstract()
         {
-            HasError("AbstractClass can't be deserialized because it has no default constructor",
+            HasError("Cannot generate writer for abstract class AbstractClass. Use a supported type or provide a custom writer",
                 "WeaverNetworkBehaviourTests.NetworkBehaviourCmdParamAbstract.NetworkBehaviourCmdParamAbstract/AbstractClass");
+            // TODO change weaver to run checks for write/read at the same time
+            //HasError("AbstractClass can't be deserialized because it has no default constructor",
+            //    "WeaverNetworkBehaviourTests.NetworkBehaviourCmdParamAbstract.NetworkBehaviourCmdParamAbstract/AbstractClass");
         }
 
         [Test]