Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Rubber Duck is greedy and feeds on VB Attributes (not a joke) #6216

Open
AstrocalcVB opened this issue Jun 11, 2024 · 10 comments
Open

The Rubber Duck is greedy and feeds on VB Attributes (not a joke) #6216

AstrocalcVB opened this issue Jun 11, 2024 · 10 comments
Labels
bug Identifies work items for known bugs

Comments

@AstrocalcVB
Copy link

Rubberduck version information
The info below can be copy-paste-completed from the first lines of Rubberduck's log or the About box:

Version 2.5.9.6316
OS: Microsoft Windows NT 6.2.9200.0, x64
Host Product: Visual Basic x86
Host Version: 6.00.9782
Host Executable: VB6.EXE

Description
In Code Inspections, applying a quick fix to Language Opportunities: "Assignment uses obsolete 'Call' modifier" - all user added Attributes like "Attribute Procedure_name.VB_Description = "Some text..." are removed from the modules file.

Not only VB_Description attributes are removed.
That's greedy I would say, ducks should stick to eating grass ;-)

Furthermore, there seems to be a second bug here as well; Choosing "Remove obsolete statement ->Selected occurrence or Occurrences in module" does what it says but if you choose "Occurrences in procedure" the Duck goes for the whole project.

To Reproduce
Steps to reproduce the behavior:

  1. Make sure to have a module with one or more Call modifier and VB_Description Attribute
  2. Open Code Inspections from Rubberduck menu.
  3. Right click item with Call modifier notice and select one of the options in the context menu "Remove obsolete statement"
  4. When RD is done, save the VB file and then check it in a text editor. All attributes are gone.

Expected behavior
It should leave my attributes alone.

Logfile
A Trace log file just gets enormously big like 50k+ rows so I just include for Debug level, which is still 6k+
RubberduckLog.txt

Additional context
Add any other context about the problem here.

@AstrocalcVB AstrocalcVB added the bug Identifies work items for known bugs label Jun 11, 2024
@AstrocalcVB
Copy link
Author

Revisiting this after 6 months as there has been no comment, adding that this still happens in 2.5.9.6321 but also when e.g. moving a file into a virtual RD folder. If the file has any row in the code section starting with "Attribute " it's removed.

Adding a sample file as proof of concept for testing, inside zip. The class file has a line starting with:
"Attribute WindowProc.VB_Description"
below Function WindowProc() but the member is also implicit public, which RD catches.

When i "Fix" this, Public is added to start of Function but the line below starting with Attribute is removed.

I think this is quite serious as it "destruct" code, code that isn't really visible unless you open the file in a text editor or use source control. I'm aware these Attributes don't really affect functionality of the code but it's still scary when things get slaughtered in the background without obvious notice.
cIBSSubclass.zip

@retailcoder
Copy link
Member

retailcoder commented Dec 25, 2024

The way Rubberduck detects when it's losing attributes, is by using annotations. They work in pairs of inspections: one detects hidden attributes without a corresponding annotation, and the other detects annotations that don't have an associated attribute. For example if a procedure has a @Description annotation then it'll expect to find a VB_Description attribute for that procedure, and if it's missing (or the description is mismatched) then there's an inspection result for it.

@AstrocalcVB
Copy link
Author

AstrocalcVB commented Dec 25, 2024

Well, doesn't seems to work quite that way, if I have understood correctly what you say. I tried to add an Annotation @description for the procedure/function with the exact same text as the present VB_Description attribute but on doing that it still removed the hidden attribute VB_Description.

The problem seems to be with a hidden attribute present in the code before RB is installed or present in imported code, that's at least how it appears to me.

BTW thanks for being here on Christmas Day - I hope you still have a Merry one :-)

@AstrocalcVB
Copy link
Author

The problem here as well is that it's not just the specific attribute related to the fixed member that is effected, but ALL attributes are removed even though the action has nothing to do with them. I'm not familiar with the RB internals so not sure what's going on, if they are deliberately removed or "left behind" in some kind of copying of data etc. but gone they are after any RD action taken on the file.

@retailcoder
Copy link
Member

Yes, this is happening exactly because the VBE isn't showing the hidden attributes. If we use the exported code, we'll generate code that VBA can't compile. So we use the code pane version of the code, which has everything where you expect it, except there aren't any attributes in it.

So the compromise is that we ask you to tell RD what the important attributes are, so it can tell you when they're being nuked, through the inspections that were developed for that purpose ☺️

I do recall something was done about this at one point though, and I'm pretty sure RD would no longer be losing attributes like this - unless they're in a document module, which cannot be exported/edited/imported-back, so any attributes in a worksheet module would indeed be lost, and there's nothing we can do about it, other than heavily discourage the use of any attributes in document modules.

@AstrocalcVB
Copy link
Author

I'm on VB6 and I can tell you that it happens, the hidden VB Attributes gets nuked right and left in .cls, .ctl and .frm files, but I understand it better now from what you are saying and found the other side of the problem "Code Quality Issues". I will test to see if I fix it there first, if that solve the "nuke" problem on "Rubberduck Opportunities" side.

@MDoerner
Copy link
Contributor

I think this might happen because of how we handle writing the exported version of the code for VB6.
For VBA, we have a feature that recovers all attributes whenever RD rewrites code. This should also work for VB6, but somehow the result does not seem to make it to the actual files.
Unfortunately, I do not have any access to an environmen for VB6. I might still have look at the code to figure out what is going on.

@AstrocalcVB
Copy link
Author

I'm happy to test/review any try to fix.

@AstrocalcVB
Copy link
Author

I thought if I picked a "Member 'MemberName' has a 'VB_yyyy' attribute with value(s) 'value', but no corresponding annotation" and applied "Fix-> Add attribute annotation Occurences in Module" that would add all missing annotations, and if VB_attributes were removed, RD would pick up on that next time and offer to add missing attributes based on the new annotations... but that didn't quite work as expected.

If I went for the first instance "has a 'VB_Createble' attribute, then it only added annotations for 5 at the top of the file "VB_Creatable, _PrediclaredId, _Exposed, _GlobalNameSpace, and _Name, left those attributes in place but removed all others in the file.

If I instead picked one further down in the file, belonging to an Event function, then RD ignored to add annotations for the attributes above, but added for all other attributes, except for a few related to Public Event declarations in the modules Declaration part. In fact, RD doesn't seem to see these declarations at all, but removes the associated VB attributes.

Now, if I Refresh Code Inspections the new annotations (for at the same time removed VB attributes) don't yield any new "Rubberduck Opportunities" to add back the missing VB attributed. However, after restarting VB6 they show up.

Now is the missing attributes are added, and interestingly the once the Public Event declarations are not removed. So a bit of a workaround the problem, so far... but now I go back to fix those 5 attributes at the top, i.e. VB_Creatable etc. and woops all the other attributes RD just added are gone, including the once it decided not to remove earlier (Public Event).

Luckily though the annotations for the removed attributes are still there. So a restart of VB6 gives me the opportunity to add back the attributes. So still a workaround but a cumbersome one and then the Public Event attributes are still a problem as RB never sees them to offer adding annotations. So have to sort them with my SC diff tool.

Ok so all well then maybe, except for quirks? Not really, because RD wants me to add the '@folder annotation, which finally removes everything again. Well, restarting VB6 again gives the option to bring it all back, so there is that.

Anyway, now I think I have given you a rather comprehensive description on that's going on. Just a final question, does it work the same if I e.g. add manually with a text editor annotations like @folder etc. ?

@AstrocalcVB
Copy link
Author

I think this might happen because of how we handle writing the exported version of the code for VB6. For VBA, we have a feature that recovers all attributes whenever RD rewrites code. This should also work for VB6, but somehow the result does not seem to make it to the actual files. Unfortunately, I do not have any access to an environmen for VB6. I might still have look at the code to figure out what is going on.

Yes, something fishy is going on here. When you "Fix all missing annotations in the module" for the already present VB attribute, the fix adds the annotations but removes said attributes. What more is that on a "Refresh" of Code Inspections the now removed attributes never show up in "Rubberduck Opportunities" offering to add them back. So it still think they are there. A restart of VB6 is needed for them to show up and are then properly added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identifies work items for known bugs
Projects
None yet
Development

No branches or pull requests

3 participants