Skip to content

Commit 4ee1d44

Browse files
Hosch250Hosch250
committed
Fires for interface params where all can by byval
1 parent 4c4a4d8 commit 4ee1d44

File tree

3 files changed

+132
-9
lines changed

3 files changed

+132
-9
lines changed

‎RetailCoder.VBE/Common/DeclarationExtensions.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,12 @@ public static IEnumerable<Declaration> FindInterfaceImplementationMembers(this I
451451
.Where(m => m.IdentifierName.EndsWith(interfaceMember));
452452
}
453453

454+
public static IEnumerable<Declaration> FindInterfaceImplementationMembers(this IEnumerable<Declaration> declarations, Declaration interfaceDeclaration)
455+
{
456+
return FindInterfaceImplementationMembers(declarations)
457+
.Where(m => m.IdentifierName == interfaceDeclaration.ComponentName + "_" + interfaceDeclaration.IdentifierName);
458+
}
459+
454460
public static Declaration FindInterfaceMember(this IEnumerable<Declaration> declarations, Declaration implementation)
455461
{
456462
var members = FindInterfaceMembers(declarations);

‎RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,50 @@ public ParameterCanBeByValInspection(RubberduckParserState state)
2121
public override IEnumerable<InspectionResultBase> GetInspectionResults()
2222
{
2323
var declarations = UserDeclarations.ToList();
24+
var issues = new List<ParameterCanBeByValInspectionResult>();
2425

25-
IEnumerable<Declaration> interfaceMembers = null;
26-
interfaceMembers = declarations.FindInterfaceMembers()
27-
.Concat(declarations.FindInterfaceImplementationMembers())
28-
.ToList();
26+
var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList();
27+
var interfaceImplementationMembers = declarations.FindInterfaceImplementationMembers().ToList();
28+
var allInterfaceMembers = interfaceImplementationMembers.Concat(interfaceDeclarationMembers);
29+
30+
foreach (var member in interfaceDeclarationMembers)
31+
{
32+
var declarationParameters =
33+
declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter &&
34+
declaration.ParentDeclaration == member)
35+
.OrderBy(o => o.Selection.StartLine)
36+
.ThenBy(t => t.Selection.StartColumn)
37+
.ToList();
38+
39+
var parametersAreByRef = declarationParameters.Select(s => true).ToList();
40+
41+
var implementations = declarations.FindInterfaceImplementationMembers(member).ToList();
42+
foreach (var implementation in implementations)
43+
{
44+
var parameters =
45+
declarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter &&
46+
declaration.ParentDeclaration == implementation)
47+
.OrderBy(o => o.Selection.StartLine)
48+
.ThenBy(t => t.Selection.StartColumn)
49+
.ToList();
50+
51+
for (var i = 0; i < parameters.Count; i++)
52+
{
53+
parametersAreByRef[i] = parametersAreByRef[i] && !IsUsedAsByRefParam(declarations, parameters[i]) &&
54+
((VBAParser.ArgContext)parameters[i].Context).BYVAL() == null &&
55+
!parameters[i].References.Any(reference => reference.IsAssignment);
56+
}
57+
}
58+
59+
for (var i = 0; i < declarationParameters.Count; i++)
60+
{
61+
if (parametersAreByRef[i])
62+
{
63+
issues.Add(new ParameterCanBeByValInspectionResult(this, declarationParameters[i],
64+
declarationParameters[i].Context, declarationParameters[i].QualifiedName));
65+
}
66+
}
67+
}
2968

3069
var formEventHandlerScopes = State.FindFormEventHandlers()
3170
.Select(handler => handler.Scope);
@@ -41,15 +80,15 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
4180

4281
var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes);
4382

44-
var issues = declarations.Where(declaration =>
83+
issues.AddRange(declarations.Where(declaration =>
4584
!declaration.IsArray
4685
&& !ignoredScopes.Contains(declaration.ParentScope)
4786
&& declaration.DeclarationType == DeclarationType.Parameter
48-
&& !interfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope)
87+
&& !allInterfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope)
4988
&& ((VBAParser.ArgContext)declaration.Context).BYVAL() == null
5089
&& !IsUsedAsByRefParam(declarations, declaration)
5190
&& !declaration.References.Any(reference => reference.IsAssignment))
52-
.Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName));
91+
.Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName)));
5392

5493
return issues;
5594
}

‎RubberduckTests/Inspections/ParameterCanBeByValInspectionTests.cs‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ Sub Foo(arg1 As String)
334334
}
335335

336336
[TestMethod]
337-
public void ParameterCanBeByVal_InterfaceMember()
337+
public void ParameterCanBeByVal_InterfaceMember_SingleParam()
338338
{
339339
//Input
340340
const string inputCode1 =
@@ -351,6 +351,43 @@ Private Sub IClass1_DoSomething(ByRef a As Integer)
351351
var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
352352
.AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1)
353353
.AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
354+
.AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
355+
.Build();
356+
var vbe = builder.AddProject(project).Build();
357+
358+
var mockHost = new Mock<IHostApplication>();
359+
mockHost.SetupAllProperties();
360+
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
361+
362+
parser.Parse(new CancellationTokenSource());
363+
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
364+
365+
var inspection = new ParameterCanBeByValInspection(parser.State);
366+
var inspectionResults = inspection.GetInspectionResults();
367+
368+
Assert.AreEqual(1, inspectionResults.Count());
369+
}
370+
371+
[TestMethod]
372+
public void ParameterCanBeByVal_InterfaceMember_SingleParamUsedByRef()
373+
{
374+
//Input
375+
const string inputCode1 =
376+
@"Public Sub DoSomething(ByRef a As Integer)
377+
End Sub";
378+
const string inputCode2 =
379+
@"Implements IClass1
380+
381+
Private Sub IClass1_DoSomething(ByRef a As Integer)
382+
a = 42
383+
End Sub";
384+
385+
//Arrange
386+
var builder = new MockVbeBuilder();
387+
var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
388+
.AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1)
389+
.AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
390+
.AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
354391
.Build();
355392
var vbe = builder.AddProject(project).Build();
356393

@@ -366,7 +403,48 @@ Private Sub IClass1_DoSomething(ByRef a As Integer)
366403

367404
Assert.IsFalse(inspectionResults.Any());
368405
}
369-
406+
407+
[TestMethod]
408+
public void ParameterCanBeByVal_InterfaceMember_MultipleParams_OneCanBeByVal()
409+
{
410+
//Input
411+
const string inputCode1 =
412+
@"Public Sub DoSomething(ByRef a As Integer, ByRef b As Integer)
413+
End Sub";
414+
const string inputCode2 =
415+
@"Implements IClass1
416+
417+
Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer)
418+
b = 42
419+
End Sub";
420+
const string inputCode3 =
421+
@"Implements IClass1
422+
423+
Private Sub IClass1_DoSomething(ByRef a As Integer, ByRef b As Integer)
424+
End Sub";
425+
426+
//Arrange
427+
var builder = new MockVbeBuilder();
428+
var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
429+
.AddComponent("IClass1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1)
430+
.AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
431+
.AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode3)
432+
.Build();
433+
var vbe = builder.AddProject(project).Build();
434+
435+
var mockHost = new Mock<IHostApplication>();
436+
mockHost.SetupAllProperties();
437+
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
438+
439+
parser.Parse(new CancellationTokenSource());
440+
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
441+
442+
var inspection = new ParameterCanBeByValInspection(parser.State);
443+
var inspectionResults = inspection.GetInspectionResults();
444+
445+
Assert.AreEqual("a", inspectionResults.Single().Target.IdentifierName);
446+
}
447+
370448
[TestMethod]
371449
public void ParameterCanBeByVal_Event()
372450
{

0 commit comments

Comments
 (0)