Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive 'Return value of fuction 'x' is never used'. #4036

Closed
daFreeMan opened this issue May 24, 2018 · 11 comments
Closed

False positive 'Return value of fuction 'x' is never used'. #4036

daFreeMan opened this issue May 24, 2018 · 11 comments
Assignees
Labels
bug Identifies work items for known bugs difficulty-01-duckling Issue where no particularly involved knowledge of the internal API is needed. feature-inspections good first issue Want to contribute? That's a good place to start! up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky

Comments

@daFreeMan
Copy link
Contributor

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

@retailcoder
Copy link
Member

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

@daFreeMan
Copy link
Contributor Author

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

@retailcoder
Copy link
Member

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...

@retailcoder retailcoder added bug Identifies work items for known bugs feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky difficulty-01-duckling Issue where no particularly involved knowledge of the internal API is needed. good first issue Want to contribute? That's a good place to start! labels May 24, 2018
@daFreeMan
Copy link
Contributor Author

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...)

@retailcoder
Copy link
Member

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.

@retailcoder
Copy link
Member

retailcoder commented May 24, 2018

            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.

@MDoerner
Copy link
Contributor

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

@retailcoder
Copy link
Member

@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!

@MDoerner
Copy link
Contributor

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

@retailcoder
Copy link
Member

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?

@MDoerner
Copy link
Contributor

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

@retailcoder retailcoder self-assigned this Nov 13, 2018
Semi-automatic bug tracker automation moved this from ToDo to Done Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identifies work items for known bugs difficulty-01-duckling Issue where no particularly involved knowledge of the internal API is needed. feature-inspections good first issue Want to contribute? That's a good place to start! up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky
Projects
Development

No branches or pull requests

3 participants