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

bclothier picture bclothier  路  33Comments

ghost picture ghost  路  39Comments

AndrewM- picture AndrewM-  路  37Comments

UnhandledCoder picture UnhandledCoder  路  32Comments

SystemsModelling picture SystemsModelling  路  25Comments