Skip to content

Commit 535a887

Browse files
MDoernerMDoerner
committed
Fix false negatives for UnassignedVariableUsageInspection
1 parent b612280 commit 535a887

File tree

3 files changed

+186
-17
lines changed

3 files changed

+186
-17
lines changed

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,10 @@ private static IEnumerable<QualifiedSelection> SingleVariableArgumentSelections(
129129

130130
protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
131131
{
132-
//FIXME: stop filtering out too many results.
133132
return reference != null
134133
&& !IsAssignedByRefArgument(reference.ParentScoping, reference, finder)
135134
&& !IsArraySubscriptAssignment(reference)
136-
&& !reference.Context.TryGetAncestor<VBAParser.RedimStmtContext>(out _);
137-
135+
&& !IsArrayReDim(reference);
138136
}
139137

140138
protected override string ResultDescription(IdentifierReference reference, dynamic properties = null)
@@ -158,12 +156,29 @@ private static bool IsAssignedByRefArgument(Declaration enclosingProcedure, Iden
158156

159157
private static bool IsArraySubscriptAssignment(IdentifierReference reference)
160158
{
161-
//FIXME: stop returning true for too many cases.
162-
var isLetAssignment = reference.Context.TryGetAncestor<VBAParser.LetStmtContext>(out var letStmt);
163-
var isSetAssignment = reference.Context.TryGetAncestor<VBAParser.SetStmtContext>(out var setStmt);
159+
var nameExpression = reference.Context;
160+
if (!(nameExpression.Parent is VBAParser.IndexExprContext indexExpression))
161+
{
162+
return false;
163+
}
164+
165+
var callingExpression = indexExpression.Parent;
166+
167+
return callingExpression is VBAParser.SetStmtContext
168+
|| callingExpression is VBAParser.LetStmtContext;
169+
}
170+
171+
private static bool IsArrayReDim(IdentifierReference reference)
172+
{
173+
var nameExpression = reference.Context;
174+
if (!(nameExpression.Parent is VBAParser.IndexExprContext indexExpression))
175+
{
176+
return false;
177+
}
178+
179+
var reDimVariableStmt = indexExpression.Parent?.Parent;
164180

165-
return isLetAssignment && letStmt.lExpression() is VBAParser.IndexExprContext
166-
|| isSetAssignment && setStmt.lExpression() is VBAParser.IndexExprContext;
181+
return reDimVariableStmt is VBAParser.RedimVariableDeclarationContext;
167182
}
168183
}
169184
}

‎RubberduckTests/Inspections/UnassignedVariableUsageInspectionTests.cs

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,22 @@ End Sub
3838

3939
[Test]
4040
[Category("Inspections")]
41-
public void IgnoresArraySubscripts()
41+
public void DoNotIgnoresArrayReDimBounds()
42+
{
43+
const string code = @"
44+
Sub Foo()
45+
Dim bar As Variant
46+
Dim baz As Variant
47+
Dim foo As Variant
48+
ReDim bar(baz To foo)
49+
End Sub
50+
";
51+
Assert.AreEqual(2, InspectionResultsForStandardModule(code).Count());
52+
}
53+
54+
[Test]
55+
[Category("Inspections")]
56+
public void IgnoresArraySubscripts_Let()
4257
{
4358
const string code = @"
4459
Sub Foo()
@@ -50,6 +65,139 @@ End Sub
5065
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
5166
}
5267

68+
[Test]
69+
[Category("Inspections")]
70+
public void DoNotIgnoreArrayIndexes_Let()
71+
{
72+
const string code = @"
73+
Sub Foo()
74+
Dim bar As Variant
75+
Dim foo As Variant
76+
ReDim bar(1 To 10)
77+
bar(foo) = 42
78+
End Sub
79+
";
80+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
81+
}
82+
83+
[Test]
84+
[Category("Inspections")]
85+
public void DoNotIgnoreValuesAssignedToArraySubscripts_Let()
86+
{
87+
const string code = @"
88+
Sub Foo()
89+
Dim bar As Variant
90+
Dim foo As Variant
91+
ReDim bar(1 To 10)
92+
bar(1) = foo
93+
End Sub
94+
";
95+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
96+
}
97+
98+
[Test]
99+
[Category("Inspections")]
100+
public void DoNotIgnoreIndexedPropertyAccess_Let()
101+
{
102+
const string code = @"
103+
Sub Foo()
104+
Dim foo As Variant
105+
ReDim bar(1 To 10)
106+
foo.Bar(1) = 42
107+
End Sub
108+
";
109+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
110+
}
111+
112+
[Test]
113+
[Category("Inspections")]
114+
public void IgnoresArraySubscripts_Set()
115+
{
116+
const string code = @"
117+
Sub Foo()
118+
Dim bar As Variant
119+
ReDim bar(1 To 10)
120+
Set bar(1) = Nothing
121+
End Sub
122+
";
123+
Assert.AreEqual(0, InspectionResultsForStandardModule(code).Count());
124+
}
125+
126+
[Test]
127+
[Category("Inspections")]
128+
public void DoNotIgnoreArrayIndexes_Set()
129+
{
130+
const string code = @"
131+
Sub Foo()
132+
Dim bar As Variant
133+
Dim foo As Variant
134+
ReDim bar(1 To 10)
135+
Set bar(foo) = Nothing
136+
End Sub
137+
";
138+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
139+
}
140+
141+
[Test]
142+
[Category("Inspections")]
143+
public void DoNotIgnoreValuesAssignedToArraySubscripts_Set()
144+
{
145+
const string code = @"
146+
Sub Foo()
147+
Dim bar As Variant
148+
Dim foo As Variant
149+
ReDim bar(1 To 10)
150+
Set bar(1) = foo
151+
End Sub
152+
";
153+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
154+
}
155+
156+
[Test]
157+
[Category("Inspections")]
158+
public void DoNotIgnoreIndexedPropertyAccess_Set()
159+
{
160+
const string code = @"
161+
Sub Foo()
162+
Dim foo As Variant
163+
ReDim bar(1 To 10)
164+
Set foo.Bar(1) = 42
165+
End Sub
166+
";
167+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
168+
}
169+
170+
[Test]
171+
[Category("Inspections")]
172+
public void DoesNotIgnoreWithBlockVariableUse()
173+
{
174+
const string code = @"
175+
Sub Foo()
176+
Dim foo As Variant
177+
With foo
178+
End With
179+
End Sub
180+
";
181+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
182+
}
183+
184+
[Test]
185+
[Category("Inspections")]
186+
public void IgnoreUseViaWithBlockVariableInWithBlock()
187+
{
188+
const string code = @"
189+
Sub Foo()
190+
Dim foo As Variant
191+
Dim bar As Variant
192+
With foo
193+
bar = .Baz + 23
194+
bar = .Baz + 42
195+
End With
196+
End Sub
197+
";
198+
Assert.AreEqual(1, InspectionResultsForStandardModule(code).Count());
199+
}
200+
53201
[Test]
54202
[Category("Inspections")]
55203
public void UnassignedVariableUsage_ReturnsResult()

‎RubberduckTests/QuickFixes/RemoveUnassignedVariableUsageQuickFixTests.cs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,29 @@ public void UnassignedVariableUsage_QuickFixWorksWithBlock()
4343
{
4444
const string inputCode =
4545
@"Sub test()
46-
Dim wb As Workbook
46+
Dim wb As Variant
4747
With wb
48-
Debug.Print .Name
49-
Debug.Print .Name
50-
Debug.Print .Name
48+
Bar .Name
49+
Bar .Name
50+
Bar .Name
5151
End With
52+
End Sub
53+
54+
Private Sub Bar(ByVal arg)
5255
End Sub";
5356

5457
const string expectedCode =
5558
@"Sub test()
56-
Dim wb As Workbook
59+
Dim wb As Variant
5760
'TODO - {0}
5861
' With wb
59-
' Debug.Print .Name
60-
' Debug.Print .Name
61-
' Debug.Print .Name
62+
' Bar .Name
63+
' Bar .Name
64+
' Bar .Name
6265
' End With
66+
End Sub
67+
68+
Private Sub Bar(ByVal arg)
6369
End Sub";
6470

6571
var (actual, inspectionDescription) =

0 commit comments

Comments
 (0)