Skip to content

Commit 47f3d58

Browse files
MDoernerMDoerner
committed
Make more declaration inspections use one of the base classes
In particular, this includes a complete rewrite of ParameterCanBeByValInspection.
1 parent 33103b5 commit 47f3d58

File tree

11 files changed

+281
-254
lines changed

11 files changed

+281
-254
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Concrete/Excel/ExcelUdfNameIsValidCellReferenceInspection.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using System.Linq;
44
using System.Text.RegularExpressions;
55
using Rubberduck.Inspections.Abstract;
6-
using Rubberduck.Inspections.Inspections.Extensions;
76
using Rubberduck.Inspections.Results;
87
using Rubberduck.Parsing.Inspections;
98
using Rubberduck.Parsing.Inspections.Abstract;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Rubberduck.Resources.Inspections;
77
using Rubberduck.Parsing.Symbols;
88
using Rubberduck.Parsing.VBA;
9-
using Rubberduck.Inspections.Inspections.Extensions;
109

1110
namespace Rubberduck.Inspections.Concrete
1211
{

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

Lines changed: 158 additions & 157 deletions
Large diffs are not rendered by default.

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using Rubberduck.Resources.Inspections;
88
using Rubberduck.Parsing.Symbols;
99
using Rubberduck.Parsing.VBA;
10-
using Rubberduck.Inspections.Inspections.Extensions;
1110

1211
namespace Rubberduck.Inspections.Concrete
1312
{

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

Lines changed: 57 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
1-
using System.Collections.Generic;
21
using System.Linq;
32
using Rubberduck.Inspections.Abstract;
43
using Rubberduck.Inspections.Inspections.Extensions;
5-
using Rubberduck.Inspections.Results;
6-
using Rubberduck.JunkDrawer.Extensions;
7-
using Rubberduck.Parsing.Inspections.Abstract;
84
using Rubberduck.Resources.Inspections;
95
using Rubberduck.Parsing.Annotations;
106
using Rubberduck.Parsing.Symbols;
117
using Rubberduck.Parsing.VBA;
8+
using Rubberduck.Parsing.VBA.DeclarationCaching;
129

1310
namespace Rubberduck.Inspections.Concrete
1411
{
@@ -43,44 +40,11 @@ namespace Rubberduck.Inspections.Concrete
4340
/// End Sub
4441
/// ]]>
4542
/// </example>
46-
public sealed class ProcedureNotUsedInspection : InspectionBase
43+
public sealed class ProcedureNotUsedInspection : DeclarationInspectionBase
4744
{
48-
public ProcedureNotUsedInspection(RubberduckParserState state) : base(state) { }
49-
50-
private static readonly string[] DocumentEventHandlerPrefixes =
51-
{
52-
"Chart_",
53-
"Worksheet_",
54-
"Workbook_",
55-
"Document_",
56-
"Application_",
57-
"Session_"
58-
};
59-
60-
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
61-
{
62-
var classes = State.DeclarationFinder.UserDeclarations(DeclarationType.ClassModule)
63-
.Concat(State.DeclarationFinder.UserDeclarations(DeclarationType.Document))
64-
.ToList();
65-
var modules = State.DeclarationFinder.UserDeclarations(DeclarationType.ProceduralModule).ToList();
66-
67-
var handlers = State.DeclarationFinder.FindEventHandlers().ToHashSet();
68-
69-
var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers().ToHashSet();
70-
var implementingMembers = State.DeclarationFinder.FindAllInterfaceImplementingMembers().ToHashSet();
71-
72-
var items = State.AllUserDeclarations
73-
.Where(item => !IsIgnoredDeclaration(item, interfaceMembers, implementingMembers, handlers, classes, modules))
74-
.ToList();
75-
var issues = items.Select(issue => new DeclarationInspectionResult(this,
76-
string.Format(InspectionResults.IdentifierNotUsedInspection, issue.DeclarationType.ToLocalizedString(), issue.IdentifierName),
77-
issue));
78-
79-
issues = DocumentEventHandlerPrefixes
80-
.Aggregate(issues, (current, item) => current.Where(issue => !issue.Description.Contains($"'{item}")));
81-
82-
return issues.ToList();
83-
}
45+
public ProcedureNotUsedInspection(RubberduckParserState state)
46+
: base(state, ProcedureTypes)
47+
{}
8448

8549
private static readonly DeclarationType[] ProcedureTypes =
8650
{
@@ -91,57 +55,82 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
9155
DeclarationType.Event
9256
};
9357

94-
private bool IsIgnoredDeclaration(Declaration declaration, IEnumerable<Declaration> interfaceMembers, IEnumerable<Declaration> interfaceImplementingMembers , IEnumerable<Declaration> handlers, IEnumerable<Declaration> classes, IEnumerable<Declaration> modules)
58+
private static readonly string[] ClassLifeCycleHandlers =
9559
{
96-
var enumerable = classes as IList<Declaration> ?? classes.ToList();
97-
var result = !ProcedureTypes.Contains(declaration.DeclarationType)
98-
|| declaration.References.Any(r => !r.IsAssignment && !r.ParentScoping.Equals(declaration)) // recursive calls don't count
99-
|| handlers.Contains(declaration)
100-
|| IsPublicModuleMember(modules, declaration)
101-
|| IsClassLifeCycleHandler(enumerable, declaration)
102-
|| interfaceMembers.Contains(declaration)
103-
|| interfaceImplementingMembers.Contains(declaration)
104-
|| declaration.Annotations.Any(x => x.Annotation is ITestAnnotation);
60+
"Class_Initialize",
61+
"Class_Terminate"
62+
};
10563

106-
return result;
64+
private static readonly string[] DocumentEventHandlerPrefixes =
65+
{
66+
"Chart_",
67+
"Worksheet_",
68+
"Workbook_",
69+
"Document_",
70+
"Application_",
71+
"Session_"
72+
};
73+
74+
protected override bool IsResultDeclaration(Declaration declaration, DeclarationFinder finder)
75+
{
76+
return !declaration.References
77+
.Any(reference => !reference.IsAssignment
78+
&& !reference.ParentScoping.Equals(declaration)) // recursive calls don't count
79+
&& !finder.FindEventHandlers().Contains(declaration)
80+
&& !IsPublicModuleMember(declaration)
81+
&& !IsClassLifeCycleHandler(declaration)
82+
&& !(declaration is ModuleBodyElementDeclaration member
83+
&& (member.IsInterfaceMember
84+
|| member.IsInterfaceImplementation))
85+
&& !declaration.Annotations
86+
.Any(pta => pta.Annotation is ITestAnnotation)
87+
&& !IsDocumentEventHandler(declaration);
10788
}
10889

10990
/// <remarks>
11091
/// We cannot determine whether exposed members of standard modules are called or not,
11192
/// so we assume they are instead of flagging them as "never called".
11293
/// </remarks>
113-
private bool IsPublicModuleMember(IEnumerable<Declaration> modules, Declaration procedure)
94+
private static bool IsPublicModuleMember(Declaration procedure)
11495
{
11596
if ((procedure.Accessibility != Accessibility.Implicit
11697
&& procedure.Accessibility != Accessibility.Public))
11798
{
11899
return false;
119100
}
120101

121-
var parent = modules.Where(item => item.ProjectId == procedure.ProjectId)
122-
.SingleOrDefault(item => item.IdentifierName == procedure.ComponentName);
123-
124-
return parent != null;
102+
var parent = Declaration.GetModuleParent(procedure);
103+
return parent != null
104+
&& parent.DeclarationType.HasFlag(DeclarationType.ProceduralModule);
125105
}
126106

127-
// TODO: Put this into grammar?
128-
private static readonly string[] ClassLifeCycleHandlers =
129-
{
130-
"Class_Initialize",
131-
"Class_Terminate"
132-
};
133-
134-
private bool IsClassLifeCycleHandler(IEnumerable<Declaration> classes, Declaration procedure)
107+
private static bool IsClassLifeCycleHandler(Declaration procedure)
135108
{
136109
if (!ClassLifeCycleHandlers.Contains(procedure.IdentifierName))
137110
{
138111
return false;
139112
}
140113

141-
var parent = classes.Where(item => item.ProjectId == procedure.ProjectId)
142-
.SingleOrDefault(item => item.IdentifierName == procedure.ComponentName);
114+
var parent = Declaration.GetModuleParent(procedure);
115+
return parent != null
116+
&& parent.DeclarationType.HasFlag(DeclarationType.ClassModule);
117+
}
143118

144-
return parent != null;
119+
private static bool IsDocumentEventHandler(Declaration declaration)
120+
{
121+
var declarationName = declaration.IdentifierName;
122+
return DocumentEventHandlerPrefixes
123+
.Any(prefix => declarationName.StartsWith(prefix));
124+
}
125+
126+
protected override string ResultDescription(Declaration declaration)
127+
{
128+
var declarationType = declaration.DeclarationType.ToLocalizedString();
129+
var declarationName = declaration.IdentifierName;
130+
return string.Format(
131+
InspectionResults.IdentifierNotUsedInspection,
132+
declarationType,
133+
declarationName);
145134
}
146135
}
147136
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
using System.Collections.Generic;
22
using System.Diagnostics.CodeAnalysis;
33
using System.Linq;
4-
using Antlr4.Runtime;
54
using Rubberduck.Inspections.Abstract;
6-
using Rubberduck.Inspections.Results;
75
using Rubberduck.JunkDrawer.Extensions;
86
using Rubberduck.Parsing;
97
using Rubberduck.Parsing.Grammar;
10-
using Rubberduck.Parsing.Inspections.Abstract;
118
using Rubberduck.Resources.Inspections;
129
using Rubberduck.Parsing.Symbols;
1310
using Rubberduck.Parsing.VBA;

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using Rubberduck.Inspections.Abstract;
55
using Rubberduck.Inspections.Inspections.Extensions;
66
using Rubberduck.Inspections.Results;
7-
using Rubberduck.JunkDrawer.Extensions;
87
using Rubberduck.Parsing.Inspections.Abstract;
98
using Rubberduck.Parsing.Symbols;
109
using Rubberduck.Parsing.VBA;
@@ -61,7 +60,7 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
6160
var whitelistedNames = settings.WhitelistedIdentifiers
6261
.Select(s => s.Identifier)
6362
.ToArray();
64-
var handlers = finder.FindEventHandlers().ToHashSet();
63+
var handlers = finder.FindEventHandlers();
6564

6665
var results = new List<IInspectionResult>();
6766
foreach (var moduleDeclaration in State.DeclarationFinder.UserDeclarations(DeclarationType.Module))
@@ -85,7 +84,7 @@ private IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleNam
8584
var whitelistedNames = settings.WhitelistedIdentifiers
8685
.Select(s => s.Identifier)
8786
.ToArray();
88-
var handlers = finder.FindEventHandlers().ToHashSet();
87+
var handlers = finder.FindEventHandlers();
8988
return DoGetInspectionResults(module, finder, whitelistedNames, handlers);
9089
}
9190

‎Rubberduck.CodeAnalysis/QuickFixes/PassParameterByValueQuickFix.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ private void FixMethods(Declaration target, IRewriteSession rewriteSession)
7979
.ToList()
8080
: _declarationFinderProvider.DeclarationFinder
8181
.FindInterfaceImplementationMembers(target.ParentDeclaration)
82-
.Cast<Declaration>()
8382
.ToList();
8483

8584
foreach (var member in members)

‎Rubberduck.Parsing/Symbols/DeclarationLoaders/SpecialFormDeclarations.cs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,22 +100,37 @@ private static FunctionDeclaration LBoundFunctionWithoutParameters(Declaration p
100100

101101
private static ParameterDeclaration ArrayNameParameter(Declaration parentModule, FunctionDeclaration parentFunction)
102102
{
103-
var arrayParam = new ParameterDeclaration(new QualifiedMemberName(parentModule.QualifiedName.QualifiedModuleName, "Arrayname"), parentFunction, "Variant", null, null, false, false, true);
103+
var arrayParam = new ParameterDeclaration(
104+
new QualifiedMemberName(parentModule.QualifiedName.QualifiedModuleName, "Arrayname"),
105+
parentFunction,
106+
"Variant",
107+
null,
108+
null,
109+
false,
110+
false,
111+
true);
104112
return arrayParam;
105113
}
106114

107115
private static ParameterDeclaration DimensionParameter(Declaration parentModule, FunctionDeclaration parentFunction)
108116
{
109-
var rankParam = new ParameterDeclaration(new QualifiedMemberName(parentModule.QualifiedName.QualifiedModuleName, "Dimension"), parentFunction, "Long", null, null, true, false);
117+
var rankParam = new ParameterDeclaration(
118+
new QualifiedMemberName(parentModule.QualifiedName.QualifiedModuleName, "Dimension"),
119+
parentFunction,
120+
"Long",
121+
null,
122+
null,
123+
true,
124+
false);
110125
return rankParam;
111126
}
112127

113128
private static FunctionDeclaration UBoundFunction(Declaration parentModule)
114129
{
115-
var uboundFunction = UBoundFunctionWithoutParameters(parentModule);
116-
uboundFunction.AddParameter(ArrayNameParameter(parentModule, uboundFunction));
117-
uboundFunction.AddParameter(DimensionParameter(parentModule, uboundFunction));
118-
return uboundFunction;
130+
var uBoundFunction = UBoundFunctionWithoutParameters(parentModule);
131+
uBoundFunction.AddParameter(ArrayNameParameter(parentModule, uBoundFunction));
132+
uBoundFunction.AddParameter(DimensionParameter(parentModule, uBoundFunction));
133+
return uBoundFunction;
119134
}
120135

121136
private static FunctionDeclaration UBoundFunctionWithoutParameters(Declaration parentModule)

‎Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class DeclarationFinder
4343
private IDictionary<QualifiedMemberName, List<IdentifierReference>> _referencesByMember;
4444

4545
private Lazy<IDictionary<DeclarationType, List<Declaration>>> _builtInDeclarationsByType;
46-
private Lazy<IDictionary<Declaration, List<Declaration>>> _handlersByWithEventsField;
46+
private Lazy<IDictionary<Declaration, List<ModuleBodyElementDeclaration>>> _handlersByWithEventsField;
4747

4848
private Lazy<IDictionary<(VBAParser.ImplementsStmtContext Context, Declaration Implementor), List<ModuleBodyElementDeclaration>>> _implementingMembers;
4949
private Lazy<IDictionary<VBAParser.ImplementsStmtContext, List<ModuleBodyElementDeclaration>>> _membersByImplementsContext;
@@ -175,7 +175,7 @@ private void InitializeLazyCollections()
175175
_formEventHandlers = new Lazy<ICollection<Declaration>>(FindAllFormEventHandlers, true);
176176
_projects = new Lazy<ICollection<Declaration>>(() => DeclarationsWithType(DeclarationType.Project).ToList(), true);
177177
_classes = new Lazy<ICollection<Declaration>>(() => DeclarationsWithType(DeclarationType.ClassModule).ToList(), true);
178-
_handlersByWithEventsField = new Lazy<IDictionary<Declaration, List<Declaration>>>(FindAllHandlersByWithEventField, true);
178+
_handlersByWithEventsField = new Lazy<IDictionary<Declaration, List<ModuleBodyElementDeclaration>>>(FindAllHandlersByWithEventField, true);
179179

180180
_implementingMembers = new Lazy<IDictionary<(VBAParser.ImplementsStmtContext Context, Declaration Implementor), List<ModuleBodyElementDeclaration>>>(FindAllImplementingMembers, true);
181181
_interfaceMembers = new Lazy<IDictionary<ClassModuleDeclaration, List<Declaration>>>(FindAllInterfaceMembersByModule, true);
@@ -252,7 +252,7 @@ private IDictionary<ClassModuleDeclaration, List<Declaration>> FindAllInterfaceM
252252
.ToList());
253253
}
254254

255-
private IDictionary<Declaration, List<Declaration>> FindAllHandlersByWithEventField()
255+
private IDictionary<Declaration, List<ModuleBodyElementDeclaration>> FindAllHandlersByWithEventField()
256256
{
257257
var withEventsFields = UserDeclarations(DeclarationType.Variable).Where(item => item.IsWithEvents);
258258
var events = withEventsFields.Select(field =>
@@ -267,9 +267,9 @@ private IDictionary<Declaration, List<Declaration>> FindAllHandlersByWithEventFi
267267
{
268268
item.WithEventsField,
269269
Handlers = item.AvailableEvents.SelectMany(evnt =>
270-
Members(item.WithEventsField.ParentDeclaration.QualifiedName.QualifiedModuleName)
271-
.Where(member => member.DeclarationType == DeclarationType.Procedure
272-
&& member.IdentifierName == item.WithEventsField.IdentifierName + "_" + evnt.IdentifierName))
270+
Members(item.WithEventsField.ParentDeclaration.QualifiedName.QualifiedModuleName, DeclarationType.Procedure)
271+
.Where(member => member.IdentifierName == $"{item.WithEventsField.IdentifierName}_{evnt.IdentifierName}"))
272+
.OfType<ModuleBodyElementDeclaration>()
273273
})
274274
.ToDictionary(item => item.WithEventsField, item => item.Handlers.ToList());
275275
return handlersByWithEventsField;
@@ -327,7 +327,7 @@ public ICollection<Declaration> FindEventHandlers()
327327
return _eventHandlers.Value;
328328
}
329329

330-
public IEnumerable<Declaration> FindEventHandlers(Declaration eventDeclaration)
330+
public IEnumerable<ModuleBodyElementDeclaration> FindEventHandlers(Declaration eventDeclaration)
331331
{
332332
var withEventsDeclarations = FindWithEventFields(eventDeclaration);
333333
return withEventsDeclarations
@@ -347,7 +347,7 @@ public IEnumerable<Declaration> FindFormControlEventHandlers(Declaration control
347347
&& handlers.IdentifierName.StartsWith(control.IdentifierName + "_"));
348348
}
349349

350-
public IEnumerable<Declaration> FindFormEventHandlers()
350+
public ICollection<Declaration> FindFormEventHandlers()
351351
{
352352
return _formEventHandlers.Value;
353353
}
@@ -382,11 +382,11 @@ public IEnumerable<Declaration> DeclarationsWithType(DeclarationType type)
382382
return BuiltInDeclarations(type).Concat(UserDeclarations(type));
383383
}
384384

385-
public IEnumerable<Declaration> FindHandlersForWithEventsField(Declaration field)
385+
public IEnumerable<ModuleBodyElementDeclaration> FindHandlersForWithEventsField(Declaration field)
386386
{
387387
return _handlersByWithEventsField.Value.TryGetValue(field, out var result)
388388
? result
389-
: Enumerable.Empty<Declaration>();
389+
: Enumerable.Empty<ModuleBodyElementDeclaration>();
390390
}
391391

392392
public IEnumerable<Declaration> FindWithEventFields()
@@ -1283,7 +1283,7 @@ bool IsHostSpecificHandler(Declaration item)
12831283
}
12841284
}
12851285

1286-
private List<Declaration> FindAllFormEventHandlers()
1286+
private HashSet<Declaration> FindAllFormEventHandlers()
12871287
{
12881288
var forms = DeclarationsWithType(DeclarationType.ClassModule).
12891289
Where(declaration => declaration.QualifiedModuleName.ComponentType == ComponentType.UserForm);
@@ -1298,7 +1298,7 @@ private List<Declaration> FindAllFormEventHandlers()
12981298
var handlers = UserDeclarations(DeclarationType.Procedure)
12991299
.Where(procedure => handlerNames.Contains(procedure.IdentifierName)
13001300
&& formScopes.Contains(procedure.ParentScope));
1301-
return handlers.ToList();
1301+
return handlers.ToHashSet();
13021302
}
13031303

13041304
/// <summary>

0 commit comments

Comments
 (0)