Analyzer redirector #11972

chsienki · 2025-06-23T23:07:58Z

This is the Razor half of dotnet/roslyn#78852

It provides a redirector that will redirect the razor generator and its dependencies.

chsienki · 2025-06-23T23:08:16Z

@dotnet/razor-compiler for reviews please.

chsienki · 2025-06-23T23:08:27Z

davidwengier

LGTM in general, very happy to see this progressing. Just not sure how strict we need to be about the version of Roslyn we use.

davidwengier · 2025-06-23T23:21:13Z

eng/Version.Details.xml

 <Dependencies>
  <Source Uri="https://github.com/dotnet/dotnet" Mapping="razor" Sha="f5705c8f4c5079bba77bae8698ba1583bde0388c" BarId="269610" />
  <ProductDependencies>
-    <Dependency Name="Microsoft.Net.Compilers.Toolset" Version="5.0.0-1.25316.1">
+    <Dependency Name="Microsoft.Net.Compilers.Toolset" Version="5.0.0-1.25321.1">


It doesn't look like this version has been inserted into VS main yet, so merging this PR could possibly break people, or our integration tests. It looks like if we downgrade this to 5.0.0-1.25319.11 then we get a version that has the Razor redirector and is in VS main.

The alternative is to wait a few days, or roll the dice that this will be fine. Or I guess we could kick off an integration test run for this branch?

Scratch that, looks like 5.0.0-1.25323.2 has been inserted now, so this version should be fine.

jjonescz · 2025-06-24T07:48:54Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

+[Export(typeof(IRazorAnalyzerAssemblyRedirector))]
+[method: ImportingConstructor]
+#pragma warning restore RS0030 // Do not use banned APIs
+internal class RazorAnalyzerAssemblyRedirector() : IRazorAnalyzerAssemblyRedirector


Suggested change

internal class RazorAnalyzerAssemblyRedirector() : IRazorAnalyzerAssemblyRedirector

internal sealed class RazorAnalyzerAssemblyRedirector() : IRazorAnalyzerAssemblyRedirector

jjonescz · 2025-06-24T07:49:24Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

+
+namespace Microsoft.VisualStudio.Razor;
+
+#pragma warning disable RS0030 // Do not use banned APIs


What's the banned API? And why is it banned?

The export attributes and its friends are MEF v2, which is banned in the LanguageServices.Razor project, which uses this shared library. I'm not sure I can remember why, but whats a little cargo culting between friends.

I'm not sure I can remember why, but whats a little cargo culting between friends.

MS.VS.LanguageServers.Razor enforces MEFv1 because Visual Studio's MEF attributes are MEFv1. VS MEF supports importing both MEFv1 and MEFv2 attributes, but things can get in certain scenarios if their attributes are mixed on the same types.

but whats a little cargo culting between friends

This precisely. It wasn't getting imported into VSCode when using one version, so I looked at the other exported parts and they had the other version and the suppression, so I copied it and it worked.

Should we consider leaving a comment about this for future readers?

It's replicated across most of the files in this shared project.

eng/Versions.props

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

DustinCampbell · 2025-06-24T15:54:25Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

+
+namespace Microsoft.VisualStudio.Razor;
+
+#pragma warning disable RS0030 // Do not use banned APIs


I'm not sure I can remember why, but whats a little cargo culting between friends.

MS.VS.LanguageServers.Razor enforces MEFv1 because Visual Studio's MEF attributes are MEFv1. VS MEF supports importing both MEFv1 and MEFv2 attributes, but things can get in certain scenarios if their attributes are mixed on the same types.

DustinCampbell

oops. Accidentally blocked the PR. 😄

davidwengier · 2025-06-25T00:34:28Z

FYI, I merged the cleanup PR so the file header on your new file will be incorrect in CI. You probably want to merge in main :)

chsienki · 2025-06-26T18:42:33Z

This is waiting on dotnet/roslyn#79154 to prevent the extra assembly load in non-razor cases.

333fred · 2025-06-26T20:36:47Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

+
+namespace Microsoft.VisualStudio.Razor;
+
+#pragma warning disable RS0030 // Do not use banned APIs


Should we consider leaving a comment about this for future readers?

333fred · 2025-06-26T20:38:21Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/RazorAnalyzerAssemblyRedirector.cs

+{
+    private static readonly ImmutableArray<Type> s_compilerAssemblyTypes = [
+
+        typeof(CodeAnalysis.Razor.CompilerFeatures), // Microsoft.CodeAnalysis.Razor.Compiler.dll


Can we assert these comments somehow? Or at least assert that they are different assemblies? Maybe there's some reflection-based asserts we can do to ensure that there's no unexpected assemblies being referenced that would need to be redirected in the face of a future refactoring?

I ended up adding a unit test that 'pins' the expected assemblies, so if they change or get moved etc it will fail and we can't accidentally break things. I added a comment explaining the purpose.

This reverts commit 7d35fca.

Mostly fixes #11942 We still currently get "Couldn't get code document" errors when the source generator is hooked up, but didn't run, but I'm leaving that because it should be fixed by #11972, and if it isn't then we either want to know, or can add handling of it in then. Commit-at-a-time is probably easiest, mostly because the 2nd commit changes every cohost endpoint to inherit from a new base class, so it's pretty noisy.

chsienki · 2025-07-02T16:53:16Z

Merging as internal insertion now shows clean metrics: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/648244

chsienki requested review from a team as code owners June 23, 2025 23:07

chsienki added the New Feature Cohosting label Jun 23, 2025

davidwengier approved these changes Jun 23, 2025

View reviewed changes

jjonescz approved these changes Jun 24, 2025

View reviewed changes

DustinCampbell requested changes Jun 24, 2025

View reviewed changes

DustinCampbell reviewed Jun 24, 2025

View reviewed changes

DustinCampbell self-requested a review June 24, 2025 15:55

DustinCampbell approved these changes Jun 25, 2025

View reviewed changes

davidwengier mentioned this pull request Jun 26, 2025

Gracefully degrade if the cohost editor isn't going to work #11981

Merged

333fred reviewed Jun 26, 2025

View reviewed changes

chsienki added 5 commits June 27, 2025 12:50

Bump roslyn version

88db71c

Add razor redirector implementation

d70b75f

PR Feedback

e0686ee

Fix build error?

7478f91

Revert "Fix build error?"

a606e79

This reverts commit 7d35fca.

chsienki force-pushed the analyzer_redirector branch from 2378993 to a606e79 Compare June 27, 2025 19:51

chsienki added 2 commits June 27, 2025 12:55

Fix file header after rebase

b5a1ec0

Add pinning test

ddab46c

chsienki merged commit 3e772db into dotnet:main Jul 2, 2025
11 checks passed

dotnet-policy-service bot added this to the Next milestone Jul 2, 2025

dotnet-bot mentioned this pull request Jul 4, 2025

[Automated] PRs inserted in VS build main-10803.82 #12014

Closed

davidwengier mentioned this pull request Jul 14, 2025

Bump Razor to 10.0.0-preview.25360.2 dotnet/vscode-csharp#8413

Merged

RikkiGibson removed this from the Next milestone Aug 20, 2025

RikkiGibson added this to the 18.0 P1 milestone Aug 20, 2025

	internal class RazorAnalyzerAssemblyRedirector() : IRazorAnalyzerAssemblyRedirector
	internal sealed class RazorAnalyzerAssemblyRedirector() : IRazorAnalyzerAssemblyRedirector


		namespace Microsoft.VisualStudio.Razor;

		#pragma warning disable RS0030 // Do not use banned APIs

Analyzer redirector #11972

Analyzer redirector #11972

Uh oh!

Conversation

chsienki commented Jun 23, 2025

Uh oh!

chsienki commented Jun 23, 2025

Uh oh!

chsienki commented Jun 23, 2025

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DustinCampbell left a comment

Choose a reason for hiding this comment

Uh oh!

davidwengier commented Jun 25, 2025

Uh oh!

chsienki commented Jun 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chsienki commented Jul 2, 2025

Uh oh!

Uh oh!

Uh oh!