Rubberduck: Inspection for unrecognized Excel.Application methods and/or members

[ThunderFrame]

Usage of the Excel.Application object is seemingly loosely compiled, presumably to accommodate things like bypassing the WorksheetFunction object with statements like Application.Pi. See #2236

The VBA compiler in Excel seems to ignore usage of unrecognized members, so it doesn't make a distinction between a valid member such as Application.Name, a worksheet function member such as Application.Pi, or non-existent members such as Application.ILikeTheSmellOfFooBarInTheMorning.

The project compiles, but a runtime error 438 occurs when executing the line.

Once Rubberduck is aware of the pseudo-worksheet functions ( #2237 ), RD should be able to identify the non-existent members and warn the user at design time.

This behavior seems to be unique to Excel.Application. For example, Word.Application enforces compilation, and compilation errors occur if a method/member isn't recognized.

[wqweto]

_Application interface in MSWord typelib is marked with nonextensible attribute while _Application in MSExcel typelib is not (so as to be "extensible"). The latter case instructs the compiler that IDispatch.Invoke on base interface is able to handle methods/props that are not explicitly specified in the typelib so this is expected behavior (no compile-time errors).

Another example of similar shenanigans is the ADODB.Connection interface where one can execute stored procedures directly with something like DbConn.usp_GetClientInfo 1, 2, 3 and use a late-bound invokation to execute usp_GetClientInfo stored procedure (highly discouraged practice).

[ThunderFrame]

@wqweto Thanks for that. Sounds like we might need to collect that interface decoration.

[comintern]

Would it make sense to generalize this inspection to _any_ unrecognised member on an interface? Or maybe a pair of inspections - MemberNotOnInterfaceInspection (error level) and UseOfExtendedInterfaceMemberInspection (warning level)?

[retailcoder]

@comintern where MemberNotOnInterfaceInspection would pop on non-extensible types, and UseOfExtendedInterfaceMemberInspection would pop on extensible types with an on-the-fly member declaration? Yes, definitely!

[comintern]

Actually, this only needs to be one inspection - attempting to use an unrecognised member of a non-extensible interface appears to always cause a compiler error for anything that's early bound, and RD has no clue about the late bound stuff (yet...).

[comintern]

Proposed Meta for the inspection:

A member access is being used that is not declared on the object's interface. This is most likely an error. If the member access is using the object's extensible interface, consider using a non-extensible equivalent to allow compile time checks that will avoid the possibility of a run-time error 438.

I'm not exactly sure how clear that is. Thoughts?

[ThunderFrame]

In the case of Excel, the extended members of Application, are all kludges for Excel 95 compatibility. The only behavioral difference between using Application.Pi and Application.WorksheetFunctions.Pi is the way that errors are handled. Maybe the inspection Meta should warn about these differences?

[comintern]

@ThunderFrame - I might be going at this backwards. The MemberNotOnInterfaceInspection will only hit if the members can't be found, but the fix for #2236 is actually adding a pseudo-interface to Excel.Application so the resolver recognizes the calls to Application.[_WSFunction] (or however it's set up). That means it won't even hit on Application.Pi and company. It _will_ pick up things like Err.Numer or Application.Visibble = True.

I'm thinking that the Application.WorksheetFunction thing should be its own host specific inspection. The alternative would be to _not_ add the pseudo-interface and lose the ability to resolve the calls that are known to exist on the extended interface, but their usage seems common enough that the benefit of having them resolve outweighs the benefit of using a single inspection.

[ThunderFrame]

@comintern I agree, the Excel extensions are at least knowable (as opposed to extensions to an ADODB.Connection which could be any stored procedure), but they're still violating the "member being used that is not declared on the object's interface" rule.

Maybe we add a properties to Declaration, that specify IsExtended and IsDeprecated, and this inspection could catch unknown extended members and known IsExtended members?

[ThunderFrame]

The IsDeprecated property being something that could be utilized for another inspection. And perhaps other known, reference-specific deprecated members could be added?

I suppose, ideally, in addition to knowing that a member was deprecated, it would be useful to know which member should be preferred. Applying the alternative as an alias seems too flaky. There might need to be some other cross-reference type.

[comintern]

I was going to implement the Excel specific inspection this afternoon. Is there any utility to adding support for a deprecated annotation for user code? AFAICT, MIDL doesn't have a "deprecated" flag, so anything that's coming from a built-in library would need to have this flag added manually (for example in ApplySpecificLibraryTweaks). This would actually make the inspection quite a bit easier to implement and would also allow for inspecting _user deprecated_ functionality. I was thinking of something similar to:

'@Deprecated NewerShinierFunction
Public Function OldCrappyFunction() As Long
    OldCrappyFunction = 6 * 7
End Function

Public Function NewerShinierFunction() As Long
    NewerShinierFunction = 42
End Function

Sub DoTheThing()
    Dim foo As Long
    'Triggers inspection result: "OldCrappyFunction is deprecated - use NewerShinierFunction
    foo = OldCrappyFunction
End Sub

If that doesn't hold any interest, I'll just implement the WorksheetFunction specific inspection with string comparisons on the built-in declarations.

[ThunderFrame]

Nice idea. In addition to the "Use of deprecated member" inspection, the @Deprecated annotation could be accompanied by a @BurnItWithFire annotation treated as an implicit TODO marker?

[retailcoder]

I like deprecating built-in members we know have better alternatives (I hear the Word object model has a few of these), but I have mixed feelings about a @Deprecated annotation for user code - after all, one should just outright remove these members. ...but the world being as imperfect as it is, I suppose there is a use case for it, and it makes useful metadata that a TODO marker doesn't convey.

Go for it! :-)

[ThunderFrame]

one should just outright remove these members

Except if the proc is in an add-in or template, and there are multiple documents that depend on that add-in. You might not fix all documents at once, so the deprecated annotation helps you find those usages at a later date.

[comintern]

I'll open it as a new issue (getting ready to close this one) - the string implementation was actually a bit faster than I expected, but if we anticipate more host specific inspections like this it should probably be switched to use annotations.