Rubberduck: False positive 'Return value of fuction 'x' is never used'.

Created on 24 May 2018  路  11Comments  路  Source: rubberduck-vba/Rubberduck

The function:

Implements ILogger

Public Function Create(ByVal loggerName As String, ByVal loggerMinLevel As logLevel) As ILogger

    Dim result As New DebugLogger
    result.Name = loggerName
    result.MinLevel = loggerMinLevel
    Set Create = result

End Function

The call site:

LogManager.Register FileLogger.Create(Strings.Join(Array("DBLog", Format$(Now, "yyyy.mm.dd hh.mm"), ".txt"), vbNullString), loggingLevel, TempVars.Item("dbPath").Value, 10)

And the inspection result:

rd retval never used

And the instant analysis:
https://chat.stackexchange.com/transcript/message/44796336#44796336

bug difficulty-01-duckling feature-inspections good first issue up-for-grabs

All 11 comments

At the call site, is Create showing up as a member of FileLogger in the RD toolbar?

Quarterly Report.DebugLogger.Create (function:ILogger) it says.

Ok so the good news is that the resolver is doing its job and correctly identifying member calls in an argument list.

The bad news is that it's not clear whether your FileLogger is actually creating a DebugLogger? Code says one thing, TD toolbar says another? I'm confused...

No, I'm confused. I went to the first *Logger.Create that the VBE's search found and forgot that there are both a FileLogger and a DebugLogger.

The DebugLogger isn't being used by my code, while the FileLogger is. The inspection takes me to DebugLogger.Create. So far, I haven't gotten to any references to FileLogger.Create not being used.

(I've got a fair few inspections to go through, but I'll hold out hope...)

Got a repro: the inspection yields a result for a function that's simply never invoked. While technically accurate, that is rather confusing.

The inspection needs to filter out functions that have !References.Any() - should be a trivial fix.

            var functions = State.DeclarationFinder
                .UserDeclarations(DeclarationType.Function)
                .Where(item => !IsIgnoringInspectionResultFor(item, AnnotationName))
                .ToList();

If that's the only problem with it, then adding && item.References.Any() to that Where predicate should do it.

Okay, a little twist: needs to exclude references that are within the scope of the function itself, since the "return value assignment" counts as a reference.

Why not simply check for the existence of references that are not assignments?

@MDoerner because we're looking for functions whose return value is never used; it's a declaration result. That said, a reference result that flags all places where a function is invoked like a sub would be rather useful too!

I think you misunderstood my comment. I was commenting on enhancement of the condition in the current inspection and referring to the preceding comment.

Right. Unless the function returns an object that has a default member.. although at this point IIRC the resolver sets the assignment flag on the default member and not the function, right?

The inspection already has a function IsReturnStatement. One could just use that.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

retailcoder picture retailcoder  路  3Comments

ThunderFrame picture ThunderFrame  路  3Comments

Gener4tor picture Gener4tor  路  3Comments

eteichm picture eteichm  路  4Comments

Hosch250 picture Hosch250  路  3Comments