Skip to content

Commit 17e364a

Browse files
retailcoderretailcoder
authored
Merge pull request #3362 from retailcoder/IllegalAnnotationFix
Fixes IllegalAnnotationInspection false positives (#3339)
2 parents 66c4d43 + d118b13 commit 17e364a

File tree

8 files changed

+88
-23
lines changed

8 files changed

+88
-23
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*.user
77
*.sln.docstates
88
*.csproj.user
9+
*.csproj.DotSettings
910

1011
# External NuGet Packages
1112
[Pp]ackages/

‎.nuget/NuGet.exe

2.8 MB
Binary file not shown.

‎RetailCoder.VBE/Rubberduck.csproj.DotSettings

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
<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">
2-
<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">CSharp60</s:String>
2+
<s:String x:Key="/Default/CodeInspection/CSharpLanguageProject/LanguageLevel/@EntryValue">CSharp70</s:String>
33
<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=ui_005Crefactorings_005Cextractinterface/@EntryIndexedValue">True</s:Boolean>
44
<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=unittesting_005Cstubs/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

‎Rubberduck.Inspections/Concrete/IllegalAnnotationInspection.cs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public IllegalAnnotationInspection(RubberduckParserState state)
2828

2929
public override CodeInspectionType InspectionType => CodeInspectionType.RubberduckOpportunities;
3030
public override IInspectionListener Listener { get; }
31-
public override ParsePass Pass => ParsePass.AttributesPass;
3231

3332
public override IEnumerable<IInspectionResult> GetInspectionResults()
3433
{
@@ -39,10 +38,11 @@ public override IEnumerable<IInspectionResult> GetInspectionResults()
3938

4039
public class IllegalAttributeAnnotationsListener : VBAParserBaseListener, IInspectionListener
4140
{
42-
private readonly IDictionary<AnnotationType, int> _annotationCounts;
43-
4441
private static readonly AnnotationType[] AnnotationTypes = Enum.GetValues(typeof(AnnotationType)).Cast<AnnotationType>().ToArray();
4542

43+
private IDictionary<Tuple<QualifiedModuleName, AnnotationType>, int> _annotationCounts =
44+
new Dictionary<Tuple<QualifiedModuleName, AnnotationType>, int>();
45+
4646
private readonly RubberduckParserState _state;
4747

4848
private Lazy<Declaration> _module;
@@ -51,32 +51,39 @@ public class IllegalAttributeAnnotationsListener : VBAParserBaseListener, IInspe
5151
public IllegalAttributeAnnotationsListener(RubberduckParserState state)
5252
{
5353
_state = state;
54-
_annotationCounts = AnnotationTypes.ToDictionary(a => a, a => 0);
5554
}
5655

5756
private readonly List<QualifiedContext<ParserRuleContext>> _contexts =
5857
new List<QualifiedContext<ParserRuleContext>>();
5958

6059
public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
6160

62-
public QualifiedModuleName CurrentModuleName { get; set; }
61+
public QualifiedModuleName CurrentModuleName
62+
{
63+
get => _currentModuleName;
64+
set
65+
{
66+
_currentModuleName = value;
67+
foreach (var type in AnnotationTypes)
68+
{
69+
_annotationCounts.Add(Tuple.Create(value, type), 0);
70+
}
71+
}
72+
}
6373

6474
private bool _isFirstMemberProcessed;
6575

6676
public void ClearContexts()
6777
{
78+
_annotationCounts = new Dictionary<Tuple<QualifiedModuleName, AnnotationType>, int>();
6879
_contexts.Clear();
6980
_isFirstMemberProcessed = false;
70-
var keys = _annotationCounts.Keys.ToList();
71-
foreach (var key in keys)
72-
{
73-
_annotationCounts[key] = 0;
74-
}
7581
}
7682

7783
#region scoping
7884
private Declaration _currentScopeDeclaration;
7985
private bool _hasMembers;
86+
private QualifiedModuleName _currentModuleName;
8087

8188
private void SetCurrentScope(string memberName = null)
8289
{
@@ -160,7 +167,8 @@ public override void ExitAnnotation(VBAParser.AnnotationContext context)
160167
{
161168
var name = Identifier.GetName(context.annotationName().unrestrictedIdentifier());
162169
var annotationType = (AnnotationType) Enum.Parse(typeof (AnnotationType), name);
163-
_annotationCounts[annotationType]++;
170+
var key = Tuple.Create(_currentModuleName, annotationType);
171+
_annotationCounts[key]++;
164172

165173
var moduleHasMembers = _members.Value.Any();
166174

@@ -174,7 +182,7 @@ public override void ExitAnnotation(VBAParser.AnnotationContext context)
174182
&& (_currentScopeDeclaration?.DeclarationType.HasFlag(DeclarationType.Member) ?? false);
175183

176184
var isIllegal = !(isMemberAnnotation && moduleHasMembers && !_isFirstMemberProcessed) &&
177-
(isModuleAnnotation && _annotationCounts[annotationType] > 1
185+
(isModuleAnnotation && _annotationCounts[key] > 1
178186
|| isMemberAnnotatedForModuleAnnotation
179187
|| isModuleAnnotatedForMemberAnnotation);
180188

‎RubberduckTests/Inspections/IllegalAnnotationsInspectionTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Linq;
23
using System.Threading;
34
using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -51,6 +52,32 @@ Option Explicit
5152
Assert.IsFalse(inspectionResults.Any());
5253
}
5354

55+
[TestMethod]
56+
[TestCategory("Inspections")]
57+
public void GivenOneIlegalModuleAnnotationAcrossModules_OneResult()
58+
{
59+
const string inputCode1 = @"
60+
Option Explicit
61+
'@Folder(""Legal"")
62+
63+
Sub DoSomething()
64+
'@Folder(""Illegal"")
65+
End Sub
66+
";
67+
const string inputCode2 = @"
68+
Option Explicit
69+
'@Folder(""Legal"")
70+
";
71+
var vbe = MockVbeBuilder.BuildFromStdModules(("Module1", inputCode1), ("Module2", inputCode2));
72+
var state = MockParser.CreateAndParse(vbe.Object);
73+
74+
var inspection = new IllegalAnnotationInspection(state);
75+
var inspector = InspectionsHelper.GetInspector(inspection);
76+
var inspectionResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
77+
78+
Assert.AreEqual(1, inspectionResults.Count());
79+
}
80+
5481
[TestMethod]
5582
[TestCategory("Inspections")]
5683
public void GivenTestModule_NoResult()

‎RubberduckTests/Mocks/MockVbeBuilder.cs

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections;
23
using System.Collections.Generic;
34
using System.Linq;
@@ -9,7 +10,7 @@
910
namespace RubberduckTests.Mocks
1011
{
1112
/// <summary>
12-
/// Builds a mock <see cref="VBE"/>.
13+
/// Builds a mock <see cref="IVBE"/>.
1314
/// </summary>
1415
public class MockVbeBuilder
1516
{
@@ -51,7 +52,7 @@ public MockVbeBuilder()
5152
/// Adds a project to the mock VBE.
5253
/// Use a <see cref="MockProjectBuilder"/> to build the <see cref="project"/>.
5354
/// </summary>
54-
/// <param name="project">A mock <see cref="VBProject"/>.</param>
55+
/// <param name="project">A mock <see cref="IVBProject"/>.</param>
5556
/// <returns>Returns the <see cref="MockVbeBuilder"/> instance.</returns>
5657
public MockVbeBuilder AddProject(Mock<IVBProject> project)
5758
{
@@ -92,22 +93,22 @@ public MockProjectBuilder ProjectBuilder(string name, string filename, string pr
9293
}
9394

9495
/// <summary>
95-
/// Gets the mock <see cref="VBE"/> instance.
96+
/// Gets the mock <see cref="IVBE"/> instance.
9697
/// </summary>
9798
public Mock<IVBE> Build()
9899
{
99100
return _vbe;
100101
}
101102

102103
/// <summary>
103-
/// Gets a mock <see cref="VBE"/> instance,
104-
/// containing a single "TestProject1" <see cref="VBProject"/>
105-
/// and a single "TestModule1" <see cref="VBComponent"/>, with the specified <see cref="content"/>.
104+
/// Gets a mock <see cref="IVBE"/> instance,
105+
/// containing a single "TestProject1" <see cref="IVBProject"/>
106+
/// and a single "TestModule1" <see cref="IVBComponent"/>, with the specified <see cref="content"/>.
106107
/// </summary>
107108
/// <param name="content">The VBA code associated to the component.</param>
108-
/// <param name="component">The created <see cref="VBComponent"/></param>
109-
/// <param name="module">The created <see cref="CodeModule"/></param>
110-
/// <param name="selection"></param>
109+
/// <param name="component">The created <see cref="IVBComponent"/></param>
110+
/// <param name="selection">Specifies user selection in the editor.</param>
111+
/// <param name="referenceStdLibs">Specifies whether standard libraries are referenced.</param>
111112
/// <returns></returns>
112113
public static Mock<IVBE> BuildFromSingleStandardModule(string content, out IVBComponent component, Selection selection = default(Selection), bool referenceStdLibs = false)
113114
{
@@ -147,6 +148,30 @@ public Mock<IVBE> Build()
147148
return vbe;
148149
}
149150

151+
/// <summary>
152+
/// Builds a mock VBE containing multiple standard modules.
153+
/// </summary>
154+
public static Mock<IVBE> BuildFromStdModules(params (string name, string content)[] modules)
155+
{
156+
var vbeBuilder = new MockVbeBuilder();
157+
158+
var builder = vbeBuilder.ProjectBuilder(TestProjectName, ProjectProtection.Unprotected);
159+
foreach (var module in modules)
160+
{
161+
builder.AddComponent(module.name, ComponentType.StandardModule, module.content);
162+
}
163+
164+
var project = builder.Build();
165+
var vbe = vbeBuilder.AddProject(project).Build();
166+
167+
var component = project.Object.VBComponents[0];
168+
169+
vbe.Object.ActiveVBProject = project.Object;
170+
vbe.Object.ActiveCodePane = component.CodeModule.CodePane;
171+
172+
return vbe;
173+
}
174+
150175
private Mock<IVBE> CreateVbeMock()
151176
{
152177
var vbe = new Mock<IVBE>();

‎RubberduckTests/RubberduckTests.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@
6161
<Reference Include="PresentationCore" />
6262
<Reference Include="PresentationFramework" />
6363
<Reference Include="System" />
64+
<Reference Include="System.ValueTuple, Version=4.0.1.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
65+
<HintPath>..\packages\System.ValueTuple.4.3.0\lib\netstandard1.0\System.ValueTuple.dll</HintPath>
66+
</Reference>
6467
<Reference Include="System.Windows.Forms" />
6568
<Reference Include="System.Xaml" />
6669
<Reference Include="WindowsBase" />

‎RubberduckTests/packages.config

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@
44
<package id="Moq" version="4.2.1507.0118" targetFramework="net45" />
55
<package id="Ninject" version="3.2.2.0" targetFramework="net45" />
66
<package id="NLog" version="4.0.1" targetFramework="net45" />
7-
</packages>
7+
<package id="System.ValueTuple" version="4.3.0" targetFramework="net45" />
8+
</packages>

0 commit comments

Comments
 (0)