Skip to content

Commit 8c6c97e

Browse files
committed
Merge branch 'rubberduck-vba/next' into RefactorExtensionMethods_5419
2 parents dda811c + 65bf91e commit 8c6c97e

File tree

6 files changed

+66
-13
lines changed

6 files changed

+66
-13
lines changed

Rubberduck.CodeAnalysis/Inspections/Abstract/MemberAccessMayReturnNothingInspectionBase.cs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,20 @@ protected MemberAccessMayReturnNothingInspectionBase(IDeclarationFinderProvider
1919
: base(declarationFinderProvider)
2020
{}
2121

22+
/// <summary>
23+
/// Members that might return Nothing
24+
/// </summary>
25+
/// <remarks>
26+
/// It must not be legal to call the members unqualified. In particular, user-defined members will not be considered.
27+
/// Moreover, this disqualifies all members on global objects.
28+
/// </remarks>
2229
public abstract IEnumerable<Declaration> MembersUnderTest(DeclarationFinder finder);
2330
public abstract string ResultTemplate { get; }
2431

2532
protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
2633
{
27-
return MembersUnderTest(finder);
34+
//This restriction is in place because the inspection currently cannot handle unqualified accesses.
35+
return MembersUnderTest(finder).Where(member => !member.IsUserDefined);
2836
}
2937

3038
protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
@@ -46,7 +54,7 @@ protected override bool IsResultReference(IdentifierReference reference, Declara
4654
{
4755
return usageContext is VBAParser.MemberAccessExprContext
4856
|| !(usageContext is VBAParser.CallStmtContext)
49-
&& !ContextIsNothingTest(usageContext);
57+
&& !ContextIsNothing(usageContext);
5058
}
5159

5260
var assignedTo = AssignmentTarget(reference, finder, setter);
@@ -65,14 +73,21 @@ private static IdentifierReference AssignmentTarget(IdentifierReference referenc
6573

6674
private static RuleContext UsageContext(IdentifierReference reference)
6775
{
68-
var access = reference.Context.GetAncestor<VBAParser.MemberAccessExprContext>();
69-
var usageContext = access.Parent is VBAParser.IndexExprContext indexExpr
76+
//We prefer the with member access over the member access, because the accesses are resolved right to left.
77+
var access = reference.Context.GetAncestor<VBAParser.WithMemberAccessExprContext>() as VBAParser.LExpressionContext
78+
?? reference.Context.GetAncestor<VBAParser.MemberAccessExprContext>();
79+
80+
if (access == null)
81+
{
82+
return null;
83+
}
84+
85+
return access.Parent is VBAParser.IndexExprContext indexExpr
7086
? indexExpr.Parent
7187
: access.Parent;
72-
return usageContext;
7388
}
7489

75-
private static bool ContextIsNothingTest(IParseTree context)
90+
private static bool ContextIsNothing(IParseTree context)
7691
{
7792
return context is VBAParser.LExprContext
7893
&& context.Parent is VBAParser.RelationalOpContext comparison
@@ -86,7 +101,7 @@ private static bool IsUsedBeforeCheckingForNothing(IdentifierReference assignedT
86101
var firstUse = GetReferenceNodes(tree).FirstOrDefault();
87102

88103
return !(firstUse is null)
89-
&& !ContextIsNothingTest(firstUse.Reference.Context.Parent);
104+
&& !ContextIsNothing(firstUse.Reference.Context.Parent);
90105
}
91106

92107
private static IEnumerable<INode> GetReferenceNodes(INode node)

Rubberduck.Core/UI/Inspections/InspectionResultsControl.xaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@
200200
SelectedItem="{Binding SelectedItem}"
201201
SelectionUnit="FullRow"
202202
ItemsSource="{Binding Results, NotifyOnSourceUpdated=True}"
203-
RequestBringIntoView="InspectionResultsGrid_RequestBringIntoView"
204203
VirtualizingPanel.IsVirtualizingWhenGrouping="True"
205204
ScrollViewer.CanContentScroll="True"
206205
ScrollViewer.VerticalScrollBarVisibility="Auto"

Rubberduck.Core/UI/Refactorings/EncapsulateField/EncapsulateFieldView.xaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@
144144
Text="{Binding Path=PropertyName, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"
145145
TabIndex="1" Margin="10,5"
146146
VerticalAlignment="Center"
147+
VerticalContentAlignment="Center"
147148
Height="22" />
148149
<Image Grid.Row="1" Style="{StaticResource InvalidNameIconStyle}"
149150
Visibility="{Binding Path=SelectionHasValidEncapsulationAttributes, Converter={StaticResource BoolToHiddenVisibility}}" />

Rubberduck.Core/UI/UnitTesting/TestExplorerViewModel.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ public bool CanExecuteIgnoreSelectedTests(object obj)
256256

257257
return false;
258258
}
259+
259260
public bool CanExecuteUnignoreSelectedTests(object obj)
260261
{
261262
if (!Model.IsBusy && obj is IList viewModels && viewModels.Count > 0)
@@ -273,12 +274,14 @@ public bool CanExecuteIgnoreGroupCommand(object obj)
273274

274275
return groupItems.Cast<TestMethodViewModel>().Count(test => test.Method.IsIgnored) != groupItems.Count;
275276
}
277+
276278
public bool CanExecuteUnignoreGroupCommand(object obj)
277279
{
278280
var groupItems = MouseOverGroup?.Items
279-
?? GroupContainingSelectedTest(MouseOverTest).Items;
280-
281-
return groupItems.Cast<TestMethodViewModel>().Any(test => test.Method.IsIgnored);
281+
?? GroupContainingSelectedTest(MouseOverTest)?.Items;
282+
283+
return groupItems != null
284+
&& groupItems.Cast<TestMethodViewModel>().Any(test => test.Method.IsIgnored);
282285
}
283286

284287
#region Commands

Rubberduck.Resources/Inspections/InspectionResults.de.resx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@
183183
<value>Der Variable '{0}' wird kein Wert zugewiesen.</value>
184184
</data>
185185
<data name="EmptyStringLiteralInspection" xml:space="preserve">
186-
<value>vbNullString' sollte statt einem leeren String-Literal verwendet werden.</value>
186+
<value>'vbNullString' sollte statt einem leeren String-Literal verwendet werden.</value>
187187
</data>
188188
<data name="ObjectVariableNotSetInspection" xml:space="preserve">
189189
<value>Objektvariable '{0}' wird ohne das 'Set'-Schlüsselwort zugewiesen.</value>
@@ -467,4 +467,4 @@ In Memoriam, 1972-2018</value>
467467
<data name="SuperfluousAnnotationArgumentInspection" xml:space="preserve">
468468
<value>Die Annotation '{0}' erwartet weniger Argumente.</value>
469469
</data>
470-
</root>
470+
</root>

RubberduckTests/Inspections/ExcelMemberMayReturnNothingInspectionTests.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,22 @@ End Sub
2626
Assert.AreEqual(1, InspectionResults(inputCode).Count());
2727
}
2828

29+
[Test]
30+
[Category("Inspections")]
31+
public void ExcelMemberMayReturnNothing_ReturnsResult_WithMemberAccessOnFind()
32+
{
33+
const string inputCode =
34+
@"Sub UnderTest()
35+
Dim ws As Worksheet
36+
Set ws = Sheet1
37+
With ws.UsedRange
38+
foo = .Find(""foo"").Row
39+
End With
40+
End Sub
41+
";
42+
Assert.AreEqual(1, InspectionResults(inputCode).Count());
43+
}
44+
2945
[Test]
3046
[Category("Inspections")]
3147
public void ExcelMemberMayReturnNothing_Ignored_DoesNotReturnResult()
@@ -111,6 +127,25 @@ End Sub
111127
Assert.AreEqual(1, InspectionResults(inputCode).Count());
112128
}
113129

130+
[Test]
131+
[Category("Inspections")]
132+
public void ExcelMemberMayReturnNothing_ReturnsResult_AssignedAndNotTested_FromWithMemberAccess()
133+
{
134+
const string inputCode =
135+
@"Sub UnderTest()
136+
Dim ws As Worksheet
137+
Set ws = Sheet1
138+
Dim result As Range
139+
With ws.UsedRange
140+
Set result = .Find(""foo"")
141+
End With
142+
result.Value = ""bar""
143+
End Sub
144+
";
145+
146+
Assert.AreEqual(1, InspectionResults(inputCode).Count());
147+
}
148+
114149
[Test]
115150
[Category("Inspections")]
116151
public void ExcelMemberMayReturnNothing_ReturnsResult_ResultIsSomethingElse()

0 commit comments

Comments
 (0)