Skip to content

Commit 69bfff3

Browse files
authored
Merge pull request #5495 from BZngr/5490_FalsePositive
ProcedureNotUsedInspection false positive for PropertyLet
2 parents cfe1ad3 + 1b088d5 commit 69bfff3

File tree

2 files changed

+121
-10
lines changed

2 files changed

+121
-10
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/ProcedureNotUsedInspection.cs

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Generic;
12
using System.Linq;
23
using Rubberduck.CodeAnalysis.Inspections.Abstract;
34
using Rubberduck.CodeAnalysis.Inspections.Extensions;
@@ -14,32 +15,92 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
1415
/// </summary>
1516
/// <why>
1617
/// Unused procedures are dead code that should probably be removed. Note, a procedure may be effectively "not used" in code, but attached to some
17-
/// Shape object in the host document: in such cases the inspection result should be ignored. An event handler procedure that isn't being
18-
/// resolved as such, may also wrongly trigger this inspection.
18+
/// Shape object in the host document: in such cases the inspection result should be ignored.
1919
/// </why>
2020
/// <remarks>
2121
/// Not all unused procedures can/should be removed: ignore any inspection results for
2222
/// event handler procedures and interface members that Rubberduck isn't recognizing as such.
23+
/// Public procedures of Standard Modules are not flagged by this inspection regardless of
24+
/// the presence or absence of user code references.
2325
/// </remarks>
2426
/// <example hasResult="true">
25-
/// <module name="MyModule" type="Standard Module">
27+
/// <module name="Module1" type="Standard Module">
2628
/// <![CDATA[
2729
/// Option Explicit
2830
///
29-
/// Public Sub DoSomething()
30-
/// ' macro is attached to a worksheet Shape.
31+
/// Private Sub DoSomething()
3132
/// End Sub
3233
/// ]]>
3334
/// </module>
3435
/// </example>
3536
/// <example hasResult="false">
36-
/// <module name="MyModule" type="Standard Module">
37+
/// <module name="Module1" type="Standard Module">
3738
/// <![CDATA[
3839
/// Option Explicit
39-
///
40+
///
4041
/// '@Ignore ProcedureNotUsed
42+
/// Private Sub DoSomething()
43+
/// End Sub
44+
/// ]]>
45+
/// </module>
46+
/// </example>
47+
/// <example hasResult="false">
48+
/// <module name="Macros" type="Standard Module">
49+
/// <![CDATA[
50+
/// Option Explicit
51+
///
52+
/// Public Sub DoSomething()
53+
/// 'a public procedure in a standard module may be a macro
54+
/// 'attached to a worksheet Shape or invoked by means other than user code.
55+
/// End Sub
56+
/// ]]>
57+
/// </module>
58+
/// </example>
59+
/// <example hasResult="true">
60+
/// <module name="Class1" type="Class Module">
61+
/// <![CDATA[
62+
/// Option Explicit
63+
///
64+
/// Public Sub DoSomething()
65+
/// End Sub
66+
///
67+
/// Public Sub DoSomethingElse()
68+
/// End Sub
69+
/// ]]>
70+
/// </module>
71+
/// <module name="Module1" type="Standard Module">
72+
/// <![CDATA[
73+
/// Option Explicit
74+
///
75+
/// Public Sub ReferenceOneClass1Procedure()
76+
/// Dim target As Class1
77+
/// Set target = new Class1
78+
/// target.DoSomething
79+
/// End Sub
80+
/// ]]>
81+
/// </module>
82+
/// </example>
83+
/// <example hasResult="false">
84+
/// <module name="Class1" type="Class Module">
85+
/// <![CDATA[
86+
/// Option Explicit
87+
///
4188
/// Public Sub DoSomething()
42-
/// ' macro is attached to a worksheet Shape.
89+
/// End Sub
90+
///
91+
/// Public Sub DoSomethingElse()
92+
/// End Sub
93+
/// ]]>
94+
/// </module>
95+
/// <module name="Module1" type="Standard Module">
96+
/// <![CDATA[
97+
/// Option Explicit
98+
///
99+
/// Public Sub ReferenceAllClass1Procedures()
100+
/// Dim target As Class1
101+
/// Set target = new Class1
102+
/// target.DoSomething
103+
/// target.DoSomethingElse
43104
/// End Sub
44105
/// ]]>
45106
/// </module>
@@ -78,8 +139,7 @@ public ProcedureNotUsedInspection(IDeclarationFinderProvider declarationFinderPr
78139
protected override bool IsResultDeclaration(Declaration declaration, DeclarationFinder finder)
79140
{
80141
return !declaration.References
81-
.Any(reference => !reference.IsAssignment
82-
&& !reference.ParentScoping.Equals(declaration)) // recursive calls don't count
142+
.Any(reference => !reference.ParentScoping.Equals(declaration)) // ignore recursive/self-referential calls
83143
&& !finder.FindEventHandlers().Contains(declaration)
84144
&& !IsPublicModuleMember(declaration)
85145
&& !IsClassLifeCycleHandler(declaration)

RubberduckTests/Inspections/ProcedureNotUsedInspectionTests.cs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,57 @@ Private Sub Foo()
182182
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
183183
}
184184

185+
//https://github.com/rubberduck-vba/Rubberduck/issues/5490
186+
[TestCase(@"Name = ""Bizz""", 0)]
187+
[TestCase(@"mName = ""Bizz""", 1)]
188+
[Category("Inspections")]
189+
public void PropertyLet(string assignmentCode, int expectedResults)
190+
{
191+
var inputCode =
192+
$@"
193+
Private mName As String
194+
195+
Private Sub Class_Initialize()
196+
{assignmentCode}
197+
End Sub
198+
199+
Private Property Let Name(ByVal value As String)
200+
mName = value
201+
End Property
202+
";
203+
204+
var modules = new(string, string, ComponentType)[]
205+
{
206+
(MockVbeBuilder.TestModuleName, inputCode, ComponentType.ClassModule),
207+
};
208+
209+
Assert.AreEqual(expectedResults, InspectionResultsForModules(modules).Count(result => result.Target.DeclarationType.HasFlag(DeclarationType.Procedure)));
210+
}
211+
212+
[Test]
213+
[Category("Inspections")]
214+
public void RecursiveReferenceOnly_ReturnsResult()
215+
{
216+
var inputCode =
217+
$@"
218+
Private mName As String
219+
220+
Private Property Let Name(ByVal value As String)
221+
mName = value
222+
If Len(mName) > 10 Then
223+
Name = Left(mName, 8)
224+
End If
225+
End Property
226+
";
227+
228+
var modules = new(string, string, ComponentType)[]
229+
{
230+
(MockVbeBuilder.TestModuleName, inputCode, ComponentType.ClassModule),
231+
};
232+
233+
Assert.AreEqual(1, InspectionResultsForModules(modules).Count(result => result.Target.DeclarationType.HasFlag(DeclarationType.Procedure)));
234+
}
235+
185236
[Test]
186237
[Category("Inspections")]
187238
public void InspectionName()

0 commit comments

Comments
 (0)