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

Trigger-happy VariableNotAssignedInspection warning #6192

Open
iVilius opened this issue Jan 11, 2024 · 2 comments
Open

Trigger-happy VariableNotAssignedInspection warning #6192

iVilius opened this issue Jan 11, 2024 · 2 comments
Labels
bug Identifies work items for known bugs

Comments

@iVilius
Copy link

iVilius commented Jan 11, 2024

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

Rubberduck version 2.5.9.6316
Operating System: Microsoft Windows NT 10.0.22631.0, x64
Host Product: Microsoft Office x64
Host Version: 16.0.17029.20108
Host Executable: EXCEL.EXE

Description
"VariableNotAssignedInspection" is firing in a manner that is not expected. There are 2 faulty behaviours:

Example 1:

CustomClassVariable.Sub subarg1

CustomClassVariable will in this case get flagged by the mentioned Warning (VariableNotAssignedInspection) even though it is being used to execute a method by utilizing one of its functions/subs.

Example 2:

Dim retVal as String * 8192
Mid$(retVal, 3) = SomeVal

retVal gets flagged by the same warning again even though it is being used for fast string concatenation.

These bugs should be fairly easy to fix.

To Reproduce
See examples above.

Expected behavior
Expected not to see the mentioned warning.

Screenshots
If applicable, add screenshots to help explain your problem.

Logfile
Rubberduck generates extensive logging in TRACE-Level. If no log was created at %APPDATA%\Rubberduck\Logs, check your settings. Include this log for bug reports about the behavior of Rubberduck.

Additional context
Add any other context about the problem here.

@iVilius iVilius added the bug Identifies work items for known bugs label Jan 11, 2024
@retailcoder
Copy link
Member

These bugs should be fairly easy to fix.

Pull requests are always welcome! 😊

These are actually two rather tricky edge cases: ByRef assignment (where an unassigned variable is passed ByRef to a procedure that assigns it) is not being tracked at all, mostly for performance reasons; we err on the side of caution and warn anyway, because there's no way to tell whether passing the parameter ByRef was intended or just an omission, given the language default.

If I recall correctly, the Mid/Mid$ statement is syntactically a special case that must be parsed as a keyword rather than a function call, so the arguments don't (and shouldn't either) resolve to Mid function parameters; I'd have to double-check but IIRC we are treating this argument as we would a ByRef assignment. This one could be a case of misunderstanding exactly how the statement operates, since it's a rather rare occurrence; I agree there should be a relatively easy way for the inspection to ignore them, if indeed it's guaranteed that the given argument is assigned after the statement is executed.

ByRef assignments are not likely to be addressed in v2.x, however the plan for v3 diagnostics is to massively expand the customization and allow each individual inspection to have its own SettingsGroup, which would make it possible to configure the inspection to either pursue ByRef assignments, or to ignore them specifically, without needing to sprinkle @Ignore annotations. Many inspections have such default behaviors that would ideally be configurable, but the 2.x settings would be painful to implement.

I think the case of the Mid statement could be fixed in 2.x, but I think it's probably best to let ByRef assignments slip for now, until diagnostics can be individually configured in v3.

@retailcoder
Copy link
Member

Actually I misread the first case, it's about a member call and it's the object variable being flagged, not the argument (correct?). In that case then the outcome depends on where the object variable is assigned. If it's global and assigned in another scope that just so happens to be invoked at some point before, then there is indeed no statically guaranteed way that the containing procedure will always be invoked in the correct global state and the warning is warranted.

If it's set relatively near its usage, there's no reason for it to be flagged. The provided code example does not show how the object variable is set though.

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

2 participants