Merge remote-tracking branch 'upstream/master' into addUsingOOP8

build/Targets/Packages.props

@@ -38,7 +38,7 @@
     <MicrosoftDiaSymReaderVersion>1.1.0</MicrosoftDiaSymReaderVersion>
     <MicrosoftDiaSymReaderConverterXmlVersion>1.0.0-beta1-61708-01</MicrosoftDiaSymReaderConverterXmlVersion>
     <MicrosoftDiaSymReaderNativeVersion>1.6.0-beta2-25304</MicrosoftDiaSymReaderNativeVersion>
-    <MicrosoftDiaSymReaderPortablePdbVersion>1.3.0-beta1-61619-01</MicrosoftDiaSymReaderPortablePdbVersion>
+    <MicrosoftDiaSymReaderPortablePdbVersion>1.3.0</MicrosoftDiaSymReaderPortablePdbVersion>
     <MicrosoftDotNetIBCMerge>4.7.2-alpha-00001</MicrosoftDotNetIBCMerge>
     <MicrosoftIdentityModelClientsActiveDirectoryVersion>3.13.8</MicrosoftIdentityModelClientsActiveDirectoryVersion>
     <MicrosoftInternalPerformanceCodeMarkersDesignTimeVersion>15.0.26507-alpha</MicrosoftInternalPerformanceCodeMarkersDesignTimeVersion>

src/EditorFeatures/CSharpTest/ExtractMethod/ExtractMethodTests.cs

@@ -10079,6 +10079,155 @@ private async Task<int> NewMethod(S s)
             await TestExtractMethodAsync(code, expected, dontPutOutOrRefOnStruct: true);
         }
 
+        [Theory]
+        [InlineData("add", "remove")]
+        [InlineData("remove", "add")]
+        [WorkItem(17474, "https://github.com/dotnet/roslyn/issues/17474")]
+        [Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        public async Task TestExtractMethodEventAccessorUnresolvedName(string testedAccessor, string untestedAccessor)
+        {
+            // This code intentionally omits a 'using System;'
+            var code =
+$@"namespace ClassLibrary9
+{{
+    public class Class
+    {{
+        public event EventHandler Event
+        {{
+            {testedAccessor} {{ [|throw new NotImplementedException();|] }}
+            {untestedAccessor} {{ throw new NotImplementedException(); }}
+        }}
+    }}
+}}";
+            var expected =
+$@"namespace ClassLibrary9
+{{
+    public class Class
+    {{
+        public event EventHandler Event
+        {{
+            {testedAccessor} {{ NewMethod(); }}
+            {untestedAccessor} {{ throw new NotImplementedException(); }}
+        }}
+
+        private static void NewMethod()
+        {{
+            throw new NotImplementedException();
+        }}
+    }}
+}}";
+
+            await TestExtractMethodAsync(code, expected);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        [WorkItem(19958, "https://github.com/dotnet/roslyn/issues/19958")]
+        public async Task TestExtractMethodRefPassThrough()
+        {
+            var code =
+@"using System;
+
+namespace ClassLibrary9
+{
+    internal class ClassExtensions
+    {
+        public static int OtherMethod(ref int x) => x;
+
+        public static void Method(ref int x)
+            => Console.WriteLine(OtherMethod(ref [|x|]));
+    }
+}";
+            var expected =
+@"using System;
+
+namespace ClassLibrary9
+{
+    internal class ClassExtensions
+    {
+        public static int OtherMethod(ref int x) => x;
+
+        public static void Method(ref int x)
+            => Console.WriteLine(OtherMethod(ref $x$));
+
+        public static ref int NewMethod(ref int x)
+        {
+            return ref x;
+        }
+    }
+}";
+
+            await TestExtractMethodAsync(code, expected, temporaryFailing: true);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
+        [WorkItem(19958, "https://github.com/dotnet/roslyn/issues/19958")]
+        public async Task TestExtractMethodRefPassThroughDuplicateVariable()
+        {
+            var code =
+@"using System;
+
+namespace ClassLibrary9
+{
+    internal interface IClass
+    {
+        bool InterfaceMethod(ref Guid x, out IOtherClass y);
+    }
+    internal interface IOtherClass
+    {
+        bool OtherInterfaceMethod();
+    }
+
+    internal static class ClassExtensions
+    {
+        public static void Method(this IClass instance, Guid guid)
+        {
+            var r = instance.InterfaceMethod(ref [|guid|], out IOtherClass guid);
+            if (!r)
+                return;
+
+            r = guid.OtherInterfaceMethod();
+            if (r)
+                throw null;
+        }
+    }
+}";
+            var expected =
+@"using System;
+
+namespace ClassLibrary9
+{
+    internal interface IClass
+    {
+        bool InterfaceMethod(ref Guid x, out IOtherClass y);
+    }
+    internal interface IOtherClass
+    {
+        bool OtherInterfaceMethod();
+    }
+
+    internal static class ClassExtensions
+    {
+        public static void Method(this IClass instance, Guid guid)
+        {
+            var r = instance.InterfaceMethod(ref NewMethod(ref guid), out IOtherClass guid);
+            if (!r)
+                return;
+
+            r = guid.OtherInterfaceMethod();
+            if (r)
+                throw null;
+        }
+
+        public static ref Guid NewMethod(ref Guid guid)
+        {
+            return ref guid;
+        }
+    }
+}";
+
+            await TestExtractMethodAsync(code, expected, temporaryFailing: true);
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.ExtractMethod)]
         public async Task ExtractMethod_Argument1()
         {

src/EditorFeatures/CSharpTest/QualifyMemberAccess/QualifyMemberAccessTests.cs

@@ -1028,5 +1028,73 @@ void Error()
 }",
 CodeStyleOptions.QualifyMethodAccess);
         }
+
+        [WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)]
+        public async Task DoNotReportToQualify_IfBaseQualificationOnField()
+        {
+            await TestMissingAsyncWithOption(
+@"class Base
+{
+    protected int field;
+}
+class Derived : Base
+{
+    void M() { [|base.field|] = 0; }
+}",
+CodeStyleOptions.QualifyFieldAccess);
+        }
+
+        [WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)]
+        public async Task DoNotReportToQualify_IfBaseQualificationOnProperty()
+        {
+            await TestMissingAsyncWithOption(
+@"class Base
+{
+    protected virtual int Property { get; }
+}
+class Derived : Base
+{
+    protected override int Property { get { return [|base.Property|]; } }
+}",
+CodeStyleOptions.QualifyPropertyAccess);
+        }
+
+        [WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)]
+        public async Task DoNotReportToQualify_IfBaseQualificationOnMethod()
+        {
+            await TestMissingAsyncWithOption(
+@"class Base
+{
+    protected virtual void M() { }
+}
+class Derived : Base
+{
+    protected override void M() { [|base.M()|]; }
+}",
+CodeStyleOptions.QualifyMethodAccess);
+        }
+
+        [WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)]
+        public async Task DoNotReportToQualify_IfBaseQualificationOnEvent()
+        {
+            await TestMissingAsyncWithOption(
+@"class Base
+{
+    protected virtual event EventHandler Event;
+}
+class Derived : Base
+{
+    protected override event EventHandler Event 
+    {
+        add { [|base.Event|] += value; }
+        remove { }
+    }
+}",
+CodeStyleOptions.QualifyEventAccess);
+        }
     }
 }
\ No newline at end of file

src/EditorFeatures/VisualBasicTest/QualifyMemberAccess/QualifyMemberAccessTests.vb

@@ -464,5 +464,102 @@ CodeStyleOptions.QualifyPropertyAccess, NotificationOption.Warning)
 CodeStyleOptions.QualifyPropertyAccess, NotificationOption.Error)
         End Function
 
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyBaseQualificationOnField() As Task
+            Await TestMissingAsyncWithOption("
+Class Base
+    Protected Field As Integer
+End Class
+Class Derived
+    Inherits Base
+    Sub M()
+        [|MyBase.Field|] = 0
+    End Sub
+End Class
+",
+CodeStyleOptions.QualifyFieldAccess)
+        End Function
+
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyClassQualificationOnField() As Task
+            Await TestMissingAsyncWithOption("
+Class C
+    Private ReadOnly Field As Integer
+    Sub M()
+        [|MyClass.Field|] = 0
+    End Sub
+End Class
+",
+CodeStyleOptions.QualifyFieldAccess)
+        End Function
+
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyBaseQualificationOnProperty() As Task
+            Await TestMissingAsyncWithOption("
+Class Base
+    Protected Overridable ReadOnly Property P As Integer
+End Class
+Class Derived
+    Inherits Base
+    Protected Overrides ReadOnly Property P As Integer
+        Get
+            Return [|MyBase.P|]
+        End Get
+End Class
+",
+CodeStyleOptions.QualifyPropertyAccess)
+        End Function
+
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyClassQualificationOnProperty() As Task
+            Await TestMissingAsyncWithOption("
+Class C
+    Dim i As Integer
+    Protected Overridable ReadOnly Property P As Integer
+    Sub M()
+        Me.i = [|MyClass.P|]
+    End Sub
+End Class
+",
+CodeStyleOptions.QualifyPropertyAccess)
+        End Function
+
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyBaseQualificationOnMethod() As Task
+            Await TestMissingAsyncWithOption("
+Class Base
+    Protected Overridable Sub M
+    End Sub
+End Class
+Class Derived
+    Inherits Base
+    Protected Overrides Sub M()
+        Get
+            Return [|MyBase.M|]()
+        End Get
+End Class
+",
+CodeStyleOptions.QualifyMethodAccess)
+        End Function
+
+        <WorkItem(17711, "https://github.com/dotnet/roslyn/issues/17711")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsQualifyMemberAccess)>
+        Public Async Function DoNotReportToQualify_IfMyClassQualificationOnMethod() As Task
+            Await TestMissingAsyncWithOption("
+Class C
+    Protected Overridable Sub M()
+    End Sub
+    Sub M2()
+        [|MyClass.M|]()
+    End Sub
+End Class
+",
+CodeStyleOptions.QualifyMethodAccess)
+        End Function
     End Class
 End Namespace
\ No newline at end of file

src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs

@@ -71,13 +71,23 @@ public override ITypeSymbol GetContainingScopeType()
                 switch (node)
                 {
                     case AccessorDeclarationSyntax access:
-                        // property case
+                        // property or event case
                         if (access.Parent == null || access.Parent.Parent == null)
                         {
                             return null;
                         }
 
-                        return ((IPropertySymbol)semanticModel.GetDeclaredSymbol(access.Parent.Parent)).Type;
+                        switch (semanticModel.GetDeclaredSymbol(access.Parent.Parent))
+                        {
+                            case IPropertySymbol propertySymbol:
+                                return propertySymbol.Type;
+
+                            case IEventSymbol eventSymbol:
+                                return eventSymbol.Type;
+
+                            default:
+                                return null;
+                        }
 
                     case MethodDeclarationSyntax method:
                         return semanticModel.GetDeclaredSymbol(method).ReturnType;

src/Features/CSharp/Portable/QualifyMemberAccess/CSharpQualifyMemberAccessDiagnosticAnalyzer.cs

@@ -13,5 +13,9 @@ protected override string GetLanguageName()
 
         protected override bool IsAlreadyQualifiedMemberAccess(SyntaxNode node)
             => node.IsKind(SyntaxKind.ThisExpression);
+
+        // If the member is already qualified with `base.`, it cannot be further qualified.
+        protected override bool CanMemberAccessBeQualified(SyntaxNode node)
+            => !node.IsKind(SyntaxKind.BaseExpression);
     }
 }
\ No newline at end of file

src/Features/Core/Portable/ExtractMethod/ExtractMethodMatrix.cs

@@ -73,6 +73,13 @@ static ExtractMethodMatrix()
                 {
                     key = new Key(dataFlowIn, /*dataFlowOut*/ true, alwaysAssigned, variableDeclared, readInside, writtenInside, readOutside, writtenOutside);
                 }
+
+                // interesting case in invalid code (bug #19136)
+                // variable is passed by reference, and another argument is an out variable with the same name
+                if (dataFlowIn && variableDeclared)
+                {
+                    key = new Key(/*dataFlowIn:*/ false, dataFlowOut, alwaysAssigned, variableDeclared, readInside, writtenInside, readOutside, writtenOutside);
+                }
             }
 
             Contract.ThrowIfFalse(s_matrix.ContainsKey(key));

src/Features/Core/Portable/QualifyMemberAccess/AbstractQualifyMemberAccessDiagnosticAnalyzer.cs

@@ -37,6 +37,14 @@ public override bool OpenFileOnly(Workspace workspace)
 
         protected abstract string GetLanguageName();
 
+        /// <summary>
+        /// Reports on whether the specified member is suitable for qualification. Some member
+        /// access expressions cannot be qualified; for instance if they begin with <c>base.</c>,
+        /// <c>MyBase.</c>, or <c>MyClass.</c>.
+        /// </summary>
+        /// <returns>True if the member access can be qualified; otherwise, False.</returns>
+        protected abstract bool CanMemberAccessBeQualified(SyntaxNode node);
+
         protected abstract bool IsAlreadyQualifiedMemberAccess(SyntaxNode node);
 
         private static MethodInfo s_registerMethod = typeof(AnalysisContext).GetTypeInfo().GetDeclaredMethod("RegisterOperationActionImmutableArrayInternal");
@@ -66,6 +74,12 @@ private void AnalyzeOperation(OperationAnalysisContext context)
                 return;
             }
 
+            // If we can't be qualified (e.g., because we're already qualified with `base.`), we're done.
+            if (!CanMemberAccessBeQualified(memberReference.Instance.Syntax))
+            {
+                return;
+            }
+
             // if we can't find a member then we can't do anything.  Also, we shouldn't qualify
             // accesses to static members.  
             if (memberReference.Member == null ||

src/Features/VisualBasic/Portable/Diagnostics/Analyzers/VisualBasicQualifyMemberAccessDiagnosticAnalyzer.vb

@@ -15,5 +15,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.QualifyMemberAccess
         Protected Overrides Function IsAlreadyQualifiedMemberAccess(node As SyntaxNode) As Boolean
             Return node.IsKind(SyntaxKind.MeExpression)
         End Function
+
+        Protected Overrides Function CanMemberAccessBeQualified(node As SyntaxNode) As Boolean
+            ' If the member is already qualified with `MyBase.`, or `MyClass.`, it cannot be further qualified.
+            Return Not (node.IsKind(SyntaxKind.MyBaseExpression) OrElse node.IsKind(SyntaxKind.MyClassExpression))
+        End Function
     End Class
 End Namespace
\ No newline at end of file