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

[release/6.0] Src gen: use GetBestTypeByMetadataName instead of GetTypeByMetadataName #59774

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

steveharter
Copy link
Member

Backport of #59092 to release/6.0

Fixes #57349

See also the related backport for System.Text.Json at #58766.

Customer Impact

If there is more than one type with the same name (namespace and class name) but in different assemblies, the conflict may cause source-generation to fail for the logging source generator. If the duplicate types are not be owned by the customer, there is no work-around without the changes in this PR.

Testing

A new test was added; System.Text.Json also has a test that uses the same GetBestTypeByMetadataName() extension method.

Risk

Low; this is a simple 1:1 method replacement which has been previously tested in System.Text.Json (which has a unit test) and by Roslyn. The guidance and extension method is provided by the Roslyn team.

@steveharter steveharter added Servicing-consider Issue for next servicing release review area-Extensions-Logging labels Sep 29, 2021
@steveharter steveharter added this to the 6.0.0 milestone Sep 29, 2021
@steveharter steveharter self-assigned this Sep 29, 2021
@ghost
Copy link

ghost commented Sep 29, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #59092 to release/6.0

Fixes #57349

See also the related backport for System.Text.Json at #58766.

Customer Impact

If there is more than one type with the same name (namespace and class name) but in different assemblies, the conflict may cause source-generation to fail for the logging source generator. If the duplicate types are not be owned by the customer, there is no work-around without the changes in this PR.

Testing

A new test was added; System.Text.Json also has a test that uses the same GetBestTypeByMetadataName() extension method.

Risk

Low; this is a simple 1:1 method replacement which has been previously tested in System.Text.Json (which has a unit test) and by Roslyn. The guidance and extension method is provided by the Roslyn team.

Author: steveharter
Assignees: steveharter
Labels:

Servicing-consider, area-Extensions-Logging

Milestone: 6.0.0

@danmoseley
Copy link
Member

OK - this is extending the proven fix from Json. Please send mail if you haven't already.

@steveharter steveharter added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 29, 2021
@steveharter
Copy link
Member Author

Servicing approved via mail.

@danmoseley danmoseley merged commit 9f66ef7 into dotnet:release/6.0 Sep 30, 2021
@steveharter steveharter deleted the Port04b6c41 branch September 30, 2021 16:28
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Logging Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants