Skip to content

Commit 999a846

Browse files
committed
Stop reporting implicit accesses to worksheets by name in SheetAccessedUsingStringInspection
Unqualified accesses and accesses through Application are accesses on the ActiveWorkbook according to the documentations. That might not be the same workbook as ThisWorkbook.
1 parent fe3641d commit 999a846

File tree

3 files changed

+7
-108
lines changed

3 files changed

+7
-108
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/Excel/SheetAccessedUsingStringInspection.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ namespace Rubberduck.Inspections.Concrete
4747
[RequiredLibrary("Excel")]
4848
public class SheetAccessedUsingStringInspection : IdentifierReferenceInspectionFromDeclarationsBase
4949
{
50-
//TODO: revisit this and its tests after clarification of intended behaviour.
51-
//This relates to the handling of implicit references to ActiveWorkbook.
52-
5350
private readonly IProjectsProvider _projectsProvider;
5451

5552
public SheetAccessedUsingStringInspection(RubberduckParserState state, IProjectsProvider projectsProvider)
@@ -65,7 +62,7 @@ public SheetAccessedUsingStringInspection(RubberduckParserState state, IProjects
6562

6663
private static readonly string[] InterestingClasses =
6764
{
68-
"_Global", "_Application", "Global", "Application", "Workbook"
65+
"Workbook"
6966
};
7067

7168
protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)

RubberduckTests/Inspections/SheetAccessedUsingStringInspectionTests.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,30 @@ public void SheetAccessedUsingString_ReturnsResult_AccessingUsingWorkbookModule(
2929

3030
[Test]
3131
[Category("Inspections")]
32-
public void SheetAccessedUsingString_ReturnsResult_AccessingUsingApplicationModule()
32+
//Access via Application is an access on the ActiveWorkbook, not necessarily ThisWorkbook.
33+
public void SheetAccessedUsingString_ReturnsNoResult_AccessingUsingApplicationModule()
3334
{
3435
const string inputCode =
3536
@"Public Sub Foo()
3637
Application.Worksheets(""Sheet1"").Range(""A1"") = ""Foo""
3738
Application.Sheets(""Sheet1"").Range(""A1"") = ""Foo""
3839
End Sub";
3940

40-
Assert.AreEqual(2, ArrangeParserAndGetResults(inputCode).Count());
41+
Assert.AreEqual(0, ArrangeParserAndGetResults(inputCode).Count());
4142
}
4243

4344
[Test]
4445
[Category("Inspections")]
45-
public void SheetAccessedUsingString_ReturnsResult_AccessingUsingGlobalModule()
46+
//Unqualified access is an access on the ActiveWorkbook, not necessarily ThisWorkbook.
47+
public void SheetAccessedUsingString_ReturnsNoResult_AccessingUsingGlobalModule()
4648
{
4749
const string inputCode =
4850
@"Public Sub Foo()
4951
Worksheets(""Sheet1"").Range(""A1"") = ""Foo""
5052
Sheets(""Sheet1"").Range(""A1"") = ""Foo""
5153
End Sub";
5254

53-
Assert.AreEqual(2, ArrangeParserAndGetResults(inputCode).Count());
55+
Assert.AreEqual(0, ArrangeParserAndGetResults(inputCode).Count());
5456
}
5557

5658
[Test]

RubberduckTests/QuickFixes/AccessSheetUsingCodeNameQuickFixTests.cs

Lines changed: 0 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -31,42 +31,6 @@ Public Sub Foo()
3131
Assert.AreEqual(expectedCode, actualCode);
3232
}
3333

34-
[Test]
35-
[Category("QuickFixes")]
36-
public void SheetAccessedUsingString_QuickFixWorks_UsingSheetThroughApplicationModule()
37-
{
38-
const string inputCode = @"
39-
Public Sub Foo()
40-
Application.Sheets(""Sheet1"").Range(""A1"") = ""foo""
41-
End Sub";
42-
43-
const string expectedCode = @"
44-
Public Sub Foo()
45-
Sheet1.Range(""A1"") = ""foo""
46-
End Sub";
47-
48-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
49-
Assert.AreEqual(expectedCode, actualCode);
50-
}
51-
52-
[Test]
53-
[Category("QuickFixes")]
54-
public void SheetAccessedUsingString_QuickFixWorks_UsingSheetThroughGlobalModule()
55-
{
56-
const string inputCode = @"
57-
Public Sub Foo()
58-
Sheets(""Sheet1"").Range(""A1"") = ""foo""
59-
End Sub";
60-
61-
const string expectedCode = @"
62-
Public Sub Foo()
63-
Sheet1.Range(""A1"") = ""foo""
64-
End Sub";
65-
66-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
67-
Assert.AreEqual(expectedCode, actualCode);
68-
}
69-
7034
[Test]
7135
[Category("QuickFixes")]
7236
public void SheetAccessedUsingString_QuickFixWorks_AssigningSheetToVariable()
@@ -220,70 +184,6 @@ Dim ws As Worksheet
220184
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
221185
Assert.AreEqual(expectedCode, actualCode);
222186
}
223-
224-
[Test]
225-
[Category("QuickFixes")]
226-
public void SheetAccessedUsingString_QuickFixWorks_TransientReferenceSetStatement()
227-
{
228-
const string inputCode = @"
229-
Sub Test()
230-
Dim ws As Worksheet
231-
Set ws = Worksheets.Add(Worksheets(""Sheet1""))
232-
Debug.Print ws.Name
233-
End Sub";
234-
235-
const string expectedCode = @"
236-
Sub Test()
237-
Dim ws As Worksheet
238-
Set ws = Worksheets.Add(Sheet1)
239-
Debug.Print ws.Name
240-
End Sub";
241-
242-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
243-
Assert.AreEqual(expectedCode, actualCode);
244-
}
245-
246-
[Test]
247-
[Category("QuickFixes")]
248-
public void SheetAccessedUsingString_QuickFixWorks_TransientReferenceNoSetStatement()
249-
{
250-
const string inputCode = @"
251-
Sub Test()
252-
If Not Worksheets.Add(Worksheets(""Sheet1"")) Is Nothing Then
253-
Debug.Print ""Added""
254-
End If
255-
End Sub";
256-
257-
const string expectedCode = @"
258-
Sub Test()
259-
If Not Worksheets.Add(Sheet1) Is Nothing Then
260-
Debug.Print ""Added""
261-
End If
262-
End Sub";
263-
264-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
265-
Assert.AreEqual(expectedCode, actualCode);
266-
}
267-
268-
[Test]
269-
[Category("QuickFixes")]
270-
public void SheetAccessedUsingString_QuickFixWorks_ImplicitVariableAssignment()
271-
{
272-
const string inputCode = @"
273-
Sub Test()
274-
Set ws = Worksheets(""Sheet1"")
275-
ws.Name = ""Foo""
276-
End Sub";
277-
278-
const string expectedCode = @"
279-
Sub Test()
280-
281-
Sheet1.Name = ""Foo""
282-
End Sub";
283-
284-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new SheetAccessedUsingStringInspection(state, state.ProjectsProvider));
285-
Assert.AreEqual(expectedCode, actualCode);
286-
}
287187

288188
protected override IQuickFix QuickFix(RubberduckParserState state)
289189
{

0 commit comments

Comments
 (0)