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

Razor synchronize loader #76357

Merged
merged 9 commits into from
Dec 12, 2024
51 changes: 29 additions & 22 deletions src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,56 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Composition;
using System.Diagnostics;
using System.Reflection;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.VisualStudio.Composition;

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor
{
[Export(typeof(IAnalyzerAssemblyResolver)), Shared]
internal class RazorAnalyzerAssemblyResolver : IAnalyzerAssemblyResolver
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal class RazorAnalyzerAssemblyResolver() : IAnalyzerAssemblyResolver
{
private static Func<AssemblyName, Assembly?>? s_assemblyResolver;

/// <summary>
/// We use this as a heuristic to catch a case where we set the resolver too
/// late and the resolver has already been asked to resolve a razor assembly.
///
/// Note this isn't perfectly accurate but is only used to trigger an assert
/// in debug builds.
/// </summary>
private static bool s_razorRequested = false;
private static readonly List<AssemblyName> s_assembliesRequested = [];
Copy link
Member

Choose a reason for hiding this comment

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

Since all of the code here just adds items to this collection and checks to see if items are present, should this be a HashSet<AssemblyName> rather than a List<AssemblyName>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, thanks.


[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public RazorAnalyzerAssemblyResolver()
{
}
private static readonly object s_resolverLock = new();

internal static Func<AssemblyName, Assembly?>? AssemblyResolver
/// <summary>
/// Attempts to set the assembly resolver. Will only succeed if the <paramref name="canaryAssembly"/> has not already been requested to load.
chsienki marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="resolver">The resolver function to set.</param>
/// <param name="canaryAssembly">The name of an assembly that is checked to see if it has been requested to load. Setting the resolver will fail if this has already been requested.</param>
/// <returns><c>true</c> when the resolver was successfully set.</returns>
internal static bool TrySetAssemblyResolver(Func<AssemblyName, Assembly?> resolver, AssemblyName canaryAssembly)
{
get => s_assemblyResolver;
set
lock (s_resolverLock)
{
Debug.Assert(s_assemblyResolver == null, "Assembly resolver should not be set multiple times.");
Debug.Assert(!s_razorRequested, "A razor assembly has already been requested before setting the resolver.");
s_assemblyResolver = value;
if (s_assemblyResolver is null && !s_assembliesRequested.Contains(canaryAssembly))
{
s_assemblyResolver = resolver;
return true;
}
return false;
}
}

public Assembly? ResolveAssembly(AssemblyName assemblyName)
{
s_razorRequested |= assemblyName.FullName.Contains("Razor");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any potential problems with razor being out of sync with this EA? i.e. razor is older than roslyn, and razor expects the old assembly hook pattern to be used, or roslyn is older than razor, and razor expects the new hook pattern.

Maybe we just have to take care to dual insert and we'll be ok?

return s_assemblyResolver?.Invoke(assemblyName);
Func<AssemblyName, Assembly?>? resolver = null;
lock (s_resolverLock)
{
s_assembliesRequested.Add(assemblyName);
resolver = s_assemblyResolver;
}

return resolver?.Invoke(assemblyName);
}
}
}
9 changes: 9 additions & 0 deletions src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,14 @@ internal static class RazorProjectExtensions
{
return project.GetSourceGeneratorRunResultAsync(cancellationToken);
}

internal static async ValueTask<IEnumerable<SourceGeneratedDocument>> GetSourceGeneratedDocumentsForGeneratorAsync(this Project project, string assemblyName, string assemblyPath, Version assemblyVersion, string typeName, CancellationToken cancellationToken)
{
var results = await project.GetSourceGeneratedDocumentsAsync(cancellationToken).ConfigureAwait(false);

var generatorIdentity = new SourceGeneratorIdentity(assemblyName, assemblyPath, assemblyVersion, typeName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SourceGeneratorIdentity isn't public, so have to pass through the requisite parts. We could make an IVT copy but doesn't seem worth it.

return results.Where(s => s.Identity.Generator == generatorIdentity);

Copy link
Member

Choose a reason for hiding this comment

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

nit: extra blank line

}
}
}
Loading