From ba9e1121c350bc82396855c40b31579da88d9b47 Mon Sep 17 00:00:00 2001
From: Rikki Gibson <rigibson@microsoft.com>
Date: Fri, 5 Apr 2019 13:22:17 -0700
Subject: [PATCH] Fixes from feedback

---
 .../Source/SourcePropertyAccessorSymbol.cs    | 21 ++++++++++++++-----
 .../Semantics/ReadOnlyStructsTests.cs         | 21 +++++++++++++++----
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
index 0db7cbbe387..c07230a33d6 100644
--- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
+++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
@@ -422,13 +422,24 @@ internal Accessibility LocalAccessibility
         /// <summary>
         /// Indicates whether this accessor is readonly due to reasons scoped to itself and its containing property.
         /// </summary>
-        internal override bool IsDeclaredReadOnly => LocalDeclaredReadOnly || _property.HasReadOnlyModifier || IsReadOnlyAutoGetter;
+        internal override bool IsDeclaredReadOnly
+        {
+            get
+            {
+                return LocalDeclaredReadOnly || _property.HasReadOnlyModifier || isReadOnlyAutoGetter();
 
-        private bool IsReadOnlyAutoGetter => ContainingType.IsStructType() && !_property.IsStatic && _isAutoPropertyAccessor && MethodKind == MethodKind.PropertyGet && !IsBadNetModule;
+                bool isReadOnlyAutoGetter() =>
+                    ContainingType.IsStructType() &&
+                    !_property.IsStatic &&
+                    _isAutoPropertyAccessor &&
+                    MethodKind == MethodKind.PropertyGet &&
+                    !isBadNetModule();
 
-        // We can't emit the synthesized attribute for netmodules, so in this case we consider auto-getters **not** implicitly readonly.
-        private bool IsBadNetModule => DeclaringCompilation.Options.OutputKind == OutputKind.NetModule &&
-            DeclaringCompilation.GetWellKnownType(WellKnownType.System_Runtime_CompilerServices_IsReadOnlyAttribute) is MissingMetadataTypeSymbol;
+                // We can't emit the synthesized attribute for netmodules, so in this case we consider auto-getters **not** implicitly readonly.
+                bool isBadNetModule() => DeclaringCompilation.Options.OutputKind == OutputKind.NetModule &&
+                    DeclaringCompilation.GetWellKnownType(WellKnownType.System_Runtime_CompilerServices_IsReadOnlyAttribute) is MissingMetadataTypeSymbol;
+            }
+        }
 
         private DeclarationModifiers MakeModifiers(AccessorDeclarationSyntax syntax, bool isExplicitInterfaceImplementation,
             bool hasBody, Location location, DiagnosticBag diagnostics, out bool modifierErrors)
diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs
index e0b025e19fe..7809d68aa50 100644
--- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs
+++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs
@@ -1453,8 +1453,18 @@ public struct S
     public int P1 { get; private set; }
 }
 ";
-            var comp = CreateCompilation(csharp, options: TestOptions.DebugModule, targetFramework: TargetFramework.Mscorlib45);
-            comp.VerifyDiagnostics();
+            var moduleMetadata = CreateCompilation(csharp, options: TestOptions.DebugModule, targetFramework: TargetFramework.Mscorlib45).EmitToImageReference();
+            var moduleComp = CreateCompilation("", new[] { moduleMetadata });
+            var moduleGetterAttributes = moduleComp.GetMember<PropertySymbol>("S.P1").GetMethod.GetAttributes();
+            Assert.Equal(1, moduleGetterAttributes.Length);
+            Assert.Equal("CompilerGeneratedAttribute", moduleGetterAttributes[0].AttributeClass.Name);
+
+            var dllMetadata = CreateCompilation(csharp, options: TestOptions.DebugDll, targetFramework: TargetFramework.Mscorlib45).EmitToImageReference();
+            var dllComp = CreateCompilation("", new[] { dllMetadata });
+            var dllGetterAttributes = dllComp.GetMember<PropertySymbol>("S.P1").GetMethod.GetAttributes();
+            Assert.Equal(2, dllGetterAttributes.Length);
+            Assert.Equal("IsReadOnlyAttribute", dllGetterAttributes[0].AttributeClass.Name);
+            Assert.Equal("CompilerGeneratedAttribute", dllGetterAttributes[1].AttributeClass.Name);
         }
 
         [Fact]
@@ -1466,11 +1476,14 @@ public struct S
     public int P1 { readonly get; private set; }
 }
 ";
-            var comp = CreateCompilation(csharp, options: TestOptions.DebugModule, targetFramework: TargetFramework.Mscorlib45);
-            comp.VerifyDiagnostics(
+            var moduleComp = CreateCompilation(csharp, options: TestOptions.DebugModule, targetFramework: TargetFramework.Mscorlib45);
+            moduleComp.VerifyDiagnostics(
                 // (4,30): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsReadOnlyAttribute' is not defined or imported
                 //     public int P1 { readonly get; private set; }
                 Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "get").WithArguments("System.Runtime.CompilerServices.IsReadOnlyAttribute").WithLocation(4, 30));
+
+            var dllComp = CreateCompilation(csharp, options: TestOptions.DebugDll, targetFramework: TargetFramework.Mscorlib45);
+            dllComp.VerifyDiagnostics();
         }
 
         [Fact]
-- 
GitLab