VBA Rubberducking

Thinking about it some more - the issue with IUnknown would be only when you have a COM interface that inherits IUnknown but not IDispatch. In that case, you would not be able to assign that interface to a Object (and maybe Variant neither) because there's no IDispatch support.

In the example code, though, we only care whether the type is IUnknown, Variant or Object; since all are compatible.

Mathieu Guindon

@SmileyFtW in the "type libraries" tab, type "rubber" :)

SmileyFtW

k, thanks!

Mathieu Guindon

6:18 PM

> Rubberduck AddIn
> C:\ProgramData\Rubberduck\Rubberduck.x64.tlb

this

Hmm. now that makes me wonder what happens if one has a IUnknown-inherited interface and try to assign it to Object. Does that fire an inspection?

@MathieuGuindon note that path may vary. That's a per-machine install. Per-user install has it somewhere else in %appdata%

Mathieu Guindon

aye

the project refs search box would find it there then

@this if Set someObj = someIUnknown is a type mismatch, then yes. otherwise, no :)

wait no the other way around I mean - Set someIUnknown = someObj

this

no, that should always succeed.

Object => IDispatch => IUnknown

it's only when you have Foo based on a IUnknown-only interface, then you can't assign it to Object because no dispatch.

No compile error for that, though. I don't have a implementation of a library that has a IUnknown-only interfaces handy to verify what happens.

Mathieu Guindon

IOW the inspection should just bail out if IUnknown is involved

this

Not quite.

Public Sub x()
    Dim f As IFoo 'an IUnknown-only interface
    Dim o As Object

    Set o = f 'should be illegal
End Sub

No IUnknown here.

we'd have to see that IFoo itself does not implement IDispatch and thus can't be assigned to any other types except IFoo or IUnknown; not sure about variants, though.

That code will compile, though. I am not confident it will run, though.

Mathieu Guindon

6:39 PM

That's just set o = nothing, no?

IFoo can only be a user class here

this

[uuid(26387857-0462-4F01-B852-8D34BBFA60BA)]
library test_iunknown
{
	[uuid(26387857-0462-4F01-B852-8D34BBFA60BC)]
	interface IFoo : IUnknown {
		HRESULT Bar();
	}
}

I didn't new it up because I have no implementation for IFoo

Mathieu Guindon

and if f is actually set, then it's totally legal downcasting

this

I need to find which interfaces implements only the IUnknown and test but I am pretty sure that it's not legal to cast f to o because Object in VBA is basically IDispatch, so that's upcasting, not downcasting.

Mathieu Guindon

Interesting

I still believe it's completely legal, can't test it atm

(assuming IFoo is a class module)

this

No, it' s not. See the IDL above.

Mathieu Guindon

6:44 PM

Ah

this

All VBA class modules always implements IDispatch

The only time you'd see a IUnknown implementation is from an external library that didn't bother to implement the IDispatch. In this case, early-binding is required to use objects from that library.

Mathieu Guindon

I'm not seeing an easy way to fix that inspection then.... not without carrying some kind of flag from the ITypeLib loader that tells us whether a type implements IDispatch

SmileyFtW

7:01 PM

@MathieuGuindon .x32 or .x64 ? Is it for the OS or the Office type?

Nevermind... just created a module and it picked x32

Mathieu Guindon

the host - it's always the host :)

M.Doerner

@this I think you think about a completely different issue than in the issue I want to fix.

Your issue is that we should not consider Object to be safe.

The issue is about the fact that IUnknown is always safe to assign to.

Cyril

@MathieuGuindon going back through the typescript learning and office-JS, that will allow me to run scripts for Office-Online applications, but will I be able to edit them online (I'm still waiting on my ticket to get resolved if they will let me have access to ScriptLab, so just gathering info atm). Noting the line in your blog about the Excel-Online team working to get things up to par, it may not currently be applicable, but is the intent for Office-Online to both design and execute scripts?

the active editing issue is one that i can't verify without having, so relying on you... sorry =/

i believe the answer is yes, but didn't see it explicitly stated (significant inference in the section about Caret locatino dropping as moving between tabs)

Mathieu Guindon

@Cyril Script Lab edits and runs, yes

doesn't debug though

Cyril

doesn't debug, but can you compile without executing?

Mathieu Guindon

7:15 PM

not that I know of

Cyril

copy

thanks again

Mathieu Guindon

maybe in VSCode though

(or actual-VS)

Cyril

intending to go through freecodecamp.org/news/… when i have a little more time to dive

trying to get some actual work things set-up atm

M.Doerner

I don't like this: my tests are passing for the wrong reason.

Oh, to really test is, I have to reference stdole.

Duga

[rubberduck-vba/Rubberduck] jmaynier starred us

2

Cyril

7:32 PM

I need to figure out if Mat's got a blog for testing... like how to and when. all i've ever done to date is to run the code (execute or step through) and debug when i have errors. i have seen testing code from iven and i simply don't comprehend that aspect, yet

Mathieu Guindon

@Cyril the thing with testing is that you simply can't do it with side-effecting code, and code with hidden dependencies - basically you need to fundamentally change how you write code, so that it's testable in the first place

i.e. every conversation about decoupling things has been about testing

Cyril

i kind of assumed the decoupling things were related to having an abstraction (classes) rather than having them be too direct to a particular scenario... learning is fun

when you say "side-effecting" as in client-side, specific effects?

FreeMan

@Duga milestoneCountdown = 8

Mathieu Guindon

@Cyril take a SUM function - the function is easily testable, because it takes all its inputs and doesn't affect any global state. SUM(2, 2) will always reliably return 4.

@FreeMan eh, countdown jinxed it the other day :)

now take a function that reads a value from some file, multiplies that value by a given parameter, and sets Application.Calculation to xlCalculationManual

we can't possibly test that function, because it's going to hit the file system and alter global state

IvenBach

@Duga Someone unstarred... It was 8 yesterday.

Mathieu Guindon

7:46 PM

it has hidden dependencies in that FileSystemObject (or Open statement) being used

and if we write a test for it and run it two times in a row, we won't be able to verify that Application.Calculation was correctly set, because the first run has just interfered with the global state we're working with

@Duga re-starred actually

so, testable code implies code written in a way that allows the test code to control the dependencies: ideally you don't want your test to fail because e.g. the network drive the function is accessing is disconnected

so instead of working directly with a FileSystemObject, you would take some IFileIO interface as a parameter, write a class that implements some IFileIO.ReadValueFromFile method using a FileSystemObject - and then your test code can provide an alternative, "test stub" implementation of that interface that won't actually hit any file system but allow your test to know that ReadValueFromFile was indeed invoked

Duga

[rubberduck-vba/Rubberduck] akcansoft starred us

2

Mathieu Guindon

...and implementing test stubs for every interface is a pain in the neck. that's why this PR is a game changer

Cyril

so to high-level capture, in cases where you can keep thing within the "box" of the direct, local realm, do that...

Mathieu Guindon

well, it all hinges on what you consider your "unit"

Cyril

the call/invoke concept regularly throws me, so i'm going to take a stab... in your example, you have the Class invokes FSO, rather than calling FSO within your code, so that you can circumvent/plan for errors and test

Mathieu Guindon

7:57 PM

maybe you're reading a value from some worksheet instead of from some text file

yes. essentially you're wrapping dependencies in code you can fully control

FreeMan

@MathieuGuindon oops... musta been a misclick.

Mathieu Guindon

note that these wrapper classes aren't testable: they're dependencies of the code/logic you want to make testable

Cyril

@MathieuGuindon i think that hits on the "when" to test and part of where my confusion comes in

Mathieu Guindon

the way I see it, cells in a worksheet are just global state

so, any code that reads from or writes to a worksheet, can't be reliably tested without abstracting the worksheet away

I touched on that in There is no worksheet a while back

note, that particular approach is very heavy-handed

M.Doerner

One example I run in frequently is that there is some configuration data on some sheet.

Instead of getting the data from the sheet directly, I have an interface IConfigurationProvider with a method to get the data.

Mathieu Guindon

8:05 PM

yep

M.Doerner

and an implementation SheetConfigurationProvider that actually gets it from the sheet.

My other code then takes a, IConfigurationProvider and does not care where the data comes from.

Mathieu Guindon

and then you can have a TestConfigProvider that doesn't care for the sheet and just lets tests tweak it as needed

M.Doerner

When tesing, I can just provide some StubConfigurationProvider that provides some data I want in the test.

^ (too slow)

Mathieu Guindon

=)

this

@M.Doerner Yes. I was trying to remember the cases where IUnknown becomes problematic, and that does not apply in your fix. I already had answered that I agreed that it's safe to assign to IUnknown; it's only when you have an IUnknown or an interface deriving from IUnknown (and does not implement IDispatch) then it's not safe to assign to an Object (or Variant?).

The problem, however is finding an actual implementation that has a IUnknown derived interface and does not implement IDispatch to test this.

M.Doerner

8:10 PM

I think you can assign absolutely everything to Variant.

this

Yes, there's a VT for IUnknown, IINM.

M.Doerner

Finally, my test all fail correctly.

this

but what I'm not sure is whether you can do someVariant.someMethod on a Variant variable referencing an IUnknown-derived interface.

I do not think that will work since that would be a dispatch call.

In fact, having a Variant/IUnknown or IUnknown is fairly useless, unless all you care about is managing the object's lifetime without knowing about it.

Mathieu Guindon

but it should still be assignable I think

this

Yes, I think so, too.

M.Doerner

8:15 PM

I think we should at least treat it as assignable, since we have no idea what it is.

Cyril

@MathieuGuindon "So you name that range Header_AccountNumber"

M.Doerner

VBA does not care about the declared object type at compile time.

The entire point of the two inspections is to warn about violations at compile time.

Cyril

will go back and read through that post, mat... only got so far, but need to finish a task before a meeting in... 14 minutes

M.Doerner

However, if we have absolutely no idea what the underlying object type might be, we should just not report a result.

this

For that inspection's purpose, yes, we should just treat IUnknown the same way we do Object / Variant

However, I can see an inspection for verifying that a variable deriving from a IUnknown interface isn't assigned to an Object or Variant variable.

Mathieu Guindon

8:21 PM

@this that would basically be specific to [_NewEnum] implementations, ...and that's all I can think of atm

this

as it happened, I was working on using some IUnknown interfaces, but the water get muddy because of this following situation:

Dim wb As ShDocVw.WebBrowser
Dim doc As MSTHML.Document
Dim customDoc As ICustomDoc 'Unknown-Derived interface

Set wb = Me.WebBrowser0.Object
Set doc = wb.Document
Set customDoc = doc 'Cast succeeds

Dim o As Object
Set o = customDoc 'Cast succeeds because implementation supports Dispatch

I have to dig a bit deeper to find an object that implements only IUnknown and see what the behavior is. As I understand, when that is the case, early-binding is the only way to use it.

Which is why I don't expect using Object or calling methods on it late-bound will succeed.

M.Doerner

Arrgh, my tests are still wrong; we do not have serialized declarations for stdole.

this

But as you see above, when the implementation implements both, even though I've cast to a IUnknown-only interface, I still can cast back to dispatch because of the implementation.

@M.Doerner I thought comintern moved us off serialized declarations?

Mathieu Guindon

for prod, not for tests :)

M.Doerner

Right, we do not have a serialized com project for stdole, then.

Mathieu Guindon

8:28 PM

~~wait, tests are also loading the actual TLBs?~~

read Mug, read

M.Doerner

And my fix also does not work because IUnknown is an actual type in a references project.

It is stdole2.tlb:stdole.IUnknown for me.

The special casing should probably not be library specific.

jcrizk

ok, so let me know if im thinking about this correctly.

a PR between a fork and the original repo basically just moves commits from the fork onto the original repo, right?

Feeds

See also: Farm animals and dinosaurs. I am so confident that there exists children's media that involves dinosaurs driving trucks on a farm that I'm writing this without even Googling to check.

IvenBach

@jcrizk Correct. It will take the commits from wherever commit you branched from and intregrate them into the "main" repo.

jcrizk

gotcha thanks lol i wanted to make sure what i said at work was correct

i started working in a fork of our repo at work because i have like 1million experimental branches that i wanted to get rid of from the main repo lol

IvenBach

8:39 PM

Don't forget to review the full manual directly.

M.Doerner

Hm, we have the test files. They are just not wired up.

Mathieu Guindon

@Feeds yep. c.f. DinoTrucks

M.Doerner

OK, now everything is wired up.

BigBen

@MathieuGuindon wtf are DinoTrucks

IvenBach

Dinotrux

Dinotrux is an American-Canadian computer-animated web series. It features a fictional prehistoric world inhabited by hybrid characters that are part dinosaur and part mechanical construction vehicle. The larger Dinotrux are accompanied by Rotilian Reptools who are smaller reptiles combined with mechanic's tools. The series debuted on August 14, 2015 on Netflix, with the second season following on March 11, 2016, and the third on October 7, 2016, and the fourth on March 31, 2017, and the fifth on August 18, 2017. Beginning in November 10, 2017, subsequent seasons are released under the tit...

Mathieu Guindon

8:48 PM

@BigBen you soooo don't want to know

BigBen

How did this thing get a 7.0 on IMDB?

IvenBach

@BigBen Dinotrux is the reason for Mugs love of whiskey.

BigBen

ahhhhhh.....

Mathieu Guindon

Nah, kids too old for it. Paw Patrol is =)

IvenBach

Rubble on the double.

BigBen

8:52 PM

completely unrelated - I think this has been discussed on Meta - but for those of us who are still closing crap questions on SO, do you downvote too or is that unwelcoming?

FreeMan

@MathieuGuindon My first thought.

@BigBen It's like Barney. Parents get a 30 minute break while the sprogs are glued to the tube.

BigBen

@FreeMan - I knew what "sprog" meant from context... but always happy to learn a new word :-)

Mathieu Guindon

@BigBen I only downvote these days (still voting on answers though, but curating Qs, meh)

BigBen

ups my Scrabble game

FreeMan

@BigBen lol

BigBen

8:56 PM

hey - Scrabble is very competitive

FreeMan

'tis a UKish term (at least in my hearing), but I liked it, so I appropriated it.

BigBen

I need any edge I can get

FreeMan

@BigBen you've never been at my in-laws on Christmas... 0_o

BigBen

:-)

The Midwestern equivalent of that scene would probably be euchre. Crazy tournaments at the relatives' house.

Cyril

@IvenBach my son loves dinotrux

he's also pretty big into paw patrol... we don't own a television, so netflix having dinotrux has been helpful

the paw patrol is when he's getting his hair cut (they have that playing constantly at the locale)

@FreeMan i need to find that rodney carrington skit about barney... put it on and go to the bedroom with the wife cus you've got some alone time

FreeMan

9:08 PM

^sump'in like that. It's about the only way you ever end up with >1 kid

Mathieu Guindon

@FreeMan alternatively, make twins the first time around

FreeMan

@MathieuGuindon generally, one has little control over that. In that vein, I do know several people who went for 3rd and had 3rd & 4th at the same time...

Mathieu Guindon

that's precisely what's giving me second thoughts about wanting a 3rd... I mean, I'd like a 3rd, but don't know that I could handle a 4th!

FreeMan

huh... random icon vertical spacing, but in a different place, this time

#BlameCarlos

@MathieuGuindon from what I hear, once you're past 2, you're in zone defense, so 3 or 5 - it doesn't really matter...

Mathieu Guindon

lol

FreeMan

9:23 PM

That had me nervous about the 3rd, as well. We ended up with just 3. Survived with the help of some Barney.

Whelp... ohs nos, it's snowing!!!one!!11! I'd better leave work!!!

Zev Spitz

Can VBA interfaces have events? Would it be possible to define an interface like INotfyPropertyChanged in VBA?

FreeMan

TTFN

Mathieu Guindon

@ZevSpitz nope

M.Doerner

Question regarding the special case in ObsoleteCallStatementInspection: we only care about the case where the Call is in the first statement on the line and it is followed by a statement separator, right?

I.e. Foo: Call Bar should return a result.

But not Call Foo: Bar.

this

@ZevSpitz I think Mat had a SO thread about that and someone answered using an adapter.

Mathieu Guindon

9:30 PM

@M.Doerner aye, removing Call in Call Foo: Bar will make Foo: parse as a line label; we don't want to be responsible for doing that :)

M.Doerner

Currently, we also do not return a result for Foo: Call Bar.

Mathieu Guindon

@this @ZevSpitz yep; and then I implemented it with the GridViewAdapter class of the Battleship project

this

hmm looks like it would break either way. We probably shouldn't for both.

M.Doerner

Why?

Mathieu Guindon

there's no reason to not flag Call in LineLabel: Call DoSomething

M.Doerner

9:32 PM

We just don't because of the implementation.

this

Sorry I got confused, thinking that Foo: Call Bar would end up making Foo an executable statement rather than a line label like the former case. Ignore me.

M.Doerner

I do not really like the implementation anyway because it uses a CodeModule.

this

Just say no to ~~drugs~~ codemodule

M.Doerner

I think it is really not that hard using the context.

Mathieu Guindon

too bad we don't have history prior to 2018-04-09, that makes me wonder just how old that inspection is!

(easy access to the history, I mean)

probably dates all the way back to the regex-based ~~parser~~ nightmare

M.Doerner

9:43 PM

Why would that history not easily be accessible?

The inspection was created 2015-02-12.

Mathieu Guindon

how did you get to it? on github the file history ends 2018-04-09 when inspections were moved to Rubberduck.CodeAnalysis

M.Doerner

git still has the history

It is all in the log, including file moves and renames.

Vogel612

git can do rename following for some cases

@MathieuGuindon do you mind if I edit your meta answer with a bit of information on how code-prettify works "internally"?

Mathieu Guindon

go ahead! feel free to make it CW if you like :)

M.Doerner

I guess I cannot do too many other things anymore before tackling ProcedureCanBeWrittenAsFunctionInspection.

That really should be a declaration inspection instead of a parse tree inspection.

Mathieu Guindon

9:54 PM

why though? IIRC at one point we were converting a bunch of declaration inspections into parse tree inspections

actually, I think I can see why

M.Doerner

It wants to know about references to its parameter.

The way it currently is written is kind of backwards.

In principle, it should not look much different than `ParameterCanBeByVal, just the other way around.

Mathieu Guindon

makes sense

Vogel612

@MathieuGuindon edit is applied

M.Doerner

Hm, why does the inspection ignore interface members?

Mathieu Guindon

10:09 PM

and event handlers?

M.Doerner

They are ignored for good reason.

Mathieu Guindon

"interface members" being on the interface class or the implementing class?

M.Doerner

The implementations are also ignored.

Mathieu Guindon

consistent, good

M.Doerner

My question is about those on the interface.

Mathieu Guindon

10:11 PM

the parameters would never even be in use on these

M.Doerner

As for ParameterCanBeByVal, the check would be on all implementations, if any.

Mathieu Guindon

@Vogel612 nice, thanks!

imo it makes sense to ignore interface members and their implementations for this

flag an interface implementation, your quickfix needs to apply everywhere impacted

M.Doerner

OK, that is easier anyway.

Mathieu Guindon

@M.Doerner oh I see - basically, inspect all known implementations before determining whether to flag or not... yeah that's kind of over-complicated... and then a referencing project that's not opened in the VBE might be implementing that interface.. we better just ignore them

M.Doerner

We flag and fix for ParameterCanBeByVal. However, that is a far less involved fix.

Mathieu Guindon

10:17 PM

> and then a referencing project that's not opened in the VBE might be implementing that interface

ignore that, it's very much an edge case

M.Doerner

Do we care about the situation where the parameter is assigned by passing it as a by ref argument?

Mathieu Guindon

we should, but ByRef assignments have been problematic since forever

M.Doerner

Ignoring it would yield false-negatives here.

Mathieu Guindon

then again, last time it came up, we didn't have half the resolver capabilities

if it can be done without involving O(MG) complexity, then sure :)

M.Doerner

I guess I will go one level deep as for the other inspections.

Mathieu Guindon

10:23 PM

sounds good

M.Doerner

Hm, what we can look at there is rather restricted to be honest.

We can only determine whether the argument is assigned to if the called method is user-defined.

Mathieu Guindon

4.20.17

oops, wrong screen lol

M.Doerner

happens

I regularly open new tabs when I want to find all.

Hm, the inspection worked a little different than I thought.

and the tests suggest.

It only cares about the first reference to the parameter.

Mathieu Guindon

11:03 PM

huh why would it do that

ttgh

M.Doerner

If the first reference is an assignment, the value passed in is never used.

Mathieu Guindon

ugh. needs CPA.

(1st ref could be conditional?)

M.Doerner

To be completely precise, yes.

I could make is more conservative for now, though.

I could require all references to be assignments.

Or assignments via ByRef argument.

Mathieu Guindon

gets tricky pretty quick!

bbl

KySoto

@this you can index views, if you try to index a view with a sub query, it berfs on you. you then have to go turn that sub query into its own view, index it, then use that for the final view you want to index. granted, that doesnt always work because of the limitations on indexing views.

another question i've been thinking about, does access compile passthrough queries?

i ask because i know if you go and change the sql property of a querydef a bunch, it will recompile the query each time and create DB bloat

but if passthrough queries dont do that...

M.Doerner

11:37 PM

@MathieuGuindon Thinking about it again, you can always write the procedure as function, as long as exactly one parameter is ByRef.

The question just is under which conditions we want to point the user to do so.

The quickfix just changes the ByRef to ByVal and adds a line at the end that assigns the byVal variable to the function result.

BTW, that will guarantee a result for ByVal parameter is assigned to.

Mathieu Guindon

good point

Transcript for