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 for implicit default member usage on RHS #2504

Closed
ThunderFrame opened this issue Jan 10, 2017 · 3 comments · Fixed by #5166
Closed

Inspection for implicit default member usage on RHS #2504

ThunderFrame opened this issue Jan 10, 2017 · 3 comments · Fixed by #5166
Assignees
Labels
enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. feature-inspections

Comments

@ThunderFrame
Copy link
Member

Following two Excel-specific examples where implicit calls to Range.[_Default] result in unexpected outcomes (where some values of unexpected involve the host application exploding into a thousand tiny pieces), it might be time to add an inspection for implicit default members on the RHS.

In both instances above, the problems are alleviated by explicitly calling the Range.[_Default] member (or its equivalent Range.Value member).

But there are lots of objects that have default members, and not all of the default members are necessarily visible or descriptive by default.

  • Excel.Application.[_Default] / Excel.Application.Name
  • Range.[_Default] / Excel.Range.Value
  • Excel.Shapes.[_Default] / Excel.Shapes.Item
  • ADODB.Connection.ConnectionString
  • ADODB.Recordset.Fields
  • ADODB.Field.Value

I'm unsure how best to implement this inspection. There are a number of approaches that could be taken:

  1. Find/Fix all implicit member usage. This might be overkill, as the number of resulting explicit usages is likely to be quite high

  2. Find/Fix all implicit, Variant-returning members. The RHS problems all seem to be related to members returning Variants. Maybe this inspection should be specific to implicit default members that return Variants?

  3. Find/Fix a user-configurable list of implicit members. Allow the use to define Excel.Range as the implicit member, and define Excel.Range.Value as the preferred/overridden explicit member. I'd certainly rather see myRange.Value in my code than myRange.[_Default], and Sheets.Item("foo") over Sheets.[_Default]("foo")

Maybe the RD settings could allow for any one of the above to be configured?

@ThunderFrame ThunderFrame changed the title Inspection for implicit default members usage on RHS Inspection for implicit default member usage on RHS Jan 10, 2017
@retailcoder
Copy link
Member

I like the configurable approach to quick-fixing; the inspection could probably be implemented anyway, as a notification that you have an implicit reference to default member - the inspection itself doesn't need any special handling though.. but it is a tricky one, that needs to dig into RHS expression trees.

@retailcoder retailcoder added the code-path-analysis Involves simulating execution paths / interpreting the user code ..to an extent. label Jan 10, 2017
@comintern
Copy link
Contributor

Rough sketch of how expression evaluation could work with the current architecture:

Collect declarations like we do now.

  • Create a lookup of <Context, Declaration>.
  • Run an "expression" pass that walks the expressions.
  • This would check all the types of the declarations in the expression.
  • Throw those into a new Expression object with it's own AsTypeName.
  • Use that for anything that is sensitive to LHS\RHS.
  • If either side is a reference type, also determine the AsTypeName of the default member if it has one (this would need to be recursive).

@Vogel612 Vogel612 added enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. and removed feature-request labels Aug 11, 2017
@retailcoder
Copy link
Member

The plot thickens - a lookup table would need to account for parameterization. Specifically:

Range.[_Default] / Excel.Range.Value

Is not necessarily true - it's true when there are no arguments provided, but Range.[_Default] appears to "redirect" the call to Excel.Range.Item given one or more arguments.

That said, we're now warning about implicit default member calls on the LHS of an assignment - we probably have what it takes to do the same about RHS too, and nevermind the lookup table: a "make default member call explicit" quickfix that just adds an explicit call to [_Default] is fine (user can tweak it to what they actually mean to call, and inspection info can mention that).

@MDoerner MDoerner self-assigned this Sep 28, 2019
@retailcoder retailcoder removed the code-path-analysis Involves simulating execution paths / interpreting the user code ..to an extent. label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. feature-inspections
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants