Merge pull request #1650 from srivatsn/fixtests

Unskip tests fixed by #662 and #679. 

src/Diagnostics/FxCop/System.Runtime.Analyzers/Core/Design/DefineAccessorsForAttributeArguments.Fixer.cs

@@ -40,9 +40,13 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
                         return;
 
                     case DefineAccessorsForAttributeArgumentsAnalyzer.MakePublicCase:
-                        context.RegisterCodeFix(new MyCodeAction(SystemRuntimeAnalyzersResources.MakeGetterPublic,
-                                                         async ct => await MakePublic(context.Document, node, ct).ConfigureAwait(false)),
-                                                diagnostic);
+                        var property = generator.GetDeclaration(node, DeclarationKind.Property);
+                        if (property != null)
+                        {
+                            context.RegisterCodeFix(new MyCodeAction(SystemRuntimeAnalyzersResources.MakeGetterPublic,
+                                                             async ct => await MakePublic(context.Document, node, property, ct).ConfigureAwait(false)),
+                                                    diagnostic);
+                        }
                         return;
 
                     case DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterCase:
@@ -96,6 +100,7 @@ private async Task<Document> AddAccessor(Document document, SyntaxNode parameter
                                                           (editor, propertyDeclaration) =>
                                                           {
                                                               editor.SetGetAccessorStatements(propertyDeclaration, null);
+                                                              editor.SetModifiers(propertyDeclaration, editor.Generator.GetModifiers(propertyDeclaration) - DeclarationModifiers.WriteOnly);
                                                           },
                                                           cancellationToken).ConfigureAwait(false);
             }
@@ -103,10 +108,31 @@ private async Task<Document> AddAccessor(Document document, SyntaxNode parameter
             return symbolEditor.GetChangedDocuments().First();
         }
 
-        private async Task<Document> MakePublic(Document document, SyntaxNode getMethod, CancellationToken cancellationToken)
+        private async Task<Document> MakePublic(Document document, SyntaxNode getMethod, SyntaxNode property, CancellationToken cancellationToken)
         {
+            // Clear the accessibility on the getter.
             DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
-            editor.SetAccessibility(getMethod, Accessibility.Public);
+            editor.SetAccessibility(getMethod, Accessibility.NotApplicable);
+
+            // If the containing property is not public, make it so
+            var propertyAccessibility = editor.Generator.GetAccessibility(property);
+            if (propertyAccessibility != Accessibility.Public)
+            {
+                editor.SetAccessibility(property, Accessibility.Public);
+
+                // Having just made the property public, if it has a setter with no accesibility set, then we've just made the setter public. 
+                // Instead restore the setter's original accessibility so that we don't fire a violation with the generated code.
+                var setter = editor.Generator.GetAccessor(property, DeclarationKind.SetAccessor);
+                if (setter != null)
+                {
+                    var setterAccesibility = editor.Generator.GetAccessibility(setter);
+                    if (setterAccesibility == Accessibility.NotApplicable)
+                    {
+                        editor.SetAccessibility(setter, propertyAccessibility);
+                    }
+                }
+            }
+
             return editor.GetChangedDocument();
         }
 

src/Diagnostics/FxCop/System.Runtime.Analyzers/Test/Design/DefineAccessorsForAttributeArgumentsTests.Fixer.cs

@@ -94,7 +94,7 @@ public string Name
 }", allowNewCompilerDiagnostics: true);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void CSharp_CA1019_MakeGetterPublic()
         {
             VerifyCSharpFix(@"
@@ -113,7 +113,7 @@ public InternalGetterTestAttribute(string name)
     public string Name
     {
         internal get { return m_name; }
-        set { m_name = value; }
+        internal set { m_name = value; }
     }
 }", @"
 using System;
@@ -131,12 +131,12 @@ public InternalGetterTestAttribute(string name)
     public string Name
     {
         get { return m_name; }
-        set { m_name = value; }
+        internal set { m_name = value; }
     }
 }");
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void CSharp_CA1019_MakeGetterPublic2()
         {
             VerifyCSharpFix(@"
@@ -173,12 +173,53 @@ public InternalGetterTestAttribute(string name)
     public string Name
     {
         get { return m_name; }
-        set { m_name = value; }
+
+        internal set { m_name = value; }
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        public void CSharp_CA1019_MakeGetterPublic3()
+        {
+            VerifyCSharpFix(@"
+using System;
+
+[AttributeUsage(AttributeTargets.All)]
+public sealed class InternalGetterTestAttribute : Attribute
+{
+    private string m_name;
+
+    public InternalGetterTestAttribute(string name)
+    {
+        m_name = name;
+    }
+
+    internal string Name
+    {
+        get { return m_name; }
+    }
+}", @"
+using System;
+
+[AttributeUsage(AttributeTargets.All)]
+public sealed class InternalGetterTestAttribute : Attribute
+{
+    private string m_name;
+
+    public InternalGetterTestAttribute(string name)
+    {
+        m_name = name;
+    }
+
+    public string Name
+    {
+        get { return m_name; }
     }
 }");
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void CSharp_CA1019_MakeSetterInternal()
         {
             VerifyCSharpFix(@"
@@ -215,6 +256,7 @@ public PublicSetterTestAttribute(string name)
     public string Name
     {
         get { return m_name; }
+
         internal set { m_name = value; }
     }
 }");
@@ -253,7 +295,7 @@ End Property
 End Class", allowNewCompilerDiagnostics: true);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/679"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void VisualBasic_CA1019_AddAccessor2()
         {
             VerifyBasicFix(@"
@@ -267,9 +309,9 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public WriteOnly Property Name() As String
-        Set
+        Friend Set
             m_name = value
         End Set
     End Property
@@ -284,9 +326,9 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public Property Name() As String
-        Set
+        Friend Set
             m_name = value
         End Set
         Get
@@ -295,7 +337,7 @@ End Property
 End Class", allowNewCompilerDiagnostics: true);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void VisualBasic_CA1019_MakeGetterPublic()
         {
             VerifyBasicFix(@"
@@ -309,12 +351,12 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public Property Name() As String
         Friend Get
             Return m_name
         End Get
-        Set
+        Friend Set
             m_name = value
         End Set
     End Property
@@ -329,19 +371,19 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public Property Name() As String
         Get
             Return m_name
         End Get
-        Set
+        Friend Set
             m_name = value
         End Set
     End Property
 End Class", allowNewCompilerDiagnostics: true);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void VisualBasic_CA1019_MakeGetterPublic2()
         {
             VerifyBasicFix(@"
@@ -355,7 +397,7 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Friend Property Name() As String
         Get
             Return m_name
@@ -375,19 +417,59 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public Property Name() As String
         Get
             Return m_name
         End Get
-        Set
+        Friend Set
             m_name = value
         End Set
     End Property
 End Class", allowNewCompilerDiagnostics: true);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/662"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        public void VisualBasic_CA1019_MakeGetterPublic3()
+        {
+            VerifyBasicFix(@"
+Imports System
+
+<AttributeUsage(AttributeTargets.All)> _
+Public NotInheritable Class InternalGetterTestAttribute
+    Inherits Attribute
+    Private m_name As String
+    
+    Public Sub New(name As String)
+        m_name = name
+    End Sub
+
+    Friend Property Name() As String
+        Get
+            Return m_name
+        End Get
+    End Property
+End Class", @"
+Imports System
+
+<AttributeUsage(AttributeTargets.All)> _
+Public NotInheritable Class InternalGetterTestAttribute
+    Inherits Attribute
+    Private m_name As String
+    
+    Public Sub New(name As String)
+        m_name = name
+    End Sub
+
+    Public Property Name() As String
+        Get
+            Return m_name
+        End Get
+    End Property
+End Class", allowNewCompilerDiagnostics: true);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void VisualBasic_CA1019_MakeSetterInternal()
         {
             VerifyBasicFix(@"
@@ -401,8 +483,8 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
-    Property Name() As String
+
+    Public Property Name() As String
         Get
             Return m_name
         End Get
@@ -421,7 +503,7 @@ Inherits Attribute
     Public Sub New(name As String)
         m_name = name
     End Sub
-    
+
     Public Property Name() As String
         Get
             Return m_name