From 80490529d55affb059ae0a21ec85673c05028bf9 Mon Sep 17 00:00:00 2001
From: MeltyPlayer <meltyplayer@gmail.com>
Date: Wed, 24 Apr 2024 10:21:37 -0500
Subject: [PATCH] Added test coverage for built-in types that should be
 mutable.

---
 Schema Tests/readOnly/BuiltInTypeTests.cs     | 63 +++++++++++++++++++
 .../readOnly/ReadOnlySubstitutionTests.cs     |  4 +-
 Schema/src/readOnly/ReadOnlyTypeGenerator.cs  | 23 ++++---
 3 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/Schema Tests/readOnly/BuiltInTypeTests.cs b/Schema Tests/readOnly/BuiltInTypeTests.cs
index bda14aa..09ca134 100644
--- a/Schema Tests/readOnly/BuiltInTypeTests.cs	
+++ b/Schema Tests/readOnly/BuiltInTypeTests.cs	
@@ -46,5 +46,68 @@ public interface IReadOnlyWrapper {
 
             """);
     }
+
+    [Test]
+    [TestCase("System.Span<int>")]
+    [TestCase("System.Collections.Generic.ICollection<int>")]
+    [TestCase("System.Collections.Generic.IList<int>")]
+    public void TestDoesNotConvertBuiltInsForMutableProperties(string mutable) {
+      ReadOnlyGeneratorTestUtil.AssertGenerated(
+          $$"""
+            using schema.readOnly;
+
+            namespace foo.bar {
+              [GenerateReadOnly]
+              public partial interface IWrapper {
+                [KeepMutableType]
+                {{mutable}} Property { get; set; }
+              }
+            }
+            """,
+          $$"""
+            namespace foo.bar {
+              public partial interface IWrapper : IReadOnlyWrapper {
+                {{mutable}} IReadOnlyWrapper.Property => Property;
+              }
+              
+              public interface IReadOnlyWrapper {
+                public {{mutable}} Property { get; }
+              }
+            }
+
+            """);
+    }
+
+    [Test]
+    [TestCase("System.Span<int>")]
+    [TestCase("System.Collections.Generic.ICollection<int>")]
+    [TestCase("System.Collections.Generic.IList<int>")]
+    public void TestDoesNotConvertBuiltInsForMutableMethods(string mutable) {
+      ReadOnlyGeneratorTestUtil.AssertGenerated(
+          $$"""
+            using schema.readOnly;
+
+            namespace foo.bar {
+              [GenerateReadOnly]
+              public partial interface IWrapper {
+                [Const]
+                [KeepMutableType]
+                {{mutable}} Convert({{mutable}} value);
+              }
+            }
+            """,
+          $$"""
+            namespace foo.bar {
+              public partial interface IWrapper : IReadOnlyWrapper {
+                {{mutable}} IReadOnlyWrapper.Convert({{mutable}} value) => Convert(value);
+              }
+              
+              public interface IReadOnlyWrapper {
+                public {{mutable}} Convert({{mutable}} value);
+              }
+            }
+
+            """);
+    }
   }
 }
\ No newline at end of file
diff --git a/Schema Tests/readOnly/ReadOnlySubstitutionTests.cs b/Schema Tests/readOnly/ReadOnlySubstitutionTests.cs
index cd7cf3e..b88c530 100644
--- a/Schema Tests/readOnly/ReadOnlySubstitutionTests.cs	
+++ b/Schema Tests/readOnly/ReadOnlySubstitutionTests.cs	
@@ -179,11 +179,11 @@ public partial interface IWrapper {
           """
           namespace foo.bar {
             public partial interface IWrapper : IReadOnlyWrapper {
-              schema.readOnly.IReadOnlyValue IReadOnlyWrapper.Foo() => Foo();
+              schema.readOnly.IValue IReadOnlyWrapper.Foo() => Foo();
             }
             
             public interface IReadOnlyWrapper {
-              public schema.readOnly.IReadOnlyValue Foo();
+              public schema.readOnly.IValue Foo();
             }
           }
 
diff --git a/Schema/src/readOnly/ReadOnlyTypeGenerator.cs b/Schema/src/readOnly/ReadOnlyTypeGenerator.cs
index a5a9379..a1826c2 100644
--- a/Schema/src/readOnly/ReadOnlyTypeGenerator.cs
+++ b/Schema/src/readOnly/ReadOnlyTypeGenerator.cs
@@ -244,7 +244,7 @@ private static void WriteMembers_(
                         memberTypeSymbol,
                         semanticModel,
                         syntax,
-                        associatedPropertySymbol))
+                        (ISymbol?) associatedPropertySymbol ?? memberSymbol))
             .Write(" ");
 
         if (interfaceName != null) {
@@ -288,6 +288,7 @@ var isIndexer
           } else {
             cbsb.Write(" => ")
                 .Write(typeSymbol.GetCStyleCastToReadOnlyIfNeeded(
+                           associatedPropertySymbol,
                            memberSymbol.ReturnType,
                            semanticModel,
                            syntax))
@@ -377,6 +378,7 @@ var isIndexer
           } else {
             cbsb.Write(" => ")
                 .Write(typeSymbol.GetCStyleCastToReadOnlyIfNeeded(
+                           memberSymbol,
                            memberSymbol.ReturnType,
                            semanticModel,
                            syntax))
@@ -532,9 +534,9 @@ private static IEnumerable<string> GetTypeConstraintNames_(
     }
 
     private static string ConvertName_(ITypeSymbol typeSymbol,
-                                       ISymbol? typeSymbolForAttributeChecks) {
+                                       ISymbol? symbolForAttributeChecks) {
       var defaultName = typeSymbol.Name.EscapeKeyword();
-      if ((typeSymbolForAttributeChecks ?? typeSymbol)
+      if ((symbolForAttributeChecks ?? typeSymbol)
           .HasAttribute<KeepMutableTypeAttribute>()) {
         return defaultName;
       }
@@ -684,13 +686,14 @@ var allParentTypes
 
     public static string GetCStyleCastToReadOnlyIfNeeded(
         this ITypeSymbol sourceSymbol,
+        ISymbol? symbolForAttributeChecks,
         ITypeSymbol symbol,
         SemanticModel semanticModel,
         TypeDeclarationSyntax syntax) {
       // TODO: Only allow casts if generics are covariant, otherwise report
       // diagnostic error
       var sb = new StringBuilder();
-      if (symbol.IsCastNeeded()) {
+      if (symbol.IsCastNeeded(symbolForAttributeChecks)) {
         sb.Append("(")
           .Append(
               sourceSymbol
@@ -704,16 +707,20 @@ public static string GetCStyleCastToReadOnlyIfNeeded(
       return sb.ToString();
     }
 
-    public static bool IsCastNeeded(this ITypeSymbol symbol) {
+    public static bool IsCastNeeded(this ITypeSymbol symbol,
+                                    ISymbol? symbolForAttributeChecks) {
+      if ((symbolForAttributeChecks ?? symbol)
+          .HasAttribute<KeepMutableTypeAttribute>()) {
+        return false;
+      }
+
       if (symbol.IsGeneric(out _, out var typeArguments) &&
           typeArguments.Any(typeArgument => typeArgument
                                 .HasAttribute<GenerateReadOnlyAttribute>())) {
         return true;
       }
 
-      return
-          !symbol.HasAttribute<KeepMutableTypeAttribute>() &&
-          symbol.HasBuiltInReadOnlyType_(out _, out var canImplicitlyConvert) &&
+      return symbol.HasBuiltInReadOnlyType_(out _, out var canImplicitlyConvert) &&
           !canImplicitlyConvert;
     }
   }