[CodeIssues] Add accessibility checking to VariableHidesMemberIssue.

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<CodeIssue> GetIssues (BaseRefactoringContext context)
+		public IEnumerable<CodeIssue> 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<TypeDeclaration> ();
+			var typeDecl = node.GetParent<TypeDeclaration>();
 			if (typeDecl == null)
 				return false;
-			var typeResolveResult = ctx.Resolve (typeDecl) as TypeResolveResult;
+			var entityDecl = node.GetParent<EntityDeclaration>();
+			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<EntityDeclaration> ();
-			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);
 
 	}
 }

ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/LocalVariableHidesMemberIssueTests.cs

@@ -108,6 +108,87 @@ class TestClass
 	{
 		int j;
 	}
+}";
+			Test<LocalVariableHidesMemberIssue> (input, 0);
+		}
+		
+		[Test]
+		public void TestAccessiblePrivate ()
+		{
+			var input = @"
+class TestClass
+{
+	int i;
+
+	void Method ()
+	{
+		int i = 0;
+	}
+}";
+			Test<LocalVariableHidesMemberIssue> (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<LocalVariableHidesMemberIssue> (input, 2);
+		}
+		
+		[Test]
+		public void TestInternalAccessibility ()
+		{
+			var input = @"
+class BaseClass
+{
+	internal int i;
+}
+class TestClass : BaseClass
+{
+	void Method ()
+	{
+		int i = 0;
+	}
+}";
+			Test<LocalVariableHidesMemberIssue> (input, 1);
+		}
+		
+		[Test]
+		public void TestInaccessiblePrivate ()
+		{
+			var input = @"
+class BaseClass
+{
+	int i;
+}
+class TestClass : BaseClass
+{
+	void Method ()
+	{
+		int i = 0;
+	}
 }";
 			Test<LocalVariableHidesMemberIssue> (input, 0);
 		}

ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterHidesMemberIssueTests.cs

@@ -88,7 +88,7 @@ class TestClass
 }";
 			Test<ParameterHidesMemberIssue> (input, 1);
 		}
-
+		
 		[Test]
 		public void TestStaticNoIssue ()
 		{
@@ -103,6 +103,78 @@ class TestClass
 	static void TestMethod2 (int j)
 	{
 	}
+}";
+			Test<ParameterHidesMemberIssue> (input, 0);
+		}
+		
+		[Test]
+		public void TestAccessiblePrivate ()
+		{
+			var input = @"
+class TestClass
+{
+	int i;
+
+	void Method (int i)
+	{
+	}
+}";
+			Test<ParameterHidesMemberIssue> (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<ParameterHidesMemberIssue> (input, 2);
+		}
+		
+		[Test]
+		public void TestInternalAccessibility ()
+		{
+			var input = @"
+class BaseClass
+{
+	internal int i;
+}
+class TestClass : BaseClass
+{
+	void Method (int i)
+	{
+	}
+}";
+			Test<ParameterHidesMemberIssue> (input, 1);
+		}
+		
+		[Test]
+		public void TestInaccessiblePrivate ()
+		{
+			var input = @"
+class BaseClass
+{
+	private int i;
+}
+class TestClass : BaseClass
+{
+	void Method (int i)
+	{
+	}
 }";
 			Test<ParameterHidesMemberIssue> (input, 0);
 		}