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

Code Inspection - Incorrect Diagnosis: Object variable is assigned without the Set keyword #1868

Closed
Stevefb opened this issue Jun 18, 2016 · 9 comments
Assignees
Labels
bug Identifies work items for known bugs feature-inspections

Comments

@Stevefb
Copy link

Stevefb commented Jun 18, 2016

In MS Word, cos ranges are different...

Sub ImTooLazy(aRange As Range)
aRange = "something"
End Sub

In this case the suggestion should have been working towards:
aRange.text = "something"

@ThunderFrame
Copy link
Member

Attack of the Default member.

@Stevefb
Copy link
Author

Stevefb commented Jun 18, 2016

And another... @ThunderFrame should be new issue, or pile them all in here?

Private Sub UserForm_KeyDown(ByVal KeyCode As MSForms.ReturnInteger, ByVal Shift As Integer)
    If KeyCode = vbKeyF1 Then
        KeyCode = 0                                                ' <-- Should be KeyCode.Value = 0
    End If
End Sub

@ThunderFrame
Copy link
Member

@Stevefb Issues should try to cover a class of problem rather than an issue for each instance. There are many, many VBA classes that have default members, so now that we know that default members remain problematic, a small number of examples is sufficient to define the problem.

Having said that, the MSForms.ReturnInteger only has one member and it is the default member, so it's a good candidate for testing.

@Stevefb
Copy link
Author

Stevefb commented Jun 18, 2016

Private Sub myTextBox_Exit(ByVal Cancel As MSForms.ReturnBoolean)
    If True Then
        Cancel = False  ' Should be Cancel.Value
    End If
End Sub

^^^^^ Funny, originally this Sub did not actually assign a value to Cancel. RD Flagged Cancel as Unused - so I used it...

@ThunderFrame
Copy link
Member

that If True Then statement is looking a little redundant, but yes, ReturnBoolean would appear to be another class with a single (default) member.

So these all have a single member that is also the default member:

MSForms.ReturnBoolean
MSForms.ReturnEffect
MSForms.ReturnInteger
MSForms.ReturnSingle
MSForms.ReturnString

@Stevefb
Copy link
Author

Stevefb commented Jun 18, 2016

Yep. But it looked better than an empty Sub... (was just showing the context as I had it in my program)....

@Hosch250 Hosch250 added bug Identifies work items for known bugs feature-inspections labels Jun 23, 2016
@retailcoder
Copy link
Member

Oh wow. So what would be a good workaround here? Should we ignore types with a default property? or try to determine if the RHS expression is an object?

@retailcoder
Copy link
Member

Hmm shouldn't the "parameter not used" inspection be not firing a result for handlers of built-in events?

@retailcoder
Copy link
Member

One problem with default properties, at least in the Excel object model, is that what we see in the COM library is just _default instead of the actual property name: it's hard to tell exactly whether the _default member is expecting an object reference or a value. Perhaps we should just assume it's a value? I don't recall seeing any default property in the Excel object model being an object. So we could warn about implicit default member assignment I guess, and then have the object reference assigned without the Set keyword inspection ignore instances where the object in question is known to have a default property.

@retailcoder retailcoder self-assigned this Dec 17, 2016
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 feature-inspections
Projects
None yet
Development

No branches or pull requests

4 participants