Enable renaming inaccessible symbols.

src/EditorFeatures/Core/Implementation/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -17,13 +17,15 @@ private class InlineRenameLocationSet : IInlineRenameLocationSet
             private readonly RenameLocations _renameLocationSet;
             private readonly SymbolInlineRenameInfo _renameInfo;
 
-            public IList<InlineRenameLocation> Locations { get; }
+            public ImmutableArray<InlineRenameLocation> Locations { get; }
 
             public InlineRenameLocationSet(SymbolInlineRenameInfo renameInfo, RenameLocations renameLocationSet)
             {
                 _renameInfo = renameInfo;
                 _renameLocationSet = renameLocationSet;
-                this.Locations = renameLocationSet.Locations.Where(l => !l.IsCandidateLocation || l.IsMethodGroupReference).Select(ConvertLocation).ToList();
+                this.Locations = renameLocationSet.Locations.Where(RenameLocation.ShouldRename)
+                                                            .Select(ConvertLocation)
+                                                            .ToImmutableArray();
             }
 
             private InlineRenameLocation ConvertLocation(RenameLocation location)
@@ -42,4 +44,4 @@ public async Task<IInlineRenameReplacementInfo> GetReplacementsAsync(string repl
             }
         }
     }
-}
+}
\ No newline at end of file

src/EditorFeatures/Core/Implementation/InlineRename/Dashboard/DashboardViewModel.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.ComponentModel;
 using System.Linq;
 using System.Runtime.CompilerServices;
@@ -49,10 +50,10 @@ public DashboardViewModel(InlineRenameSession session)
 
         public event PropertyChangedEventHandler PropertyChanged;
 
-        private void OnReferenceLocationsChanged(object sender, IList<InlineRenameLocation> renameLocations)
+        private void OnReferenceLocationsChanged(object sender, ImmutableArray<InlineRenameLocation> renameLocations)
         {
             int totalFilesCount = renameLocations.GroupBy(s => s.Document).Count();
-            int totalSpansCount = renameLocations.Count;
+            int totalSpansCount = renameLocations.Length;
 
             UpdateSearchText(totalSpansCount, totalFilesCount);
         }

src/EditorFeatures/Core/Implementation/InlineRename/IEditorInlineRenameService.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -110,7 +111,7 @@ internal interface IInlineRenameLocationSet
         /// has entered in the inline rename session.  These are the locations are all relative
         /// to the solution when the inline rename session began.
         /// </summary>
-        IList<InlineRenameLocation> Locations { get; }
+        ImmutableArray<InlineRenameLocation> Locations { get; }
 
         /// <summary>
         /// Returns the set of replacements and their possible resolutions if the user enters the

src/EditorFeatures/Core/Implementation/InlineRename/InlineRenameSession.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
 using System.Threading;
@@ -265,7 +266,7 @@ private void UpdateReferenceLocationsTask(Task<IInlineRenameLocationSet> allRena
 
         public IInlineRenameUndoManager UndoManager { get; }
 
-        public event EventHandler<IList<InlineRenameLocation>> ReferenceLocationsChanged;
+        public event EventHandler<ImmutableArray<InlineRenameLocation>> ReferenceLocationsChanged;
         public event EventHandler<IInlineRenameReplacementInfo> ReplacementsComputed;
         public event EventHandler ReplacementTextChanged;
 
@@ -341,14 +342,14 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs args)
             }
         }
 
-        private void RaiseSessionSpansUpdated(IList<InlineRenameLocation> locations)
+        private void RaiseSessionSpansUpdated(ImmutableArray<InlineRenameLocation> locations)
         {
             AssertIsForeground();
             SetReferenceLocations(locations);
             ReferenceLocationsChanged?.Invoke(this, locations);
         }
 
-        private void SetReferenceLocations(IEnumerable<InlineRenameLocation> locations)
+        private void SetReferenceLocations(ImmutableArray<InlineRenameLocation> locations)
         {
             AssertIsForeground();
 

src/EditorFeatures/Test2/Rename/RenameEngineTests.vb

@@ -20,8 +20,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
         End Sub
 
         <WorkItem(543661, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/543661")>
-        <Fact>
-        <Trait(Traits.Feature, Traits.Features.Rename)>
+        <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
         Public Sub CannotRenameNamespaceAlias()
             Using result = RenameEngineResult.Create(_outputHelper,
                    <Workspace>
@@ -41,6 +40,54 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
             End Using
         End Sub
 
+        <WorkItem(4904, "https://github.com/dotnet/roslyn/issues/4904")>
+        <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub RenameInaccessibleMember1()
+            Using result = RenameEngineResult.Create(_outputHelper,
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class C
+{
+    void Main()
+    {
+        D.[|field|] = 8;
+    }
+}
+ 
+class D
+{
+    private static int [|$$field|];
+}                             </Document>
+                       </Project>
+                   </Workspace>, renameTo:="_field")
+            End Using
+        End Sub
+
+        <WorkItem(4904, "https://github.com/dotnet/roslyn/issues/4904")>
+        <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub RenameInaccessibleMember2()
+            Using result = RenameEngineResult.Create(_outputHelper,
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class C
+{
+    void Main()
+    {
+        D.[|$$field|] = 8;
+    }
+}
+ 
+class D
+{
+    private static int [|field|];
+}                             </Document>
+                       </Project>
+                   </Workspace>, renameTo:="_field")
+            End Using
+        End Sub
+
         <WorkItem(813409, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/813409")>
         <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
         Public Sub RenameTypeDoesNotRenameGeneratedConstructorCalls()

src/VisualStudio/Xaml/Impl/Features/InlineRename/XamlEditorInlineRenameService.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Composition;
 using System.Linq;
 using System.Threading;
@@ -66,14 +67,16 @@ public async Task<IInlineRenameLocationSet> FindRenameLocationsAsync(OptionSet o
             {
                 var references = new List<InlineRenameLocation>();
 
-                IList<DocumentSpan> renameLocations = await _renameInfo.FindRenameLocationsAsync(
+                var renameLocations = await _renameInfo.FindRenameLocationsAsync(
                     renameInStrings: optionSet.GetOption(RenameOptions.RenameInStrings),
                     renameInComments: optionSet.GetOption(RenameOptions.RenameInComments),
                     cancellationToken: cancellationToken).ConfigureAwait(false);
 
-                references.AddRange(renameLocations.Select(ds => new InlineRenameLocation(ds.Document, ds.TextSpan)));
+                references.AddRange(renameLocations.Select(
+                    ds => new InlineRenameLocation(ds.Document, ds.TextSpan)));
 
-                return new InlineRenameLocationSet(_renameInfo, _document.Project.Solution, references);
+                return new InlineRenameLocationSet(_renameInfo, _document.Project.Solution,
+                    references.ToImmutableArray());
             }
 
             public TextSpan? GetConflictEditSpan(InlineRenameLocation location, string replacementText, CancellationToken cancellationToken)
@@ -129,14 +132,14 @@ private class InlineRenameLocationSet : IInlineRenameLocationSet
                 private readonly IXamlRenameInfo _renameInfo;
                 private readonly Solution _oldSolution;
 
-                public InlineRenameLocationSet(IXamlRenameInfo renameInfo, Solution solution, IList<InlineRenameLocation> locations)
+                public InlineRenameLocationSet(IXamlRenameInfo renameInfo, Solution solution, ImmutableArray<InlineRenameLocation> locations)
                 {
                     _renameInfo = renameInfo;
                     _oldSolution = solution;
                     Locations = locations;
                 }
 
-                public IList<InlineRenameLocation> Locations { get; }
+                public ImmutableArray<InlineRenameLocation> Locations { get; }
 
                 public bool IsReplacementTextValid(string replacementText)
                 {

src/Workspaces/Core/Portable/FindSymbols/ReferenceLocation.cs

@@ -67,13 +67,7 @@ internal ReferenceLocation(Document document, IAliasSymbol alias, Location locat
         /// "Foo()" could show up as an error tolerance location to a method "Foo(int i)" if no
         /// actual "Foo()" method existed.
         /// </summary>
-        public bool IsCandidateLocation
-        {
-            get
-            {
-                return this.CandidateReason != CandidateReason.None;
-            }
-        }
+        public bool IsCandidateLocation => this.CandidateReason != CandidateReason.None;
 
         public static bool operator ==(ReferenceLocation left, ReferenceLocation right)
         {

src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs

@@ -715,7 +715,7 @@ private async Task AddDocumentsWithPotentialConflicts(IEnumerable<Document> docu
                 Solution originalSolution,
                 Solution partiallyRenamedSolution,
                 HashSet<DocumentId> documentIdsToRename,
-                IEnumerable<RenameLocation> renameLocations,
+                ISet<RenameLocation> renameLocations,
                 RenamedSpansTracker renameSpansTracker,
                 bool replacementTextValid)
             {
@@ -725,15 +725,13 @@ private async Task AddDocumentsWithPotentialConflicts(IEnumerable<Document> docu
                     {
                         _cancellationToken.ThrowIfCancellationRequested();
 
-                        // We try to rewrite all locations that are not candidate locations. If there is only one location 
-                        // it must be the correct one (the symbol is ambiguous to something else) and we always try to rewrite it.
                         var document = originalSolution.GetDocument(documentId);
                         var semanticModel = await document.GetSemanticModelAsync(_cancellationToken).ConfigureAwait(false);
                         var originalSyntaxRoot = await semanticModel.SyntaxTree.GetRootAsync(_cancellationToken).ConfigureAwait(false);
 
                         // Get all rename locations for the current document.
                         var allTextSpansInSingleSourceTree = renameLocations
-                            .Where(l => l.DocumentId == documentId && !l.IsRenameInStringOrComment && (renameLocations.Count() == 1 || !l.IsCandidateLocation || l.IsMethodGroupReference))
+                            .Where(l => l.DocumentId == documentId && ShouldIncludeLocation(renameLocations, l))
                             .ToDictionary(l => l.Location.SourceSpan);
 
                         // All textspan in the document documentId, that requires rename in String or Comment
@@ -790,6 +788,25 @@ private async Task AddDocumentsWithPotentialConflicts(IEnumerable<Document> docu
                     throw ExceptionUtilities.Unreachable;
                 }
             }
+
+            /// We try to rewrite all locations that are invalid candidate locations. If there is only
+            /// one location it must be the correct one (the symbol is ambiguous to something else)
+            /// and we always try to rewrite it.  If there are multiple locations, we only allow it
+            /// if the candidate reason allows for it).
+            private bool ShouldIncludeLocation(ISet<RenameLocation> renameLocations, RenameLocation location)
+            {
+                if (location.IsRenameInStringOrComment)
+                {
+                    return false;
+                }
+
+                if (renameLocations.Count == 1)
+                {
+                    return true;
+                }
+
+                return RenameLocation.ShouldRename(location);
+            }
         }
     }
-}
+}
\ No newline at end of file

src/Workspaces/Core/Portable/Rename/RenameLocation.ReferenceProcessing.cs

@@ -298,7 +298,10 @@ private static string TrimNameToAfterLastDot(string name)
                 {
                     if (location.IsInSource)
                     {
-                        results.Add(new RenameLocation(location, solution.GetDocument(location.SourceTree).Id, isRenamableAccessor: isRenamableAccessor));
+                        results.Add(new RenameLocation(
+                            location, 
+                            solution.GetDocument(location.SourceTree).Id, 
+                            isRenamableAccessor: isRenamableAccessor));
                     }
                 }
 
@@ -375,7 +378,7 @@ internal static async Task<IEnumerable<RenameLocation>> GetRenamableReferenceLoc
                         if (location.Alias.Name == referencedSymbol.Name)
                         {
                             results.Add(new RenameLocation(location.Location, location.Document.Id,
-                                isCandidateLocation: location.IsCandidateLocation, isRenamableAliasUsage: true, isWrittenTo: location.IsWrittenTo));
+                                candidateReason: location.CandidateReason, isRenamableAliasUsage: true, isWrittenTo: location.IsWrittenTo));
 
                             // We also need to add the location of the alias itself
                             var aliasLocation = location.Alias.Locations.Single();
@@ -389,8 +392,7 @@ internal static async Task<IEnumerable<RenameLocation>> GetRenamableReferenceLoc
                             location.Location,
                             location.Document.Id,
                             isWrittenTo: location.IsWrittenTo,
-                            isCandidateLocation: location.IsCandidateLocation,
-                            isMethodGroupReference: location.IsCandidateLocation && location.CandidateReason == CandidateReason.MemberGroup,
+                            candidateReason: location.CandidateReason,
                             isRenamableAccessor: await IsPropertyAccessorOrAnOverride(referencedSymbol, solution, cancellationToken).ConfigureAwait(false)));
                     }
                 }

src/Workspaces/Core/Portable/Rename/RenameLocation.cs

@@ -10,7 +10,7 @@ internal struct RenameLocation : IEquatable<RenameLocation>
     {
         public readonly Location Location;
         public readonly DocumentId DocumentId;
-        public readonly bool IsCandidateLocation;
+        public readonly CandidateReason CandidateReason;
         public readonly bool IsRenamableAliasUsage;
         public readonly bool IsRenamableAccessor;
         public readonly TextSpan ContainingLocationForStringOrComment;
@@ -18,13 +18,10 @@ internal struct RenameLocation : IEquatable<RenameLocation>
 
         public bool IsRenameInStringOrComment { get { return ContainingLocationForStringOrComment != default(TextSpan); } }
 
-        public bool IsMethodGroupReference { get; }
-
         public RenameLocation(
             Location location,
             DocumentId documentId,
-            bool isCandidateLocation = false,
-            bool isMethodGroupReference = false,
+            CandidateReason candidateReason = CandidateReason.None,
             bool isRenamableAliasUsage = false,
             bool isRenamableAccessor = false,
             bool isWrittenTo = false,
@@ -32,8 +29,7 @@ internal struct RenameLocation : IEquatable<RenameLocation>
         {
             Location = location;
             DocumentId = documentId;
-            IsCandidateLocation = isCandidateLocation;
-            IsMethodGroupReference = isMethodGroupReference;
+            CandidateReason = candidateReason;
             IsRenamableAliasUsage = isRenamableAliasUsage;
             IsRenamableAccessor = isRenamableAccessor;
             IsWrittenTo = isWrittenTo;
@@ -42,8 +38,7 @@ internal struct RenameLocation : IEquatable<RenameLocation>
 
         public RenameLocation(ReferenceLocation referenceLocation, DocumentId documentId)
             : this(referenceLocation.Location, documentId,
-                   isCandidateLocation: referenceLocation.IsCandidateLocation && referenceLocation.CandidateReason != CandidateReason.LateBound,
-                   isMethodGroupReference: referenceLocation.IsCandidateLocation && referenceLocation.CandidateReason == CandidateReason.MemberGroup,
+                   candidateReason: referenceLocation.CandidateReason,
                    isWrittenTo: referenceLocation.IsWrittenTo)
         {
         }
@@ -69,5 +64,118 @@ public override int GetHashCode()
         {
             return Location.GetHashCode();
         }
+
+        internal static bool ShouldRename(RenameLocation location)
+            => ShouldRename(location.CandidateReason);
+
+        internal static bool ShouldRename(CandidateReason candidateReason)
+        {
+            if (candidateReason != CandidateReason.None)
+            {
+                // When we have a CandidateReason that means (for most reasons) the compiler 
+                // encountered some sort of issue when binding the node.  This means we're 
+                // less certain about what the code meant and if the node bound to the actual
+                // symbol that we're trying to rename.  However, for many of these reasons,
+                // even if the code is in error, we can still be confident enough that the 
+                // node bound to the symbol we care about.
+
+                switch (candidateReason)
+                {
+                    case CandidateReason.NotATypeOrNamespace:
+                        // We had a reference to the symbol in a location where we needed a 
+                        // type or namespace.  This is usually a wildly broken situation.  i.e.
+                        // now due to hiding, something like a field/property is being referenced
+                        // in a type location.  It is highly likely that this should not be
+                        // renamed.
+                        return false;
+
+                    case CandidateReason.NotAnEvent:
+                    case CandidateReason.NotAWithEventsMember:
+                        // it's unlikely that someone would be referencing a non-event in an 
+                        // event context.  Likely this location should be included.
+                        return false;
+
+                    case CandidateReason.NotAnAttributeType:
+                        // It's feasible that someon was referencing some type in an attribute
+                        // location before making that type itself descend from System.Attribute.
+                        // Still allow this type to be renamed.
+                        return true;
+
+                    case CandidateReason.WrongArity:
+                        // Someone may have provided the wrong number of type arguments to
+                        // a type/method when calling it. We should still allow the reference
+                        // to be updated.
+                        return true;
+
+                    case CandidateReason.NotCreatable:
+                        // Can happen when someone tries to do something like 'new' an 
+                        // abstract type.  We still want to allow renaming this location.
+                        return true;
+
+                    case CandidateReason.NotReferencable:
+                        // Happens when the user does something like directly accessing
+                        // the accessor of a normal property.  In this case, we do still
+                        // want to allow the rename to happen.
+                        return true;
+
+                    case CandidateReason.Inaccessible:
+                        // Can trivially occur in code that is in an initially broken state
+                        // where inaccessible members are being referenced.  We still want
+                        // to update these references.
+                        return true;
+
+                    case CandidateReason.NotAValue:
+                    case CandidateReason.NotAVariable:
+                        // Happens with code like "NS = 1".  If "NS" is binding now to a 
+                        // namespace, then it's likely something has gone very wrong (similar to
+                        // NotATypeOrNamespace), and we shouldn't update this reference.
+                        return false;
+
+                    case CandidateReason.NotInvocable:
+                        // Happens when something like a variable is being invoked, but the variable
+                        // isn't a delegate type.  This may be because the user intends to give 
+                        // this value a delegate type, but hasn't done so yet.  We should still allow
+                        // renaming this reference for now.
+                        return false;
+
+                    case CandidateReason.StaticInstanceMismatch:
+                        // Similar to 'Inaccessible', the code is currently broken, but it's fairly
+                        // clear what the user's intent was.  In this case, we want to update the
+                        // references, even though the code isn't valid.
+                        return true;
+
+                    case CandidateReason.OverloadResolutionFailure:
+                        // Here we were renaming a method, and have a reference location that didn't
+                        // bind properly to any methods.  This case is simply hard to reason about.
+                        // There are times where we might want to rename this, and times when we 
+                        // don't.  As overloading methods is very common, we won't update this location
+                        // as it might update code the user really doesn't want us to be touching.
+                        return false;
+
+                    case CandidateReason.LateBound:
+                        // This is a late bound call, so we should not update this location. 
+                        return false;
+
+                    case CandidateReason.Ambiguous:
+                        // we shoudl not touch amgiguous code.  We have no way to feel confident that
+                        // this really is a location that we should be updating.
+                        return false;
+
+                    case CandidateReason.MemberGroup:
+                        // MemberGroup is not an error case.  It happens in completely legal code,
+                        // like nameof(int.ToString).  Because of that, we do want to update the
+                        // reference here.
+                        return true;
+
+                    default:
+                        // For cases added in the future, conservatively presume we can't update them.
+                        // If we need to we can just add a case above this.
+                        return false;
+                }
+            }
+
+            // If there is no candidate reason, we can rename this reference.
+            return true;
+        }
     }
-}
+}
\ No newline at end of file

src/Workspaces/Core/Portable/Rename/RenameLocations.cs

@@ -100,7 +100,7 @@ private class SearchResult
             {
                 mergedLocations.AddRange(renameMethodGroupReferences
                     ? result.Locations
-                    : result.Locations.Where(x => !x.IsMethodGroupReference));
+                    : result.Locations.Where(x => x.CandidateReason != CandidateReason.MemberGroup));
 
                 mergedImplicitLocations.AddRange(result.ImplicitLocations);
                 mergedReferencedSymbols.AddRange(result.ReferencedSymbols);
@@ -109,7 +109,7 @@ private class SearchResult
             _mergedResult = new SearchResult(mergedLocations, mergedImplicitLocations, mergedReferencedSymbols);
         }
 
-        public ISet<RenameLocation> Locations { get { return _mergedResult.Locations; } }
+        public ISet<RenameLocation> Locations => _mergedResult.Locations;
         public SymbolAndProjectId SymbolAndProjectId => _symbolAndProjectId;
         public ISymbol Symbol => _symbolAndProjectId.Symbol;
         public Solution Solution { get { return _solution; } }

src/Workspaces/Core/Portable/Rename/RenameUtilities.cs

@@ -136,6 +136,15 @@ private static bool ShouldRenameOnlyAffectDeclaringProject(ISymbol symbol)
                 return TokenRenameInfo.CreateMemberGroupTokenInfo(symbolInfo.CandidateSymbols);
             }
 
+            if (RenameLocation.ShouldRename(symbolInfo.CandidateReason) &&
+                symbolInfo.CandidateSymbols.Length == 1)
+            {
+                // TODO(cyrusn): We're allowing rename here, but we likely should let the user
+                // know that there is an error in the code and that rename results might be
+                // inaccurate.
+                return TokenRenameInfo.CreateSingleSymbolTokenInfo(symbolInfo.CandidateSymbols[0]);
+            }
+
             return TokenRenameInfo.NoSymbolsTokenInfo;
         }
     }