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

Extend member access analysis to events #907

Merged
merged 2 commits into from Sep 10, 2021

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Sep 7, 2021

Presently, only methods and properties are handled.

Caveat: only += and -= are handled - raw event accesses didn't fit nicely, since there's no method invocation, and didn't seem high-priority (since they can only occur within the declaring type).

Presently, only methods and properties are handled.

Caveat: only += and -= are handled - raw event accesses didn't fit
nicely, since there's no method invocation, and didn't seem
high-priority (since they can only occur within the declaring type).
@amcasey
Copy link
Member Author

amcasey commented Sep 7, 2021

FYI @jessetrinity

@amcasey
Copy link
Member Author

amcasey commented Sep 7, 2021

I expect the tests to fail. I'm getting a MissingMethodException (for IEventAssignmentOperation.get_EventReference) at runtime because the product code targets Roslyn 2.8.2 and the tests target 3.11.0. The only difference I see is in nullability attributes and, if that's the problem, I'm not sure why the existing code is unaffected.

@amcasey
Copy link
Member Author

amcasey commented Sep 7, 2021

This might fix #232?

@AArnott AArnott self-assigned this Sep 10, 2021
@AArnott AArnott added this to the v17.0 milestone Sep 10, 2021
@AArnott
Copy link
Member

AArnott commented Sep 10, 2021

The missing method is:

[Microsoft.CodeAnalysis]Microsoft.CodeAnalysis.Operations.IEventReferenceOperation [Microsoft.CodeAnalysis]Microsoft.CodeAnalysis.Operations.IEventAssignmentOperation::get_EventReference()

The method that actually exists is:

.get instance class Microsoft.CodeAnalysis.IOperation Microsoft.CodeAnalysis.Operations.IEventAssignmentOperation::get_EventReference()

Note the difference in return type. The runtime version returns IOperation whereas the compile-time ref assembly defined a return type of IEventReferenceOperation.

This binary breaking change was made between 2.8.2 and 2.10.0. It was introduced here and @mavasani recognized that breaking change, so it was not an accident.

I think we'll have to target 2.10 instead of 2.8 to make this work. I don't know which VS version that ties to, but it's probably still a Dev15-era version so I don't think it'll be a problem.

@AArnott AArnott merged commit 4e61fc7 into microsoft:main Sep 10, 2021
@AArnott
Copy link
Member

AArnott commented Sep 10, 2021

This might fix #232?

I don't think so. That issue is about how we interpret the invocations of the event handlers, rather than how we invoke the add and remove methods on the event itself.

@amcasey
Copy link
Member Author

amcasey commented Sep 10, 2021

@AArnott Thanks! I missed the return type.

@amcasey amcasey deleted the EventAssignment branch September 10, 2021 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants