Skip to content

Make Razor redirect code more robust #78039

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Make Razor redirect code more robust #78039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39f467d
Make Razor redirect code more robust
jaredpar Apr 8, 2025
3cd85a3
more
jaredpar Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 187 additions & 53 deletions ...sualStudio/Core/Test/ProjectSystemShim/VisualStudioProjectTests/AnalyzerReferenceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
Imports System.ComponentModel.Composition
Imports System.IO
Imports System.Threading
Imports Microsoft.Build.Construction
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.Host.Mef
Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Microsoft.CodeAnalysis.Workspaces.AnalyzerRedirecting
Imports Microsoft.CodeAnalysis.Workspaces.ProjectSystem
Imports Microsoft.VisualStudio.LanguageServices.Implementation.Diagnostics
Imports Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics
Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework
Expand All @@ -17,6 +20,26 @@ Imports Roslyn.Test.Utilities
Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
<[UseExportProvider]>
Public Class AnalyzerReferenceTests

Private Shared Function CreateAnalyzerDir(ParamArray fileNames As String()) As String
Dim dir = Path.Combine(TempRoot.Root, Guid.NewGuid().ToString())
Directory.CreateDirectory(dir)
For Each fileName In fileNames
Dim filePath = Path.Combine(dir, fileName)
File.WriteAllBytes(filePath, {})
Next
Return dir
End Function

Private Shared Function GetAnalyzerFilePaths(analyzers As IEnumerable(Of AnalyzerReference)) As List(Of String)
Dim list = New List(Of String)
For Each a In analyzers
Dim filePath = DirectCast(a, AnalyzerFileReference).FullPath
list.Add(filePath)
Next
Return list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the OrderBy that's used by all callers here

End Function

<WpfFact>
Public Async Function RemoveAndReAddInSameBatchWorksCorrectly() As Task
Using environment = New TestEnvironment(GetType(TestDynamicFileInfoProviderThatProducesNoFiles))
Expand Down Expand Up @@ -99,84 +122,195 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
End Function

<WpfFact>
Public Async Function RazorSourceGenerator_FromVsix() As Task
Public Async Function RazorSourceGenerator_FromVsix_VsixHasMoreDlls() As Task
Using environment = New TestEnvironment()

Dim vsixDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't Util.dll be listed here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should really get refactored so the test setup is one helper for all tests to avoid this oops.


Dim providerFactory = DirectCast(environment.ExportProvider.GetExportedValue(Of IVisualStudioDiagnosticAnalyzerProviderFactory), MockVisualStudioDiagnosticAnalyzerProviderFactory)
providerFactory.Extensions =
{
({
Path.Combine(TempRoot.Root, "RazorVsix", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency1.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency2.dll")
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Analyzer.dll"),
Path.Combine(vsixDir, "Util.dll")
},
"Microsoft.VisualStudio.RazorExtension"),
({
Path.Combine(TempRoot.Root, "File.dll")
},
"AnotherExtension")
ProjectSystemProject.RazorVsixExtensionId)
}

Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync(
"Project", LanguageNames.CSharp, CancellationToken.None)

' adding just Razor dependency and not the main source generator is a no-op
project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "SdkDependency1.dll"))
Dim getAnalyzerRefs =
Function() As IReadOnlyList(Of AnalyzerReference)
Return environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences
End Function
Comment on lines +144 to +147
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just inline this logic into GetAnalyzerFilePaths since it's being repeated everywhere.


Assert.Empty(environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences)
' Deliberately left Util.dll as we want to verify the behavior when the sdk and vsix disagree on total
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
' Deliberately left Util.dll as we want to verify the behavior when the sdk and vsix disagree on total
' Deliberately left out Util.dll as we want to verify the behavior when the sdk and vsix disagree on total

' set of dependencies
Dim sdkDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll")

' removing just Razor dependency and not the main source generator is a no-op
project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "SdkDependency1.dll"))
project.AddAnalyzerReference(Path.Combine(sdkDir, "Analyzer.dll"))
Assert.Empty(getAnalyzerRefs())

Assert.Empty(environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences)
project.AddAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Equal(3, getAnalyzerRefs().Count)
Assert.Equal(
{
Path.Combine(vsixDir, "Analyzer.dll"),
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Util.dll")
}, GetAnalyzerFilePaths(getAnalyzerRefs()).OrderBy(Function(x) Path.GetFileName(x), StringComparer.Ordinal))

' add Razor source generator and a couple more other analyzer files:
project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "SdkDependency1.dll"))
project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"))
project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Some other directory", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"))
project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Dir", "File.dll"))
project.RemoveAnalyzerReference(Path.Combine(sdkDir, "Analyzer.dll"))
Assert.Equal(3, getAnalyzerRefs().Count)

AssertEx.Equal(
{
Path.Combine(TempRoot.Root, "RazorVsix", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency1.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency2.dll"),
Path.Combine(TempRoot.Root, "Some other directory", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"),
Path.Combine(TempRoot.Root, "Dir", "File.dll")
}, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath))
project.RemoveAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Empty(getAnalyzerRefs())
End Using
End Function

' add Razor source generator again:
Assert.Throws(Of ArgumentException)(
"fullPath",
Sub() project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll")))
<WpfFact>
Public Async Function RazorSourceGenerator_FromVsix_SdkHasMoreDlls() As Task
Using environment = New TestEnvironment()

AssertEx.Equal(
Dim vsixDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Util.dll to match the vsix setup below?

Applies to the tests below as well.


Dim providerFactory = DirectCast(environment.ExportProvider.GetExportedValue(Of IVisualStudioDiagnosticAnalyzerProviderFactory), MockVisualStudioDiagnosticAnalyzerProviderFactory)
providerFactory.Extensions =
{
Path.Combine(TempRoot.Root, "RazorVsix", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency1.dll"),
Path.Combine(TempRoot.Root, "RazorVsix", "VsixDependency2.dll"),
Path.Combine(TempRoot.Root, "Some other directory", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"),
Path.Combine(TempRoot.Root, "Dir", "File.dll")
}, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath))
({
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Util.dll")
},
ProjectSystemProject.RazorVsixExtensionId)
}
Comment on lines +179 to +187
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting this to a helper.


' remove:
project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "SdkDependency1.dll"))
project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"))
project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Some other directory", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll"))
Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync(
"Project", LanguageNames.CSharp, CancellationToken.None)

AssertEx.Equal(
Dim getAnalyzerRefs =
Function() As IReadOnlyList(Of AnalyzerReference)
Return environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences
End Function

Dim sdkDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll", "Util.dll")

project.AddAnalyzerReference(Path.Combine(sdkDir, "Analyzer.dll"))
Assert.Empty(getAnalyzerRefs())

project.AddAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))
Assert.Empty(getAnalyzerRefs())

project.AddAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Equal(2, getAnalyzerRefs().Count)
Assert.Equal(
{
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Util.dll")
}, GetAnalyzerFilePaths(getAnalyzerRefs()).OrderBy(Function(x) Path.GetFileName(x), StringComparer.Ordinal))
Comment on lines +206 to +211
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding just a "assert the file names are" helper; then you could just use that one helper everywhere, and also replace all the count assertions with that which makes it a bit clearer you expect things didn't change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm also not a fan of the count assertion before the set assertion: if it's got not-2, it'll fail, but the failure assertion won't make it obvious what happened.)


project.RemoveAnalyzerReference(Path.Combine(sdkDir, "Analyzer.dll"))
Assert.Equal(2, getAnalyzerRefs().Count)

project.RemoveAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))
Assert.Equal(2, getAnalyzerRefs().Count)

project.RemoveAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Empty(getAnalyzerRefs())
End Using
End Function

<WpfFact>
Public Async Function RazorSourceGenerator_FromVsix_OtherGenerators() As Task
Using environment = New TestEnvironment()

Dim vsixDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll")

Dim providerFactory = DirectCast(environment.ExportProvider.GetExportedValue(Of IVisualStudioDiagnosticAnalyzerProviderFactory), MockVisualStudioDiagnosticAnalyzerProviderFactory)
providerFactory.Extensions =
{
Path.Combine(TempRoot.Root, "Dir", "File.dll")
}, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath))
({
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Util.dll")
},
ProjectSystemProject.RazorVsixExtensionId)
}

' remove again:
Assert.Throws(Of ArgumentException)(
"fullPath",
Sub() project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk.Razor", "source-generators", "Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll")))
Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync(
"Project", LanguageNames.CSharp, CancellationToken.None)

AssertEx.Equal(
Dim getAnalyzerRefs =
Function() As IReadOnlyList(Of AnalyzerReference)
Return environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences
End Function

Dim sdkDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Util.dll")
Dim otherDir = CreateAnalyzerDir("SpellChecker.dll")

project.AddAnalyzerReference(Path.Combine(otherDir, "SpellChecker.dll"))
Assert.Equal(1, getAnalyzerRefs().Count)
project.AddAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Equal(3, getAnalyzerRefs().Count)
project.AddAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))
Assert.Equal(3, getAnalyzerRefs().Count)

Assert.Equal(
{
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(otherDir, "SpellChecker.dll"),
Path.Combine(vsixDir, "Util.dll")
}, GetAnalyzerFilePaths(getAnalyzerRefs()).OrderBy(Function(x) Path.GetFileName(x), StringComparer.Ordinal))

project.RemoveAnalyzerReference(Path.Combine(otherDir, "SpellChecker.dll"))
Assert.Equal(2, getAnalyzerRefs().Count)
project.RemoveAnalyzerReference(Path.Combine(vsixDir, "Util.dll"))
Assert.Equal(2, getAnalyzerRefs().Count)
project.RemoveAnalyzerReference(Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Empty(getAnalyzerRefs())
End Using
End Function

<WpfFact>
Public Async Function RazorSourceGenerator_FromVsix_ErrorCases() As Task
Using environment = New TestEnvironment()

Dim vsixDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Analyzer.dll")

Dim providerFactory = DirectCast(environment.ExportProvider.GetExportedValue(Of IVisualStudioDiagnosticAnalyzerProviderFactory), MockVisualStudioDiagnosticAnalyzerProviderFactory)
providerFactory.Extensions =
{
Path.Combine(TempRoot.Root, "Dir", "File.dll")
}, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath))
({
Path.Combine(vsixDir, ProjectSystemProject.RazorSourceGenaratorDllName),
Path.Combine(vsixDir, "Util.dll")
},
ProjectSystemProject.RazorVsixExtensionId)
}

Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync(
"Project", LanguageNames.CSharp, CancellationToken.None)

Dim getAnalyzerRefs =
Function() As IReadOnlyList(Of AnalyzerReference)
Return environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences
End Function

Dim sdkDir = CreateAnalyzerDir(ProjectSystemProject.RazorSourceGenaratorDllName, "Util.dll")

project.AddAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
project.AddAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))

project.RemoveAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))

' This does not throw because within the razor generator only the actual generator dll triggers real
' add / removes. At a glance this may seem problematic but it's an unavoidable outcome once the
' project system begins mapping / mangling analyzer paths. There will be cases where existing names
' do not map to analyzers and it ends up with this behavior.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be fixed by putting a bit more logic into ProjectSystemProject where by we keep track of the original paths that were mapped to empty. Then we could have the Add / Remove calls manipulate that table and get back throwing behavior for double remove. Ended up deciding against that because it's a half measure. Yes we get back the Add / Remove semantics but it doesn't fix the fact that AnalyzerReferences in the project don't change.

I could be convinced to go the other direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this, since at some point I hope this gets properly refactored. 😄

project.RemoveAnalyzerReference(Path.Combine(sdkDir, "Util.dll"))

project.RemoveAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName))
Assert.Throws(Of ArgumentException)(
Sub() project.RemoveAnalyzerReference(Path.Combine(sdkDir, ProjectSystemProject.RazorSourceGenaratorDllName)))
End Using
End Function

Expand Down
Loading
Loading