Rubberduck: False Positive 'Assignment is not used' inspection

Created on 12 Apr 2019  路  8Comments  路  Source: rubberduck-vba/Rubberduck

Rubberduck version information
Version 2.4.1.4661
OS: Microsoft Windows NT 10.0.15063.0, x64
Host Product: Microsoft Office 2016 x64
Host Version: 16.0.4822.1000
Host Executable: EXCEL.EXE

Description
False positive Assignment not used inspection on the following use cases.
The actual code where it surfaced:

  Dim sqlString As String
  If AllClinicFlag Then
    sqlString = selectStatementBase & Table & " WHERE " & dateColumnName & " BETWEEN '" & startDate & "' AND '" & endDate & "'"
  Else
    sqlString = selectStatementBase & Table & whereClause & clinicID
    sqlString = sqlString & " AND " & dateColumnName & " BETWEEN '" & startDate & "' AND '" & endDate & "'"
  End If
  If Len(Extra) > 0 Then
    sqlString = sqlString & " AND " & Extra
  End If

  LogManager.Log TraceLevel, "SQLString: " & sqlString

An MCVE

  Dim foo As String
  foo = "bar"
  foo = foo & " baz"

To Reproduce
Steps to reproduce the behavior:

  1. See above

Expected behavior
no inspection necessary in these cases

Screenshots
n/a

Logfile
RubberduckLog.txt

Additional context
I had quite a number of other legitimate, valid Assignment is not used inspection results, so it's not totally borked.

Possibly related: #4754

_Yes, I know, Bobby Tables... It's on the list of refactorings to do. I'm the only one in the code and the only user. Since I have no intention of causing self-harm, it's_ low _on the list to fix_

bug code-path-analysis feature-inspections inspection-false-positive

All 8 comments

Just for funs, here's another example I just discovered in my code:

  If markerLocation > 0 Then
    Dim workingName As String
    workingName = Right$(componentName, Len(componentName) - markerLocation - 1)
  Else
    workingName = componentName
  End If
  markerLocation = InStr(1, workingName, ".")

The first assignment is the one that's highlighted.

What happens if the declaration is not inside the "Then-Block"?

Interestingly, that resolves the issue for this new instance. However, the original code has the declaration outside the If...End If, so the issue as a whole still stands.

Yes, but it could imply that it's a different underlying issue. I'm not sure whether this needs to be in a separate issue now... Let's leave it in here for now and see whether the same fix applies for both parts.

Noting another that I just came across, to see if there is One Change to Fix Them All:

ErrorHandler:
  With Err
    LogManager.Log ErrorLevel, Strings.Join(Array("Navigation failed for: ", this.SiteValues(CStr(ClinicIDLinkKey)), ". Number: ", .Number, " Description: ", .Description), vbNullString)
    Dim errNum As Long
    errNum = .Number
    Dim errDesc As String
    errDesc = .Description
  End With

  IWebDownloader_NavigateToDownloadLocation = False
  Err.Raise FailToNavigate, "ApptPlusWebDownloader.IWebDownloader_NavigateToDownloadLocation", "Number: " & CStr(errNum) & "  Desc: " & errDesc
  Resume CleanExit

Both errNum = .Number and errDesc = .Description are flagged

This is the same issue, just with a with block instead of an if then block.

The underlying problem seems to be that the inspection makes the completely wrong assumption that variables in VBA are scoped to the containing block.

  Dim foo As String
  foo = "bar"
  foo = foo & " baz"

Most of this discussion is around If and With blocks, but this example, where a String variable is assigned a new value that includes the old value, is the one haunting my inspection results.

See the duplicate issue for some more discussion.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

A9G-Data-Droid picture A9G-Data-Droid  路  21Comments

UnhandledCoder picture UnhandledCoder  路  32Comments

SystemsModelling picture SystemsModelling  路  25Comments

ishita799 picture ishita799  路  28Comments

zspitz picture zspitz  路  28Comments