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.91k stars 299 forks source link

Find All References could use a functionality update #4848

Open daFreeMan opened 5 years ago

daFreeMan commented 5 years ago

Rubberduck version information Version 2.4.0.4586 OS: Microsoft Windows NT 10.0.15063.0, x64 Host Product: Microsoft Office 2016 x64 Host Version: 16.0.4810.1000 Host Executable: MSACCESS.EXE

Description Find All References seems to be missing some references when they're on the LHS of an assignment

To Reproduce Steps to reproduce the behavior:

  1. Create a class with members and appropriate Getters/Letters
  2. Assign a class member a value (put it on the LHS)
  3. Use that class member (put it on the RHS)
  4. Find all references for that class member
  5. The RHS use appears in the list, the LHS use does not

Expected behavior All uses (LHS & RHS) of a class member should appear in the list of "All references"

Screenshots 2019-03-07 08_11_08-microsoft visual basic for applications - roireport - archivedataprocess code

Logfile RubberduckLog.txt

Additional context

comintern commented 5 years ago

If you look at the identifier at the top of the tab, you'll see that the declaration you're finding references for is the property Get. The one in the green square is the Let. This appears to be working as designed.

As to whether it should (optionally?) aggregate the Let, Get, and Set in this dialog would be up for discussion - I could see the usefulness of adding that functionality.

daFreeMan commented 5 years ago

Ah, yes, that does make sense. I had seen that, but it hadn't fully registered.

For the naive user (waives hand), having it default to finding Get, Set/Let, with an option to have it find only one side or the other would be quite useful. Grouping them by Get and Set/Let would help ejumacate said naive users while reducing the confusion for less naive users.

daFreeMan commented 3 years ago

This cropped up again, and when I found this issue I'd posted a while back, I thought it had bit me again. (It also made me miss @comintern again).

However, if you'll look at these two screen shots, it's not a Get/Let issue as identified above - the status bar indicates that they're both of type variable:WebDownloaderStatus, yet only one is being found and the other isn't. There are quite a number of places where downloaderStatus is referenced throughout the code, yet only 2 uses are identified by Find All References.

The declaration (earlier in the same method, also not identified in Find All References):

  Dim downloaderStatus As WebDownloaderStatus
  Set downloaderStatus = New WebDownloaderStatus

The found usage: 2021-05-14 07_16_55-Search Results

The not found usage: 2021-05-14 07_17_22-Search Results

Note too, that the status bar correctly (I believe) identifies 10 references to downloaderStatus no matter which use is identified here. Until/unless someone explains to me where I am, again, misunderstanding what the duck's telling me, I believe this belongs back in the bug category.

Loading the code into Notepad++ identifies 16 occurrences of the text downloaderStatus:

    Line 223:   Dim downloaderStatus As WebDownloaderStatus
    Line 224:   Set downloaderStatus = New WebDownloaderStatus
    Line 230:       Set Downloader = PrepareForSnapSurveyDownload(downloaderStatus)
    Line 233:       downloaderStatus.downloadStatus = dlOK
    Line 237:   If downloaderStatus.okToContinue Then
    Line 241:       clinicUsesNRC = GetNRCFlag(clinicList, index, downloaderStatus, LoadExcelDataRC)
    Line 247:             GetSnapSurveyFile Downloader, clinicList.Keys(index), clinicList.Items(index), downloaderStatus
    Line 250:           LoadExcelDataRC = ProcessApptPlusByApi(clinicList.Keys(index), downloaderStatus)
    Line 253:       If downloaderStatus.okToContinue And Not clinicUsesNRC Then
    Line 287:       SendCompletioneMail downloaderStatus
    Line 294:       Downloader.LogoutOfWebPage downloaderStatus
    Line 312: Private Function GetNRCFlag(ByRef clinicList As Scripting.Dictionary, ByRef index As Long, ByRef downloaderStatus As WebDownloaderStatus, ByRef LoadExcelDataRC As Long) As Boolean
    Line 339: Private Sub SendCompletioneMail(ByVal downloaderStatus As WebDownloaderStatus)
    Line 359:   With downloaderStatus
    Line 420: Private Function PrepareForSnapSurveyDownload(ByRef downloaderStatus As WebDownloaderStatus) As IWebDownloader
    Line 426:   Set Downloader = SnapSurveyWebDownloader.Create(Chrome, downloaderStatus)

I understand that in the one method, where the text downloaderStatus is found 11 times, it's identified as "10 references" because the 11th is the declaration, not a reference. I also understand that downloaderStatus as WebDownloaderStatus occurs in method signatures, and is correctly identified as a parameter in those methods, and is used in them as well. However, this still seems to leave 14 occurrences of the text downloaderStatus (all Dim As WebDownloaderStatus) missing from the Find All References results, and I would think that a reasonable person would expect it to find them all, whether it's a variable or a parameter,

retailcoder commented 3 years ago

It seems you are looking for a text search. The references of a parameter can never be the references of another variable, even if they happen to have the same name: they're not the same symbol. Find all references isn't looking at the text at all. Now with that said when the commandbar says 10, clicking that button should be listing 10 items indeed. This does need attention.