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/8.0] Disambiguate type names by containing assembly in WellKnownTypes #52130

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 16, 2023

Backport of #50969 to release/8.0

/cc @captainsafia

Description

ASP.NET Core's analyzer implementations use a well-known type cache to store TypeSymbols associated with key types in the ASP.NET Core space. The implementation of this cache currently uses Roslyn's GetTypeByMetadataName to resolve the type symbol from the current compilation by its name. The behavior of this API is such that it throws if it encounters two types with the same fully-qualified name in different assemblies.

This has posed a problem for certain types like IEndpointRouteBuilderExtensions which are defined in the same namespace in both ASP.NET Core and 3rd-party packages like AspNetCore.HealthChecksUI and HotChocolate.

To resolve this issue, we resolve all types with a given name in the users compilation but filter by containing assemblies under the Microsoft.*and System.* assembly names.

Closes #50836

Customer Impact

When users try to build or edit projects in Visual Studio that contain references to impacted packages (like AspNetCore.HealthChecksUI) they will encounter unhandled exceptions. Customers can workaround this by manually disabling analyzers that ship as part of the ASP.NET Core runtime and target Minimal APIs.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This change is low-risk given that (a) the affected code does not affect app's runtime behavior and (b) existing workarounds are still viable if users run into issues with this fix.

Verification

  • Manual (required)
  • Automated

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Nov 16, 2023
@ghost ghost added this to the 8.0.x milestone Nov 16, 2023
@ghost
Copy link

ghost commented Nov 16, 2023

Hi @github-actions[bot]. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@ghost
Copy link

ghost commented Nov 16, 2023

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

@captainsafia captainsafia added analyzer Indicates an issue which is related to analyzer experience area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc Servicing-consider Shiproom approval is required for the issue and removed area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework labels Nov 16, 2023
@ghost
Copy link

ghost commented Nov 16, 2023

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Nov 16, 2023
@leecow leecow modified the milestones: 8.0.x, 8.0.1 Nov 16, 2023
@wtgodbe wtgodbe merged commit fa7f7ba into release/8.0 Nov 16, 2023
25 checks passed
@wtgodbe wtgodbe deleted the backport/pr-50969-to-release/8.0 branch November 16, 2023 18:49
@ghost ghost modified the milestones: 8.0.1, 8.0.0 Nov 16, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Nov 16, 2023
@wtgodbe wtgodbe modified the milestones: 8.0.0, 8.0.1 Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer Indicates an issue which is related to analyzer experience area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc Servicing-approved Shiproom has approved the issue
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants