Address review comments. Tweak DeclarationFinder to use readonly list instead of building.

Author: comintern

Rubberduck.Core/UI/Command/FindAllImplementationsCommand.cs

@@ -162,7 +162,7 @@ private SearchResultsViewModel CreateViewModel(Declaration target)
             {
                 implementations = _state.DeclarationFinder.FindAllImplementationsOfInterface(classModule);
             }
-            else if (target is ICanBeInterfaceMember member && member.IsInterfaceMember)
+            else if (target is IInterfaceExposable member && member.IsInterfaceMember)
             {
                 implementations = _state.DeclarationFinder.FindInterfaceImplementationMembers(target);
             }

Rubberduck.Parsing/Rubberduck.Parsing.csproj

@@ -156,7 +156,7 @@
     <Compile Include="PreProcessing\ICompilationArgumentsProvider.cs" />
     <Compile Include="PreProcessing\ICompilationArgumentsCache.cs" />
     <Compile Include="Rewriter\ModuleRewriterFactory.cs" />
-    <Compile Include="Symbols\ICanBeInterfaceMember.cs" />
+    <Compile Include="Symbols\IInterfaceExposable.cs" />
     <Compile Include="Symbols\ModuleBodyElementDeclaration.cs" />
     <Compile Include="Symbols\ModuleDeclaration.cs" />
     <Compile Include="Symbols\ParsingExceptions\ExceptionErrorListenerFactory.cs" />

Rubberduck.Parsing/Symbols/DeclarationFinder.cs

@@ -33,7 +33,7 @@ public class DeclarationFinder
         private IDictionary<DeclarationType, List<Declaration>> _userDeclarationsByType;
         private IDictionary<QualifiedSelection, List<Declaration>> _declarationsBySelection;
         private IDictionary<QualifiedSelection, List<IdentifierReference>> _referencesBySelection;
-        private IDictionary<QualifiedModuleName, List<IdentifierReference>> _referencesByModule;
+        private IReadOnlyDictionary<QualifiedModuleName, IReadOnlyList<IdentifierReference>> _referencesByModule;
         private IDictionary<QualifiedMemberName, List<IdentifierReference>> _referencesByMember;
 
         private Lazy<IDictionary<DeclarationType, List<Declaration>>> _builtInDeclarationsByType;
@@ -43,7 +43,7 @@ public class DeclarationFinder
         private Lazy<IDictionary<VBAParser.ImplementsStmtContext, List<ModuleBodyElementDeclaration>>> _membersByImplementsContext;
         private Lazy<IDictionary<ClassModuleDeclaration, List<Declaration>>> _interfaceMembers;
         private Lazy<IDictionary<ClassModuleDeclaration, List<ClassModuleDeclaration>>> _interfaceImplementions;
-        private Lazy<IDictionary<ICanBeInterfaceMember, List<ModuleBodyElementDeclaration>>> _implementationsByMember;
+        private Lazy<IDictionary<IInterfaceExposable, List<ModuleBodyElementDeclaration>>> _implementationsByMember;
 
         private Lazy<List<Declaration>> _nonBaseAsType;
         private Lazy<List<Declaration>> _eventHandlers;
@@ -129,7 +129,7 @@ private List<Action> CollectionConstructionActions(IReadOnlyList<Declaration> de
                         .SelectMany(declaration => declaration.References)
                         .GroupBy(reference =>
                             Declaration.GetModuleParent(reference.ParentScoping).QualifiedName.QualifiedModuleName)
-                        .ToDictionary(),
+                        .ToReadonlyDictionary(),
                 () =>
                     _referencesByMember = declarations
                         .SelectMany(declaration => declaration.References)
@@ -169,7 +169,7 @@ private void InitializeLazyCollections()
             _interfaceMembers = new Lazy<IDictionary<ClassModuleDeclaration, List<Declaration>>>(FindAllIinterfaceMembersByModule, true);
             _membersByImplementsContext = new Lazy<IDictionary<VBAParser.ImplementsStmtContext, List<ModuleBodyElementDeclaration>>>(FindAllImplementingMembersByImplementsContext, true);
             _interfaceImplementions = new Lazy<IDictionary<ClassModuleDeclaration, List<ClassModuleDeclaration>>>(FindAllImplementionsByInterface, true);
-            _implementationsByMember = new Lazy<IDictionary<ICanBeInterfaceMember, List<ModuleBodyElementDeclaration>>>(FindAllImplementingMembersByMember, true);
+            _implementationsByMember = new Lazy<IDictionary<IInterfaceExposable, List<ModuleBodyElementDeclaration>>>(FindAllImplementingMembersByMember, true);
         }
 
         private IDictionary<(VBAParser.ImplementsStmtContext Context, Declaration Implementor), List<ModuleBodyElementDeclaration>> FindAllImplementingMembers()
@@ -195,7 +195,8 @@ private void InitializeLazyCollections()
                 output.Add((impl.Context, impl.IdentifierReference.ParentScoping),
                     ((ClassModuleDeclaration) impl.IdentifierReference.ParentScoping).Members.Where(item =>
                         item is ModuleBodyElementDeclaration member && ReferenceEquals(member.InterfaceImplemented,
-                            impl.IdentifierReference.Declaration)).Cast<ModuleBodyElementDeclaration>().ToList());
+                            impl.IdentifierReference.Declaration))
+                    .Cast<ModuleBodyElementDeclaration>().ToList());
             }
 
             return output;
@@ -210,11 +211,12 @@ private Dictionary<ClassModuleDeclaration, List<ClassModuleDeclaration>> FindAll
                         .Where(type => type.ImplementedInterfaces.Contains(intrface)).ToList());
         }
 
-        private IDictionary<ICanBeInterfaceMember, List<ModuleBodyElementDeclaration>> FindAllImplementingMembersByMember()
+        private IDictionary<IInterfaceExposable, List<ModuleBodyElementDeclaration>> FindAllImplementingMembersByMember()
         {
             var implementations = _implementingMembers.Value.AllValues();
             return implementations
-                .GroupBy(member => (ICanBeInterfaceMember)member.InterfaceMemberImplemented)
+                
+                .GroupBy(member => (IInterfaceExposable)member.InterfaceMemberImplemented)
                 .ToDictionary(member => member.Key, member => member.ToList());
         }
 
@@ -231,7 +233,7 @@ private IDictionary<ClassModuleDeclaration, List<Declaration>> FindAllIinterface
                 .ToDictionary(
                     module => module,
                     module => module.Members
-                        .Where(member => member is ICanBeInterfaceMember candidate && candidate.IsInterfaceMember)
+                        .Where(member => member is IInterfaceExposable candidate && candidate.IsInterfaceMember)
                         .ToList());
         }
 
@@ -402,9 +404,10 @@ public IEnumerable<Declaration> FindInterfaceMembersForImplementsContext(VBAPars
         /// <returns>The selected interface if found, null if not found.</returns>
         public ClassModuleDeclaration FindInterface(QualifiedSelection selection)
         {
-            return FindAllUserInterfaces().FirstOrDefault(declaration => declaration.References
-                .Where(refrnce => refrnce.Context.GetAncestor<VBAParser.ImplementsStmtContext>() != null)
-                .Any(reference => ReferenceEquals(reference.Declaration, declaration)));
+            return FindAllUserInterfaces()
+                .FirstOrDefault(declaration => declaration.References
+                    .Any(reference => reference.Context.GetAncestor<VBAParser.ImplementsStmtContext>() != null 
+                                      && ReferenceEquals(reference.Declaration, declaration)));
         }
 
         /// <summary>
@@ -454,7 +457,7 @@ public IEnumerable<ModuleBodyElementDeclaration> FindAllInterfaceImplementingMem
         /// <returns>All concrete implementations of the passed interface declaration.</returns>
         public IEnumerable<ModuleBodyElementDeclaration> FindInterfaceImplementationMembers(Declaration interfaceMember)
         {
-            if (!(interfaceMember is ICanBeInterfaceMember member))
+            if (!(interfaceMember is IInterfaceExposable member))
             {
                 return Enumerable.Empty<ModuleBodyElementDeclaration>();
             }
@@ -1131,18 +1134,17 @@ private static bool IsSubroutineOrProperty(Declaration declaration)
         /// <summary>
         /// Creates a dictionary of identifier references, keyed by module.
         /// </summary>
-        public IReadOnlyDictionary<QualifiedModuleName,IEnumerable<IdentifierReference>> IdentifierReferences()
+        public IReadOnlyDictionary<QualifiedModuleName, IReadOnlyList<IdentifierReference>> IdentifierReferences()
         {
-            return new ReadOnlyDictionary<QualifiedModuleName, IEnumerable<IdentifierReference>>(
-                _referencesByModule.ToDictionary(kvp => kvp.Key, kvp => kvp.Value.AsEnumerable()));
+            return _referencesByModule;
         }
 
         /// <summary>
         /// Gets all identifier references in the specified module.
         /// </summary>
         public IEnumerable<IdentifierReference> IdentifierReferences(QualifiedModuleName module)
         {
-            return _referencesByModule.TryGetValue(module, out List<IdentifierReference> value)
+            return _referencesByModule.TryGetValue(module, out var value)
                 ? value
                 : Enumerable.Empty<IdentifierReference>();
         }

Rubberduck.Parsing/Symbols/FunctionDeclaration.cs

@@ -65,7 +65,7 @@ public FunctionDeclaration(ComMember member, Declaration parent, QualifiedModule
         }
 
         /// <inheritdoc/>
-        protected override bool Implements(ICanBeInterfaceMember member)
+        protected override bool Implements(IInterfaceExposable member)
         {
             if (ReferenceEquals(member, this))
             {
@@ -74,8 +74,8 @@ protected override bool Implements(ICanBeInterfaceMember member)
 
             return member.DeclarationType == DeclarationType.Function
                 && member.IsInterfaceMember
-                && IsInterfaceImplementation
-                && IdentifierName.Equals(member.ImplementingIdentifierName);
+                && IdentifierName.Equals(member.ImplementingIdentifierName)
+                   && ((ClassModuleDeclaration)member.ParentDeclaration).Subtypes.Any(implementation => ReferenceEquals(implementation, ParentDeclaration));
         }
     }
 }

Rubberduck.Parsing/Symbols/ICanBeInterfaceMember.cs renamed to Rubberduck.Parsing/Symbols/IInterfaceExposable.cs

@@ -1,6 +1,6 @@
 namespace Rubberduck.Parsing.Symbols
 {
-    public interface ICanBeInterfaceMember
+    public interface IInterfaceExposable
     {
         /// <summary>
         /// Returns true if the member is part of an interface definition.
@@ -22,31 +22,31 @@ public interface ICanBeInterfaceMember
         string AsTypeName { get; }
     }
 
-    internal static class InterfaceMemberExtensions
+    internal static class InterfaceExposableExtensions
     {
         /// <summary>
-        /// Provides a default implementation of ICanBeInterfaceMember.IsInterfaceMember
+        /// Provides a default implementation of IInterfaceExposable.IsInterfaceMember
         /// </summary>
         /// <param name="member">The member to test.</param>
         /// <returns>Returns true if the member is part of an interface definition.</returns>
-        internal static bool IsInterfaceMember(this ICanBeInterfaceMember member) =>
+        internal static bool IsInterfaceMember(this IInterfaceExposable member) =>
             (member.Accessibility == Accessibility.Public || member.Accessibility == Accessibility.Implicit) &&
             member.InterfaceDeclaration != null;
 
         /// <summary>
-        /// Provides a default implementation of ICanBeInterfaceMember.InterfaceDeclaration
+        /// Provides a default implementation of IInterfaceExposable.InterfaceDeclaration
         /// </summary>
         /// <param name="member">The member to find the InterfaceDeclaration of.</param>
         /// <returns>Tthe Declaration of the interface that this is a member of, or null if IsInterfaceMember is false.</returns>
-        internal static ClassModuleDeclaration InterfaceDeclaration(this ICanBeInterfaceMember member) =>
+        internal static ClassModuleDeclaration InterfaceDeclaration(this IInterfaceExposable member) =>
             member.ParentDeclaration is ClassModuleDeclaration parent && parent.IsInterface ? parent : null;
 
         /// <summary>
-        /// Provides a default implementation of ICanBeInterfaceMember.ImplementingIdentifierName
+        /// Provides a default implementation of IInterfaceExposable.ImplementingIdentifierName
         /// </summary>
         /// <param name="member">The member to find the ImplementingIdentifierName of.</param>
         /// <returns>The identifier name of members implementing this member, or null if IsInterfaceMember is false.</returns>
-        internal static string ImplementingIdentifierName(this ICanBeInterfaceMember member) =>
+        internal static string ImplementingIdentifierName(this IInterfaceExposable member) =>
             member.InterfaceDeclaration != null
             ? $"{member.InterfaceDeclaration.IdentifierName}_{member.IdentifierName}"
             : null;

Rubberduck.Parsing/Symbols/ModuleBodyElementDeclaration.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Xml;
 using Antlr4.Runtime;
 using Rubberduck.Parsing.Annotations;
 using Rubberduck.Parsing.Grammar;
@@ -9,7 +10,7 @@
 
 namespace Rubberduck.Parsing.Symbols
 {
-    public abstract class ModuleBodyElementDeclaration : Declaration, IParameterizedDeclaration, ICanBeInterfaceMember, ICanBeDefaultMember
+    public abstract class ModuleBodyElementDeclaration : Declaration, IParameterizedDeclaration, IInterfaceExposable, ICanBeDefaultMember
     {
         protected ModuleBodyElementDeclaration(
             QualifiedMemberName name,
@@ -100,7 +101,7 @@ protected void AddParameters(IEnumerable<ParameterDeclaration> parameters)
         /// </summary>
         /// <param name="interfaceMember">The member to test for implementation of.</param>
         /// <returns>True if this Declaration is a concrete implementation of interfaceMember.</returns>
-        protected abstract bool Implements(ICanBeInterfaceMember interfaceMember);
+        protected abstract bool Implements(IInterfaceExposable interfaceMember);
 
         /// <inheritdoc/>
         public bool IsInterfaceMember => this.IsInterfaceMember();
@@ -133,18 +134,32 @@ private static (bool IsInterfaceImplementation, ClassModuleDeclaration Interface
                 return (false, null);
             }
 
+            /*
+             * The following MS-VBAL rule effectively limits the the IdentifierName of an implemented interface member to the last token in 'identifiers':
+             *
+             * 5.2.4.2 - A class may not be used as an interface class if the names of any of its public variable or method
+             * methods contain an underscore character (Unicode u+005F).
+             *
+             * Note that there is NOT a corresponding restriction on the <class-type-name>. 
+             */
+
             var supertype = string.Join("_", identifiers.Take(identifiers.Length - 1));
 
             var implements = classModule.Supertypes.Cast<ClassModuleDeclaration>().FirstOrDefault(intrface =>
                 intrface.IdentifierName.Equals(supertype)
                 && intrface.References.Any(reference => ReferenceEquals(reference.ParentScoping, classModule)));
 
-            return (implements != null, implements);
+            if (implements == null)
+            {
+                return (false, null);
+            }
+
+            return implements.Members.Any(member => element.Implements(member as IInterfaceExposable)) ? (true, implements) : (false, null);
         }
 
         private static Declaration MemberImplemented(ModuleBodyElementDeclaration element)
         {
-            return element.InterfaceImplemented?.Members.FirstOrDefault(member => element.Implements(member as ICanBeInterfaceMember));
+            return element.InterfaceImplemented?.Members.FirstOrDefault(member => element.Implements(member as IInterfaceExposable));
         }
     }
 }

Rubberduck.Parsing/Symbols/ModuleDeclaration.cs

@@ -43,7 +43,7 @@ protected ModuleDeclaration(
         private readonly List<Declaration> _members = new List<Declaration>();
         public IReadOnlyList<Declaration> Members => _members;
 
-        public void AddMember(Declaration member)
+        internal void AddMember(Declaration member)
         {
             _members.Add(member);
         }

Rubberduck.Parsing/Symbols/PropertyDeclaration.cs

@@ -48,6 +48,6 @@ protected PropertyDeclaration(
             base.IsObject || (Parameters.OrderBy(p => p.Selection).LastOrDefault()?.IsObject ?? false);
 
         /// <inheritdoc/>
-        protected abstract override bool Implements(ICanBeInterfaceMember member);
+        protected abstract override bool Implements(IInterfaceExposable member);
     }
 }

Rubberduck.Parsing/Symbols/PropertyGetDeclaration.cs

@@ -83,7 +83,7 @@ public PropertyGetDeclaration(ComField field, Declaration parent, QualifiedModul
         { }
 
         /// <inheritdoc/>
-        protected override bool Implements(ICanBeInterfaceMember member)
+        protected override bool Implements(IInterfaceExposable member)
         {
             if (ReferenceEquals(member, this))
             {
@@ -92,7 +92,7 @@ protected override bool Implements(ICanBeInterfaceMember member)
 
             return (member.DeclarationType == DeclarationType.PropertyGet || member.DeclarationType == DeclarationType.Variable)
                    && member.IsInterfaceMember
-                   && IsInterfaceImplementation
+                   && ((ClassModuleDeclaration)member.ParentDeclaration).Subtypes.Any(implementation => ReferenceEquals(implementation, ParentDeclaration))
                    && IdentifierName.Equals(member.ImplementingIdentifierName);
         }
     }

Rubberduck.Parsing/Symbols/PropertyLetDeclaration.cs

@@ -73,16 +73,16 @@ public PropertyLetDeclaration(ComField field, Declaration parent, QualifiedModul
         { }
 
         /// <inheritdoc/>
-        protected override bool Implements(ICanBeInterfaceMember member)
+        protected override bool Implements(IInterfaceExposable member)
         {
             if (ReferenceEquals(member, this))
             {
                 return false;
             }
 
             return member.IsInterfaceMember
-                   && IsInterfaceImplementation
                    && IdentifierName.Equals(member.ImplementingIdentifierName)
+                   && ((ClassModuleDeclaration)member.ParentDeclaration).Subtypes.Any(implementation => ReferenceEquals(implementation, ParentDeclaration))
                    && (member.DeclarationType == DeclarationType.PropertyLet
                        || member.DeclarationType == DeclarationType.Variable
                        && !member.IsObject);