Skip to content

Commit b8da683

Browse files
authored
Merge pull request #5652 from BZngr/5628_MisleadingByRefFalsePositive
Fix MisleadingByRefParameterInspection false positive: array parameters
2 parents 0b86dd3 + 62cd4ac commit b8da683

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/MisleadingByRefParameterInspection.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,17 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
1313
/// <why>
1414
/// Regardless of the presence or absence of an explicit ByRef or ByVal modifier, the value-parameter
1515
/// of a property mutator is always treated as though it had an explicit ByVal modifier.
16-
/// Exception: UserDefinedType parameters are always passed by reference.
16+
/// Exception: UserDefinedType and Array parameters are always passed by reference.
1717
/// </why>
1818
/// <example hasResult="true">
1919
/// <module name="MyModule" type="Standard Module">
2020
/// <![CDATA[
2121
/// Private fizzField As Long
2222
/// Public Property Get Fizz() As Long
23-
/// Fizz = fizzFiled
23+
/// Fizz = fizzField
2424
/// End Property
2525
/// Public Property Let Fizz(ByRef arg As Long)
26-
/// fizzFiled = arg
26+
/// fizzField = arg
2727
/// End Property
2828
/// ]]>
2929
/// </module>
@@ -33,10 +33,10 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
3333
/// <![CDATA[
3434
/// Private fizzField As Long
3535
/// Public Property Get Fizz() As Long
36-
/// Fizz = fizzFiled
36+
/// Fizz = fizzField
3737
/// End Property
3838
/// Public Property Let Fizz(arg As Long)
39-
/// fizzFiled = arg
39+
/// fizzField = arg
4040
/// End Property
4141
/// ]]>
4242
/// </module>
@@ -50,14 +50,18 @@ public MisleadingByRefParameterInspection(IDeclarationFinderProvider declaration
5050
protected override bool IsResultDeclaration(Declaration declaration, DeclarationFinder finder)
5151
{
5252
return declaration is ParameterDeclaration parameter
53-
&& !(parameter.AsTypeDeclaration?.DeclarationType.HasFlag(DeclarationType.UserDefinedType) ?? false)
54-
&& parameter.ParentDeclaration is ModuleBodyElementDeclaration enclosingMethod
53+
&& !IsAlwaysByRef(declaration)
54+
&& declaration.ParentDeclaration is ModuleBodyElementDeclaration enclosingMethod
5555
&& (enclosingMethod.DeclarationType.HasFlag(DeclarationType.PropertyLet)
5656
|| enclosingMethod.DeclarationType.HasFlag(DeclarationType.PropertySet))
5757
&& enclosingMethod.Parameters.Last() == parameter
5858
&& parameter.IsByRef && !parameter.IsImplicitByRef;
5959
}
6060

61+
private static bool IsAlwaysByRef(Declaration parameter)
62+
=> parameter.IsArray
63+
|| (parameter.AsTypeDeclaration?.DeclarationType.HasFlag(DeclarationType.UserDefinedType) ?? false);
64+
6165
protected override string ResultDescription(Declaration declaration)
6266
{
6367
return string.Format(

RubberduckTests/Inspections/MisleadingByRefParameterInspectionTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,31 @@ End Property
5454
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
5555
}
5656

57+
//https://github.com/rubberduck-vba/Rubberduck/issues/5628
58+
[TestCase("ArrayToStore")]
59+
[TestCase("ByRef ArrayToStore")]
60+
[Category("QuickFixes")]
61+
[Category(nameof(MisleadingByRefParameterInspection))]
62+
public void ArrayEdgeCase(string parameterMechanismAndParam)
63+
{
64+
var inputCode =
65+
$@"
66+
Option Explicit
67+
68+
Private InternalArray() As Variant
69+
70+
Public Property Get StoredArray() As Variant()
71+
StoredArray = InternalArray
72+
End Property
73+
74+
Public Property Let StoredArray({parameterMechanismAndParam}() As Variant)
75+
InternalArray = ArrayToStore
76+
End Property
77+
";
78+
79+
Assert.AreEqual(0, InspectionResultsForStandardModule(inputCode).Count());
80+
}
81+
5782
[Test]
5883
[Category("QuickFixes")]
5984
[Category(nameof(MisleadingByRefParameterInspection))]

0 commit comments

Comments
 (0)