Merge pull request #3362 from retailcoder/IllegalAnnotationFix

retailcoder · web-flow · commit 17e364ab17f0 · 2017-09-08T23:49:11.000-04:00
Fixes IllegalAnnotationInspection false positives (#3339)
diff --git a/.gitignore b/.gitignore
@@ -6,6 +6,7 @@
 *.user
 *.sln.docstates
 *.csproj.user
+*.csproj.DotSettings
 
 # External NuGet Packages
 [Pp]ackages/
diff --git a/.nuget/NuGet.exe b/.nuget/NuGet.exe
diff --git a/RetailCoder.VBE/Rubberduck.csproj.DotSettings b/RetailCoder.VBE/Rubberduck.csproj.DotSettings
@@ -1,4 +1,4 @@
 ﻿<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
-	<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">CSharp60</s:String>
+	<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">CSharp70</s:String>
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=ui_005Crefactorings_005Cextractinterface/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=unittesting_005Cstubs/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
diff --git a/Rubberduck.Inspections/Concrete/IllegalAnnotationInspection.cs b/Rubberduck.Inspections/Concrete/IllegalAnnotationInspection.cs
@@ -28,7 +28,6 @@ public IllegalAnnotationInspection(RubberduckParserState state)
 
         public override CodeInspectionType InspectionType => CodeInspectionType.RubberduckOpportunities;
         public override IInspectionListener Listener { get; }
-        public override ParsePass Pass => ParsePass.AttributesPass;
 
         public override IEnumerable<IInspectionResult> GetInspectionResults()
         {
@@ -39,10 +38,11 @@ public override IEnumerable<IInspectionResult> GetInspectionResults()
 
         public class IllegalAttributeAnnotationsListener : VBAParserBaseListener, IInspectionListener
         {
-            private readonly IDictionary<AnnotationType, int> _annotationCounts;
-
             private static readonly AnnotationType[] AnnotationTypes = Enum.GetValues(typeof(AnnotationType)).Cast<AnnotationType>().ToArray();
 
+            private IDictionary<Tuple<QualifiedModuleName, AnnotationType>, int> _annotationCounts = 
+                new Dictionary<Tuple<QualifiedModuleName, AnnotationType>, int>();
+
             private readonly RubberduckParserState _state;
 
             private Lazy<Declaration> _module;
@@ -51,32 +51,39 @@ public class IllegalAttributeAnnotationsListener : VBAParserBaseListener, IInspe
             public IllegalAttributeAnnotationsListener(RubberduckParserState state)
             {
                 _state = state;
-                _annotationCounts = AnnotationTypes.ToDictionary(a => a, a => 0);
             }
 
             private readonly List<QualifiedContext<ParserRuleContext>> _contexts =
                 new List<QualifiedContext<ParserRuleContext>>();
 
             public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
 
-            public QualifiedModuleName CurrentModuleName { get; set; }
+            public QualifiedModuleName CurrentModuleName
+            {
+                get => _currentModuleName;
+                set
+                {
+                    _currentModuleName = value;
+                    foreach (var type in AnnotationTypes)
+                    {
+                        _annotationCounts.Add(Tuple.Create(value, type), 0);
+                    }
+                }
+            }
 
             private bool _isFirstMemberProcessed;
 
             public void ClearContexts()
             {
+                _annotationCounts = new Dictionary<Tuple<QualifiedModuleName, AnnotationType>, int>();
                 _contexts.Clear();
                 _isFirstMemberProcessed = false;
-                var keys = _annotationCounts.Keys.ToList();
-                foreach (var key in keys)
-                {
-                    _annotationCounts[key] = 0;
-                }
             }
 
             #region scoping
             private Declaration _currentScopeDeclaration;
             private bool _hasMembers;
+            private QualifiedModuleName _currentModuleName;
 
             private void SetCurrentScope(string memberName = null)
             {
@@ -160,7 +167,8 @@ public override void ExitAnnotation(VBAParser.AnnotationContext context)
             {
                 var name = Identifier.GetName(context.annotationName().unrestrictedIdentifier());
                 var annotationType = (AnnotationType) Enum.Parse(typeof (AnnotationType), name);
-                _annotationCounts[annotationType]++;
+                var key = Tuple.Create(_currentModuleName, annotationType);
+                _annotationCounts[key]++;
 
                 var moduleHasMembers = _members.Value.Any();
 
@@ -174,7 +182,7 @@ public override void ExitAnnotation(VBAParser.AnnotationContext context)
                     && (_currentScopeDeclaration?.DeclarationType.HasFlag(DeclarationType.Member) ?? false);
 
                 var isIllegal = !(isMemberAnnotation && moduleHasMembers && !_isFirstMemberProcessed) &&
-                                (isModuleAnnotation && _annotationCounts[annotationType] > 1
+                                (isModuleAnnotation && _annotationCounts[key] > 1
                                  || isMemberAnnotatedForModuleAnnotation
                                  || isModuleAnnotatedForMemberAnnotation);
 
diff --git a/RubberduckTests/Inspections/IllegalAnnotationsInspectionTests.cs b/RubberduckTests/Inspections/IllegalAnnotationsInspectionTests.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Linq;
 using System.Threading;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -51,6 +52,32 @@ Option Explicit
             Assert.IsFalse(inspectionResults.Any());
         }
 
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void GivenOneIlegalModuleAnnotationAcrossModules_OneResult()
+        {
+            const string inputCode1 = @"
+Option Explicit
+'@Folder(""Legal"")
+
+Sub DoSomething()
+'@Folder(""Illegal"")
+End Sub
+";
+            const string inputCode2 = @"
+Option Explicit
+'@Folder(""Legal"")
+";
+            var vbe = MockVbeBuilder.BuildFromStdModules(("Module1", inputCode1), ("Module2", inputCode2));
+            var state = MockParser.CreateAndParse(vbe.Object);
+
+            var inspection = new IllegalAnnotationInspection(state);
+            var inspector = InspectionsHelper.GetInspector(inspection);
+            var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+
+            Assert.AreEqual(1, inspectionResults.Count());
+        }
+
         [TestMethod]
         [TestCategory("Inspections")]
         public void GivenTestModule_NoResult()
diff --git a/RubberduckTests/Mocks/MockVbeBuilder.cs b/RubberduckTests/Mocks/MockVbeBuilder.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
@@ -9,7 +10,7 @@
 namespace RubberduckTests.Mocks
 {
     /// <summary>
-    /// Builds a mock <see cref="VBE"/>.
+    /// Builds a mock <see cref="IVBE"/>.
     /// </summary>
     public class MockVbeBuilder
     {
@@ -51,7 +52,7 @@ public MockVbeBuilder()
         /// Adds a project to the mock VBE.
         /// Use a <see cref="MockProjectBuilder"/> to build the <see cref="project"/>.
         /// </summary>
-        /// <param name="project">A mock <see cref="VBProject"/>.</param>
+        /// <param name="project">A mock <see cref="IVBProject"/>.</param>
         /// <returns>Returns the <see cref="MockVbeBuilder"/> instance.</returns>
         public MockVbeBuilder AddProject(Mock<IVBProject> project)
         {
@@ -92,22 +93,22 @@ public MockProjectBuilder ProjectBuilder(string name, string filename, string pr
         }
 
         /// <summary>
-        /// Gets the mock <see cref="VBE"/> instance.
+        /// Gets the mock <see cref="IVBE"/> instance.
         /// </summary>
         public Mock<IVBE> Build()
         {
             return _vbe;
         }
 
         /// <summary>
-        /// Gets a mock <see cref="VBE"/> instance, 
-        /// containing a single "TestProject1" <see cref="VBProject"/>
-        /// and a single "TestModule1" <see cref="VBComponent"/>, with the specified <see cref="content"/>.
+        /// Gets a mock <see cref="IVBE"/> instance, 
+        /// containing a single "TestProject1" <see cref="IVBProject"/>
+        /// and a single "TestModule1" <see cref="IVBComponent"/>, with the specified <see cref="content"/>.
         /// </summary>
         /// <param name="content">The VBA code associated to the component.</param>
-        /// <param name="component">The created <see cref="VBComponent"/></param>
-        /// <param name="module">The created <see cref="CodeModule"/></param>
-        /// <param name="selection"></param>
+        /// <param name="component">The created <see cref="IVBComponent"/></param>
+        /// <param name="selection">Specifies user selection in the editor.</param>
+        /// <param name="referenceStdLibs">Specifies whether standard libraries are referenced.</param>
         /// <returns></returns>
         public static Mock<IVBE> BuildFromSingleStandardModule(string content, out IVBComponent component, Selection selection = default(Selection), bool referenceStdLibs = false)
         {
@@ -147,6 +148,30 @@ public Mock<IVBE> Build()
             return vbe;
         }
 
+        /// <summary>
+        /// Builds a mock VBE containing multiple standard modules.
+        /// </summary>
+        public static Mock<IVBE> BuildFromStdModules(params (string name, string content)[] modules)
+        {
+            var vbeBuilder = new MockVbeBuilder();
+
+            var builder = vbeBuilder.ProjectBuilder(TestProjectName, ProjectProtection.Unprotected);
+            foreach (var module in modules)
+            {
+                builder.AddComponent(module.name, ComponentType.StandardModule, module.content);
+            }
+
+            var project = builder.Build();
+            var vbe = vbeBuilder.AddProject(project).Build();
+
+            var component = project.Object.VBComponents[0];
+
+            vbe.Object.ActiveVBProject = project.Object;
+            vbe.Object.ActiveCodePane = component.CodeModule.CodePane;
+
+            return vbe;
+        }
+
         private Mock<IVBE> CreateVbeMock()
         {
             var vbe = new Mock<IVBE>();
diff --git a/RubberduckTests/RubberduckTests.csproj b/RubberduckTests/RubberduckTests.csproj
@@ -61,6 +61,9 @@
     <Reference Include="PresentationCore" />
     <Reference Include="PresentationFramework" />
     <Reference Include="System" />
+    <Reference Include="System.ValueTuple, Version=4.0.1.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
+      <HintPath>..\packages\System.ValueTuple.4.3.0\lib\netstandard1.0\System.ValueTuple.dll</HintPath>
+    </Reference>
     <Reference Include="System.Windows.Forms" />
     <Reference Include="System.Xaml" />
     <Reference Include="WindowsBase" />
diff --git a/RubberduckTests/packages.config b/RubberduckTests/packages.config
@@ -4,4 +4,5 @@
   <package id="Moq" version="4.2.1507.0118" targetFramework="net45" />
   <package id="Ninject" version="3.2.2.0" targetFramework="net45" />
   <package id="NLog" version="4.0.1" targetFramework="net45" />
-</packages>
+  <package id="System.ValueTuple" version="4.3.0" targetFramework="net45" />
+</packages>
