Rework analyzer assembly loading #77004

jaredpar · 2025-02-03T15:07:04Z

This change restructures our analyzer assembly loading to allow for composable customizations to meet the demands of customers like VBCSCompiler, VS IDE, Razor and VS Code. Specifically, these layers want to customize two aspects of assembly loading:

The location where an assembly is loaded from. For example shadow copying does not change the content of what is loaded but it does change where it is loaded from
The Assembly loaded for a given path / name. For example Razor wants to control exactly which Assembly from which AssemblyLoadContext is loaded when running inside the VS OOP process.

The existing customization approach was to derive from AnalyzerAssemblyLoader, override the right virtual and customize the behavior. That had limits, particularly because different parts of the VS IDE want to override these behaviors in different ways. That forced us to centralize logic that we didn't want to centralize (baking Razor into workspaces) and was challenging to integrate with our MEF composition.

The new approach is to have a sealed type that allows customization through interfaces.

This PR only establishes the new architecture and leaves the IDE unchanged. I originally attempted to do both at one time and the PR was hard to follow, the perf implications were challenging to track down and verify. Instead going to approach this by getting the basics in place then start migrating parts of the IDE over one at a time.

jaredpar · 2025-02-03T15:11:00Z

src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs

-/// in-proc and OOP e.g. in-proc (VS) running on desktop clr and OOP running on ServiceHub .Net6
-/// host. We need to make sure to use the ones from the same location as the remote.
-/// </summary>
-internal sealed class RemoteAnalyzerAssemblyLoader : AnalyzerAssemblyLoader


Became RemoteAnalyzerPathResolver ... at the same time though, it's very unclear where this was actually used. There are unit tests for this but not any use in production code that I could see.

jaredpar · 2025-02-03T15:30:37Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs

@@ -11,60 +11,67 @@
 using Roslyn.Utilities;


I will rename this file to match type once the change is signed off on. Trying to keep the diff here.

jaredpar · 2025-02-05T00:59:21Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

@@ -5,6 +5,7 @@
 using System;


This file changed substantially as it moved from using inheritance to control behaviors to a composable plugin system. It may be easiest to just review the new contents vs. the diff.

jaredpar · 2025-02-18T17:22:49Z

src/Tools/SemanticSearch/ReferenceAssemblies/Apis/Microsoft.CodeAnalysis.CSharp.txt

@@ -1177,6 +1177,7 @@ Microsoft.CodeAnalysis.CSharp.ForEachStatementInfo.get_GetEnumeratorMethod
 Microsoft.CodeAnalysis.CSharp.ForEachStatementInfo.get_IsAsynchronous
 Microsoft.CodeAnalysis.CSharp.ForEachStatementInfo.get_MoveNextMethod
 Microsoft.CodeAnalysis.CSharp.InterceptableLocation
+Microsoft.CodeAnalysis.CSharp.InterceptableLocation.Equals(Microsoft.CodeAnalysis.CSharp.InterceptableLocation)


Not sure why this is just getting picked up with my change here, think the full rebuild locally added this from the PR which added the operator. Keeping unless someone says it should be removed.

jaredpar · 2025-02-18T17:23:45Z

src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs

-internal sealed class DefaultAnalyzerAssemblyLoaderProvider(
-    [ImportMany] IEnumerable<IAnalyzerAssemblyResolver> externalResolvers)
-    : AbstractAnalyzerAssemblyLoaderProvider(externalResolvers);
+internal sealed class DefaultAnalyzerAssemblyLoaderProvider : AbstractAnalyzerAssemblyLoaderProvider


Moving IAnalyzerAssemblyResolver to be .NET Core only was correct but I hate the bifuraction it caused in the MEF layer here. I did give consideration for adding a new factory interface that had a #if NET method for producing this value. That would simplify the MEF import here but eventually decided to go this route. Happy to change if you all feel differently.

@jasonmalinowski

I'll think about this more next week if there's some nicer alternative, but it's constrained enough here to not be too much of a mess. No concerns with merging at this point.

jaredpar · 2025-02-18T17:25:48Z

src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerPathResolver.cs

+/// in-proc and OOP e.g. in-proc (VS) running on desktop clr and OOP running on ServiceHub .Net6
+/// host. We need to make sure to use the ones from the same location as the remote.
+/// </summary>
+internal sealed class RemoteAnalyzerPathResolver(string baseDirectory) : IAnalyzerPathResolver


This was the RemoteAnalyzerAssemblyLoader type . Given that AnalyzerAssemblyLoader is sealed now it couldn't be a derivation anymore. The natural change was for it to be IAnalyzerPathResolver. At the same time I can find no where in our code base where this is used, except unit tests, so I'm not 100% sure what to do here.

@jasonmalinowski

Agreed this appears to be dead code. Nothing is still consuming this in this new PR, right? Then I'd say just delete it -- if something is depending on this...we already broken them whether this stays in the new form or not.

jaredpar · 2025-02-18T17:27:15Z

@jjonescz, @333fred, @chsienki, @RikkiGibson, @jasonmalinowski PTAL

CyrusNajmabadi · 2025-02-18T17:36:49Z

This PR might benefit from an OP stating the overall idea being implemented. Tnx :)

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs

Co-authored-by: Fred Silberberg <[email protected]>

jaredpar · 2025-02-24T23:07:14Z

@333fred, @CyrusNajmabadi, @jasonmalinowski PR feedback addressed.

jasonmalinowski

I'm getting as far as I can get today, so thoughts so far:

The design patterns and introducing the various interfaces to split apart all the behaviors is very well done; maybe there's a few places where I wish we had different names, but that's really my only quibble.
I got through most stuff except the actual AnalyzerAssemblyLoader.cs main file, which is unfortunately the most interesting bit. So boo. 😢
Your questions about the RemoteAnalyzer* stuff which you think might be test only isn't something I'm familiar with. I can dig later if you need me to.

src/Compilers/Core/Portable/DiagnosticAnalyzer/IAnalyzerPathResolver.cs

src/Compilers/Core/Portable/DiagnosticAnalyzer/ProgramFilesAnalyzerPathResolver.cs

jasonmalinowski · 2025-02-26T00:29:33Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/ProgramFilesAnalyzerPathResolver.cs

+    private ProgramFilesAnalyzerPathResolver()
+    {
+        var programFilesPath = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
+        DotNetPath = Path.Combine(programFilesPath, "dotnet");


We could also do a similar optimization for any paths under the Visual Studio install path, right? That can of course live in the VS layer but I'm not seeing anything over in that folder if you already did something there.

I originally had Microsoft Visual Studio as one of the paths but there was some push back like "what if we installed off of C:" which caused me to pull that. If you all want me to put it back that's fine but I was mostly responding to IDE team feedback. 😄

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs

jasonmalinowski · 2025-02-26T08:25:45Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs

        }

        public bool IsHostAssembly(Assembly assembly)
        {
            CheckIfDisposed();

            var alc = AssemblyLoadContext.GetLoadContext(assembly);
-            return alc == _compilerLoadContext || alc == AssemblyLoadContext.Default;
+            return alc == CompilerLoadContext || alc == AssemblyLoadContext.Default;


Not a question for this PR, but I'm surprised to see AssemblyLoadContext.Default being mentioned here since it's fairly ambiguous what that might be in more complicated scenarios. (I'm reminded of cases where MSBuild assumes it's something and that causes some pain too.)

I tried making this an explicit v.s optional parameter to force the issue of "you must be specific about this" but the churn got a bit too big. And honestly, in all our production hosting now Deafult is the compiler context.

jasonmalinowski · 2025-02-26T08:32:16Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/IAnalyzerAssemblyResolver.cs

-    /// Allows a host to override how assembly resolution is performed by the <see cref="AnalyzerAssemblyLoader"/>.
+    /// This interface allows hosts to control exactly how a given <see cref="AssemblyName"/> is resolved to an
+    /// <see cref="Assembly"/> instance. This is useful for hosts that need to load assemblies in a custom way like
+    /// Razor or stream based loading.
    /// </summary>
    internal interface IAnalyzerAssemblyResolver


I regret you can't call this interface IAnalyzerAssemblyLoader since it's actually doing the actual loading more or less. (i.e. the implementations all become some variant of ALC.Load*), but the names are already taken. :frowning>

jasonmalinowski · 2025-02-26T08:33:34Z

src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs

    {
-        _externalResolvers = [.. externalResolvers];
+        _assemblyResolvers = [.. assemblyResolvers];
        _shadowCopyLoader = new(CreateNewShadowCopyLoader);


Yep, makes sense for me to keep it lazy for now. I imagine this will still look different eventually, but there's some indirection/laziness going to be present even in the new thing.

jasonmalinowski · 2025-02-26T08:34:26Z

src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs

-internal sealed class DefaultAnalyzerAssemblyLoaderProvider(
-    [ImportMany] IEnumerable<IAnalyzerAssemblyResolver> externalResolvers)
-    : AbstractAnalyzerAssemblyLoaderProvider(externalResolvers);
+internal sealed class DefaultAnalyzerAssemblyLoaderProvider : AbstractAnalyzerAssemblyLoaderProvider


I'll think about this more next week if there's some nicer alternative, but it's constrained enough here to not be too much of a mess. No concerns with merging at this point.

jasonmalinowski · 2025-02-27T06:40:02Z

src/Compilers/Core/CodeAnalysisTest/CompilerResolverTests.cs

+    public void Dispose()
+    {
+        // This test should not pollute the default load context and hence interfere with other tests.
+        Assert.Equal(DefaultLoadContextCount, AssemblyLoadContext.Default.Assemblies.Count());


You have no idea how many bugs I have found with this approach in various compiler changes 😦

jasonmalinowski

I haven't gone through the AnalyzerAssemblyLoaderTests.cs, but everything else seems great.

src/Compilers/Core/CodeAnalysisTest/ShadowCopyAnalyzerPathResolverTests.cs

jasonmalinowski · 2025-02-27T06:41:52Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

+    /// places where differentiating between the two is important, the original path will be referred to as the "original" and
+    /// the latter is referred to as "resolved".


jasonmalinowski · 2025-02-27T06:44:45Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

+        /// These are paths generated by the loader, or one of its plugins. They need no normalization and hence
+        /// should be compared ordinally.
+        /// </summary>
+        internal static readonly StringComparer GeneratedPathComparer = StringComparer.Ordinal;


It would seem to me that as long as we count original paths via ordinal comparison (which I agree is good), then we'd also have to always make this ordinal too. Otherwise, A.dll and a.dll from the same directory would shadow copy to the same directory, and as "generated" paths those still have to be ordinal. So I'm not entirely sure why we'd ever need two static readonlys here for ultimately the same comparer....it seems like we can't really ever change either one without bugs. Or, if we were to, we still have to audit the entire loading system.

We go back and forth on what we want to do with the original paths. We are not consistent and have changed our minds a few times. Hence I like to be clear about which is being used in case we need to quickly change our minds.

With path comparison it's all about choosing how to fail 😦

jasonmalinowski · 2025-02-27T06:49:04Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs

@@ -83,6 +96,60 @@ private partial Assembly Load(AssemblyName assemblyName, string assemblyOriginal
            return loadContext.LoadFromAssemblyName(assemblyName);


To make sure I understand the flow here, this LoadFromAssemblyName triggers the ALC resolve, and that'll call back to the actual IAnalyzerAssemblyResolvers at this point? There's no way it'll resolve on it's own in some way, right?

jasonmalinowski · 2025-02-27T06:52:51Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

-                    _knownAssemblyPathsBySimpleName[simpleName] = paths.Add(fullPath);
+                    // The above can fail when the assembly doesn't exist because it's corrupted, 
+                    // doesn't exist on disk, or is a native DLL. Those failures are handled when 
+                    // the actual load is attempted. Just record the failure now.


I'd agree in this case -- this could be an I/O exception, security exception, or some corrupted data exception here. Just too many types...

jasonmalinowski · 2025-02-27T06:54:55Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

            if (assemblyName is null)
            {
-                throw new ArgumentException($"Not a valid assembly: {originalAnalyzerPath}");
+                // Not a managed assembly, nothing else to do
+                throw new ArgumentException($"Not a valid assembly: {originalPath}");


Not new to your change, but that could mean we had some I/O error for the assembly, right? Should we capture the exception that was thrown at that other point and hold onto it to re-report it here?

jasonmalinowski · 2025-02-27T06:58:50Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs

                {
                    return (null, null);
                }
+
+                originalPaths = set.OrderBy(x => x).ToList();


Nit: consider putting outside the lock since it's not needed to be synchronized, although I presume this won't be slow since it'd be rare to have more than a few...

jasonmalinowski · 2025-02-27T07:05:43Z

src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerPathResolver.cs

+/// in-proc and OOP e.g. in-proc (VS) running on desktop clr and OOP running on ServiceHub .Net6
+/// host. We need to make sure to use the ones from the same location as the remote.
+/// </summary>
+internal sealed class RemoteAnalyzerPathResolver(string baseDirectory) : IAnalyzerPathResolver


Agreed this appears to be dead code. Nothing is still consuming this in this new PR, right? Then I'd say just delete it -- if something is depending on this...we already broken them whether this stays in the new form or not.

jasonmalinowski · 2025-02-27T07:09:15Z

src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs

+            // When debugging, the debugger will load this DLL and that can throw off the debugging 
+            // session so exclude it here.
+            if (Debugger.IsAttached)
+            {
+                loadedAssemblies = loadedAssemblies.Where(x => x.GetName().Name != "Microsoft.VisualStudio.Debugger.Runtime.Desktop");
+            }
+


You had a similar assertion elsewhere, but it didn't have the debug check. Should we just have a little "assert nothing new in the ALC" helper? (can defer after the PR merges)

This is part of the set of helpers that verify nothing new in the ALC / AppDomain. There is a core helper method and a few others that feed into it. This is one of the core helpers though hence the best place to put this check.

Co-authored-by: Jason Malinowski <[email protected]>

Few changes here: 1. Jason had suggested I add an assert into the shadow copy loader, I did but apparently forgot to save. 2. Rename the file to match type name Follow up to dotnet#77004

* Feedback from analyzer loading change Few changes here: 1. Jason had suggested I add an assert into the shadow copy loader, I did but apparently forgot to save. 2. Rename the file to match type name Follow up to #77004 * PR feedback * Apply suggestions from code review Co-authored-by: Jan Jones <[email protected]> --------- Co-authored-by: Jan Jones <[email protected]>

* Change vs deps flow (#77651) * Add tests * Add version check * Fix type argument removal * Add tests * Add tests * Add tests * Add tests * Add tests * Add tests * Add tests * Docs * move to local functions * Remove unused using * Docs * Add tests * Add tests * Add tests * Loc strings * Loc strings * Fix generation of attribute with array constant * Make ISemanticSearchCopilotUIProvider import lazy to avoid loading VS.EA.Copilot (#77516) * Fix watch window completion window upon manual completion invocation (#77656) * Fix watch window completion window upon manual completion invocation Addresses https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2287966 This recently regressed due to this recent PR: #77204 The issue here is that I changed the csharp debugger context's projection buffer creation code from always having a one char string as the second sourceSpan to sometimes having that string be empty. This allowed the completion context to include items for which a semicolon would close that scope. However, by having a zero length span, a seam was created for a tracking span at priorTrackingSpan to be introduced into the set of tracking positions that can contribute to completion. Editor completion doesn't handle this well, so we're best off always sending a non-zero length source Span, so we do so and just use a space as the value. * Extensions: flesh out behavior for some pattern-based binding scenarios (#77608) * Generate Documentation - Bug Fixes (#77641) * fix some bugs * Feedback * More receiverType check out of EnumerateAllExtensionMembersInSingleBinder * Address feedback * Remove TODO * Fix formatting * Update maintenance-packages versions conditioned to sourcebuild only * Fix reflection * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250318.1 (#77672) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.616701 -> To Version 10.0.616801 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix typo (#77678) disalloed => disallowed. * Extensions: Adjust accessibility and file-type checks for extension members (#77657) * Avoid adding duplicate suppressors to the host analyzer arrays * Remove unnecessary code * Add docs * Simplify * Add docs * Strong types * move removed api * Fix SkipApplyOptimizationData parameter (#77677) * Avoid alloc * Add repro for IDE issue * Don't check constraint in explicit type argument scenario for now * Support textDocument/semanticTokens/full * Revert "Don't check constraint in explicit type argument scenario for now" This reverts commit e3af843. * Use type parameter definitions and avoid creating new type maps in checkConstraintsIncludingExtension * Add more tests * Upgrade from VSTelemetryApi 17.14.2 to 17.14.8 * Cleanup and make semantic token processing and testing code more consistent * Fix test * Add CI validation of Semantic Search API lists (#77535) * raw strings * Fix bug where exact path match would throw for additional files (#77583) From /pull/77517/files/45c0e103f76f36bed6004f836d3dcfeae4bfae0d#r1992506030 * Extensions: update semantic model APIs (#77619) * Update to latest in nuget.org * Update CSharpCopilotCodeAnalysisService.cs (#77691) * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250319.2 (#77694) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.616801 -> To Version 10.0.616902 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Disable integration tests on PRs to main-vs-deps and above * Make the ci build happy * Update aps * Simplify * Update compiler * Update src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs Co-authored-by: Jan Jones <[email protected]> * Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb * Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb * Update src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb * Remove old vs4mac code * Files * Fix copilot layering * Fix tests * Fix tests * Fix * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250320.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.616902 -> To Version 10.0.617002 * Do not consider `ref` argument to `in`/`ref readonly` parameter as being written to (#77682) Fixes #77528. Related to test plan #68056. * Update dotnet format instructions (#77699) * Add -solution option to build.ps1 (#77698) * Update infra * Implement scoping and shadowing rules for extension parameter and type parameters (#77704) See dotnet/csharplang#9229 (commit 9) * Fix updating committed solution with insignificant changes (#77648) * Extensions: only look for new extensions in new LangVer (#77690) * Improve raw string completion * Fix * Fix speeling * Extensions: definite assignment and region analysis (#77675) * Make things more strongly typed in compilation tracker * Show lightbulb on member name as well as on throw node * Rework analyzer assembly loading (#77004) This change restructures our analyzer assembly loading to allow for composable customizations to meet the demands of customers like VBCSCompiler, VS IDE, Razor and VS Code. Specifically, these layers want to customize two aspects of assembly loading: 1. The location where an assembly is loaded from. For example shadow copying does not change the content of what is loaded but it does change where it is loaded from 2. The `Assembly` loaded for a given path / name. For example Razor wants to control exactly which `Assembly` from which `AssemblyLoadContext` is loaded when running inside the VS OOP process. The existing customization approach was to derive from `AnalyzerAssemblyLoader`, override the right `virtual` and customize the behavior. That had limits, particularly because different parts of the VS IDE want to override these behaviors in different ways. That forced us to centralize logic that we didn't want to centralize (baking Razor into workspaces) and was challenging to integrate with our MEF composition. The new approach is to have a `sealed` type that allows customization through interfaces. This PR only establishes the new architecture and leaves the IDE unchanged. I originally attempted to do both at one time and the PR was hard to follow, the perf implications were challenging to track down and verify. Instead going to approach this by getting the basics in place then start migrating parts of the IDE over one at a time. * Remove all the existing workaround checks All of these have been fixed long ago (or we suspect they've long been fixed.) Closes #35123 * Ensure we also package Microsoft.Extensions.DependencyInjection.Abstractions * Add new skips for Codelens For reasons I'm unable to diagnose the codelens ServiceHub service is failing, possibly because it's missing a reference. I'm unable to sort out why, but it's not critical for integration tests so just ignore the failure. * Remove exclusion of main-vs-deps for running integration tests * Cleanup for code review * Improve the lightbulb detection * Fix handling of partial definition deletes (#77735) * Extensions: account for new extensions in function type scenarios (#77755) * Do not allow System.CommandLine to swallow all exceptions * Set the default namespace for the project and proper folders for documents. Resovles dotnet/vscode-csharp#8091 * Fix move to namespace offering for primary constructor (#77751) * add comment * Update src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs Co-authored-by: Jason Malinowski <[email protected]> * Update tests to verify Folders change. * Clean up branches that we're excluding * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250324.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.617002 -> To Version 10.0.617402 * Update infra * Fix issue with disappearing newlines after multiline documentation comment (#77521) * Fix issue with disappearing newlines after multiline documentation comments * Fix formatting issues * Convert LineBreaksAfterLeading to local function * Update function syntax to match conventions Co-authored-by: Jan Jones <[email protected]> * Fix compilation errors --------- Co-authored-by: Jan Jones <[email protected]> * Disallow complex forms of IndexerName attributes in extensions (#77781) * Implement name an signature conflict check across extension containers with the same receiver type (#77747) Implements the following rule from the spec: "Within a given enclosing static class, the set of extension member declarations with the same receiver type (modulo identity conversion and type parameter name substitution) are treated as a single declaration space similar to the members within a class or struct declaration, and are subject to the same rules about uniqueness." * Address PROTOTYPE comment in ExtensionMethodBodyRewriter (#77782) * Simplify tests * Simplify tests * Simplify tests * Simplify tests * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Remove unnecessary nullable directives * Update behavior to only report when throw in member * Fix tests * Remove unnecessary nullable directives * fixed test * Generate Documentation - Add remarks if available (#77783) * add remarks to documentation if available * add named parameter for nulls * Reduce likelihood of needing a dual insertion in VS Code (#77715) * Create new folder structure for Razor EA. /Shared : will eventually be the area for all source items shared between the current EA and the new EA.Razor.Features /Features : will have an EA that is restricted to depending only on the features layer. This will help provide a way to ship a more sane EA structure for VS Code as well as reduce the likelyhood and work required for a dual insertion /EditorFeatures : contains the current EA. The project name is kept the same for now but the folder is moved. This denotes that the binary does depend on the EditorFeatures layer in Roslyn * Update to new behavior - always show on throw * nit cleanup * Remove whitespaces * Remove extra blank line * Remove allThrows array * Simplify logic * Simplify * Comment * Workaround issue related to crossgen and specific .NET 9 runtimes * Show fleshed out example * Show fleshed out example * CommentS * Fixes tests Left to do: => case * Fixes => case * use patterns * use patterns * formatting * Complete fix * Set PackageProjectUrl (#77824) Avoids the SDK setting this to dotnet/dotnet automatically, which is generally not useful for customers. * [main] Update dependencies from dotnet/source-build-reference-packages (#77829) This pull request updates the following dependencies [marker]: <> (Begin:4e88e544-5668-4a45-0f48-08dc0c57b4c0) ## From https://github.com/dotnet/source-build-reference-packages - **Subscription**: 4e88e544-5668-4a45-0f48-08dc0c57b4c0 - **Build**: [20250325.1](https://dev.azure.com/dnceng/internal/_build/results?buildId=2671647) - **Date Produced**: March 25, 2025 5:28:05 PM UTC - **Commit**: [d2fc98192bb9780acbe2ad3df284da19203cc26d](dotnet/source-build-reference-packages@d2fc981) - **Branch**: refs/heads/main [DependencyUpdate]: <> (Begin) - **Updates**: - **Microsoft.SourceBuild.Intermediate.source-build-reference-packages**: [from 10.0.617402 to 10.0.617501][1] [1]: dotnet/source-build-reference-packages@4a8b582...d2fc981 [DependencyUpdate]: <> (End) [marker]: <> (End:4e88e544-5668-4a45-0f48-08dc0c57b4c0) * Extensions: check inferrability (#77815) * Extensions: Review/adjust call sites of `MethodSymbol.IsExtensionMethod` API (#77820) * Extensions: compact error codes (#77832) * Review call sites of extension methods related APIs (#77838) - BoundCollectionElementInitializer.InvokedAsExtensionMethod - BoundCall.InvokedAsExtensionMethod - BoundDelegateCreationExpression.IsExtensionMethod - MethodGroup.IsExtensionMethodGroup - MethodGroupResolution.IsExtensionMethodGroup * Update Ubuntu and MacOS images (#77833) * Apply PR feedback * Block use of extension properties in unsupported scenarios (#77844) * Extensions: remove PROTOTYPE comments (#77836) * Update src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionDiagnosticAnalyzer.cs Co-authored-by: Cyrus Najmabadi <[email protected]> * Log trace messages as "debug" so it formats right Otherwise this gets logged by the LSP client as a generic message, which isn't prefixed with a severity like everything else. * Support ILogger.BeginScope() for LSP logging This surrounds the processing of each LSP request with a call to ILogger.BeginScope() with the name of the method that's being invoked. This is prefixed to the message we send back to the LSP client, so it's much easier to understand the source of various pieces of logging. ILspLogger.LogStartContext/LogEndContext are deprecated here, as setting scopes plus an explicit start/end as appropriate makes things work much better when we're handling requests in parallel. * Completely delete the obsolete methods * Resolve merge conflicts * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250326.1 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.617501 -> To Version 10.0.617601 * Skip failing tests on macOS * Parse ignored directives (#77696) Speclet: https://github.com/dotnet/csharplang/blob/main/proposals/ignored-directives.md Public API: Resolves #77697. * Null-conditional assignment: Add tests from test plan review (#77717) * Implement support for Semantic Search agent and tool (#77784) * Another attempt to fix the SDK insertion issue related to runtime/apphost downloads (#77845) Use the EnableRuntimePackDownload/EnableAppHostPackDownload flags to indicate whether the restore should download those packages./ * Upgrade to VSSDK.BuildTools 17.14.1043-preview2 The original reason to upgrade to this is carries along updates to the Fast Up to Date Check support, which means Roslyn can delete our own copy of the targets. The upgrade caused one break: there's newer support in the VS SDK now where they look for ForceIncludeInVSIX metadata on PackageReferences as a way to bypass the suppression logic that exists to ensure certain references don't end up in the final VSIX. We had our own custom logic where you'd set just IncludeInVSIX="true" as metadata, which would produce items under teh covers with ForceIncludeInVSIX=true. There's arguably a bug in the targets that they should be respecting our underlying metadata rather than looking back to the PackageReference metadata, but it still seems reasonable for us just to adopt the same metadata name as the VS SDK. At this point it's not clear to me if we even need our custom logic anymore or if it's now obsolete. # Conflicts: # eng/Versions.props # src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj # Conflicts: # src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj * Generate Documentation - Add loading state (#77718) * wip * loading state works * clean up * feedback/comments * fix formatting * fix formatting * feedback * change enqueueaction * last feedback * feedback * commented out line of code * Use --opt-cross-module only for assemblies within the servicing bubble * Fix up comment * Remove condition * Fix up urls in docs * Add rules docs * Regenerate documentation * Silence warning about release note header * Update default .NET Core metadata references * Allow the new Razor project to access protocol types * Remove richnav pipeline and package (#77902) The service no longer exists. * Update the link to the file Portable PDB v1.0: Format Specification (#77911) * Feedback from analyzer loading change (#77780) * Feedback from analyzer loading change Few changes here: 1. Jason had suggested I add an assert into the shadow copy loader, I did but apparently forgot to save. 2. Rename the file to match type name Follow up to #77004 * PR feedback * Apply suggestions from code review Co-authored-by: Jan Jones <[email protected]> --------- Co-authored-by: Jan Jones <[email protected]> * Use NoWarn for RS2007 * Skip tests * Razor EA: load from ServiceHubCore (#77720) * Razor EA: load from ServiceHubCore * Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs Co-authored-by: Jan Jones <[email protected]> * Trim trailing slashes * Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs Co-authored-by: Jan Jones <[email protected]> * Fix trim call * PR feedback --------- Co-authored-by: Jan Jones <[email protected]> Co-authored-by: Jared Parsons <[email protected]> --------- Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: dotnet-policy-service[bot] <123482357+dotnet-policy-service[bot]@users.noreply.github.com> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Tomáš Matoušek <[email protected]> Co-authored-by: Todd Grunke <[email protected]> Co-authored-by: AlekseyTs <[email protected]> Co-authored-by: Ankita Khera <[email protected]> Co-authored-by: Julien Couvreur <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: Carlos Sánchez López <[email protected]> Co-authored-by: David Wengier <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Bill Wagner <[email protected]> Co-authored-by: Sam Harwell <[email protected]> Co-authored-by: PaddiM8 <[email protected]> Co-authored-by: Arun Chander <[email protected]> Co-authored-by: Andrew Hall <[email protected]> Co-authored-by: Julien Couvreur <[email protected]> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Maryam Ariyan <[email protected]> Co-authored-by: Jared Parsons <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: Dimitris Gounaris <[email protected]> Co-authored-by: Matt Mitchell <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: Alexander Köplinger <[email protected]> Co-authored-by: Dong A. <[email protected]> Co-authored-by: Chris Sienkiewicz <[email protected]>

ghost added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 3, 2025

dotnet-policy-service bot added the VSCode label Feb 3, 2025

jaredpar commented Feb 3, 2025

View reviewed changes

jaredpar mentioned this pull request Feb 3, 2025

Revert "Reduce File I/O under the AnalyzerAssemblyLoader folder (#724… #75761

Open

jaredpar commented Feb 5, 2025

View reviewed changes

jaredpar added 13 commits February 18, 2025 08:55

Rework analyzer assembly loading

fde224f

tests passing

a1fcb8c

more

5e9c686

more

e4d944b

more

aec482e

more

f992a17

formatting

d47a88e

fix

e7a0716

more

865191b

more

7c7d88d

more

3f398a0

more

a09f29a

more

c7a0ee5

jaredpar force-pushed the load branch from c5401f4 to c7a0ee5 Compare February 18, 2025 17:21

jaredpar commented Feb 18, 2025

View reviewed changes

jaredpar marked this pull request as ready for review February 18, 2025 17:26

jaredpar requested review from a team as code owners February 18, 2025 17:26

CyrusNajmabadi reviewed Feb 18, 2025

View reviewed changes

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 18, 2025

View reviewed changes

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 18, 2025

View reviewed changes

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs Outdated Show resolved Hide resolved

pr feedback

431014e

jjonescz approved these changes Feb 21, 2025

View reviewed changes

jaredpar and others added 2 commits February 21, 2025 11:08

Apply suggestions from code review

b21926c

Co-authored-by: Fred Silberberg <[email protected]>

pr feedback

7578830

333fred approved these changes Feb 24, 2025

View reviewed changes

RikkiGibson removed their assignment Feb 25, 2025

jasonmalinowski reviewed Feb 26, 2025

View reviewed changes

jasonmalinowski reviewed Feb 27, 2025

View reviewed changes

jasonmalinowski approved these changes Feb 27, 2025

View reviewed changes

jaredpar and others added 4 commits March 21, 2025 13:40

Rename the document

9697d90

fixup

5499f7e

Apply suggestions from code review

5288c11

Co-authored-by: Jason Malinowski <[email protected]>

PR feedback

bcd40bd

333fred approved these changes Mar 24, 2025

View reviewed changes

jaredpar merged commit 28b8657 into dotnet:main Mar 24, 2025
28 checks passed

jaredpar deleted the load branch March 24, 2025 17:11

dotnet-policy-service bot added this to the Next milestone Mar 24, 2025

jaredpar mentioned this pull request Mar 24, 2025

Feedback from analyzer loading change #77780

Merged

This was referenced Mar 25, 2025

[Automated] PRs inserted in VS build main-35925.14 #77804

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35927.15 #77863

Closed

JoeRobich mentioned this pull request Mar 28, 2025

Update Roslyn to 4.14.0-3.25178.1 dotnet/vscode-csharp#8103

Merged

jjonescz modified the milestones: Next, 17.14 P3 Apr 1, 2025

dotnet-bot mentioned this pull request Apr 3, 2025

[Automated] PRs inserted in VS build feature.d18initial-10502.05 #77975

Closed

		/// places where differentiating between the two is important, the original path will be referred to as the "original" and
		/// the latter is referred to as "resolved".

		@@ -83,6 +96,60 @@ private partial Assembly Load(AssemblyName assemblyName, string assemblyOriginal
		return loadContext.LoadFromAssemblyName(assemblyName);

Rework analyzer assembly loading #77004

Rework analyzer assembly loading #77004

Uh oh!

Conversation

jaredpar commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

jasonmalinowski Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jaredpar commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyrusNajmabadi commented Feb 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaredpar commented Feb 24, 2025

Uh oh!

jasonmalinowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

jasonmalinowski Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jasonmalinowski left a comment

Choose a reason for hiding this comment

Uh oh!

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

jaredpar commented Feb 3, 2025 •

edited

Loading

jasonmalinowski Feb 26, 2025 •

edited

Loading

jaredpar commented Feb 18, 2025 •

edited

Loading

jasonmalinowski Feb 26, 2025 •

edited

Loading