Skip to content

Commit c12f606

Browse files
committed
Use visitor in NonReturningFunctionInspection properly
Previously, it was kind of used like a listener, but calling it a visitor. Now, it specifies the result aggregation method and actually short-circuits once the result is known.
1 parent d0f9ef7 commit c12f606

File tree

1 file changed

+27
-16
lines changed

1 file changed

+27
-16
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Linq;
2+
using Antlr4.Runtime.Tree;
23
using Rubberduck.CodeAnalysis.Inspections.Abstract;
34
using Rubberduck.Parsing;
45
using Rubberduck.Parsing.Grammar;
@@ -122,48 +123,58 @@ protected override string ResultDescription(Declaration declaration)
122123
private class FunctionReturnValueAssignmentLocator : VBAParserBaseVisitor<bool>
123124
{
124125
private readonly string _name;
125-
private bool _result;
126126
private bool _inFunctionReturnWithExpression;
127127

128128
public FunctionReturnValueAssignmentLocator(string name)
129129
{
130130
_name = name;
131+
_inFunctionReturnWithExpression = false;
131132
}
132133

133-
public override bool VisitBlock(VBAParser.BlockContext context)
134+
protected override bool DefaultResult => false;
135+
136+
protected override bool ShouldVisitNextChild(IRuleNode node, bool currentResult)
134137
{
135-
base.VisitBlock(context);
136-
return _result;
138+
return !currentResult;
137139
}
140+
141+
//This is actually the default implementation, but for explicities sake stated here.
142+
protected override bool AggregateResult(bool aggregate, bool nextResult)
143+
{
144+
return nextResult;
145+
}
146+
138147
public override bool VisitWithStmt(VBAParser.WithStmtContext context)
139148
{
140149
var oldInFunctionReturnWithExpression = _inFunctionReturnWithExpression;
141150
_inFunctionReturnWithExpression = context.expression().GetText() == _name;
142-
base.VisitWithStmt(context);
151+
var result = base.VisitWithStmt(context);
143152
_inFunctionReturnWithExpression = oldInFunctionReturnWithExpression;
144-
return _result;
153+
return result;
145154
}
146155

147156
public override bool VisitLetStmt(VBAParser.LetStmtContext context)
148157
{
149158
var LHS = context.lExpression();
159+
if (_inFunctionReturnWithExpression
160+
&& LHS is VBAParser.WithMemberAccessExprContext)
161+
{
162+
return true;
163+
}
150164
var leftmost = LHS.GetChild(0).GetText();
151-
_result = _result
152-
|| leftmost == _name
153-
|| (_inFunctionReturnWithExpression
154-
&& LHS is VBAParser.WithMemberAccessExprContext);
155-
return _result;
165+
return leftmost == _name;
156166
}
157167

158168
public override bool VisitSetStmt(VBAParser.SetStmtContext context)
159169
{
160170
var LHS = context.lExpression();
171+
if (_inFunctionReturnWithExpression
172+
&& LHS is VBAParser.WithMemberAccessExprContext)
173+
{
174+
return true;
175+
}
161176
var leftmost = LHS.GetChild(0).GetText();
162-
_result = _result
163-
|| leftmost == _name
164-
|| (_inFunctionReturnWithExpression
165-
&& LHS is VBAParser.WithMemberAccessExprContext);
166-
return _result;
177+
return leftmost == _name;
167178
}
168179
}
169180
}

0 commit comments

Comments
 (0)