rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.92k stars 301 forks source link

False positive FunctionReturnValueDiscardedInspection with Err.Raise Sub #5480

Closed IvenBach closed 4 years ago

IvenBach commented 4 years ago

Rubberduck version information Version 2.5.0.29079 OS: Microsoft Windows NT 10.0.18362.0, x64 Host Product: Microsoft Office 2016 x64 Host Version: 16.0.4978.1000 Host Executable: EXCEL.EXE

Description The inspection is triggering on a Sub.

Public Sub MVCE()
    Err.Raise -1, "Foo", "Bar"
End Sub

To Reproduce Steps to reproduce the behavior:

  1. Insert module, Add above code, Parse & view inspections

Expected behavior Have inspection trigger only on functions.

MDoerner commented 4 years ago

The inspection result is not for Err.Raise, it is for Err itself. The thing the inspection does not anticipate its that there is a valid scenario for a throw-away object returned from a function. Technically, Err is not an object reference but a function returning the ErrObject. The inspection complains about the fact that you just make a member call and then silently discard the object.

Because of the particular design of this, it is actually valid to do that. For user code, having a function (not property) providing throw-away objects would be kind of a code smell.

My suggestion to handle this is the make a member call on the returned object sufficient as long as the function is not user-defined.

IvenBach commented 4 years ago

My thinking was so much shallower than that. I opened this issue since on the ErrorObject the member Raise(...) is, according to the object browser, a Sub instead of a Function.

retailcoder commented 4 years ago

Merged, but part of me thinks there's valuable info being silenced, ...OTOH we don't have per-inspection configuration settings, so only flagging user code seems the best compromise.