Merge pull request #124 from riviti/master

Fixes for a couple of issues

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs

@@ -128,6 +128,9 @@ static bool IsBannedInsertionPoint(AstNode anchorNode)
 				// Don't allow moving the declaration into the resource aquisition of a using statement
 				if (parent is UsingStatement)
 					return true;
+				// Don't allow moving things into arbitrary positions of for statements
+				if (parent is ForStatement && anchorNode.Role != Roles.EmbeddedStatement)
+					return true;
 				return false;
 			}
 

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 @@ static void TestMethod2 ()
 	{
 		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 @@ static void TestMethod2 (int i)
 }";
 			Test<ParameterHidesMemberIssue> (input, 1);
 		}
-
+		
 		[Test]
 		public void TestStaticNoIssue ()
 		{
@@ -103,6 +103,78 @@ void TestMethod (int i)
 	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);
 		}

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

@@ -391,6 +391,24 @@ public void DoesNotExpandElseClause ()
 ", 0);
 		}
 
+		[Test]
+		public void DoesNotExpandForStatement ()
+		{	
+			TestStatements(@"
+		var val = GetValue ();
+		if (false) {
+			for (var i = GetValue (); ; i++)
+				System.Console.WriteLine (val);
+		}
+", 1, @"
+		if (false) {
+			var val = GetValue ();
+			for (var i = GetValue (); ; i++)
+				System.Console.WriteLine (val);
+		}
+");
+		}
+
 		[Test]
 		public void DoesNotInsertBlockStatementInResourceAquisition ()
 		{	

ICSharpCode.NRefactory.Tests/Utils/CompositeFormatStringParser/CompositeFormatStringParserTests.cs

@@ -37,15 +37,16 @@ public class CompositeFormatStringParserTests
 		IList<IFormatStringSegment> ParseTest(string format, params IFormatStringSegment[] expectedFormatSegments)
 		{
 			var parser = new CompositeFormatStringParser();
-			var actualFormatSegments = parser.Parse(format).Segments;
+			var formatStringParseResult = parser.Parse(format);
+			var actualFormatSegments = formatStringParseResult.Segments;
 
 			Console.WriteLine("Expected format segments:");
 			foreach (var item in expectedFormatSegments) {
-				Console.WriteLine(item.ToString());
+				Console.WriteLine(item);
 			}
 			Console.WriteLine("Actual format segments:");
 			foreach (var item in actualFormatSegments) {
-				Console.WriteLine(item.ToString());
+				Console.WriteLine(item);
 				foreach (var error in item.Errors) {
 					Console.WriteLine("\t{0}", error);
 				}
@@ -117,7 +118,8 @@ public void MultipleCompleteFormatItems()
 		[Test]
 		public void BraceEscape()
 		{
-			ParseTest("{{}}", new TextSegment("{}"));
+			var segments = ParseTest("{{}}", new TextSegment("{}"));
+			Assert.IsFalse(segments.First().HasErrors);
 		}
 
 		[Test]