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.92k stars 302 forks source link

"Move module-level variable to a smaller scope" autofix breaks on one-line if statements #4117

Closed Inarion closed 6 years ago

Inarion commented 6 years ago

Consider (or try it yourself) the following MCVE:

Option Explicit

Private TooBigAScopeForMe As String
Private ImGoingToBreak As String
Private IllBeFine As String

Public Sub SmallerScope()

    TooBigAScopeForMe = "Give my declaration back!"
    Debug.Print TooBigAScopeForMe

End Sub

Public Sub AnotherSmallScopeRightHere()

    If ImGoingToBreak = vbNullString Then ImGoingToBreak = "Don't autofix me, please!"
    Debug.Print ImGoingToBreak

End Sub

Public Sub YetAnotherSmallScope()

    If IllBeFine = vbNullString Then
        IllBeFine = "See? I'm fine."
    End If
    Debug.Print IllBeFine

End Sub

This is going to get autofixed to

Option Explicit

Public Sub SmallerScope()

    Dim TooBigAScopeForMe As String
    TooBigAScopeForMe = "Give my declaration back!"
    Debug.Print TooBigAScopeForMe

End Sub

Public Sub AnotherSmallScopeRightHere()

    If ImGoingToBreak = vbNullString Then Dim ImGoingToBreak As String
    ImGoingToBreak = "Don't autofix me, please!"
    Debug.Print ImGoingToBreak

End Sub

Public Sub YetAnotherSmallScope()

    Dim IllBeFine As String
    If IllBeFine = vbNullString Then
        IllBeFine = "See? I'm fine."
    End If
    Debug.Print IllBeFine

End Sub

The first variable/sub proves the autofix is generally working. The second one shows the breaking scenario. Number three narrows it down to one-liners.

EDIT: Version information: Version 2.2.6738.40126 OS: Microsoft Windows NT 6.1.7601 Service Pack 1, x64 Host Product: Microsoft Office 2013 x86 Host Version: 15.0.5023.1000 Host Executable: MSACCESS.EXE

Reproducible also in Excel. Should be host-independent.

MDoerner commented 6 years ago

The problem here is that a sameLineStatement currently is a blockStmt but should be a mainBlockStmt. (Statement labels can only appear at the start of a line.)