From 8d70d17dee4b4923c0f3339da605edcbbde4ab22 Mon Sep 17 00:00:00 2001 From: Carol Hu Date: Tue, 31 Jul 2018 16:09:17 -0700 Subject: [PATCH] add check for symbol.IsOverride when renaming a symbol (#28952) Add check for symbol.IsOverride when renaming a symbol --- .../Test2/Rename/CSharp/OverrideTests.vb | 141 ++++++++++++++++++ .../Core/Portable/Rename/RenameUtilities.cs | 26 +++- 2 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 src/EditorFeatures/Test2/Rename/CSharp/OverrideTests.vb diff --git a/src/EditorFeatures/Test2/Rename/CSharp/OverrideTests.vb b/src/EditorFeatures/Test2/Rename/CSharp/OverrideTests.vb new file mode 100644 index 00000000000..b0ea3e5966f --- /dev/null +++ b/src/EditorFeatures/Test2/Rename/CSharp/OverrideTests.vb @@ -0,0 +1,141 @@ +' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename.CSharp + <[UseExportProvider]> + Public Class OverrideTests + Private ReadOnly _outputHelper As Abstractions.ITestOutputHelper + + Public Sub New(outputHelper As Abstractions.ITestOutputHelper) + _outputHelper = outputHelper + End Sub + + + Public Sub RenameOverrideMemberFromDerivedClass() + Using result = RenameEngineResult.Create(_outputHelper, + + + +namespace ClassLibrary1 +{ + public class Class1 + { + public virtual void [|M|]() { } + } +} + + + + ClassLibrary1 + +namespace ClassLibrary2 +{ + public class Class2 : ClassLibrary1.Class1 + { + public override void [|$$M|]() { } + } +} + + + , renameTo:="A") + + End Using + End Sub + + + + Public Sub RenameOverrideMemberFromDerivedClassWhenMemberIsPrivate() + Using result = RenameEngineResult.Create(_outputHelper, + + + +namespace ClassLibrary1 +{ + public class Class1 + { + public virtual void [|M|]() { } + } +} + + + + ClassLibrary1 + +namespace ClassLibrary2 +{ + public class Class2 : ClassLibrary1.Class1 + { + override void [|$$M|]() { } + } +} + + + , renameTo:="A") + + End Using + End Sub + + + Public Sub RenameOverrideMemberFromDerivedClass_abstract_virtual() + Using result = RenameEngineResult.Create(_outputHelper, + + + +namespace ClassLibrary1 +{ + public abstract class Class1 + { + public abstract void M(); + } +} + + + + ClassLibrary1 + +namespace ClassLibrary2 +{ + public class Class2 : ClassLibrary1.Class1 + { + virtual void [|$$M|]() { } + } +} + + + , renameTo:="A") + + End Using + End Sub + + + Public Sub RenameOverrideMemberFromDerivedClass_abstract_override() + Using result = RenameEngineResult.Create(_outputHelper, + + + +namespace ClassLibrary1 +{ + public abstract class Class1 + { + public virtual void [|M|](); + } +} + + + + ClassLibrary1 + +namespace ClassLibrary2 +{ + public class Class2 : ClassLibrary1.Class1 + { + override void [|$$M|]() { } + } +} + + + , renameTo:="A") + + End Using + End Sub + End Class +End Namespace diff --git a/src/Workspaces/Core/Portable/Rename/RenameUtilities.cs b/src/Workspaces/Core/Portable/Rename/RenameUtilities.cs index 1b708ac1727..7f570991946 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameUtilities.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameUtilities.cs @@ -84,6 +84,7 @@ internal static IEnumerable GetDocumentsAffectedByRename(ISymbol symbo .Concat(documentsOfRenameSymbolDeclaration.First().Id) .Select(d => d.ProjectId).Distinct(); + // perf optimization: only look in declaring project when possible if (ShouldRenameOnlyAffectDeclaringProject(symbol)) { var isSubset = renameLocations.Select(l => l.DocumentId.ProjectId).Distinct().Except(projectIdsOfRenameSymbolDeclaration).IsEmpty(); @@ -92,7 +93,7 @@ internal static IEnumerable GetDocumentsAffectedByRename(ISymbol symbo } else { - // We are trying to figure out the projects that directly depend on the project that contains the declaration for + // We are trying to figure out the projects that directly depend on the project that contains the declaration for // the rename symbol. Other projects should not be affected by the rename. var relevantProjects = projectIdsOfRenameSymbolDeclaration.Concat(projectIdsOfRenameSymbolDeclaration.SelectMany(p => solution.GetProjectDependencyGraph().GetProjectsThatDirectlyDependOnThisProject(p))).Distinct(); @@ -108,8 +109,27 @@ internal static IEnumerable GetDocumentsAffectedByRename(ISymbol symbo /// private static bool ShouldRenameOnlyAffectDeclaringProject(ISymbol symbol) { - // Explicit interface implementations can cascade to other projects - return symbol.DeclaredAccessibility == Accessibility.Private && !symbol.ExplicitInterfaceImplementations().Any(); + if (symbol.DeclaredAccessibility != Accessibility.Private) + { + // non-private members can influence other projects. + return false; + } + + if (symbol.ExplicitInterfaceImplementations().Any()) + { + // Explicit interface implementations can cascade to other projects + return false; + } + + if (symbol.IsOverride) + { + // private-overrides aren't actually legal. But if we see one, we tolerate it and search other projects in case + // they override it. + // https://github.com/dotnet/roslyn/issues/25682 + return false; + } + + return true; } internal static TokenRenameInfo GetTokenRenameInfo( -- GitLab