Skip to content

Commit 06cd3c9

Browse files
rkapkarkapka
committed
Duplicated annotation enhancements
1 parent 99241e5 commit 06cd3c9

File tree

5 files changed

+148
-60
lines changed

5 files changed

+148
-60
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Concrete/DuplicatedAnnotationInspection.cs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22
using System.Linq;
33
using Rubberduck.Inspections.Abstract;
44
using Rubberduck.Inspections.Results;
5-
using Rubberduck.Parsing.Annotations;
65
using Rubberduck.Parsing.Inspections.Abstract;
7-
using Rubberduck.Parsing.Symbols;
86
using Rubberduck.Parsing.VBA;
97
using Rubberduck.Resources.Inspections;
108

@@ -24,16 +22,19 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
2422
{
2523
var duplicateAnnotations = declaration.Annotations
2624
.GroupBy(annotation => annotation.AnnotationType)
27-
.Where(group => !group.First().AllowMultiple && group.Count() > 1 &&
28-
(group.Key.HasFlag(AnnotationType.ModuleAnnotation) &&
29-
declaration.DeclarationType.HasFlag(DeclarationType.Module) ||
30-
group.Key.HasFlag(AnnotationType.MemberAnnotation) &&
31-
declaration.DeclarationType.HasFlag(DeclarationType.Member)));
25+
.Where(group => !group.First().AllowMultiple && group.Count() > 1);
3226

33-
issues.AddRange(duplicateAnnotations.Select(duplicate => new DeclarationInspectionResult(
34-
this,
35-
string.Format(InspectionResults.DuplicatedAnnotationInspection, duplicate.Key.ToString()),
36-
declaration)));
27+
issues.AddRange(duplicateAnnotations.Select(duplicate =>
28+
{
29+
var result = new DeclarationInspectionResult(
30+
this,
31+
string.Format(InspectionResults.DuplicatedAnnotationInspection, duplicate.Key.ToString()),
32+
declaration);
33+
34+
result.Properties.AnnotationType = duplicate.Key;
35+
36+
return result;
37+
}));
3738
}
3839

3940
return issues;

‎Rubberduck.CodeAnalysis/QuickFixes/RemoveDuplicatedAnnotationQuickFix.cs

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Linq;
34
using Rubberduck.Inspections.Abstract;
45
using Rubberduck.Inspections.Concrete;
6+
using Rubberduck.Parsing.Annotations;
57
using Rubberduck.Parsing.Grammar;
68
using Rubberduck.Parsing.Inspections.Abstract;
9+
using Rubberduck.Parsing.Rewriter;
710
using Rubberduck.Parsing.VBA;
811

912
namespace Rubberduck.Inspections.QuickFixes
@@ -23,18 +26,34 @@ public override void Fix(IInspectionResult result)
2326
var rewriter = _state.GetRewriter(result.QualifiedSelection.QualifiedName);
2427

2528
var duplicateAnnotations = result.Target.Annotations
26-
.GroupBy(annotation => annotation.AnnotationType)
27-
.Where(group => !group.First().AllowMultiple && group.Count() > 1)
28-
.SelectMany(group => group.Skip(1));
29+
.Where(annotation => annotation.AnnotationType == result.Properties.AnnotationType)
30+
.OrderBy(annotation => annotation.Context.Start.StartIndex)
31+
.Skip(1);
32+
33+
var duplicatesPerAnnotationList = duplicateAnnotations
34+
.Select(annotation => (VBAParser.AnnotationListContext) annotation.Context.Parent)
35+
.Distinct()
36+
.ToDictionary(list => list, _ => 0);
2937

3038
foreach (var annotation in duplicateAnnotations)
3139
{
32-
// Remove also the annotation marker
3340
var annotationList = (VBAParser.AnnotationListContext)annotation.Context.Parent;
34-
var index = Array.IndexOf(annotationList.annotation(), annotation.Context);
35-
rewriter.Remove(annotationList.AT(index));
41+
42+
RemoveAnnotationMarker(annotationList, annotation, rewriter);
43+
RemoveWhiteSpaceAfterAnnotation(annotationList, annotation, rewriter);
3644

3745
rewriter.Remove(annotation.Context);
46+
47+
duplicatesPerAnnotationList[annotationList]++;
48+
}
49+
50+
foreach (var pair in duplicatesPerAnnotationList)
51+
{
52+
if (OnlyQuoteRemainedFromAnnotationList(pair))
53+
{
54+
rewriter.Remove(pair.Key);
55+
rewriter.Remove(((VBAParser.CommentOrAnnotationContext) pair.Key.Parent).NEWLINE());
56+
}
3857
}
3958
}
4059

@@ -44,5 +63,29 @@ public override string Description(IInspectionResult result) =>
4463
public override bool CanFixInProcedure => true;
4564
public override bool CanFixInModule => true;
4665
public override bool CanFixInProject => true;
66+
67+
private static void RemoveAnnotationMarker(VBAParser.AnnotationListContext annotationList,
68+
IAnnotation annotation, IModuleRewriter rewriter)
69+
{
70+
var index = Array.IndexOf(annotationList.annotation(), annotation.Context);
71+
rewriter.Remove(annotationList.AT(index));
72+
}
73+
74+
private static void RemoveWhiteSpaceAfterAnnotation(VBAParser.AnnotationListContext annotationList,
75+
IAnnotation annotation, IModuleRewriter rewriter)
76+
{
77+
var whitespace = annotationList.whiteSpace().FirstOrDefault(ws =>
78+
ws.Start.StartIndex == annotation.Context.Stop.StopIndex + 1);
79+
80+
if (whitespace != null)
81+
{
82+
rewriter.Remove(whitespace);
83+
}
84+
}
85+
86+
private static bool OnlyQuoteRemainedFromAnnotationList(KeyValuePair<VBAParser.AnnotationListContext, int> pair)
87+
{
88+
return pair.Key.annotation().Length == pair.Value && pair.Key.commentBody() == null;
89+
}
4790
}
4891
}

‎Rubberduck.Parsing/Annotations/AnnotationType.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ namespace Rubberduck.Parsing.Annotations
88
public enum AnnotationType
99
{
1010
/// <summary>
11-
/// A flag indicating that the annotation type is valid once per module.
11+
/// A flag indicating that the annotation type is valid for module.
1212
/// </summary>
1313
ModuleAnnotation = 1 << 1,
1414
/// <summary>
15-
/// A flag indicating that the annotation type is valid once per member.
15+
/// A flag indicating that the annotation type is valid for member.
1616
/// </summary>
1717
MemberAnnotation = 1 << 2,
1818

‎RubberduckTests/Inspections/DuplicatedAnnotationInspectionTests.cs

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ public class DuplicatedAnnotationInspectionTests
1414
public void AnnotationDuplicated_ReturnsResult()
1515
{
1616
const string inputCode = @"
17+
Public Sub Bar()
18+
End Sub
19+
1720
'@Obsolete
1821
'@Obsolete
1922
Public Sub Foo()
@@ -34,6 +37,9 @@ Public Sub Foo()
3437
public void AnnotationDuplicatedTwice_ReturnsSingleResult()
3538
{
3639
const string inputCode = @"
40+
Public Sub Bar()
41+
End Sub
42+
3743
'@Obsolete
3844
'@Obsolete
3945
'@Obsolete
@@ -52,30 +58,16 @@ Public Sub Foo()
5258

5359
[Test]
5460
[Category("Inspections")]
55-
public void AnnotationNotDuplicated_DoesNotReturnResult()
61+
public void MultipleAnnotationsDuplicated_ReturnsResult()
5662
{
5763
const string inputCode = @"
58-
'@Obsolete
59-
Public Sub Foo()
60-
End Sub";
61-
62-
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out _);
63-
using (var state = MockParser.CreateAndParse(vbe.Object))
64-
{
65-
var inspection = new DuplicatedAnnotationInspection(state);
66-
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
67-
68-
Assert.AreEqual(0, inspectionResults.Count());
69-
}
70-
}
64+
Public Sub Bar()
65+
End Sub
7166
72-
[Test]
73-
[Category("Inspections")]
74-
public void AnnotationAllowingMultipleApplicationsDuplicated_DoesNotReturnResult()
75-
{
76-
const string inputCode = @"
77-
'@Ignore(Bar)
78-
'@Ignore(Baz)
67+
'@Obsolete
68+
'@Obsolete
69+
'@TestMethod
70+
'@TestMethod
7971
Public Sub Foo()
8072
End Sub";
8173

@@ -85,20 +77,19 @@ Public Sub Foo()
8577
var inspection = new DuplicatedAnnotationInspection(state);
8678
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
8779

88-
Assert.AreEqual(0, inspectionResults.Count());
80+
Assert.AreEqual(2, inspectionResults.Count());
8981
}
9082
}
9183

9284
[Test]
9385
[Category("Inspections")]
94-
public void MemberAnnotationDuplicatedForModuleDeclaration_DoesNotReturnResult()
86+
public void AnnotationNotDuplicated_DoesNotReturnResult()
9587
{
9688
const string inputCode = @"
97-
'@TestInitialize
98-
'@TestInitialize
99-
100-
Public S as String
89+
Public Sub Bar()
90+
End Sub
10191
92+
'@Obsolete
10293
Public Sub Foo()
10394
End Sub";
10495

@@ -114,15 +105,15 @@ Public Sub Foo()
114105

115106
[Test]
116107
[Category("Inspections")]
117-
public void ModuleAnnotationDuplicatedForMemberDeclaration_DoesNotReturnResult()
108+
public void AnnotationAllowingMultipleApplicationsDuplicated_DoesNotReturnResult()
118109
{
119110
const string inputCode = @"
120-
Public Sub Foo()
111+
Public Sub Bar()
121112
End Sub
122113
123-
'@Folder(Baz)
124-
'@Folder(Baz)
125-
Public Sub Bar()
114+
'@Ignore(Bar)
115+
'@Ignore(Baz)
116+
Public Sub Foo()
126117
End Sub";
127118

128119
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out _);

‎RubberduckTests/QuickFixes/RemoveDuplicatedAnnotationQuickFixTests.cs

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using NUnit.Framework;
44
using Rubberduck.Inspections.Concrete;
55
using Rubberduck.Inspections.QuickFixes;
6+
using Rubberduck.Parsing.Annotations;
67
using RubberduckTests.Mocks;
78

89
namespace RubberduckTests.QuickFixes
@@ -21,7 +22,6 @@ Public Sub Foo
2122
End Sub";
2223

2324
const string expectedCode = @"
24-
'
2525
'@Obsolete
2626
Public Sub Foo
2727
End Sub";
@@ -49,8 +49,6 @@ Public Sub Foo
4949
End Sub";
5050

5151
const string expectedCode = @"
52-
'
53-
'
5452
'@Obsolete
5553
Public Sub Foo
5654
End Sub";
@@ -71,14 +69,14 @@ Public Sub Foo
7169
public void RemoveDuplicatedAnnotation_QuickFixWorks_RemoveDuplicateWithComment()
7270
{
7371
const string inputCode = @"
74-
'@Obsolete: Foo
7572
'@Obsolete
73+
'@Obsolete: Foo
7674
Public Sub Foo
7775
End Sub";
7876

7977
const string expectedCode = @"
80-
': Foo
8178
'@Obsolete
79+
': Foo
8280
Public Sub Foo
8381
End Sub";
8482

@@ -128,7 +126,7 @@ Public Sub Foo
128126
End Sub";
129127

130128
const string expectedCode = @"
131-
'@Obsolete
129+
'@Obsolete
132130
Public Sub Foo
133131
End Sub";
134132

@@ -148,14 +146,14 @@ Public Sub Foo
148146
public void RemoveDuplicatedAnnotation_QuickFixWorks_RemoveDuplicateFromOtherAnnotationList()
149147
{
150148
const string inputCode = @"
151-
'@Obsolete @NoIndent
152149
'@Obsolete
150+
'@Obsolete @NoIndent
153151
Public Sub Foo
154152
End Sub";
155153

156154
const string expectedCode = @"
157-
' @NoIndent
158155
'@Obsolete
156+
'@NoIndent
159157
Public Sub Foo
160158
End Sub";
161159

@@ -175,13 +173,38 @@ Public Sub Foo
175173
public void RemoveDuplicatedAnnotation_QuickFixWorks_RemoveMultipleDuplicatesFromOtherAnnotationList()
176174
{
177175
const string inputCode = @"
176+
'@Obsolete
178177
'@Obsolete @NoIndent @Obsolete
178+
Public Sub Foo
179+
End Sub";
180+
181+
const string expectedCode = @"
179182
'@Obsolete
183+
'@NoIndent
184+
Public Sub Foo
185+
End Sub";
186+
187+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component);
188+
using (var state = MockParser.CreateAndParse(vbe.Object))
189+
{
190+
var inspection = new DuplicatedAnnotationInspection(state);
191+
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
192+
193+
new RemoveDuplicatedAnnotationQuickFix(state).Fix(inspectionResults.First());
194+
Assert.AreEqual(expectedCode, state.GetRewriter(component).GetText());
195+
}
196+
}
197+
198+
[Test]
199+
[Category("QuickFixes")]
200+
public void RemoveDuplicatedAnnotation_QuickFixWorks_RemoveDuplicatesWithoutWhitespaceFromAnnotationList()
201+
{
202+
const string inputCode = @"
203+
'@Obsolete@Obsolete
180204
Public Sub Foo
181205
End Sub";
182206

183207
const string expectedCode = @"
184-
' @NoIndent
185208
'@Obsolete
186209
Public Sub Foo
187210
End Sub";
@@ -196,5 +219,35 @@ Public Sub Foo
196219
Assert.AreEqual(expectedCode, state.GetRewriter(component).GetText());
197220
}
198221
}
222+
223+
[Test]
224+
[Category("QuickFixes")]
225+
public void RemoveDuplicatedAnnotation_QuickFixWorks_RemoveDuplicatesOfOnlyOneAnnotation()
226+
{
227+
const string inputCode = @"
228+
'@Obsolete
229+
'@Obsolete
230+
'@TestMethod
231+
'@TestMethod
232+
Public Sub Foo
233+
End Sub";
234+
235+
const string expectedCode = @"
236+
'@Obsolete
237+
'@TestMethod
238+
'@TestMethod
239+
Public Sub Foo
240+
End Sub";
241+
242+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component);
243+
using (var state = MockParser.CreateAndParse(vbe.Object))
244+
{
245+
var inspection = new DuplicatedAnnotationInspection(state);
246+
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
247+
248+
new RemoveDuplicatedAnnotationQuickFix(state).Fix(inspectionResults.First(result => result.Properties.AnnotationType == AnnotationType.Obsolete));
249+
Assert.AreEqual(expectedCode, state.GetRewriter(component).GetText());
250+
}
251+
}
199252
}
200253
}

0 commit comments

Comments
 (0)