Unlike quite a number of Rubberduck releases, this time we’re not boasting we above a thousand commits: this time we’re looking at well under 300 changes, but if the last you’ve seen of Rubberduck was 2.3.0 or prior, …trying this release you’ll quickly realize why we originally wanted to release it around Christmas. So, here’s… Continue reading Rubberdu…
> They’re back, and this time it does work, and it’s another game changer: Rubberduck users no longer need to export any code file to modify module & member attributes!
I think that's inaccurate - we still have an open PR to do that automatically
which will definitely come in next version.
So for now they get inspected and flagged but fixes will be manual (I think?)
Cool, we'll have to work out what goes in next now. I know @Comintern's been pining for the deployment PR to go in sooner than later but I'd rather get others' opinions.
@this My open PR about recovering member attributes tries to fix the following problem. Whenever we rewrite a module, we replace the whole text in the module. Doing this via the code pane makes the VBE remove all member attributes.
The PR introduces an automatic second rewrite that recovers the member attributes.
This service is a provides access to the active selection and selection of individual modules without involving COM wrappers directly; the interface only uses QMNs and selections.
Out of 9 open PRs, only 5 are neither WIP nor conflicting. That's 4072 (Refactoring dialog refactor), 4582 (Deployment enhancements), 4699 (Clean up persistence service), and 4717 (Inspection toolwindows overhaul). Both 4072 and 4582 are more than a month old and will likely have impacting changes. I'd like to merge those 2 first to ensure they all get field tested as soon as possible.
However, if we merge them, then it means that if we discover any hotfixes needed for 2.4.0, they'd have to go into a separate branch in order to merge into 2.4.X release separately to ensure those don't get into the green release due to hotfixes.
@M.Doerner No problem. If there is no dissenting opinions, I will create a hotfix branch before merging any more PRs. I think that best complement our existing workflow.
@FreeMan IKR?
At least with the QR code, you can use your phone to get a KB article (I think?). In oldies day, you had to scrabble down the pertinent information and if you were oh, so fortunate to have another computer handy, to look it up.
I'm still getting the occasional crash on exit but I've been otherwise occupied and haven't managed to remember to gather the WER reports. I will make a note to self to remember to do so.
however, I have to do my annual self-evalutation. yech. so it'll be a little while longer...
> I recently upgraded from 2.3.1.4441 to 2.4.0.4488 and I'm reasonably confident that these false positives haven't been showing up before the upgrade.
For reference, the inspection tells me: *An assignment is immediately overridden by another assignment or is never used.* I hope upon inspecting the following procedures you will agree with me that the above assessment is objectively false. Also please disregard their mostly nonsensical nature - I tried to condense them as far as possible.
> While I'm not sure if that's the intention of the current inspection, I think this definitely should be a different altogether inspection. Something like `InconsistentDimScopingInspection` -- the main problems is that the `Dim` statements are apparently used outside the blocks. For that reason, dimming them inside a block doesn't really make sense.
This inspection is necessary because otherwise it hides a subtle "feature" of VBA -- the `Dim` statement is "global" to the entire procedure, e
> While I'm not sure if that's the intention of the current inspection, I think this definitely should be a different altogether inspection. Something like `InconsistentDimScopingInspection` -- the main problem is that the `Dim`med variables are apparently used outside the blocks where the statements are. For that reason, dimming them inside a block doesn't really make sense.
This inspection is necessary because otherwise it hides a subtle "feature" of VBA -- the `Dim` statement is "global"
> Alright, so I'm totally aware that there's no thing like different scopes within a procedure (`With` blocks may be an exception). The only reason I put the `Dim`s in those blocks is because it seemed the most logical place. (And some of them might even be the result of the `Move Closer to Usage` QF.)
But what about the `IssueWithStaticVariable` procedure? Is this a similar case? If so, how?
> Alright, so I'm totally aware that there's no thing like different scopes within a procedure (`With` blocks may be an exception). The only reason I put the `Dim`s in those blocks is because it seemed the most logical place. (And some of them might even be the result of the `Move Closer to Usage` QF.)
But what about the `IssueWithStaticVariable` procedure? Is this a similar case? If so, how?
Edit: And of course, @bclothier is totally correct. Moving the `Dim` outside of the blocks will ma
> It seems like the code analysis does not take Static locals into account. I'd assume this specific case never worked, though...
Note that the inspection is not about Dim statements, but about assignments. It seems that the code path analysis incorrectly scopes Dim statements to their direct parent node instead of propagating them up the "scope tree".
> I'm pretty sure `IssueWithStaticVariable` is a separate issue (and a legitimate one at that); it shouldn't have had fired any results.
As for inspection results disappearing when the `Dim` statement has been moved outside the block, this is more of a readability issue and a benefit for the programmer, not for the compiler. Seeing it inside a block makes me think it should be confined to only that block. So I want to be warned if I'm misusing a variable outside the block. One way we can do
@Duga This makes me think, though - do we want to also inspect for conditional assignments of the variable which may not be expected?
I've seen code that wrongly assumes that a variable will always have a value assigned due to it being inside a If...Then block to guard against a runtime error or something like that.
> > Seeing it inside a block makes me think it should be confined to only that block. So I want to be warned if I'm misusing a variable outside the block.
While I can follow your reasoning, I don't think it's globally applicable. Can I assume your primary coding background is in a language that offers such different scopes? As someone who basically learned coding in/from VBA, I wouldn't have called that misuse. (What, you can tell that from my MCVE? *cough*) I can see the merit in handling it
> Indeed - VBA is in the minority when it comes to scoping resolution. Several languages normally scope the declarations to the same block it is declared and consider them invalid outside the block.
Even so, I started with VBA long before I learnt C# so I have been accustomed to the famous "wall of the declarations" that is very commonplace throughout VBA. `Move closer to usage` is one way to fix it but to put it at very first assignment when they are used later somewhere in the procedure fe
Pretty sure those were the last 2 outstanding issues for the inspection results that aren't already covered by #4717, although I'll do another check when I go through and make sure the CE pulls closed everything it should have.
> Just tossing my $.02 in here, the Assignment is not used inspection should not care about the location of the `Dim` within the procedure at all. It is inspecting for a superfluous assignment and nothing more.
Re another inspection for where the `Dim` is placed within the module, if we want to inspect that, IMO it should be opened as another issue and tagged with enhancement. For that one, I'd suggest something more along the lines of "Variable used outside of implied scope", at a hint lev
[rubberduck-vba/Rubberduck]bclothier pushed commit dcfd96f2 to next: Clean up the csproj files and try to use solution; put in comment block for future fix and dry the current approach of enumerating the projects
> So, to summarize: All of the presented cases in the OP are in need of some smartening-up? And examples 1, 2, 4 and 5 warrant a new inspection about *Hungarian Location*, err.. *Implied Scope*.
> So, to summarize: All of the presented cases in the OP are in need of some smartening-up? And examples 1, 2, 4 and 5 warrant a new inspection about *Hungarian Location*, err.. *Implied Scope*.
Edit: > I would add that declaring inside the "implied scope" makes it much easier to later refactor and extract a method manually.
Agreed. Also, I've started to notice a certain smell when the same `VariableThatIFoundHardToNameCorrectly` appeared again in later sections of the code: There alw
> @Inarion TBH it looks like the inspection is somehow mistakenly tracking declarations instead of assignments; the reported false positives are indeed false positives - the goal behind this inspection is to warn about cases such as these:
```vb 'If bar Then '' conditional block makes no difference foo = 42 ' assignment not used 'End If foo = 74 ```
In other words, it means to identify instructions where a variable is assigned, *and then assigned again before the value is read*, i
> @Inarion TBH it looks like the inspection is somehow mistakenly tracking declarations instead of assignments; the reported false positives are indeed false positives - the goal behind this inspection is to warn about cases such as these:
```vb 'If bar Then '' conditional block makes no difference foo = 42 ' assignment not used 'End If foo = 74 ```
In other words, it means to identify instructions where a variable is assigned, *and then assigned again before the value is read*, i
> @retailcoder but conditional block should be considered for this case, no?
``` foo = 74 If bar Then foo = 42 End If ```
Granted, this might be a case where it can be better rewritten as:
``` If bar Then foo = 42 Else foo = 74 End If ```
but the assignment before the conditional block is a common pattern for ensuring there's a sane value given in case where conditional block don't apply or something like that.
@Comintern I kind of agree in principle that we might not want to count the Debug calls. OTOH, I'm not sure if we should bother until we have AvalonEdit so that we can dim out the Debug statements like R#(?)/VS(?) does.
@Comintern FWIW, it's somehow a less problem w/ Access, especially if you ship ACCDE by default. In that case, all Debug statements are automatically stripped out.
Can this be done more efficiently?
The code does the following:
Do Stuff
Show UserForm and ask user to confirm the shown data is ok
User can manipulate and controll excel as usual
As soon as user clicks the "yes" button "do some other stuff" is called
The Module
Sub ControlDataUI()
Dim Ui ...
> @retailcoder I think that the definition you gave of what to report is slightly off, too. I think the following would make more sense.
It should report all assignments in any code path such that in all subsequent code paths the variable is either reassigned before use or never used at all.
More likely, the tutorials they used contained only a brief paragraph and obscure example in the last chapter, and they went "meh, I guess I don't need it."
Makes me think - does any language used by the same crowd (e.g. non-professional programmers) have the same tendencies? (would say Python but not sure if that's a fair comparison)
The problem arises when lack of familiarity with language features compels the non-programmer to "re-invent a wheel", which invariably comes out square instead of round.
There was an SO question over the weekend about how to use Excel's Choose function on a VBA array, and I was like "um... there's a VBA Choose function".
OK, since nobody else has mentioned it, I'll bite and ask why you need to know the order that worksheets were added. It sounds like you're trying to treat the worksheets themselves as data, which indicates a design problem. — Comintern49 secs ago
It just struck me as an odd thing to need - OP is probably doing something weird, and relying on the ordinal of the worksheet is kind of like relying on the name of a database table.
hmm, not sure if there's actually a difference. I'm sure VB has its appobject stuff - I'm thinking more about the project types, the fact that the file doesn't exist independent, the document module types which must be provided by the application.
IINM, VB6 does not have a equivalent for document module?
(there's a UserDocument but that's slightly different, I've been told?)
@MathieuGuindon I have to double check but I think Access reports can use those. Never used it (and I doubt majority of Access users even know about those)
The thing is that how it's implemented, it's "OK" to add by whole numbers. But fractional part don't work the same way.
But the real point is that when you abuse dates like that, you've violated the encapsulation of the Date data type and are now treating it like a just another Double which may not have the same outcomes when you do use correct date arithmetic functions.
@Greedo the "default interface" of a class is the interface presented by an object declared As [ClassName] - in other words As UserForm1 includes the members of UserForm inherited from the base class, and the members of UserForm1 defined in UserForm1, but not the members of IDialogView defined in some hypothetical IDialogView class that UserForm1 would be implementing with Implements IDialogView. — Mathieu Guindon14 mins ago
it's not just epoch, though. You can get the same behavior with dates predating the epoch, too. I don't think you can get that after the epoch.
But if doing arithmetic on the date "leaks", that's good enough reason to stop violating the encapsulation that the Date data type and its related functions provides, no?
Not all valid values of a double are also a valid value of Date.
> 5.5.1.2.3 Let-coercion to and from Date A Date value may be converted to or from a standard Double representation of a date/time, defined as the fractional number of days after 12/30/1899 00:00:00. As Date values representing times with no date are represented as times within the date 12/30/1899, their standard Double representation becomes a Double value greater than or equal to 0 and less than 1.
Can this be done more efficiently?
The code does the following:
Do Stuff
Show UserForm and ask user to confirm the shown data is ok
User can manipulate and controll excel as usual
As soon as user clicks the "yes" button "do some other stuff" is called
The Module
Sub ControlDataUI()
Dim Ui ...
VBA Rubberducking
VBA Rubberducking