Merge pull request #2772 from retailcoder/rd-next

retailcoder · web-flow · commit e5e692f95781 · 2017-03-02T11:05:32.000-05:00
resolver / inspection fixes
diff --git a/RetailCoder.VBE/Inspections/ImplicitByRefParameterInspection.cs b/RetailCoder.VBE/Inspections/ImplicitByRefParameterInspection.cs
@@ -23,7 +23,7 @@ public ImplicitByRefParameterInspection(RubberduckParserState state)
         public override IEnumerable<InspectionResultBase> GetInspectionResults()
         {
             var interfaceMembers = UserDeclarations.FindInterfaceImplementationMembers();
-            var builtinEventHandlers = State.DeclarationFinder.FindBuiltinEventHandlers();
+            var builtinEventHandlers = State.DeclarationFinder.FindEventHandlers();
 
             var issues = State.DeclarationFinder
                 .UserDeclarations(DeclarationType.Parameter)
diff --git a/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs b/RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs
@@ -33,7 +33,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
 
             var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList();
             var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope);
-            var eventHandlerScopes = State.DeclarationFinder.FindBuiltinEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope);
+            var eventHandlerScopes = State.DeclarationFinder.FindEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope);
             var eventScopes = eventMembers.Select(s => s.Scope)
                 .Concat(formEventHandlerScopes)
                 .Concat(eventHandlerScopes);
diff --git a/RetailCoder.VBE/Inspections/ParameterNotUsedInspection.cs b/RetailCoder.VBE/Inspections/ParameterNotUsedInspection.cs
@@ -1,6 +1,5 @@
 using System.Collections.Generic;
 using System.Linq;
-using Rubberduck.Common;
 using Rubberduck.Inspections.Abstract;
 using Rubberduck.Inspections.Resources;
 using Rubberduck.Inspections.Results;
@@ -29,7 +28,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
             var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers();
             var interfaceImplementationMembers = State.DeclarationFinder.FindAllInterfaceImplementingMembers();
 
-            var builtInHandlers = State.DeclarationFinder.FindBuiltinEventHandlers();
+            var handlers = State.DeclarationFinder.FindEventHandlers();
 
             var parameters = State.DeclarationFinder
                 .UserDeclarations(DeclarationType.Parameter)
@@ -39,7 +38,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
                                     && parameter.ParentDeclaration.DeclarationType != DeclarationType.LibraryFunction
                                     && parameter.ParentDeclaration.DeclarationType != DeclarationType.LibraryProcedure
                                     && !interfaceMembers.Contains(parameter.ParentDeclaration)
-                                    && !builtInHandlers.Contains(parameter.ParentDeclaration))
+                                    && !handlers.Contains(parameter.ParentDeclaration))
                 .ToList();
 
             var issues = from issue in parameters
diff --git a/RetailCoder.VBE/Inspections/ProcedureCanBeWrittenAsFunctionInspection.cs b/RetailCoder.VBE/Inspections/ProcedureCanBeWrittenAsFunctionInspection.cs
@@ -43,7 +43,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
             }
 
             var userDeclarations = UserDeclarations.ToList();
-            var builtinHandlers = State.DeclarationFinder.FindBuiltinEventHandlers().ToList();
+            var builtinHandlers = State.DeclarationFinder.FindEventHandlers().ToList();
 
             var contextLookup = userDeclarations.Where(decl => decl.Context != null).ToDictionary(decl => decl.Context);
 
diff --git a/RetailCoder.VBE/Inspections/ProcedureNotUsedInspection.cs b/RetailCoder.VBE/Inspections/ProcedureNotUsedInspection.cs
@@ -41,7 +41,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
             var handlers = State.DeclarationFinder.UserDeclarations(DeclarationType.Control)
                 .SelectMany(control => declarations.FindEventHandlers(control)).ToList();
 
-            var builtInHandlers = State.DeclarationFinder.FindBuiltinEventHandlers();
+            var builtInHandlers = State.DeclarationFinder.FindEventHandlers();
             handlers.AddRange(builtInHandlers);
 
             var withEventFields = State.DeclarationFinder.UserDeclarations(DeclarationType.Variable).Where(item => item.IsWithEvents).ToList();
diff --git a/RetailCoder.VBE/Inspections/UseMeaningfulNameInspection.cs b/RetailCoder.VBE/Inspections/UseMeaningfulNameInspection.cs
@@ -42,7 +42,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
             var settings = _settings.Load(new CodeInspectionSettings()) ?? new CodeInspectionSettings();
             var whitelistedNames = settings.WhitelistedIdentifiers.Select(s => s.Identifier).ToArray();
 
-            var handlers = State.DeclarationFinder.FindBuiltinEventHandlers();
+            var handlers = State.DeclarationFinder.FindEventHandlers();
 
             var issues = UserDeclarations
                             .Where(declaration => !string.IsNullOrEmpty(declaration.IdentifierName) &&
diff --git a/RetailCoder.VBE/Root/RubberduckModule.cs b/RetailCoder.VBE/Root/RubberduckModule.cs
@@ -466,7 +466,9 @@ private IMenuItem GetRefactoringsParentMenu()
             var items = new IMenuItem[]
             {
                 KernelInstance.Get<CodePaneRefactorRenameCommandMenuItem>(),
+#if DEBUG
                 KernelInstance.Get<RefactorExtractMethodCommandMenuItem>(),
+#endif
                 KernelInstance.Get<RefactorReorderParametersCommandMenuItem>(),
                 KernelInstance.Get<RefactorRemoveParametersCommandMenuItem>(),
                 KernelInstance.Get<RefactorIntroduceParameterCommandMenuItem>(),
diff --git a/Rubberduck.Parsing/Symbols/BoundExpressionVisitor.cs b/Rubberduck.Parsing/Symbols/BoundExpressionVisitor.cs
@@ -1,5 +1,6 @@
 ﻿using Rubberduck.Parsing.Annotations;
 using Rubberduck.Parsing.Binding;
+using Rubberduck.Parsing.Grammar;
 using Rubberduck.VBEditor;
 
 namespace Rubberduck.Parsing.Symbols
@@ -47,6 +48,13 @@ private void Visit(
             bool isAssignmentTarget,
             bool hasExplicitLetStatement)
         {
+            if (isAssignmentTarget && expression.Context.Parent is VBAParser.IndexExprContext && !expression.ReferencedDeclaration.IsArray)
+            {
+                // 'SomeDictionary' is not the assignment target in 'SomeDictionary("key") = 42'
+                // ..but we want to treat array index assignment as assignment to the array itself.
+                isAssignmentTarget = false;
+            }
+
             var callSiteContext = expression.Context;
             var identifier = expression.Context.GetText();
             var callee = expression.ReferencedDeclaration;
diff --git a/Rubberduck.Parsing/Symbols/DeclarationFinder.cs b/Rubberduck.Parsing/Symbols/DeclarationFinder.cs
@@ -64,7 +64,7 @@ public DeclarationFinder(IReadOnlyList<Declaration> declarations, IEnumerable<IA
             _parametersByParent = declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter)
                 .GroupBy(declaration => declaration.ParentDeclaration).ToConcurrentDictionary();
             _userDeclarationsByType = declarations.Where(declaration => !declaration.IsBuiltIn).GroupBy(declaration => declaration.DeclarationType).ToConcurrentDictionary();
-            _builtinEvents = new Lazy<ConcurrentBag<Declaration>>(() => FindBuiltInEventHandlers(declarations), true);
+            _eventHandlers = new Lazy<ConcurrentBag<Declaration>>(() => FindEventHandlers(declarations), true);
 
             _projects = _projects = new Lazy<ConcurrentBag<Declaration>>(() => new ConcurrentBag<Declaration>(declarations.Where(d => d.DeclarationType == DeclarationType.Project)), true);
             _classes = new Lazy<ConcurrentBag<Declaration>>(() => new ConcurrentBag<Declaration>(declarations.Where(d => d.DeclarationType == DeclarationType.ClassModule)), true);
@@ -165,12 +165,12 @@ public IEnumerable<Declaration> FindDeclarationsWithNonBaseAsType()
             }
         }
 
-        private readonly Lazy<ConcurrentBag<Declaration>> _builtinEvents; 
-        public IEnumerable<Declaration> FindBuiltinEventHandlers()
+        private readonly Lazy<ConcurrentBag<Declaration>> _eventHandlers; 
+        public IEnumerable<Declaration> FindEventHandlers()
         {
             lock (ThreadLock)
             {
-                return _builtinEvents.Value;
+                return _eventHandlers.Value;
             }
         }
 
@@ -695,7 +695,23 @@ private IEnumerable<Declaration> FindAllInReferencedProjectByPriority(Declaratio
             }
         }
 
-        public ConcurrentBag<Declaration> FindBuiltInEventHandlers(IEnumerable<Declaration> declarations)
+        private IEnumerable<Declaration> FindFormControlHandlers(IReadOnlyList<Declaration> declarations)
+        {
+            var controls = declarations
+                .Where(declaration => declaration.DeclarationType == DeclarationType.Control);
+            var handlerNames = declarations
+                .Where(declaration => declaration.IsBuiltIn && declaration.DeclarationType == DeclarationType.Event)
+                .SelectMany(e => controls.Select(c => c.IdentifierName + "_" + e.IdentifierName));
+            if (!_userDeclarationsByType.ContainsKey(DeclarationType.Procedure))
+            {
+                return Enumerable.Empty<Declaration>();
+            }
+            var handlers = _userDeclarationsByType[DeclarationType.Procedure]
+                .Where(procedure => handlerNames.Contains(procedure.IdentifierName));
+            return handlers;
+        }
+
+        private ConcurrentBag<Declaration> FindEventHandlers(IEnumerable<Declaration> declarations)
         {
             var declarationList = declarations.ToList();
 
@@ -725,7 +741,9 @@ public ConcurrentBag<Declaration> FindBuiltInEventHandlers(IEnumerable<Declarati
                 (!item.IsBuiltIn &&
                  item.DeclarationType == DeclarationType.Procedure &&
                  handlerNames.Contains(item.IdentifierName))
-                );
+                )
+                .Concat(_handlersByWithEventsField.Value.SelectMany(kvp => kvp.Value))
+                .Concat(FindFormControlHandlers(declarationList));
 
             return new ConcurrentBag<Declaration>(handlers);
         }
diff --git a/RubberduckTests/Grammar/ResolverTests.cs b/RubberduckTests/Grammar/ResolverTests.cs
@@ -902,6 +902,30 @@ End Sub
                 && !item.IsAssignment));
         }
 
+        [TestMethod]
+        public void SubscriptWrite_IsNotAssignmentReferenceToObjectDeclaration()
+        {
+            var code = @"
+Public Sub DoSomething()
+    Dim foo As Object
+    Set foo = CreateObject(""Scripting.Dictionary"")
+    foo(""key"") = 42
+End Sub
+";
+            // act
+            var state = Resolve(code);
+
+            // assert
+            var declaration = state.AllUserDeclarations.Single(item =>
+                item.DeclarationType == DeclarationType.Variable
+                && item.IdentifierName == "foo");
+
+            Assert.IsNotNull(declaration.References.SingleOrDefault(item =>
+                item.ParentScoping.DeclarationType == DeclarationType.Procedure
+                && item.ParentScoping.IdentifierName == "DoSomething"
+                && !item.IsAssignment));
+        }
+
         [TestMethod]
         public void ArraySubscriptWrite_IsAssignmentReferenceToArrayDeclaration()
         {
diff --git a/RubberduckTests/Inspections/ObjectVariableNotSetInpsectionTests.cs b/RubberduckTests/Inspections/ObjectVariableNotSetInpsectionTests.cs
@@ -16,6 +16,34 @@ namespace RubberduckTests.Inspections
     [TestClass]
     public class ObjectVariableNotSetInpsectionTests
     {
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void ObjectVariableNotSet_GivenIndexerObjectAccess_ReturnsNoResult()
+        {
+            const string inputCode = @"
+Private Sub DoSomething()
+    Dim target As Object
+    Set target = CreateObject(""Scripting.Dictionary"")
+    target(""foo"") = 42
+End Sub
+";
+            //Arrange
+            var builder = new MockVbeBuilder();
+            IVBComponent component;
+            var vbe = builder.BuildFromSingleStandardModule(inputCode, out component);
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
+
+            parser.Parse(new CancellationTokenSource());
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new ObjectVariableNotSetInspection(parser.State);
+            var inspectionResults = inspection.GetInspectionResults();
+
+            Assert.AreEqual(0, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void ObjectVariableNotSet_GivenStringVariable_ReturnsNoResult()

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public ImplicitByRefParameterInspection(RubberduckParserState state)`
`23`	`23`	`public override IEnumerable<InspectionResultBase> GetInspectionResults()`
`24`	`24`	`{`
`25`	`25`	`var interfaceMembers = UserDeclarations.FindInterfaceImplementationMembers();`
`26`		`- var builtinEventHandlers = State.DeclarationFinder.FindBuiltinEventHandlers();`
	`26`	`+ var builtinEventHandlers = State.DeclarationFinder.FindEventHandlers();`
`27`	`27`
`28`	`28`	`var issues = State.DeclarationFinder`
`29`	`29`	`.UserDeclarations(DeclarationType.Parameter)`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`var userDeclarations = UserDeclarations.ToList();`
`46`		`- var builtinHandlers = State.DeclarationFinder.FindBuiltinEventHandlers().ToList();`
	`46`	`+ var builtinHandlers = State.DeclarationFinder.FindEventHandlers().ToList();`
`47`	`47`
`48`	`48`	`var contextLookup = userDeclarations.Where(decl => decl.Context != null).ToDictionary(decl => decl.Context);`
`49`	`49`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public DeclarationFinder(IReadOnlyList<Declaration> declarations, IEnumerable<IA`
`64`	`64`	`_parametersByParent = declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter)`
`65`	`65`	`.GroupBy(declaration => declaration.ParentDeclaration).ToConcurrentDictionary();`
`66`	`66`	`_userDeclarationsByType = declarations.Where(declaration => !declaration.IsBuiltIn).GroupBy(declaration => declaration.DeclarationType).ToConcurrentDictionary();`
`67`		`- _builtinEvents = new Lazy<ConcurrentBag<Declaration>>(() => FindBuiltInEventHandlers(declarations), true);`
	`67`	`+ _eventHandlers = new Lazy<ConcurrentBag<Declaration>>(() => FindEventHandlers(declarations), true);`
`68`	`68`
`69`	`69`	`_projects = _projects = new Lazy<ConcurrentBag<Declaration>>(() => new ConcurrentBag<Declaration>(declarations.Where(d => d.DeclarationType == DeclarationType.Project)), true);`
`70`	`70`	`_classes = new Lazy<ConcurrentBag<Declaration>>(() => new ConcurrentBag<Declaration>(declarations.Where(d => d.DeclarationType == DeclarationType.ClassModule)), true);`
`@@ -165,12 +165,12 @@ public IEnumerable<Declaration> FindDeclarationsWithNonBaseAsType()`
`165`	`165`	`}`
`166`	`166`	`}`
`167`	`167`
`168`		`- private readonly Lazy<ConcurrentBag<Declaration>> _builtinEvents;`
`169`		`- public IEnumerable<Declaration> FindBuiltinEventHandlers()`
	`168`	`+ private readonly Lazy<ConcurrentBag<Declaration>> _eventHandlers;`
	`169`	`+ public IEnumerable<Declaration> FindEventHandlers()`
`170`	`170`	`{`
`171`	`171`	`lock (ThreadLock)`
`172`	`172`	`{`
`173`		`- return _builtinEvents.Value;`
	`173`	`+ return _eventHandlers.Value;`
`174`	`174`	`}`
`175`	`175`	`}`
`176`	`176`
`@@ -695,7 +695,23 @@ private IEnumerable<Declaration> FindAllInReferencedProjectByPriority(Declaratio`
`695`	`695`	`}`
`696`	`696`	`}`
`697`	`697`
`698`		`- public ConcurrentBag<Declaration> FindBuiltInEventHandlers(IEnumerable<Declaration> declarations)`
	`698`	`+ private IEnumerable<Declaration> FindFormControlHandlers(IReadOnlyList<Declaration> declarations)`
	`699`	`+ {`
	`700`	`+ var controls = declarations`
	`701`	`+ .Where(declaration => declaration.DeclarationType == DeclarationType.Control);`
	`702`	`+ var handlerNames = declarations`
	`703`	`+ .Where(declaration => declaration.IsBuiltIn && declaration.DeclarationType == DeclarationType.Event)`
	`704`	`+ .SelectMany(e => controls.Select(c => c.IdentifierName + "_" + e.IdentifierName));`
	`705`	`+ if (!_userDeclarationsByType.ContainsKey(DeclarationType.Procedure))`
	`706`	`+ {`
	`707`	`+ return Enumerable.Empty<Declaration>();`
	`708`	`+ }`
	`709`	`+ var handlers = _userDeclarationsByType[DeclarationType.Procedure]`
	`710`	`+ .Where(procedure => handlerNames.Contains(procedure.IdentifierName));`
	`711`	`+ return handlers;`
	`712`	`+ }`
	`713`	`+`
	`714`	`+ private ConcurrentBag<Declaration> FindEventHandlers(IEnumerable<Declaration> declarations)`
`699`	`715`	`{`
`700`	`716`	`var declarationList = declarations.ToList();`
`701`	`717`
`@@ -725,7 +741,9 @@ public ConcurrentBag<Declaration> FindBuiltInEventHandlers(IEnumerable<Declarati`
`725`	`741`	`(!item.IsBuiltIn &&`
`726`	`742`	`item.DeclarationType == DeclarationType.Procedure &&`
`727`	`743`	`handlerNames.Contains(item.IdentifierName))`
`728`		`- );`
	`744`	`+ )`
	`745`	`+ .Concat(_handlersByWithEventsField.Value.SelectMany(kvp => kvp.Value))`
	`746`	`+ .Concat(FindFormControlHandlers(declarationList));`
`729`	`747`
`730`	`748`	`return new ConcurrentBag<Declaration>(handlers);`
`731`	`749`	`}`