Rubberduck: ParameterCanBeByVal throws ArgumentOutOfRange

Created on 9 Apr 2018  路  2Comments  路  Source: rubberduck-vba/Rubberduck

Found in the logs for #3905:

2018-04-09 08:25:58.0942;WARN-2.2.6672.28001;Rubberduck.Inspections.Rubberduck.Inspections.Inspector;System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at Rubberduck.Inspections.Concrete.ParameterCanBeByValInspection.<GetResults>d__3.MoveNext() in C:\projects\rubberduck\Rubberduck.Inspections\Concrete\ParameterCanBeByValInspection.cs:line 94
   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)
   at Rubberduck.Inspections.Concrete.ParameterCanBeByValInspection.DoGetInspectionResults() in C:\projects\rubberduck\Rubberduck.Inspections\Concrete\ParameterCanBeByValInspection.cs:line 29
   at Rubberduck.Inspections.Abstract.InspectionBase.GetInspectionResults(CancellationToken token) in C:\projects\rubberduck\Rubberduck.Inspections\Abstract\InspectionBase.cs:line 166
   at Rubberduck.Inspections.Rubberduck.Inspections.Inspector.RunInspection(IInspection inspection, ConcurrentBag`1 allIssues, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Inspections\Inspector.cs:line 157;System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at Rubberduck.Inspections.Concrete.ParameterCanBeByValInspection.<GetResults>d__3.MoveNext() in C:\projects\rubberduck\Rubberduck.Inspections\Concrete\ParameterCanBeByValInspection.cs:line 94
   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)
   at Rubberduck.Inspections.Concrete.ParameterCanBeByValInspection.DoGetInspectionResults() in C:\projects\rubberduck\Rubberduck.Inspections\Concrete\ParameterCanBeByValInspection.cs:line 29
   at Rubberduck.Inspections.Abstract.InspectionBase.GetInspectionResults(CancellationToken token) in C:\projects\rubberduck\Rubberduck.Inspections\Abstract\InspectionBase.cs:line 166
   at Rubberduck.Inspections.Rubberduck.Inspections.Inspector.RunInspection(IInspection inspection, ConcurrentBag`1 allIssues, CancellationToken token) in C:\projects\rubberduck\Rubberduck.Inspections\Inspector.cs:line 157

We should not be throwing that exception when analyzing any code. Let's find out where this comes from and fix it. No bandaids, please :)

bug difficulty-01-duckling feature-inspections up-for-grabs

Most helpful comment

As the grand winner of the test case finder contest, I am obliged by @comintern to provide a MVCE.

This apparently involves implementation of an interface; there is no problem when it's a straight property but as an implementation, this causes the assertion to fail. Furthermore, the property must have an extra parameter (e.g. an indexer for instance). Implemented property with no indexer does not seem to trip the assertion.

Class1 (aka the interface to implement)

Public Enum Bar
    a
    b
    c
End Enum

Public Property Get Foo(Bar As Bar) As String
End Property

Public Property Let Foo(Bar As Bar, NewValue As String)
End Property

Class2 (aka the implementing class)

Implements Class1

Private Property Let Class1_Foo(Bar As Bar, RHS As String)
End Property

Private Property Get Class1_Foo(Bar As Bar) As String
End Property

This fails the debug assertion; having one parameter too short.

All 2 comments

The problem is here:

for (var i = 0; i < parameters.Count; i++)
{
    parametersAreByRef[i] = parametersAreByRef[i] &&
                            !IsUsedAsByRefParam(declarations, parameters[i]) &&
                            ((VBAParser.ArgContext) parameters[i].Context).BYVAL() == null &&
                            !parameters[i].References.Any(reference => reference.IsAssignment);
}

Note that the indexing of the parameters List is bounded by the condition of the for loop. The indexing of the parametersAreByRef List is unchecked. I can't spot the error with casual static analysis, but this will throw the exeception above if parametersAreByRef has less members than parameters

As the grand winner of the test case finder contest, I am obliged by @comintern to provide a MVCE.

This apparently involves implementation of an interface; there is no problem when it's a straight property but as an implementation, this causes the assertion to fail. Furthermore, the property must have an extra parameter (e.g. an indexer for instance). Implemented property with no indexer does not seem to trip the assertion.

Class1 (aka the interface to implement)

Public Enum Bar
    a
    b
    c
End Enum

Public Property Get Foo(Bar As Bar) As String
End Property

Public Property Let Foo(Bar As Bar, NewValue As String)
End Property

Class2 (aka the implementing class)

Implements Class1

Private Property Let Class1_Foo(Bar As Bar, RHS As String)
End Property

Private Property Get Class1_Foo(Bar As Bar) As String
End Property

This fails the debug assertion; having one parameter too short.

Was this page helpful?
0 / 5 - 0 ratings