-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add analyzer redirecting API #74820
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
Add analyzer redirecting API #74820
Changes from all commits
1a317da
bebcf7d
f108d19
659d124
93b8e10
9c68c45
10f96ea
7e3faba
0f3bb48
1481640
7f7eda4
74f9a76
1261e70
61f96b6
6315300
d489573
23e0f47
fbf85ed
889d6fe
bc99564
2d1fe3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,6 +146,7 @@ | |
| <RestrictedInternalsVisibleTo Include="Microsoft.VisualStudio.IntelliCode" Partner="Pythia" Key="$(IntelliCodeKey)" /> | ||
| <RestrictedInternalsVisibleTo Include="Microsoft.VisualStudio.IntelliCode.CSharp" Partner="Pythia" Key="$(IntelliCodeCSharpKey)" /> | ||
| <RestrictedInternalsVisibleTo Include="Microsoft.VisualStudio.IntelliCode.CSharp.Extraction" Partner="Pythia" Key="$(IntelliCodeCSharpKey)" /> | ||
| <RestrictedInternalsVisibleTo Include="Microsoft.Net.Sdk.AnalyzerRedirecting" Namespace="Microsoft.CodeAnalysis.Workspaces.AnalyzerRedirecting" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to be 100% clear here, this Microsoft.Net.Sdk.AnalyzerRedirecting assembly is something that ships with VS, and is versioned with VS, and does not ship inside the .NET SDK?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is built and versioned from the .NET SDK repo, then inserted into VS and shipped with VS. |
||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <!-- TODO: Remove the below IVTs to CodeStyle Unit test projects once all analyzer/code fix tests are switched to Microsoft.CodeAnalysis.Testing --> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Workspaces.AnalyzerRedirecting; | ||
|
|
||
| /// <summary> | ||
| /// Any MEF component implementing this interface will be used to redirect analyzer assemblies. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a bit here that drives home this happens before shadow copy? Essentially this redirects the path that we pass to the compiler and the compiler will then do as it normally does with the path?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should have a comment explicitly stating what threading expectations exist here. Since it's not async that means no thread switching would be allowed. (And if it was async, it might still require the stronger guarantee that no thread switching is allowed.) Per the conversation I had with @jaredpar earlier today, I'm not entirely convinced this interface could actually be implemented safely, especially if to actually implement it you need to fetch any VS service, which would require asynchrony. |
||
| /// </summary> | ||
| /// <remarks> | ||
| /// The redirected path is passed to the compiler where it is processed in the standard way, | ||
| /// e.g., the redirected assembly is shadow copied before it's loaded | ||
| /// (this could be improved in the future since shadow copying redirected assemblies is usually unnecessary). | ||
| /// </remarks> | ||
| internal interface IAnalyzerAssemblyRedirector | ||
| { | ||
| /// <param name="fullPath"> | ||
| /// Original full path of the analyzer assembly. | ||
| /// </param> | ||
| /// <returns> | ||
| /// The redirected full path of the analyzer assembly | ||
| /// or <see langword="null"/> if this instance cannot redirect the given assembly. | ||
| /// </returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// If two redirectors return different paths for the same assembly, no redirection will be performed. | ||
| /// </para> | ||
| /// <para> | ||
| /// No thread switching inside this method is allowed. | ||
| /// </para> | ||
| /// </remarks> | ||
| string? RedirectPath(string fullPath); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this ever need to be async? Or is the assumption that any async would be to create the redirector in the first place?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haven't thought about that. Currently, no async is needed. When we need async during creation or redirecting, we can update the code, I think. We have just one implementation and don't expect to have more for now. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Collections; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
| using Microsoft.CodeAnalysis.ErrorReporting; | ||
| using Microsoft.CodeAnalysis.Host; | ||
| using Microsoft.CodeAnalysis.Internal.Log; | ||
| using Microsoft.CodeAnalysis.PooledObjects; | ||
|
|
@@ -1126,9 +1127,43 @@ private OneOrMany<string> GetMappedAnalyzerPaths(string fullPath) | |
| return GetMappedRazorSourceGenerator(fullPath); | ||
| } | ||
|
|
||
| if (TryRedirectAnalyzerAssembly(fullPath) is { } redirectedPath) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If @jaredpar's work doesn't come first, we should then refactor this method away by converting the IsSdkRazorSourceGenerator/GetMappedRazorSourceGenerator methods into a redirector.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think Jared's work is going to refactor all this and we just want to get this in so we can continue on the SDK side etc. |
||
| { | ||
| return OneOrMany.Create(redirectedPath); | ||
| } | ||
|
|
||
| return OneOrMany.Create(fullPath); | ||
| } | ||
|
|
||
| private string? TryRedirectAnalyzerAssembly(string fullPath) | ||
| { | ||
| string? redirectedPath = null; | ||
|
|
||
| foreach (var redirector in _hostInfo.AnalyzerAssemblyRedirectors) | ||
| { | ||
| try | ||
| { | ||
| if (redirector.RedirectPath(fullPath) is { } currentlyRedirectedPath) | ||
| { | ||
| if (redirectedPath == null) | ||
| { | ||
| redirectedPath = currentlyRedirectedPath; | ||
| } | ||
| else if (redirectedPath != currentlyRedirectedPath) | ||
| { | ||
| throw new InvalidOperationException($"Multiple redirectors disagree on the path to redirect '{fullPath}' to ('{redirectedPath}' vs '{currentlyRedirectedPath}')."); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.General)) | ||
| { | ||
| // Ignore if the external redirector throws. | ||
| } | ||
| } | ||
|
|
||
| return redirectedPath; | ||
| } | ||
|
|
||
| private static readonly string s_csharpCodeStyleAnalyzerSdkDirectory = CreateDirectoryPathFragment("Sdks", "Microsoft.NET.Sdk", "codestyle", "cs"); | ||
| private static readonly string s_visualBasicCodeStyleAnalyzerSdkDirectory = CreateDirectoryPathFragment("Sdks", "Microsoft.NET.Sdk", "codestyle", "vb"); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.