Skip to content

Commit f65094b

Browse files
authored
Merge pull request #6022 from MDoerner/FixNonReturningFunctionOnUdtWithAssignment
2 parents b53f353 + c12f606 commit f65094b

File tree

2 files changed

+161
-10
lines changed

2 files changed

+161
-10
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs

Lines changed: 38 additions & 10 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,31 +123,58 @@ protected override string ResultDescription(Declaration declaration)
122123
private class FunctionReturnValueAssignmentLocator : VBAParserBaseVisitor<bool>
123124
{
124125
private readonly string _name;
125-
private bool _result;
126+
private bool _inFunctionReturnWithExpression;
126127

127128
public FunctionReturnValueAssignmentLocator(string name)
128129
{
129130
_name = name;
131+
_inFunctionReturnWithExpression = false;
130132
}
131133

132-
public override bool VisitBlock(VBAParser.BlockContext context)
134+
protected override bool DefaultResult => false;
135+
136+
protected override bool ShouldVisitNextChild(IRuleNode node, bool currentResult)
137+
{
138+
return !currentResult;
139+
}
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+
147+
public override bool VisitWithStmt(VBAParser.WithStmtContext context)
133148
{
134-
base.VisitBlock(context);
135-
return _result;
149+
var oldInFunctionReturnWithExpression = _inFunctionReturnWithExpression;
150+
_inFunctionReturnWithExpression = context.expression().GetText() == _name;
151+
var result = base.VisitWithStmt(context);
152+
_inFunctionReturnWithExpression = oldInFunctionReturnWithExpression;
153+
return result;
136154
}
137155

138156
public override bool VisitLetStmt(VBAParser.LetStmtContext context)
139157
{
140-
var leftmost = context.lExpression().GetChild(0).GetText();
141-
_result = _result || leftmost == _name;
142-
return _result;
158+
var LHS = context.lExpression();
159+
if (_inFunctionReturnWithExpression
160+
&& LHS is VBAParser.WithMemberAccessExprContext)
161+
{
162+
return true;
163+
}
164+
var leftmost = LHS.GetChild(0).GetText();
165+
return leftmost == _name;
143166
}
144167

145168
public override bool VisitSetStmt(VBAParser.SetStmtContext context)
146169
{
147-
var leftmost = context.lExpression().GetChild(0).GetText();
148-
_result = _result || leftmost == _name;
149-
return _result;
170+
var LHS = context.lExpression();
171+
if (_inFunctionReturnWithExpression
172+
&& LHS is VBAParser.WithMemberAccessExprContext)
173+
{
174+
return true;
175+
}
176+
var leftmost = LHS.GetChild(0).GetText();
177+
return leftmost == _name;
150178
}
151179
}
152180
}

RubberduckTests/Inspections/NonReturningFunctionInspectionTests.cs

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,129 @@ End Sub
192192
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
193193
}
194194

195+
[Test]
196+
[Category("Inspections")]
197+
//ref issue #5964
198+
public void NonReturningFunction_NoResult_AssignmenToUDTMembersInWithBlock()
199+
{
200+
const string inputCode = @"
201+
Private Type tipo
202+
one As Long
203+
two As Long
204+
End Type
205+
206+
Function assigner() As tipo
207+
With assigner
208+
.one = 1
209+
.two = 2
210+
End With
211+
End Function
212+
";
213+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
214+
}
215+
216+
[Test]
217+
[Category("Inspections")]
218+
//ref issue #5964
219+
public void NonReturningFunction_NoResult_AssignmenToUDTMembersInWithBlock_NestedWith_Inside()
220+
{
221+
const string inputCode = @"
222+
Private Type tipo
223+
one As Long
224+
two As Long
225+
End Type
226+
227+
Function assigner() As tipo
228+
Dim bar As tipo
229+
With bar
230+
.one = 3
231+
.two = 2
232+
With assigner
233+
.one = 1
234+
.two = 2
235+
End With
236+
End With
237+
End Function
238+
";
239+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
240+
}
241+
242+
[Test]
243+
[Category("Inspections")]
244+
//ref issue #5964
245+
public void NonReturningFunction_NoResult_AssignmenToUDTMembersInWithBlock_NestedWith_Start()
246+
{
247+
const string inputCode = @"
248+
Private Type tipo
249+
one As Long
250+
two As Long
251+
End Type
252+
253+
Function assigner() As tipo
254+
Dim bar As tipo
255+
With assigner
256+
.one = 3
257+
.two = 2
258+
With bar
259+
.one = 1
260+
.two = 2
261+
End With
262+
End With
263+
End Function
264+
";
265+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
266+
}
267+
268+
[Test]
269+
[Category("Inspections")]
270+
//ref issue #5964
271+
public void NonReturningFunction_NoResult_AssignmenToUDTMembersInWithBlock_NestedWith_End()
272+
{
273+
const string inputCode = @"
274+
Private Type tipo
275+
one As Long
276+
two As Long
277+
End Type
278+
279+
Function assigner() As tipo
280+
Dim bar As tipo
281+
With assigner
282+
With bar
283+
.one = 1
284+
.two = 2
285+
End With
286+
.one = 3
287+
.two = 2
288+
End With
289+
End Function
290+
";
291+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
292+
}
293+
294+
[Test]
295+
[Category("Inspections")]
296+
//ref issue #5964
297+
public void NonReturningFunction_OneResult_AssignmenToUDTMembersOfOtherVariableInNestedWith()
298+
{
299+
const string inputCode = @"
300+
Private Type tipo
301+
one As Long
302+
two As Long
303+
End Type
304+
305+
Function assigner() As tipo
306+
Dim bar As tipo
307+
With assigner
308+
With bar
309+
.one = 1
310+
.two = 2
311+
End With
312+
End With
313+
End Function
314+
";
315+
Assert.AreEqual(1, InspectionResultsForStandardModule(inputCode).Count());
316+
}
317+
195318
[Test]
196319
[Category("Inspections")]
197320
public void NonReturningFunction_ReturnsResult_InterfaceImplementation()

0 commit comments

Comments
 (0)