[CodeIssues] Fix some false positives in ParameterCanBeDemotedIssue.

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

@@ -57,6 +57,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			{
 				base.VisitMethodDeclaration(methodDeclaration);
 
+				var declarationResolveResult = context.Resolve(methodDeclaration) as MemberResolveResult;
+				if (declarationResolveResult == null)
+					return;
+				var member = declarationResolveResult.Member;
+				if (member.IsOverride || member.IsOverridable)
+					return;
+
 				var collector = new TypeCriteriaCollector(context);
 				methodDeclaration.AcceptVisitor(collector);
 				

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ParameterCanBeDemotedIssue/TypeCriteriaCollector.cs

@@ -55,15 +55,33 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 				TypeCriteria[variable] = new List<ITypeCriterion>();
 			TypeCriteria[variable].Add(criterion);
 		}
-		
+
+		public override void VisitMemberReferenceExpression(MemberReferenceExpression memberReferenceExpression)
+		{
+			base.VisitMemberReferenceExpression(memberReferenceExpression);
+
+			var memberResolveResult = context.Resolve(memberReferenceExpression) as MemberResolveResult;
+			if (memberResolveResult == null)
+				return;
+			var targetResolveResult = memberResolveResult.TargetResult as LocalResolveResult;
+			if (targetResolveResult == null)
+				return;
+			var variable = targetResolveResult.Variable;
+			AddCriterion(variable, new HasMemberCriterion(memberResolveResult.Member));
+		}
+
 		public override void VisitInvocationExpression(InvocationExpression invocationExpression)
 		{
 			base.VisitInvocationExpression(invocationExpression);
+
 			var resolveResult = context.Resolve(invocationExpression);
 			var invocationResolveResult = resolveResult as InvocationResolveResult;
 			if (invocationResolveResult == null)
 				return;
 			CollectRestrictionsFromNodes(invocationExpression.Arguments);
+
+			// invocationExpression.Target resolves to a method group and VisitMemberReferenceExpression
+			// only handles members, so handle method groups here
 			var targetResolveResult = invocationResolveResult.TargetResult as LocalResolveResult;
 			if (targetResolveResult == null)
 				return;
@@ -73,18 +91,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 
 		void CollectRestrictionsFromNodes(IEnumerable<Expression> expressions)
 		{
-			var identifiers =
+			foreach (var expression in expressions) {
-				from expression in expressions
+				var resolveResult = context.Resolve(expression);
-					from node in expression.DescendantsAndSelf
+				var argumentResolveResult = resolveResult as LocalResolveResult;
-					let identifier = node as IdentifierExpression
-					where identifier != null
-					select identifier;
-			foreach (var identifier in identifiers) {
-				var resolveResult2 = context.Resolve(identifier);
-				var argumentResolveResult = resolveResult2 as LocalResolveResult;
 				if (argumentResolveResult == null)
 					continue;
-				var expectedType = context.GetExpectedType(identifier);
+				var expectedType = context.GetExpectedType(expression);
 				AddCriterion(argumentResolveResult.Variable, new IsTypeCriterion(expectedType));
 			}
 		}

ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ParameterCanBeDemotedIssue/ParameterCanBeDemotedTests.cs

@@ -92,6 +92,88 @@ class A
 			Assert.AreEqual(0, issues.Count);
 		}
 
+		[Test]
+		public void IgnoresOverrides()
+		{
+			var input = @"
+interface IA
+{
+	void Foo();
+}
+class B : IA
+{
+	public virtual void Foo() {}
+	public virtual void Bar() {}
+}
+class TestBase
+{
+	public void F(B b) {
+		b.Foo();
+		b.Bar();
+	}
+}
+class TestClass : TestBase
+{
+	public override void F(B b)
+	{
+		b.Foo();
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new ParameterCanBeDemotedIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+		
+		[Test]
+		public void IgnoresOverridables()
+		{
+			var input = @"
+interface IA
+{
+	void Foo();
+}
+class B : IA
+{
+	public virtual void Foo() {}
+	public virtual void Bar() {}
+}
+class TestClass
+{
+	public virtual void F(B b)
+	{
+		b.Foo();
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new ParameterCanBeDemotedIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+		
+		[Test]
+		public void HandlesNeededProperties()
+		{
+			var input = @"
+interface IA
+{
+	void Foo(string s);
+}
+class B : IA
+{
+	public virtual void Foo(string s) {}
+	public string Property { get; }
+}
+class TestClass
+{
+	public void F(B b)
+	{
+		b.Foo(b.Property);
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new ParameterCanBeDemotedIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+
 		[Test]
 		public void InterfaceTest()
 		{
@@ -280,6 +362,52 @@ class Test
 	void DemandType(D d)
 	{
 	}
+}");
+		}
+		
+		[Test]
+		public void RespectsOutgoingCallsTypeRestrictionsWhenPassingResultFromMember()
+		{
+			var input = @"
+interface IA
+{
+	void Foo(string s);
+	string Property { get; }
+}
+class B : IA
+{
+	public virtual void Foo(string s) {}
+	public string Property { get; }
+}
+class TestClass
+{
+	public void F(B b)
+	{
+		b.Foo(b.Property);
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new ParameterCanBeDemotedIssue(), input, out context);
+			Assert.AreEqual(1, issues.Count);
+			var issue = issues [0];
+			Assert.AreEqual(1, issue.Actions.Count);
+			CheckFix(context, issues [0], @"
+interface IA
+{
+	void Foo(string s);
+	string Property { get; }
+}
+class B : IA
+{
+	public virtual void Foo(string s) {}
+	public string Property { get; }
+}
+class TestClass
+{
+	public void F(IA b)
+	{
+		b.Foo(b.Property);
+	}
 }");
 		}
 	}