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

@OptionStrict annotation #4936

Open retailcoder opened 5 years ago

retailcoder commented 5 years ago

Justification

VBA is very forgiving in terms of implicit type conversions. Late-bound member calls move glaring compile-time errors from compile-time to run-time; variables don't even need a declared type! VB.NET addressed these loose ends with Option Strict, and I think we have everything we need to make VBA do the same.

Description

Microsoft Docs describes Option Strict as follows:

Option Strict Statement
Restricts implicit data type conversions to only widening conversions, disallows late binding, and disallows implicit typing that results in an Object type.

https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/option-strict-statement

In a VBA context, "Object" means "Object or Variant".

Let's introduce a new @OptionStrict annotation, a MissingOptionStrictInspection "Rubberduck Opportunity" inspection that finds modules without it, and quickfix to add it in this module... and then another error-level OptionStrictInspection that point to all violations.

Bonus: a checkbox setting to "use 'Option Strict' everywhere", and have the annotation added automatically to every new (& existing?) module.

Additional context These annotations should ideally all be valid (with or without the parentheses):

'@OptionStrict
'@OptionStrict(On)
'@OptionStrict("On")
'@OptionStrict Off
'@OptionStrict "Off"

...but it's completely fine to start with just a single @OptionStrict annotation, and have it support a parameter later: if the annotation is there, it's on, if not, it's off.

When '@OptionStrict is on, the following inspections are in effect:

Note that explicit Variant and Object declarations are legal with '@OptionStrict: what's not allowed is to make a member call against it without first casting it to an early-bound interface.

The advantages are:


The OptionStrict inspections should be public nested types in an all-encompassing OptionStrictInspection, so that '@Ignore OptionStrict works for all OptionStrict results. Disabling the OptionStrictInspection could fire an OptionStrictDisabledInspection result that flags '@OptionStrict annotations when the OptionStrictInspection is disabled, warning about the option not being enacted. Or (perhaps simpler, and less... recursive "what it that one is disabled?"), we special-case OptionStrictInspection and disable changing/configuring that inspection's severity level (Error).

Wiki should also be updated with a table of Widening Conversions adapted to VBA data types.

retailcoder commented 5 years ago

VBA doesn't have an explicit cast operator: a new @DirectCast annotation could be used to decorate assignments that would otherwise trip implicit object type conversions (linking #3580):

Dim a As Foo
Set a = New Foo
Dim b As Bar
'@DirectCast
Set b = a
SmileyFtW commented 5 years ago

My only issue with enforcing early binding is the problem it causes users with different versions and therefore requiring different references… I’d rather develop in early bound mode and then go late-bound when deploying. Maybe that is what you are suggesting and I am missing the point (which is usually the case… ).

retailcoder commented 5 years ago

@SmileyFtW the version-specific issues of early binding are real, but also wildly overstated. Scripting runtime, for one, has no reason to be late-bound: every single Windows machine built this century is running the same version. Ditto with any XML library, or regex, and several others. That said we can't hook the compiler (that I know of) and enforce it at compile-time; best we can do is trigger inspections, and offer quickfixes where appropriate. The vast majority of VBA code uses only standard libraries, and "late binding" has nothing to do with library references (another widely spread piece of misinformation).

This Range call is late-bound:

Worksheets("Sheet1").Range("A1") = 42

It's late bound because Sheets returns Object. The fix is to cast that object to an early-bound interface:

Dim sh As Worksheet
Set sh = Worksheets("Sheet1")

And now a Range call against sh is compile-time validated. That is mostly what this idea is all about. But yes, late-bound library references would also be "forbidden"... and then, moving such library-specific calls into their own module (without @OptionStrict specified) would only make the code more modular and robust overall.

SmileyFtW commented 5 years ago

This is why I love this RD community… I learn something virtually every time I read a post, and ALWAYS when I post a reply. Thanks to you and all who have moved me way beyond where I was a few months ago.