UnassignedVariableUsage Doesn't support variable being assigned as an "out" parameter from a property/sub/function

[jhiston]

Short form:

Appears that if a variable is assigned during a call to a function/sub/property, the variable not assigned inspection is triggered (correctly in one view, since within the procedure being inspected there is no assignment, but missing possibility that assignment is occurring as part of a byref passing of a parameter to a subprocedure/function/property). More specifics below, and not sure this is a bug per se but seemed most appropriate category (possibly enhancement?)

Rubberduck version information The info below can be copy-paste-completed from the first lines of Rubberduck's log or the About box:

Version 2.5.9.6316 OS: Microsoft Windows NT 10.0.19045.0, x64 Host Product: Microsoft Office x64 Host Version: 16.0.17126.20132 Host Executable: MSACCESS.EXE

Description RE: Code Inspection: Unassigned Variable Usage

Inspection triggers on following code:

"variable 'aViaGroupsDisposer' is used but not assigned."

Private Sub I_Is_Disposable_Dispose()

Dim aViaGroupsDisposer As I_Func_Disposer_ViaGroups
If DisposerFunctionality.HasGroupedDisposingMethods(aViaGroupsDisposer) Then
    With aViaGroupsDisposer
       .TryDisposingGroupOfObjects Me, Me _
            , This.MyDependencies.Services.ClassShared _

    End With
End If

End Sub

as well as following code

'aDefaultSubstitutes' is used but not assigned

Private Sub I_4LocalUse_LocalConfiguration___COMMON_ReceiveVisitCOMMON(aVisitor As I_Visitor2_4LocalUse_LocalConfiguration)

 Dim aDefaultSubstitutes As I_4LocalUse_Substitutes
 If aVisitor.CanSupplyDefaultLocalSubstitutes(aDefaultSubstitutes) Then
    Set DefaultSubstitutes = aDefaultSubstitutes
End If

End Sub

I commonly use a pattern of (simplified) if [Object].HasPropertyOfInterest(outPropertyOfInterest as object) as boolean then .... End if

Advantage is to make more readable (and encompass more potential checks) than

if not Object.PropertyOfInterest is nothing then set outPropertyOfInterest = Object.PropertyOfInterest ... end if

But this coding pattern is triggering the unassigned variable usage inspection. I can disable so not a huge deal, but surprised it was flagged.

To Reproduce Steps to reproduce the behavior: see above

Expected behavioUr (:)) No inspection result if variable previously passed by Ref (or implicitly byref given objects) to a property/sub/function that explicitly labels the incoming parameter with an 'out' prefix (e.g. public property get HasPropertyOfInterest(outPropertyOfInterest as object) as boolean)

This is relying on a convention with the labelling so maybe not supportable and ignoring is proper approach (not sure this is a 'bug' per se)

Screenshots If applicable, add screenshots to help explain your problem.

Logfile N/A

Additional context N/A

[IvenBach]

The convention of a prefix for ByRef parameters outFoo had been previously talked about in chat. Lack of developer time buried the idea. Thanks for writing this up. :+1:

[retailcoder]

Indeed, it doesn't handle ByRef returns, deliberately so: before we started resolving argument expressions to their respective parameter declarations, we tried to think of a way of making the inspection track these assignments and accurately report them, but following a ByRef argument into a procedure scope and tracking if the parameter is assigned... or passed ByRef to another scope, ...is a rabbit hole that easily kills performance, especially since ByRef is the unfortunate implicit default - so a conscious decision was made to keep evaluations within the declaration scope, knowing it would mean false positives in ByRef assignment cases; at the time this was deemed a better outcome than the alternative, which would have been to have false negatives for the presumed vast majority of cases involving a ByRef argument.

On top of developer time, implementing a naming convention -based exception in the inspection itself has not happened, because inspections in RD2 cannot really be individually configured (not without quite a bit of pain!), and hard-coding the "out" prefix was deemed too much of a nudge towards a certain coding style, which ironically is something we usually want to avoid in Rubberduck: we don't want to be telling our users how they should be naming things! ....meanwhile, we kinda do it anyway 🤔

That said, since then we've seen added support for private type definitions in the Encapsulate Field refactoring, which is absolutely a coding style element of mine that ended up in Rubberduck, so the basis for not hard-coding a bail-out when the parameter has an out prefix, is essentially gone: I think it would be the simplest way to address this, and then it could be configured similarly to the naming and prefixing exceptions, at the Inspection Settings level; RD3 can then move that setting with the specific inspection/diagnostic configuration options.

In the meantime, the inspection could be disabled locally using @Ignore annotations, at module level using @IgnoreModule annotations, or by outright disabling it by making its severity level "DoNotShow" (in which case the inspection won't run at all); see quick-fix options in the Inspection Results toolwindow for the commands to do this automatically.

[jhiston]

I totally get the potential rabbit hole in the unconstrained case. Appreciate comments above and will be using the ignore approach. Thank you for all the work so far, looking forward to RD3!

[jhiston]

Aack - can edit comment, but not sure 'closed' is the right status for this if there was interest in pursuing the approach @retailcoder describes in 3rd paragraph. Trying this as a 'reopen with comment', I would defer to you as experts on whether this should be listed as closed since there are workarounds / not really a bug or whatever appropriate status would be.