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

IOperation - IOperation API for BoundMethodGroup and BoundPropertyGroup #19965

Closed
CyrusNajmabadi opened this issue Jun 1, 2017 · 12 comments
Closed
Labels
Area-Analyzers Bug Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature - IOperation IOperation Need Design Review The end user experience design needs to be reviewed and approved. _Product-level triaged
Milestone

Comments

@CyrusNajmabadi
Copy link
Member

Would be valuable for things like INameOfExpression: #19954

Probably also want this for IBadInvocationExpression where the member group can specify the members that the compiler considered.

@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/analyzer-ioperation

@CyrusNajmabadi CyrusNajmabadi changed the title IOperation - We probably want an operation that represents a MemberGroup IOperation - IOperation API for MemberGroups Jun 1, 2017
@jinujoseph jinujoseph added the Feature - IOperation IOperation label Jun 1, 2017
@jinujoseph jinujoseph added this to the 15.5 milestone Jun 27, 2017
@jinujoseph
Copy link
Contributor

@CyrusNajmabadi needed for V1 ?

@jinujoseph jinujoseph changed the title IOperation - IOperation API for MemberGroups IOperation - IOperation API for BoundMethodGroup and BoundPropertyGroup Jul 4, 2017
@jinujoseph
Copy link
Contributor

jinujoseph commented Jul 13, 2017

No. We don't need this for V1. However, whoever does Nameof needs to make sure it exposes the list of members that were bound.

@mavasani
Copy link
Contributor

I think it is weird to expose members within a nameof expression - in fact the BoundNameOfOperator node doesn't even include any BoundMethodGroup or BoundPropertyGroup: http://source.roslyn.io/#Microsoft.CodeAnalysis.CSharp/Generated/BoundNodes.xml.Generated.cs,985506451dde049a.

Instead a better alternative is to just expose the nameof argument as an IOperation, and in future expose these groups. Until then, the recommendation across our entire API surface should be to revert to semantic model to get bound method and property groups.

@CyrusNajmabadi
Copy link
Member Author

I think it is weird to expose members within a nameof expression

I don't think it's weird. It's what the entity inside the nameof ends up meaning semantically.

Until then, the recommendation across our entire API surface should be to revert to semantic model to get bound method and property groups.

This is undesirable because now you have to go back to syntax, and handle the syntax for each language separately just to ask a question that can be answered at our common ioperation layer.

That said, if this is hard to provide (due to how the bound nodes are structured), then i think it's ok if we punt on having the INameOfOperation actually expose any additional information. We can design and add such a thing later.

@jinujoseph
Copy link
Contributor

Design Team Decision

Lets leave this as is for V1 release.

@jinujoseph jinujoseph removed this from the 15.5 milestone Jul 29, 2017
@mavasani
Copy link
Contributor

@heejaechang Wants to discuss this in the design meeting.

@mavasani
Copy link
Contributor

We need to fix this for 15.5 so that children of bound method and property group are not cut - should be a trivial bug fix.

@tmat tmat added this to the 15.later milestone Sep 6, 2017
@jinujoseph jinujoseph added the Concept-API This issue involves adding, removing, clarification, or modification of an API. label Sep 26, 2017
@jinujoseph jinujoseph modified the milestones: 15.5, 15.later Sep 26, 2017
@jinujoseph jinujoseph modified the milestones: 15.6, Unknown Nov 2, 2017
@jinujoseph jinujoseph modified the milestones: Unknown, 16.0 Jun 23, 2018
@jinujoseph jinujoseph added _Product-level triaged Need Design Review The end user experience design needs to be reviewed and approved. labels Jun 23, 2018
mavasani added a commit to mavasani/roslyn that referenced this issue Dec 6, 2018
Detect references to method/property in bound method group/property group using semantic model APIs. Also provide a different message for methods which have name only references.
@jinujoseph jinujoseph modified the milestones: 16.0, 16.1 Jan 16, 2019
@jinujoseph jinujoseph modified the milestones: 16.1, Backlog Apr 24, 2019
@sharwell sharwell moved this to In Queue in IDE: Design review Aug 22, 2023
@CyrusNajmabadi CyrusNajmabadi moved this from In Queue to Complete in IDE: Design review Oct 26, 2024
@333fred
Copy link
Member

333fred commented Jan 7, 2025

Closing as a duplicate of a community-driven issue, #76620.

@333fred 333fred closed this as completed Jan 7, 2025
@CyrusNajmabadi
Copy link
Member Author

WFM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers Bug Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature - IOperation IOperation Need Design Review The end user experience design needs to be reviewed and approved. _Product-level triaged
Projects
Status: Complete
Development

No branches or pull requests

7 participants