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

Prefix type names with global and use static TypeSyntax and NameSyntax instead of Parse(Type)Name #90339

Merged
merged 5 commits into from
Aug 11, 2023

Conversation

jtschuster
Copy link
Member

Fixes #90299

There were a few places where we would use the type names without the global:: prefix that causes issues when there is another "System" symbol in the context. To avoid this, we can use a static TypeSyntax and NameSyntax for well-known types instead of calling ParseTypeName or ParseName and remembering to prefix "global::". Though of course now we need to remember to use these instead of ParseTypeName...

@ghost
Copy link

ghost commented Aug 10, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #90299

There were a few places where we would use the type names without the global:: prefix that causes issues when there is another "System" symbol in the context. To avoid this, we can use a static TypeSyntax and NameSyntax for well-known types instead of calling ParseTypeName or ParseName and remembering to prefix "global::". Though of course now we need to remember to use these instead of ParseTypeName...

Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

@jtschuster
Copy link
Member Author

Looks like there's a few I missed, like Span and ThrowForHR.

@jtschuster
Copy link
Member Author

Still some code cleanup that can be done, but the generated code looks good and this should probably make it into 8. I'll do the follow up for 9.

Copy link
Contributor

@IDisposable IDisposable left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@jtschuster
Copy link
Member Author

Failures are known

@jtschuster jtschuster merged commit 05dfb7e into dotnet:main Aug 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LibraryImport cannot handle a property named "System"
5 participants