feat#550375: Support ref and ref readonly return (#611)

* Support ref and ref readonly return

* Add test cases

* Update test

mdoc/Consts.cs

@@ -48,5 +48,6 @@ namespace Mono.Documentation
         public const string CompilerGeneratedAttribute = "System.Runtime.CompilerServices.CompilerGeneratedAttribute";
         public const string IsByRefLikeAttribute = "System.Runtime.CompilerServices.IsByRefLikeAttribute";
         public const string IsReadOnlyAttribute = "System.Runtime.CompilerServices.IsReadOnlyAttribute";
+        public const string InAttribute = "System.Runtime.InteropServices.InAttribute";
     }
 }

mdoc/Mono.Documentation/Updater/Formatters/CSharpFullMemberFormatter.cs

@@ -517,18 +517,6 @@ namespace Mono.Documentation.Updater.Formatters
             if (method.IsFinal) modifiers += " sealed";
             if (modifiers == " virtual sealed") modifiers = "";
 
-            if ((method.ReturnType.IsRequiredModifier
-                  && ((RequiredModifierType)method.ReturnType).ElementType.IsByReference)
-                || method.ReturnType.IsByReference)
-            {
-                modifiers += " ref";
-            }
-
-            if (method.ReturnType.IsRequiredModifier && DocUtils.HasCustomAttribute(method.MethodReturnType, Consts.IsReadOnlyAttribute))
-            {
-                modifiers += " readonly";
-            }
-
             switch (method.Name)
             {
                 case "op_Implicit":
@@ -542,6 +530,24 @@ namespace Mono.Documentation.Updater.Formatters
             return buf.Append (modifiers);
         }
 
+        protected override StringBuilder AppendRefTypeName(StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
+        {
+            buf.Append("ref ");
+            return base.AppendRefTypeName(buf, type, context);
+        }
+
+        protected override StringBuilder AppendRequiredModifierTypeName(
+            StringBuilder buf, RequiredModifierType type, IAttributeParserContext context)
+        {
+            if (type.ModifierType.FullName == Consts.InAttribute && type.ElementType is ByReferenceType refType)
+            {
+                buf.Append("ref readonly ");
+                return _AppendTypeName(buf, refType.ElementType, context);
+            }
+
+            return base.AppendRequiredModifierTypeName(buf, type, context);
+        }
+
         protected override StringBuilder AppendGenericMethod (StringBuilder buf, MethodDefinition method)
         {
             if (method.IsGenericMethod ())
@@ -585,19 +591,23 @@ namespace Mono.Documentation.Updater.Formatters
 
         private StringBuilder AppendParameter (StringBuilder buf, ParameterDefinition parameter)
         {
-            if (parameter.ParameterType is ByReferenceType)
+            TypeReference parameterType = parameter.ParameterType;
+
+            if (parameterType is ByReferenceType byReferenceType)
             {
                 if (parameter.IsOut)
                 {
                     buf.Append ("out ");
                 }
+                else if(parameter.IsIn)
+                {
+                    buf.Append("in ");
+                }
                 else
                 {
-                    if (parameter.HasCustomAttributes && parameter.CustomAttributes.Any (ca => ca.AttributeType.Name == "IsReadOnlyAttribute"))
-                        buf.Append ("in ");
-                    else
-                        buf.Append ("ref ");
+                    buf.Append("ref ");
                 }
+                parameterType = byReferenceType.ElementType;
             }
 
             if (parameter.HasCustomAttributes)
@@ -609,7 +619,7 @@ namespace Mono.Documentation.Updater.Formatters
 
             var context = AttributeParserContext.Create (parameter);
             var isNullableType = context.IsNullable ();
-            buf.Append (GetTypeName (parameter.ParameterType, context));
+            buf.Append (GetTypeName (parameterType, context));
             buf.Append (GetTypeNullableSymbol (parameter.ParameterType, isNullableType));
             buf.Append (" ");
             buf.Append (parameter.Name);
@@ -673,9 +683,6 @@ namespace Mono.Documentation.Updater.Formatters
                 modifiers = "";
             buf.Append (modifiers).Append (' ');
 
-            if (property.PropertyType.IsByReference)
-                buf.Append("ref ");
-
             var context = AttributeParserContext.Create (property);
             var isNullableType = context.IsNullable ();
             var propertyReturnTypeName = GetTypeName (property.PropertyType, context);

mdoc/Mono.Documentation/Updater/Formatters/CppFormatters/CppFullMemberFormatter.cs

@@ -776,10 +776,9 @@ namespace Mono.Documentation.Updater.Formatters.CppFormatters
             return buf;
         }
 
-        protected override StringBuilder AppendRefTypeName(StringBuilder buf, TypeReference type, IAttributeParserContext context)
+        protected override StringBuilder AppendRefTypeName(StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
         {
-            TypeSpecification spec = type as TypeSpecification;
-            _AppendTypeName(buf, spec != null ? spec.ElementType : type.GetElementType(), context);
+            _AppendTypeName(buf, type.ElementType, context);
             AppendHat(buf, type);
             buf.Append(RefTypeModifier);
 

mdoc/Mono.Documentation/Updater/Formatters/FSharpFormatter.cs

@@ -613,10 +613,9 @@ namespace Mono.Documentation.Updater
             return buf;
         }
 
-        protected override StringBuilder AppendRefTypeName(StringBuilder buf, TypeReference type, IAttributeParserContext context)
+        protected override StringBuilder AppendRefTypeName(StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
         {
-            ByReferenceType reftype = type as ByReferenceType;
-            return AppendTypeName(buf, reftype?.ElementType, context);
+            return AppendTypeName(buf, type.ElementType, context);
         }
 
         protected override StringBuilder AppendModifiers(StringBuilder buf, MethodDefinition method)

mdoc/Mono.Documentation/Updater/Formatters/MemberFormatter.cs

@@ -143,14 +143,14 @@ namespace Mono.Documentation.Updater
 
             StringBuilder interimBuilder = new StringBuilder();
 
-            if (ShouldStripModFromTypeName && type is RequiredModifierType)
+            if (ShouldStripModFromTypeName && type is RequiredModifierType requiredModifierType)
             {
-                AppendRequiredModifierTypeName(interimBuilder, type as RequiredModifierType, context);
+                AppendRequiredModifierTypeName(interimBuilder, requiredModifierType, context);
                 return SetBuffer(buf, interimBuilder, useTypeProjection: useTypeProjection);
             }
-            if (ShouldStripModFromTypeName && type is OptionalModifierType)
+            if (ShouldStripModFromTypeName && type is OptionalModifierType optionalModifierType)
             {
-                AppendOptionalModifierTypeName(interimBuilder, type as OptionalModifierType, context);
+                AppendOptionalModifierTypeName(interimBuilder, optionalModifierType, context);
                 return SetBuffer(buf, interimBuilder, useTypeProjection: useTypeProjection);
             }
             if (type is ArrayType)
@@ -158,9 +158,9 @@ namespace Mono.Documentation.Updater
                 AppendArrayTypeName(interimBuilder, type, context);
                 return SetBuffer(buf, interimBuilder, useTypeProjection: useTypeProjection);
             }
-            if (type is ByReferenceType)
+            if (type is ByReferenceType byReferenceType)
             {
-                AppendRefTypeName (interimBuilder, type, context);
+                AppendRefTypeName (interimBuilder, byReferenceType, context);
                 return SetBuffer(buf, interimBuilder, useTypeProjection: useTypeProjection);
             }
             if (type is PointerType)
@@ -290,10 +290,9 @@ namespace Mono.Documentation.Updater
             get { return "@"; }
         }
 
-        protected virtual StringBuilder AppendRefTypeName (StringBuilder buf, TypeReference type, IAttributeParserContext context)
+        protected virtual StringBuilder AppendRefTypeName (StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
         {
-            TypeSpecification spec = type as TypeSpecification;
-            return _AppendTypeName(buf, spec != null ? spec.ElementType : type.GetElementType(), context);
+            return _AppendTypeName(buf, type.ElementType, context);
         }
 
         protected virtual string PointerModifier

mdoc/Mono.Documentation/Updater/Formatters/MsxdocSlashDocMemberFormatter.cs

@@ -7,10 +7,9 @@ namespace Mono.Documentation.Updater
     {
         public MsxdocSlashDocMemberFormatter(TypeMap map) : base(map) { }
 
-        protected override StringBuilder AppendRefTypeName(StringBuilder buf, TypeReference type, IAttributeParserContext context)
+        protected override StringBuilder AppendRefTypeName(StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
         {
-            TypeSpecification spec = type as TypeSpecification;
-            return _AppendTypeName(buf, spec != null ? spec.ElementType : type.GetElementType(), context).Append(RefTypeModifier);
+            return _AppendTypeName(buf, type.ElementType, context).Append(RefTypeModifier);
         }
     }
 }

mdoc/Mono.Documentation/Updater/Formatters/SlashDocMemberFormatter.cs

@@ -82,7 +82,7 @@ namespace Mono.Documentation.Updater
             return buf;
         }
 
-        protected override StringBuilder AppendRefTypeName (StringBuilder buf, TypeReference type, IAttributeParserContext context)
+        protected override StringBuilder AppendRefTypeName (StringBuilder buf, ByReferenceType type, IAttributeParserContext context)
         {
             return base.AppendRefTypeName (buf, type, context).Append (RefTypeModifier);
         }

mdoc/mdoc.Test/BasicTests.cs

@@ -88,6 +88,15 @@ namespace mdoc.Test
             return GetMethod(GetType(type), i => i.Name == name);
         }
 
+        protected PropertyDefinition GetProperty(Type type, Func<PropertyDefinition, bool> query)
+        {
+            var properties = GetType(type).Properties;
+            var member = properties.FirstOrDefault(query)?.Resolve();
+            if (member == null)
+                throw new Exception("Did not find the member in the test class");
+            return member;
+        }
+
         protected Dictionary<string, List<MemberReference>> GetClassInterface(TypeDefinition type)
         {
             return DocUtils.GetImplementedMembersFingerprintLookup(type);

mdoc/mdoc.Test/FormatterTests.cs

@@ -256,22 +256,28 @@ namespace mdoc.Test
             Assert.AreEqual("public void SomeMethod4 (out string a, T t, object b = default);", sig);
         }
 
-        [Test]
-        public void CSharpReadonlyRefReturn()
+        [TestCase(typeof(ReadonlyRefClass), "Ref", "public ref int Ref ();")]
+        [TestCase(typeof(ReadonlyRefClass), "ReadonlyRef", "public ref readonly int ReadonlyRef ();")]
+        [TestCase(typeof(ReadonlyRefClass), "RefInAndOutMethod", "public void RefInAndOutMethod (ref int a, in int b, out int c);")]
+        [TestCase(typeof(GenericRefClass<>), "Ref", "public ref T Ref ();")]
+        [TestCase(typeof(GenericRefClass<>), "ReadonlyRef", "public ref readonly T ReadonlyRef ();")]
+        [TestCase(typeof(GenericRefClass<>), "RefInAndOutMethod", "public void RefInAndOutMethod (ref T a, in T b, out T c);")]
+        public void CSharpRefReturnMethodTest(Type type, string methodName, string expectedSignature)
         {
-            var member = GetMethod(typeof(ReadonlyRefClass), m => m.Name == "ReadonlyRef");
-            var formatter = new CSharpFullMemberFormatter();
-            var sig = formatter.GetDeclaration(member);
-            Assert.AreEqual("public ref readonly int ReadonlyRef ();", sig);
+            var member = GetMethod(type, m => m.Name == methodName);
+            var actualSignature = formatter.GetDeclaration(member);
+            Assert.AreEqual(expectedSignature, actualSignature);
         }
 
-        [Test]
-        public void CSharpRefReturn()
+        [TestCase(typeof(ReadonlyRefClass), "RefProperty", "public ref int RefProperty { get; }")]
+        [TestCase(typeof(ReadonlyRefClass), "Item", "public ref readonly int this[int index] { get; }")]
+        [TestCase(typeof(GenericRefClass<>), "RefProperty", "public ref T RefProperty { get; }")]
+        [TestCase(typeof(GenericRefClass<>), "Item", "public ref readonly T this[int index] { get; }")]
+        public void CSharpRefReturnPropertyTest(Type type, string propertyName, string expectedSignature)
         {
-            var member = GetMethod(typeof(ReadonlyRefClass), m => m.Name == "Ref");
-            var formatter = new CSharpFullMemberFormatter();
-            var sig = formatter.GetDeclaration(member);
-            Assert.AreEqual("public ref int Ref ();", sig);
+            var member = GetProperty(type, p => p.Name == propertyName);
+            var actualSignature = formatter.GetDeclaration(member);
+            Assert.AreEqual(expectedSignature, actualSignature);
         }
 
         [Test]

mdoc/mdoc.Test/SampleClasses/ReadonlyRefClass.cs

@@ -3,8 +3,27 @@
     public class ReadonlyRefClass
     {
         int i;
-
         public ref int Ref() => ref i;
-        public ref readonly int ReadonlyRef() => ref i;       
+
+        public ref readonly int ReadonlyRef() => ref i;
+
+        public ref int RefProperty { get { return ref i; } }
+
+        public ref readonly int this[int index] => throw null;
+
+        public void RefInAndOutMethod(ref int a, in int b, out int c) => throw null;
+    }
+
+    public class GenericRefClass<T>
+    {
+        public ref T Ref() => throw null;
+
+        public ref readonly T ReadonlyRef() => throw null;
+
+        public ref T RefProperty => throw null;
+
+        public ref readonly T this[int index] => throw null;
+
+        public void RefInAndOutMethod(ref T a, in T b, out T c) => throw null;
     }
 }