-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add analyzer redirecting proposal #42437
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
e855a47
Add analyzer redirecting proposal
jjonescz 192a7c5
Improve wording
jjonescz 5306d3b
Change clash to an error
jjonescz f2f6f9f
Fix formatting
jjonescz ed6b89c
Simplify by moving the resolver to SDK
jjonescz 13535d9
Match minor version as well
jjonescz ccdc041
Update roslyn API
jjonescz ec748bb
Improve wording
jjonescz 684582f
Clarify that versions come from the paths
jjonescz f6523bf
Merge branch 'main' into tearing-redirect
jjonescz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # Redirecting analyzers in SDK to VS | ||
|
|
||
| We will redirect analyzers from the SDK to ones deployed in the VS to avoid the [torn SDK][torn-sdk] issue at design time. | ||
| Only matching major+minor versions will be redirected because different versions of the same analyzer cannot be assumed to be compatible. | ||
| So this applies to a situation like: | ||
| - Having an analyzer in SDK 9.0.1 referencing Roslyn 4.12. That gets deployed to VS 17.12. | ||
| - Having an analyzer in SDK 9.0.7 referencing Roslyn 4.13. | ||
| - User loads project with SDK 9.0.7 in VS 17.12. | ||
| Previously the analyzers would fail to load as they reference newer Roslyn version (4.13) than what is part of VS (4.12). | ||
|
jjonescz marked this conversation as resolved.
Outdated
|
||
| Now we will redirect the analyzers to those from SDK 9.0.1 that are deployed as part of VS and reference the matching Roslyn (4.12). | ||
|
|
||
| Loading older analyzer versions should not be a problem because they must reference an older version of Roslyn. | ||
|
|
||
| Targeting an SDK (and hence also loading analyzers) with newer major version in an old VS already results in an error like: | ||
|
|
||
| > error NETSDK1045: The current .NET SDK does not support targeting .NET 10.0. | ||
| > Either target .NET 9.0 or lower, or use a version of the .NET SDK that supports .NET 10.0. | ||
| > Download the .NET SDK from https://aka.ms/dotnet/download | ||
|
|
||
| ## Overview | ||
|
|
||
| - The SDK will contain a VSIX with the analyzer DLLs and an MEF-exported implementation of `IAnalyzerAssemblyRedirector`. | ||
| Implementations of this interface are imported by Roslyn and can redirect analyzer DLL loading. | ||
|
|
||
| - The SDK's implementation of `IAnalyzerAssemblyRedirector` will redirect any analyzer DLL matching some pattern | ||
| to the corresponding DLL deployed via the VSIX. | ||
| Details of this process are described below. | ||
|
|
||
| - Note that when `IAnalyzerAssemblyRedirector` is involved, Roslyn is free to not use shadow copy loading and instead load the DLLs directly. | ||
|
|
||
| ## Details | ||
|
|
||
| The VSIX contains some analyzers, for example: | ||
|
|
||
| ``` | ||
| AspNetCoreAnalyzers\9.0.0-preview.5.24306.11\analyzers\dotnet\cs\Microsoft.AspNetCore.App.Analyzers.dll | ||
| NetCoreAnalyzers\9.0.0-preview.5.24306.7\analyzers\dotnet\cs\System.Text.RegularExpressions.Generator.dll | ||
| WindowsDesktopAnalyzers\9.0.0-preview.5.24306.8\analyzers\dotnet\System.Windows.Forms.Analyzers.dll | ||
| SDKAnalyzers\9.0.100-dev\Sdks\Microsoft.NET.Sdk\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll | ||
| WebSDKAnalyzers\9.0.100-dev\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Analyzers.dll | ||
| ``` | ||
|
|
||
| Given an analyzer assembly load going through our `IAnalyzerAssemblyRedirector`, | ||
| we will redirect it if the original path of the assembly being loaded matches the path of a VSIX-deployed analyzer - | ||
| only segments of these paths starting after the version segment are compared, | ||
| plus the major and minor component of the versions must match. | ||
|
|
||
| For example, analyzer | ||
|
jjonescz marked this conversation as resolved.
Outdated
|
||
|
|
||
| ``` | ||
| C:\Program Files\dotnet\sdk\9.0.100-preview.5.24307.3\Sdks\Microsoft.NET.Sdk\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll | ||
| ``` | ||
|
|
||
| will be redirected to | ||
|
|
||
| ``` | ||
| {VSIX}\SDKAnalyzers\9.0.100-dev\Sdks\Microsoft.NET.Sdk\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll | ||
| ``` | ||
|
|
||
| because | ||
| 1. the suffix `Sdks\Microsoft.NET.Sdk\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll` matches, and | ||
| 2. the version `9.0.100-preview.5.24307.3` has the same major and minor component (`9.0`) as the version `9.0.100-dev`. | ||
|
333fred marked this conversation as resolved.
Outdated
|
||
|
|
||
| Analyzers that cannot be matched will continue to be loaded from the SDK | ||
| (and will fail to load if they reference Roslyn that is newer than the VS). | ||
|
jjonescz marked this conversation as resolved.
Outdated
|
||
|
|
||
| [torn-sdk]: https://github.com/dotnet/sdk/issues/42087 | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.