diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableHidesMemberIssue/VariableHidesMemberIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableHidesMemberIssue/VariableHidesMemberIssue.cs index 6b1dffdf6c..5fb0de1215 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableHidesMemberIssue/VariableHidesMemberIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableHidesMemberIssue/VariableHidesMemberIssue.cs @@ -23,36 +23,83 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. - using System.Collections.Generic; using System.Linq; using ICSharpCode.NRefactory.Semantics; +using ICSharpCode.NRefactory.TypeSystem; +using System; namespace ICSharpCode.NRefactory.CSharp.Refactoring { public abstract class VariableHidesMemberIssue : ICodeIssueProvider { - public IEnumerable GetIssues (BaseRefactoringContext context) + public IEnumerable GetIssues(BaseRefactoringContext context) { - return GetGatherVisitor (context).GetIssues (); + return GetGatherVisitor(context).GetIssues(); } protected static bool HidesMember(BaseRefactoringContext ctx, AstNode node, string variableName) { - var typeDecl = node.GetParent (); + var typeDecl = node.GetParent(); if (typeDecl == null) return false; - var typeResolveResult = ctx.Resolve (typeDecl) as TypeResolveResult; + var entityDecl = node.GetParent(); + var memberResolveResult = ctx.Resolve(entityDecl) as MemberResolveResult; + if (memberResolveResult == null) + return false; + var typeResolveResult = ctx.Resolve(typeDecl) as TypeResolveResult; if (typeResolveResult == null) return false; - var entityDecl = node.GetParent (); - var isStatic = (entityDecl.Modifiers & Modifiers.Static) == Modifiers.Static; + var sourceMember = memberResolveResult.Member; + + return typeResolveResult.Type.GetMembers(m => m.Name == variableName).Any(m2 => IsAccessible(sourceMember, m2)); + } + + static bool IsAccessible(IMember sourceMember, IMember targetMember) + { + if (sourceMember.IsStatic != targetMember.IsStatic) + return false; - return typeResolveResult.Type.GetMembers (m => m.Name == variableName && m.IsStatic == isStatic).Any (); + var sourceType = sourceMember.DeclaringType; + var targetType = targetMember.DeclaringType; + switch (targetMember.Accessibility) { + case Accessibility.None: + return false; + case Accessibility.Private: + // check for members of outer classes (private members of outer classes can be accessed) + var targetTypeDefinition = targetType.GetDefinition(); + for (var t = sourceType.GetDefinition(); t != null; t = t.DeclaringTypeDefinition) { + if (t.Equals(targetTypeDefinition)) + return true; + } + return false; + case Accessibility.Public: + return true; + case Accessibility.Protected: + return IsProtectedAccessible(sourceType, targetType); + case Accessibility.Internal: + return IsInternalAccessible(sourceMember.ParentAssembly, targetMember.ParentAssembly); + case Accessibility.ProtectedOrInternal: + return IsInternalAccessible(sourceMember.ParentAssembly, targetMember.ParentAssembly) || IsProtectedAccessible(sourceType, targetType); + case Accessibility.ProtectedAndInternal: + return IsInternalAccessible(sourceMember.ParentAssembly, targetMember.ParentAssembly) && IsProtectedAccessible(sourceType, targetType); + default: + throw new Exception("Invalid value for Accessibility"); + } + } + + static bool IsProtectedAccessible(IType sourceType, IType targetType) + { + return sourceType.GetAllBaseTypes().Any(type => targetType.Equals(type)); + } + + static bool IsInternalAccessible(IAssembly sourceAssembly, IAssembly targetAssembly) + { + return sourceAssembly.InternalsVisibleTo(targetAssembly); } - internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context); + internal abstract GatherVisitorBase GetGatherVisitor(BaseRefactoringContext context); } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/LocalVariableHidesMemberIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/LocalVariableHidesMemberIssueTests.cs index 41b118eedf..cbdf040a6f 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/LocalVariableHidesMemberIssueTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/LocalVariableHidesMemberIssueTests.cs @@ -108,6 +108,87 @@ class TestClass { int j; } +}"; + Test (input, 0); + } + + [Test] + public void TestAccessiblePrivate () + { + var input = @" +class TestClass +{ + int i; + + void Method () + { + int i = 0; + } +}"; + Test (input, 1); + } + + [Test] + public void TestAccessiblePrivateDueToTypeNesting () + { + var input = @" +class RootClass +{ + int i; + + class NestedClass : RootClass + { + // Issue 1 + void Method () + { + int i = 0; + } + + class NestedNestedClass : NestedClass + { + // Issue 2 + void OtherMethod () + { + int i = 0; + } + } + } +}"; + Test (input, 2); + } + + [Test] + public void TestInternalAccessibility () + { + var input = @" +class BaseClass +{ + internal int i; +} +class TestClass : BaseClass +{ + void Method () + { + int i = 0; + } +}"; + Test (input, 1); + } + + [Test] + public void TestInaccessiblePrivate () + { + var input = @" +class BaseClass +{ + int i; +} +class TestClass : BaseClass +{ + void Method () + { + int i = 0; + } }"; Test (input, 0); } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterHidesMemberIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterHidesMemberIssueTests.cs index 40b62739e5..982182d02f 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterHidesMemberIssueTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterHidesMemberIssueTests.cs @@ -88,7 +88,7 @@ class TestClass }"; Test (input, 1); } - + [Test] public void TestStaticNoIssue () { @@ -103,6 +103,78 @@ class TestClass static void TestMethod2 (int j) { } +}"; + Test (input, 0); + } + + [Test] + public void TestAccessiblePrivate () + { + var input = @" +class TestClass +{ + int i; + + void Method (int i) + { + } +}"; + Test (input, 1); + } + + [Test] + public void TestAccessiblePrivateDueToTypeNesting () + { + var input = @" +class RootClass +{ + int i; + + class NestedClass : RootClass + { + // Issue 1 + void Method (int i) {} + + class NestedNestedClass : NestedClass + { + // Issue 2 + void OtherMethod (int i) {} + } + } +}"; + Test (input, 2); + } + + [Test] + public void TestInternalAccessibility () + { + var input = @" +class BaseClass +{ + internal int i; +} +class TestClass : BaseClass +{ + void Method (int i) + { + } +}"; + Test (input, 1); + } + + [Test] + public void TestInaccessiblePrivate () + { + var input = @" +class BaseClass +{ + private int i; +} +class TestClass : BaseClass +{ + void Method (int i) + { + } }"; Test (input, 0); }