/
FunctionReturnValueAlwaysDiscardedInspection.cs
163 lines (145 loc) · 6.14 KB
/
FunctionReturnValueAlwaysDiscardedInspection.cs
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
using System.Collections.Generic;
using System.Linq;
using Antlr4.Runtime;
using Rubberduck.Inspections.Abstract;
using Rubberduck.JunkDrawer.Extensions;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Resources.Inspections;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Parsing.VBA;
using Rubberduck.Parsing.VBA.DeclarationCaching;
namespace Rubberduck.Inspections.Concrete
{
/// <summary>
/// Warns when a user function's return value is discarded at all its call sites.
/// </summary>
/// <why>
/// A 'Function' procedure normally means its return value to be captured and consumed by the calling code.
/// It's possible that not all call sites need the return value, but if the value is systematically discarded then this
/// means the function is side-effecting, and thus should probably be a 'Sub' procedure instead.
/// </why>
/// <example hasResults="true">
/// <![CDATA[
/// Public Sub DoSomething()
/// GetFoo ' return value is not captured
/// End Sub
///
/// Private Function GetFoo() As Long
/// GetFoo = 42
/// End Function
/// ]]>
/// </example>
/// <example hasResults="false">
/// <![CDATA[
/// Public Sub DoSomething()
/// GetFoo ' return value is discarded
/// End Sub
///
/// Public Sub DoSomethingElse()
/// Dim foo As Long
/// foo = GetFoo ' return value is captured
/// End Sub
///
/// Private Function GetFoo() As Long
/// GetFoo = 42
/// End Function
/// ]]>
/// </example>
public sealed class FunctionReturnValueAlwaysDiscardedInspection : DeclarationInspectionBase
{
public FunctionReturnValueAlwaysDiscardedInspection(RubberduckParserState state)
:base(state, DeclarationType.Function)
{}
protected override bool IsResultDeclaration(Declaration declaration)
{
if (!(declaration is ModuleBodyElementDeclaration moduleBodyElementDeclaration))
{
return false;
}
//We only report the interface itself.
if (moduleBodyElementDeclaration.IsInterfaceImplementation)
{
return false;
}
var finder = DeclarationFinderProvider.DeclarationFinder;
if (moduleBodyElementDeclaration.IsInterfaceMember)
{
return IsInterfaceIssue(moduleBodyElementDeclaration, finder);
}
return IsIssueItself(moduleBodyElementDeclaration);
}
private bool IsIssueItself(ModuleBodyElementDeclaration declaration)
{
var procedureCallReferences = ProcedureCallReferences(declaration).ToHashSet();
if (!procedureCallReferences.Any())
{
return false;
}
return declaration.References
.All(reference => procedureCallReferences.Contains(reference)
|| reference.IsAssignment && IsReturnStatement(declaration, reference));
}
private bool IsReturnStatement(Declaration function, IdentifierReference assignment)
{
return assignment.ParentScoping.Equals(function) && assignment.Declaration.Equals(function);
}
private bool IsInterfaceIssue(ModuleBodyElementDeclaration declaration, DeclarationFinder finder)
{
if (!IsIssueItself(declaration))
{
return false;
}
var implementations = finder.FindInterfaceImplementationMembers(declaration);
return implementations.All(implementation => IsIssueItself(implementation)
|| implementation.References.All(reference =>
reference.IsAssignment
&& IsReturnStatement(implementation, reference)));
}
private static IEnumerable<IdentifierReference> ProcedureCallReferences(Declaration declaration)
{
return declaration.References
.Where(IsProcedureCallReference);
}
private static bool IsProcedureCallReference(IdentifierReference reference)
{
return reference?.Declaration != null
&& !reference.IsAssignment
&& !reference.IsArrayAccess
&& !reference.IsInnerRecursiveDefaultMemberAccess
&& IsCalledAsProcedure(reference.Context);
}
private static bool IsCalledAsProcedure(ParserRuleContext context)
{
var callStmt = context.GetAncestor<VBAParser.CallStmtContext>();
if (callStmt == null)
{
return false;
}
//If we are in an argument list, the value is used somewhere in defining the argument.
var argumentListParent = context.GetAncestor<VBAParser.ArgumentListContext>();
if (argumentListParent != null)
{
return false;
}
//Member accesses are parsed right-to-left, e.g. 'foo.Bar' is the parent of 'foo'.
//Thus, having a member access parent means that the return value is used somehow.
var ownFunctionCallExpression = context.Parent is VBAParser.MemberAccessExprContext methodCall
? methodCall
: context;
var memberAccessParent = ownFunctionCallExpression.GetAncestor<VBAParser.MemberAccessExprContext>();
if (memberAccessParent != null)
{
return false;
}
//If we are in an output list, the value is used somewhere in defining the argument.
var outputListParent = context.GetAncestor<VBAParser.OutputListContext>();
return outputListParent == null;
}
protected override string ResultDescription(Declaration declaration)
{
var functionName = declaration.QualifiedName.ToString();
return string.Format(InspectionResults.FunctionReturnValueAlwaysDiscardedInspection, functionName);
}
}
}