Rubberduck: Parser errors aren't consistently reported as such

[inDeev]

Hi, I wasn't ever able to use Code inspector, it every time only Parsing and Parsing - minutes, half hour, hours, without any result... I've created log, maybe it will help you to fix it.
RubberduckLog.txt

[Vogel612]

2017-11-16 13:51:46.6717;TRACE-2.1.1.2461;Rubberduck.Common.LogLevelHelper;
    Rubberduck version 2.1.1.2461 loading:
    Operating System: Microsoft Windows NT 6.1.7601 Service Pack 1 x64
    Host Product: Microsoft Office 2016 x86
    Host Version: 16.0.8326.2107
    Host Executable: EXCEL.EXE;

There's two SLL mode exceptions in the log. Neither results in a final ParseException...
But both are annoyed at an Else token.

[retailcoder]

Stupid question: does the code compile (Alt+D,Enter)?

[inDeev]

Hi, yes, code compile without any error, this is something, what I'm doing often.

[retailcoder]

Can you share a bit of the code at or around formInsertRow L725C5?

2017-11-16 13:52:13.5204;WARN-2.1.1.2461;Rubberduck.Parsing.VBA.VBAModuleParser;SLL mode failed while parsing the code pane version of module formInsertRow at symbol Else at L725C5.

[inDeev]

Wow, there it is, i see it, double else.

Private Sub tbNoRows_Change()
  activateWb
  If isNumber(tbNoRows.value) Then
    If StrToInt(tbNoRows.value) >= 1 And StrToInt(tbNoRows.value) <= 30 Then
      tempItem.NoRows = StrToInt(tbNoRows.value)

      ReDim guids(tempItem.NoRows) As String
      For i = 1 To tempItem.NoRows
        guids(i) = GetGUID()
      Next
      tempItem.guid = guids

      tbNoRows.BackColor = matColor("white")
      prepareAndInsert
    Else
      tbNoRows.BackColor = matColor("red")
      Call MsgBox("Number of Rows have wrong value (" & StrToInt(tbNoRows.value) & ")" & vbCrLf & "Acceptable values are positive integers > 0 and <=30", vbCritical + vbOKOnly, "engBoM 2017")
      If StrToInt(tbNoRows.value) > 30 Then: tbNoRows.value = "30": SpinButton1.value = 30
        If StrToInt(tbNoRows.value) < 1 Then tbNoRows.value = "1": SpinButton1.value = 1
      End If
    Else ' ### LINE 725 here
      If tbNoRows.value = vbNullString Then
        tbNoRows.BackColor = matColor("red")
      Else
        Call MsgBox("Wrong Number of Rows value(" & tbNoRows.value & ")" & vbCrLf & "Only numbers are allowed", vbCritical + vbOKOnly, "engBoM 2017")
        tbNoRows.value = "1"
        SpinButton1.value = 1
      End If
    End If
  End Sub

I don't understand how it can be compiled. Maybe, this piece of code is no longer required in project, that is why I'm trying to user Rubberduck more then only for code indentation now.

[retailcoder]

@Vogel612 the 2nd exception is from the attributes pass:

Token: Else at L725C5
Component: formInsertRow (code pane version)
ParseType: Main parse

Token: Else at L740C5
Component: formInsertRow (exported version)
ParseType: Main parse

Both are about the same token.

In any case the log appears to be incomplete:

2017-11-16 13:52:16.5987;DEBUG-2.1.1.2461;Rubberduck.Parsing.VBA.RubberduckParserState;Module 'formRawDataParser' state is changing to 'Parsed' (thread 19);

Or, something is silently stopping the parser right there and then. I don't think we swallow any exception in the process...

BTW @MDoerner big huge kudos for the changes to the parser logging (ref. #3523) - it's absolutely awesome to be able to tell exactly where it's failing, without logging any of the user's code.

[retailcoder]

@inDeev huh, indeed, that would be an "Else without If" compile error!

We still have a bug though: that should have resulted in a proper exception being thrown, parser state going to Error, and reporting the faulty token in the "Parser Errors" search results toolwindow tab.

Thanks a bunch!

[inDeev]

I found exact reason, there was missing end if above line 725.
Now there is no SLL mode exception, but parsing still "stuck" - no error, it still "Parsing..." for about 10 minutes now but nothing was added to log after first minute,
RubberduckLog.txt
(that last line I only tried Code Inspection button but there was still only message "Rubberduck doesn't see anything yet")

[retailcoder]

hmm, annoying. what if you click the "Refresh" button on the code inspections toolwindow?

I expect the already-parsed modules to be skipped (their parse trees are cached already), and hopefully something to get logged... let's see.. we have these modules and states:

• 'XML_Export' (Parsed)
• 'formPrintFormat' (Parsed)
• 'cCell' (Parsed)
• 'materialColors' (Parsed)
• 'formRootAssys' (Parsed)
• 'formItemMovement' (Parsed)
• 'List1' (Parsed)
• 'ThisWorkbook' (Parsed)
• 'RawDataParser' (Parsed)
• 'List2' (Parsed)
• 'formHideShowSect' (Parsed)
• 'ProgressBar' (Parsed)
• 'cCells' (Parsed)
• 'formAssembliesMatrix' (Parsed)
• 'rowFunctions' (Parsed)
• 'List6' (Parsed)
• 'GlobalFunctions' (Parsed)
• 'formCmnStructs' (Parsed)
• 'printModule' (Parsing)
• 'formInsertRow' (Parsed)
• 'formDeltaBoM' (Parsed)
• 'Sheet3' (Parsed)
• 'engBoMgenerator' (Parsed)
• 'List21' (Parsed)
• 'List7' (Parsed)
• 'formRemoveRow' (Parsed)
• 'formEngBoMInfo' (Parsed)
• 'engBoMItem' (Parsed)
• 'formRawDataParser' (Parsed)
• 'Sheet1' (Parsed)
• 'setupModule' (Parsed)
• 'GlobalVariables' (Parsed)
• 'formItemEdit' (Parsed)

Per the original logs, the only module that isn't in a Parsed state, would be printModule.

Can you export that module and import it into a separate VBA project on its own and see if it ends up parsed? Hopefully the logs for it contain something we can chew on...

[retailcoder]

Alternatively, if you can attach it here, we can try to parse it with a debugger attached, and see if perhaps maybe we're forgetting to log an exception somewhere..

[inDeev]

OK, I'll do it, firstly, here is log after click on refresh inside inspection window (from time 19:37):
RubberduckLog.txt

[inDeev]

And here is log of new excel with added printModule (it really still Parsing... but not Parse):
RubberduckLog.txt
and printModule itself:
printModule.zip

[retailcoder]

Excellent, repro confirmed off v2.1.0 release build. I can't use a debug build right now, but will update later - there's definitely something going on here.

Thanks again for sharing the code!

[MDoerner]

Just to clarify why there has only been a parser error in SLL mode for the procedure posted above: the code is perfectly valid. It is just that the indentation is a lie.

The EndIf statements match the Else statements. The two other if statements are one line if statements and, hence, do not need an EndIf. They actually would not make too much sense if the EndIf belonged to the first of the two like the indentation seems to imply.

[inDeev]

MDoerner, no, there is really missing one End If, between lies 724 and 725. then if you re-indent it, it starts to make a sense.

[MDoerner]

Let me just provide the code indented as the VBE sees it:

Private Sub tbNoRows_Change()
    activateWb
    If IsNumber(tbNoRows.Value) Then
        If StrToInt(tbNoRows.Value) >= 1 And StrToInt(tbNoRows.Value) <= 30 Then
            tempItem.NoRows = StrToInt(tbNoRows.Value)

            ReDim guids(tempItem.NoRows) As String
            For i = 1 To tempItem.NoRows
                guids(i) = GetGUID()
            Next
            tempItem.GUID = guids

            tbNoRows.BackColor = matColor("white")
            prepareAndInsert
        Else
            tbNoRows.BackColor = matColor("red")
            Call MsgBox("Number of Rows have wrong value (" & StrToInt(tbNoRows.Value) & ")" & vbCrLf & "Acceptable values are positive integers > 0 and <=30", vbCritical + vbOKOnly, "engBoM 2017")
            If StrToInt(tbNoRows.Value) > 30 Then: tbNoRows.Value = "30": SpinButton1.Value = 30    'One line if statement; no End If needed
            If StrToInt(tbNoRows.Value) < 1 Then tbNoRows.Value = "1": SpinButton1.Value = 1    'One line if statement; no End If needed
        End If
    Else                                     ' ### LINE 725 here
        If tbNoRows.Value = vbNullString Then
            tbNoRows.BackColor = matColor("red")
        Else
            Call MsgBox("Wrong Number of Rows value(" & tbNoRows.Value & ")" & vbCrLf & "Only numbers are allowed", vbCritical + vbOKOnly, "engBoM 2017")
            tbNoRows.Value = "1"
            SpinButton1.Value = 1
        End If
    End If
End Sub

However, this is not really the main point of this issue, I think. We will have to see why the other module does not finish parsing.

[inDeev]

Sure, here is the problem:
If StrToInt(tbNoRows.Value) > 30 Then : tbNoRows.Value = "30": SpinButton1.Value = 30
Extra colon after then, so it missing End If. when semicolon is removed, it works. but it is about SLL problem, not problem with print module, which still freeze parsing of whole project.

[retailcoder]

I noticed quite a lot of instructions separators, a lot of which are actually misplaced or redundant. Regardless of RD being able or not to handle them correctly, IMO that would be a habit to lose :wink:

Ref. #3175

[MDoerner]

I have done some testing and found out that our parser has a very hard time parsing the condition in the following if statement (reformatted for better readibility):

If preserveUserColor = True _
   And rCell.row > 6 _
   And Not cColor = matColor("blue", 50) _
   And Not cColor = matColor("blue", 100) _
   And Not cColor = matColor("blue", 200) _
   And Not cColor = matColor("blue", 300) _
   And Not cColor = matColor("blue", 400) _
   And Not cColor = matColor("blue", 500) _
   And Not cColor = matColor("blue", 600) _
   And Not cColor = matColor("blue", 700) _
   And Not cColor = matColor("blue", 800) _
   And Not cColor = matColor("blue", 900) _
   And Not cColor = matColor("amber", 100) _
   And Not cColor = matColor("amber", 200) _
   And Not cColor = matColor("lime", 100) _
   And Not cColor = matColor("lime", 200) _
   And Not cColor = matColor("bluegrey", 100) _
   And Not cColor = matColor("bluegrey", 200) _
   And Not cColor = matColor("deeppurple", 100) _
   And Not cColor = matColor("deeppurple", 200) _
   And Not cColor = matColor("yellow", 200) _
   And Not cColor = matColor("teal", 200) _
Then GoTo skiprCellFor

The runtime of the parse seems to be exponential in the number of conditions. REmoving everything before _bluegray_ leads to a tolerable time, adding _lime_ makes it painful and the entire condition probably takes kours to parse.

I do not really know how to increase the performance of the parser here since expressions are inhererntly left-recursive. This means that Antlr does some nifty transformation to ba able to deal with it. However, this transformation seems to be rather inefficient if the recursion goes very deep.

In this particular example, the parse time can be redically shortened by inverting the condition, which also alows to remove the GoTo statement and somewhat better comunicates the intend of the code, in my humble opinion:

If Not preserveUserColor _
   Or rCell.row <= 6 _
   Or cColor = matColor("blue", 50) _
   Or cColor = matColor("blue", 100) _
   Or cColor = matColor("blue", 200) _
   Or cColor = matColor("blue", 300) _
   Or cColor = matColor("blue", 400) _
   Or cColor = matColor("blue", 500) _
   Or cColor = matColor("blue", 600) _
   Or cColor = matColor("blue", 700) _
   Or cColor = matColor("blue", 800) _
   Or cColor = matColor("blue", 900) _
   Or cColor = matColor("amber", 100) _
   Or cColor = matColor("amber", 200) _
   Or cColor = matColor("lime", 100) _
   Or cColor = matColor("lime", 200) _
   Or cColor = matColor("bluegrey", 100) _
   Or cColor = matColor("bluegrey", 200) _
   Or cColor = matColor("deeppurple", 100) _
   Or cColor = matColor("deeppurple", 200) _
   Or cColor = matColor("yellow", 200) _
   Or cColor = matColor("teal", 200) _
Then
    rCell.Interior.color = RGB(cColorArray(1) + ((255 - cColorArray(1)) / 1.2), _
                           cColorArray(2) + ((255 - cColorArray(2)) / 1.2), _
                           cColorArray(3) + ((255 - cColorArray(3)) / 1.2))
    rCell.Font.color = matColor("black")
End If

Still , it is in general not good having to make code conform to a tool in order to make it work properly.

[inDeev]

Well, thanks, I splitted that condition and it now works good and quickly, i will kept it for keep rubberduck working:

        If preserveUserColor = True And rCell.row > 6 Then
            If Not cColor = matColor("blue", 50) _
               And Not cColor = matColor("blue", 100) _
               And Not cColor = matColor("blue", 200) _
               And Not cColor = matColor("blue", 300) _
               And Not cColor = matColor("blue", 400) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("blue", 500) _
               And Not cColor = matColor("blue", 600) _
               And Not cColor = matColor("blue", 700) _
               And Not cColor = matColor("blue", 800) _
               And Not cColor = matColor("blue", 900) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("amber", 100) _
               And Not cColor = matColor("amber", 200) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("lime", 100) _
               And Not cColor = matColor("lime", 200) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("bluegrey", 100) _
               And Not cColor = matColor("bluegrey", 200) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("deeppurple", 100) _
               And Not cColor = matColor("deeppurple", 200) _
               Then GoTo skiprCellFor
            If Not cColor = matColor("yellow", 200) _
               And Not cColor = matColor("teal", 200) _
               Then GoTo skiprCellFor
        End If

[MDoerner]

Since the apparent issue here was the one for which we opened the separate issue #3634 and because that issue is fixed now via PR #3715, I close this issue. Should the problem still exist for the project, please feel free to reopen the issue.