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 301 forks source link

Inspection for repeated/multiple usage of pure functions with identical arguments #2388

Open ThunderFrame opened 8 years ago

ThunderFrame commented 8 years ago

Repeated calls to a pure function like RGB, with identical inputs, could be better assigned to a variable, and then re-used.

Identifying pure functions in type libraries might require some TLB-specific rules.

In special cases, the result could be assigned to a constant, and the function call removed altogether

Eg:

ActiveCell.Interior.Color = RGB(255,0,0)
ActiveCell.Offset(2,2).Interior.Color = RGB(255,0,0)

Could become:

Const COLOR_WARNING As Long = 255
ActiveCell.Interior.Color = COLOR_WARNING
ActiveCell.Offset(2,2).Interior.Color = COLOR_WARNING

Or :

Dim WarningColor As Long
WarningColor =RGB(255, 0, 0)
ActiveCell.Interior.Color = WarningColor
ActiveCell.Offset(2,2).Interior.Color = WarningColor
retailcoder commented 8 years ago

How do we identify a pure function that isn't user code?

Vogel612 commented 8 years ago

how do we Identify a pure function that is user code? Basically the only way to know that a function is pure is to check in the source code, whether it calls an impure function. All I/O is impure by definition. That's about it. There's some things like collection updating that are also impure depending on how they are implemented

comintern commented 8 years ago

This would be difficult for user code. It would require knowing that the function is both purely deterministic and has no side-effects.

ThunderFrame commented 8 years ago

Identifying a pure function that isn't user code would require hard-coding in Rubberduck.

Many of the VBA functions are pure, and we already special case many VBA functions.

retailcoder commented 8 years ago

@comintern wouldn't that simply be a function that doesn't access state outside of its scope?

comintern commented 8 years ago

@retailcoder - The best example would be a random number generator:

Public Function Foo() As Double
    Foo = Rnd()  'Your favorite PRNG here.
End Function 

Others would be functions that retain their own state...

Public Function Foo() As Long
    Static bar As Long
    bar = bar + 1
    Foo = bar
End Function

...anything ByRef...

Public Function Foo(ByRef bar As Long) As Long
    bar = bar + 1
    Foo = bar
End Function

...and anything that touches anything like it in it's execution graph.

Vogel612 commented 8 years ago

actually we don't need a full execution graph for it. It should be sufficient to successively mark IdentifierReferences as "impure". Start with Variable Class Members, then add ByRef and Static References in their own context. Additionally there's things like MsgBox, FSO usage and all the like and then just propagate backwards through the call graph we already have (called-by that is)

retailcoder commented 7 years ago

ref. #236