Skip to content

Commit ee93e40

Browse files
MDoernerMDoerner
authored
Merge b56b19d into fbfff3c 
2 parents fbfff3c + b56b19d commit ee93e40

File tree

5 files changed

+208
-12
lines changed

5 files changed

+208
-12
lines changed

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,23 @@ private static bool IsCalledAsProcedure(ParserRuleContext context)
141141
}
142142

143143
//Member accesses are parsed right-to-left, e.g. 'foo.Bar' is the parent of 'foo'.
144-
//Thus, having a member access parent means that the return value is used somehow.
145-
var ownFunctionCallExpression = context.Parent is VBAParser.MemberAccessExprContext methodCall
146-
? methodCall
147-
: context;
148-
var memberAccessParent = ownFunctionCallExpression.GetAncestor<VBAParser.MemberAccessExprContext>();
144+
//Thus, having a member access parent and being contained in its lExpression on the left of the dot
145+
//or having a further member access parent means that the return value is used somehow.
146+
var memberAccessParent = context.GetAncestor<VBAParser.MemberAccessExprContext>();
149147
if (memberAccessParent != null)
150148
{
151-
return false;
149+
//This case is necessary for member accesses in particular on simple name expressions since the context is the simpleNameExpression there and not the identifier.
150+
if (memberAccessParent.lExpression().Contains(context))
151+
{
152+
return false;
153+
}
154+
155+
//This case is necessary if the context is itself the unrestricted identifier in a member access.
156+
var furtherMemberAccessParent = memberAccessParent.GetAncestor<VBAParser.MemberAccessExprContext>();
157+
if (furtherMemberAccessParent != null)
158+
{
159+
return false;
160+
}
152161
}
153162

154163
//If we are in an output list, the value is used somewhere in defining the argument.

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,23 @@ private static bool IsCalledAsProcedure(ParserRuleContext context)
7777
}
7878

7979
//Member accesses are parsed right-to-left, e.g. 'foo.Bar' is the parent of 'foo'.
80-
//Thus, having a member access parent means that the return value is used somehow.
81-
var ownFunctionCallExpression = context.Parent is VBAParser.MemberAccessExprContext methodCall
82-
? methodCall
83-
: context;
84-
var memberAccessParent = ownFunctionCallExpression.GetAncestor<VBAParser.MemberAccessExprContext>();
80+
//Thus, having a member access parent and being contained in its lExpression on the left of the dot
81+
//or having a further member access parent means that the return value is used somehow.
82+
var memberAccessParent = context.GetAncestor<VBAParser.MemberAccessExprContext>();
8583
if (memberAccessParent != null)
8684
{
87-
return false;
85+
//This case is necessary for member accesses in particular on simple name expressions since the context is the simpleNameExpression there and not the identifier.
86+
if (memberAccessParent.lExpression().Contains(context))
87+
{
88+
return false;
89+
}
90+
91+
//This case is necessary if the context is itself the unrestricted identifier in a member access.
92+
var furtherMemberAccessParent = memberAccessParent.GetAncestor<VBAParser.MemberAccessExprContext>();
93+
if (furtherMemberAccessParent != null)
94+
{
95+
return false;
96+
}
8897
}
8998

9099
//If we are in an output list, the value is used somewhere in defining the argument.

‎Rubberduck.Parsing/ParserRuleContextExtensions.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ public static Selection GetSelection(this ParserRuleContext context)
3030
context.Stop.EndColumn() + 1);
3131
}
3232

33+
public static bool Contains(this ParserRuleContext context, ParserRuleContext otherContext)
34+
{
35+
return context.start.TokenIndex <= otherContext.start.TokenIndex
36+
&& context.stop.TokenIndex >= otherContext.stop.TokenIndex;
37+
}
38+
3339
/// <summary>
3440
/// Gets the tokens belonging to the context from the token stream.
3541
/// </summary>

‎RubberduckTests/Inspections/FunctionReturnValueAlwaysDiscardedInspectionTests.cs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,24 @@ End Sub
286286
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
287287
}
288288

289+
[Test]
290+
[Category("Inspections")]
291+
[Category("Unused Value")]
292+
public void WithStatement_DoesNotReturnResult()
293+
{
294+
const string code = @"
295+
Public Function Foo() As Object
296+
End Function
297+
298+
Public Sub Baz()
299+
With Foo
300+
'Do Whatever
301+
End With
302+
End Sub
303+
";
304+
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
305+
}
306+
289307
[Test]
290308
[Category("Inspections")]
291309
[Category("Unused Value")]
@@ -403,6 +421,74 @@ Public Sub Baz()
403421
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
404422
}
405423

424+
425+
[Test]
426+
[Category("Inspections")]
427+
[Category("Unused Value")]
428+
public void ChainedParameterlessMemberAccess_ReturnsNoResult()
429+
{
430+
const string inputCode = @"
431+
Public Function GetIt() As Object
432+
End Function
433+
434+
Public Sub Baz()
435+
GetIt.Select
436+
End Sub";
437+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
438+
}
439+
440+
[Test]
441+
[Category("Inspections")]
442+
[Category("Unused Value")]
443+
//See issue #5853 at https://github.com/rubberduck-vba/Rubberduck/issues/5853
444+
public void CallToMemberOfFunctionReturnValueInBody_NoResult()
445+
{
446+
const string returnTypeClassCode = @"
447+
Public Sub Bar(ByVal arg As String)
448+
End Sub
449+
";
450+
const string moduleCode =
451+
@"
452+
Public Function Foo() As Class1
453+
Set Foo = New Class1
454+
Foo.Bar ""SomeArgument""
455+
End Function
456+
";
457+
var modules = new (string, string, ComponentType)[]
458+
{
459+
("Class1", returnTypeClassCode, ComponentType.ClassModule),
460+
("Module1", moduleCode, ComponentType.StandardModule)
461+
};
462+
463+
Assert.AreEqual(0, InspectionResultsForModules(modules).Count());
464+
}
465+
466+
[Test]
467+
[Category("Inspections")]
468+
[Category("Unused Value")]
469+
//See issue #5853 at https://github.com/rubberduck-vba/Rubberduck/issues/5853
470+
public void ExplicitCallToMemberOfFunctionReturnValueInBody_NoResult()
471+
{
472+
const string returnTypeClassCode = @"
473+
Public Sub Bar(ByVal arg As String)
474+
End Sub
475+
";
476+
const string moduleCode =
477+
@"
478+
Public Function Foo() As Class1
479+
Set Foo = New Class1
480+
Call Foo.Bar(""SomeArgument"")
481+
End Function
482+
";
483+
var modules = new (string, string, ComponentType)[]
484+
{
485+
("Class1", returnTypeClassCode, ComponentType.ClassModule),
486+
("Module1", moduleCode, ComponentType.StandardModule)
487+
};
488+
489+
Assert.AreEqual(0, InspectionResultsForModules(modules).Count());
490+
}
491+
406492
[Test]
407493
[Category("Inspections")]
408494
[Category("Unused Value")]

‎RubberduckTests/Inspections/FunctionReturnValueDiscardedInspectionTests.cs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,24 @@ End Sub
186186
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
187187
}
188188

189+
[Test]
190+
[Category("Inspections")]
191+
[Category("Unused Value")]
192+
public void FunctionReturnValueDiscarded_DoesNotReturnResult_WithStatement()
193+
{
194+
const string code = @"
195+
Public Function Foo() As Object
196+
End Function
197+
198+
Public Sub Baz()
199+
With Foo
200+
'Do Whatever
201+
End With
202+
End Sub
203+
";
204+
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
205+
}
206+
189207
[Test]
190208
[Category("Inspections")]
191209
[Category("Unused Value")]
@@ -374,6 +392,22 @@ Public Sub Baz()
374392
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
375393
}
376394

395+
396+
[Test]
397+
[Category("Inspections")]
398+
[Category("Unused Value")]
399+
public void ChainedParameterlessMemberAccess_ReturnsNoResult()
400+
{
401+
const string inputCode = @"
402+
Public Function GetIt() As Object
403+
End Function
404+
405+
Public Sub Baz()
406+
GetIt.Select
407+
End Sub";
408+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
409+
}
410+
377411
[Test]
378412
[Category("Inspections")]
379413
[Category("Unused Value")]
@@ -458,6 +492,58 @@ End Function
458492
Assert.AreEqual(0, InspectionResultsForModules(modules).Count());
459493
}
460494

495+
[Test]
496+
[Category("Inspections")]
497+
[Category("Unused Value")]
498+
//See issue #5853 at https://github.com/rubberduck-vba/Rubberduck/issues/5853
499+
public void CallToMemberOfFunctionReturnValueInBody_NoResult()
500+
{
501+
const string returnTypeClassCode = @"
502+
Public Sub Bar(ByVal arg As String)
503+
End Sub
504+
";
505+
const string moduleCode =
506+
@"
507+
Public Function Foo() As Class1
508+
Set Foo = New Class1
509+
Foo.Bar ""SomeArgument""
510+
End Function
511+
";
512+
var modules = new (string, string, ComponentType)[]
513+
{
514+
("Class1", returnTypeClassCode, ComponentType.ClassModule),
515+
("Module1", moduleCode, ComponentType.StandardModule)
516+
};
517+
518+
Assert.AreEqual(0, InspectionResultsForModules(modules).Count());
519+
}
520+
521+
[Test]
522+
[Category("Inspections")]
523+
[Category("Unused Value")]
524+
//See issue #5853 at https://github.com/rubberduck-vba/Rubberduck/issues/5853
525+
public void ExplicitCallToMemberOfFunctionReturnValueInBody_NoResult()
526+
{
527+
const string returnTypeClassCode = @"
528+
Public Sub Bar(ByVal arg As String)
529+
End Sub
530+
";
531+
const string moduleCode =
532+
@"
533+
Public Function Foo() As Class1
534+
Set Foo = New Class1
535+
Call Foo.Bar(""SomeArgument"")
536+
End Function
537+
";
538+
var modules = new (string, string, ComponentType)[]
539+
{
540+
("Class1", returnTypeClassCode, ComponentType.ClassModule),
541+
("Module1", moduleCode, ComponentType.StandardModule)
542+
};
543+
544+
Assert.AreEqual(0, InspectionResultsForModules(modules).Count());
545+
}
546+
461547
[Test]
462548
[Category("Inspections")]
463549
[Category("Unused Value")]

0 commit comments

Comments
 (0)