Merge pull request #5494 from MDoerner/FixTwoNREs

Fixes two null reference exceptions

Author: retailcoder

Rubberduck.CodeAnalysis/Inspections/Abstract/MemberAccessMayReturnNothingInspectionBase.cs

@@ -19,12 +19,20 @@ protected MemberAccessMayReturnNothingInspectionBase(IDeclarationFinderProvider
             : base(declarationFinderProvider)
         {}
 
+        /// <summary>
+        /// Members that might return Nothing
+        /// </summary>
+        /// <remarks>
+        /// It must not be legal to call the members unqualified. In particular, user-defined members will not be considered.
+        /// Moreover, this disqualifies all members on global objects. 
+        /// </remarks>
         public abstract IEnumerable<Declaration> MembersUnderTest(DeclarationFinder finder);
         public abstract string ResultTemplate { get; }
 
         protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
         {
-            return MembersUnderTest(finder);
+            //This restriction is in place because the inspection currently cannot handle unqualified accesses.
+            return MembersUnderTest(finder).Where(member => !member.IsUserDefined);
         }
 
         protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
@@ -46,7 +54,7 @@ protected override bool IsResultReference(IdentifierReference reference, Declara
             {
                 return usageContext is VBAParser.MemberAccessExprContext 
                         || !(usageContext is VBAParser.CallStmtContext)
-                            && !ContextIsNothingTest(usageContext);
+                            && !ContextIsNothing(usageContext);
             }
 
             var assignedTo = AssignmentTarget(reference, finder, setter);
@@ -65,14 +73,21 @@ private static IdentifierReference AssignmentTarget(IdentifierReference referenc
 
         private static RuleContext UsageContext(IdentifierReference reference)
         {
-            var access = reference.Context.GetAncestor<VBAParser.MemberAccessExprContext>();
-            var usageContext = access.Parent is VBAParser.IndexExprContext indexExpr
+            //We prefer the with member access over the member access, because the accesses are resolved right to left.
+            var access = reference.Context.GetAncestor<VBAParser.WithMemberAccessExprContext>() as VBAParser.LExpressionContext
+                ?? reference.Context.GetAncestor<VBAParser.MemberAccessExprContext>();
+
+            if (access == null)
+            {
+                return null;
+            }
+
+            return access.Parent is VBAParser.IndexExprContext indexExpr
                 ? indexExpr.Parent
                 : access.Parent;
-            return usageContext;
         }
 
-        private static bool ContextIsNothingTest(IParseTree context)
+        private static bool ContextIsNothing(IParseTree context)
         {
             return context is VBAParser.LExprContext 
                    && context.Parent is VBAParser.RelationalOpContext comparison 
@@ -86,7 +101,7 @@ private static bool IsUsedBeforeCheckingForNothing(IdentifierReference assignedT
             var firstUse = GetReferenceNodes(tree).FirstOrDefault();
 
             return !(firstUse is null)
-                   && !ContextIsNothingTest(firstUse.Reference.Context.Parent);
+                   && !ContextIsNothing(firstUse.Reference.Context.Parent);
         }
 
         private static IEnumerable<INode> GetReferenceNodes(INode node)

Rubberduck.Core/UI/UnitTesting/TestExplorerViewModel.cs

@@ -256,6 +256,7 @@ public bool CanExecuteIgnoreSelectedTests(object obj)
 
             return false;
         }
+
         public bool CanExecuteUnignoreSelectedTests(object obj)
         {
             if (!Model.IsBusy && obj is IList viewModels && viewModels.Count > 0)
@@ -273,12 +274,14 @@ public bool CanExecuteIgnoreGroupCommand(object obj)
 
             return groupItems.Cast<TestMethodViewModel>().Count(test => test.Method.IsIgnored) != groupItems.Count;
         }
+
         public bool CanExecuteUnignoreGroupCommand(object obj)
         {
             var groupItems = MouseOverGroup?.Items
-                             ?? GroupContainingSelectedTest(MouseOverTest).Items;
-
-            return groupItems.Cast<TestMethodViewModel>().Any(test => test.Method.IsIgnored);
+                             ?? GroupContainingSelectedTest(MouseOverTest)?.Items;
+            
+            return groupItems != null 
+                   && groupItems.Cast<TestMethodViewModel>().Any(test => test.Method.IsIgnored);
         }
 
         #region Commands

RubberduckTests/Inspections/ExcelMemberMayReturnNothingInspectionTests.cs

@@ -26,6 +26,22 @@ End Sub
             Assert.AreEqual(1, InspectionResults(inputCode).Count());
         }
 
+        [Test]
+        [Category("Inspections")]
+        public void ExcelMemberMayReturnNothing_ReturnsResult_WithMemberAccessOnFind()
+        {
+            const string inputCode =
+                @"Sub UnderTest()
+    Dim ws As Worksheet
+    Set ws = Sheet1
+    With ws.UsedRange
+        foo = .Find(""foo"").Row
+    End With
+End Sub
+";
+            Assert.AreEqual(1, InspectionResults(inputCode).Count());
+        }
+
         [Test]
         [Category("Inspections")]
         public void ExcelMemberMayReturnNothing_Ignored_DoesNotReturnResult()
@@ -111,6 +127,25 @@ End Sub
             Assert.AreEqual(1, InspectionResults(inputCode).Count());
         }
 
+        [Test]
+        [Category("Inspections")]
+        public void ExcelMemberMayReturnNothing_ReturnsResult_AssignedAndNotTested_FromWithMemberAccess()
+        {
+            const string inputCode =
+                @"Sub UnderTest()
+    Dim ws As Worksheet
+    Set ws = Sheet1
+    Dim result As Range
+    With ws.UsedRange
+        Set result = .Find(""foo"")
+    End With
+    result.Value = ""bar""
+End Sub
+";
+
+            Assert.AreEqual(1, InspectionResults(inputCode).Count());
+        }
+
         [Test]
         [Category("Inspections")]
         public void ExcelMemberMayReturnNothing_ReturnsResult_ResultIsSomethingElse()