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

Inspection warning about procedure coercion calls #5165

Merged

Conversation

MDoerner
Copy link
Contributor

The new ObjectWhereProcedureIsRequiredInspection warns about places in which a procedure coercion takes place, i.e. places in which an object is provided in a call statement. This leads to a call to the default member, provided it is a procedure or function or the default member cannot be bound at compile time. (Properties yield a compilation error, since they are not allowed in call statements.) This type of indirection is next to never really necessary and much more likely is caused by erroneously forgetting a member call.

This PR also provides the ExpandDefaultMemberQuickFiz, which does the obvious thing. Note that it will always expand the full default member chain for recursive default member reolutions. Moreover, it is deactivated for any such chain that cannot fully be bound at compile time. This is the first step towards fixing issue #2504; just the corresponding inspection is missing.

NOTE:
Because of the infrastructure introduced in PR #5164, this PR is based on it. Please do not merge before that PR. I will rebase this one once the other PR is merged.

Note that the parameterized versions are technically not procedure coercions, but indexed default member accesses.
@MDoerner MDoerner force-pushed the ProcedureCoercionCallInspection branch from ff6966b to 6ca5b84 Compare October 1, 2019 20:39
@bclothier
Copy link
Contributor

LGTM; just one suggestion - In test cases we consider only the scenario of cls (where it should be cls.Foo. However, would it not be more common to see the misuses in cases like Bar(cls)? Having an explicit test for that case would at least guarantee that the inspection is not influenced by the different syntax.

MDoerner added a commit to MDoerner/Rubberduck that referenced this pull request Oct 2, 2019
@MDoerner MDoerner force-pushed the ProcedureCoercionCallInspection branch from 0e15f77 to ba80ad9 Compare October 3, 2019 01:28
/// Identifies places in which an object is used but a procedure is required and a default member exists on the object.
/// </summary>
/// <why>
/// Providing an object in a place in which a procedure is required leads to a call to the objects default member.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Providing an object in a place in which a procedure is required leads to a call to the objects default member.
/// Providing an object where a procedure is required leads to an implicit call to the object's default member.

/// </summary>
/// <why>
/// Providing an object in a place in which a procedure is required leads to a call to the objects default member.
/// This behavior is not obvious and most likely an error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// This behavior is not obvious and most likely an error.
/// This behavior is not obvious, and most likely unintended.

@bclothier bclothier merged commit e8f8d1a into rubberduck-vba:next Oct 3, 2019
@MDoerner MDoerner deleted the ProcedureCoercionCallInspection branch February 19, 2020 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants