Fix analyzer consistency checks

src/Compilers/Server/VBCSCompiler/AnalyzerConsistencyChecker.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#if NETFRAMEWORK
+
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
@@ -90,13 +92,32 @@ private static bool CheckCore(
             }
 
             // Third, check that the MVIDs of the files on disk match the MVIDs of the loaded assemblies.
+            var comparer = PathUtilities.Comparer;
+            var compilerDirectory = Path.GetDirectoryName(typeof(AnalyzerConsistencyChecker).Assembly.CodeBase);
+
             for (int i = 0; i < resolvedPaths.Count; i++)
             {
                 var resolvedPath = resolvedPaths[i];
                 var loadedAssembly = loadedAssemblies[i];
+
+                // When an assembly is loaded from the GAC then the load result would be the same if 
+                // this ran on command line compiler. So there is no consistency issue here, this 
+                // is just runtime rules expressing themselves.
+                if (loadedAssembly.GlobalAssemblyCache)
+                {
+                    continue;
+                }
+
+                // When an assembly is loaded from the compiler directory then this means it's assembly
+                // binding redirects taking over. For example it's moving from an older version of System.Memory
+                // to the one shipping in the compiler. This is not a consistency issue.
+                if (PathUtilities.Comparer.Equals(compilerDirectory, Path.GetDirectoryName(loadedAssembly.CodeBase)))
+                {
+                    continue;
+                }
+
                 var resolvedPathMvid = AssemblyUtilities.ReadMvid(resolvedPath);
                 var loadedAssemblyMvid = loadedAssembly.ManifestModule.ModuleVersionId;
-
                 if (resolvedPathMvid != loadedAssemblyMvid)
                 {
                     var message = $"analyzer assembly '{resolvedPath}' has MVID '{resolvedPathMvid}' but loaded assembly '{loadedAssembly.FullName}' has MVID '{loadedAssemblyMvid}'";
@@ -109,3 +130,5 @@ private static bool CheckCore(
         }
     }
 }
+
+#endif

src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs

@@ -74,11 +74,6 @@ internal CompilerServerHost(string clientDirectory, string sdkDirectory, ICompil
             Logger = logger;
         }
 
-        private bool CheckAnalyzers(string baseDirectory, ImmutableArray<CommandLineAnalyzerReference> analyzers, [NotNullWhen(false)] out List<string>? errorMessages)
-        {
-            return AnalyzerConsistencyChecker.Check(baseDirectory, analyzers, AnalyzerAssemblyLoader, Logger, out errorMessages);
-        }
-
         public bool TryCreateCompiler(in RunRequest request, BuildPaths buildPaths, [NotNullWhen(true)] out CommonCompiler? compiler)
         {
             switch (request.Language)
@@ -141,16 +136,18 @@ public BuildResponse RunCompilation(in RunRequest request, CancellationToken can
                 return new RejectedBuildResponse(message);
             }
 
-            bool utf8output = compiler.Arguments.Utf8Output;
-            if (!CheckAnalyzers(request.WorkingDirectory, compiler.Arguments.AnalyzerReferences, out List<string>? errorMessages))
+#if NETFRAMEWORK
+            if (!AnalyzerConsistencyChecker.Check(request.WorkingDirectory, compiler.Arguments.AnalyzerReferences, AnalyzerAssemblyLoader, Logger, out List<string?> errorMessages))
             {
                 Logger.Log($"Rejected: {request.RequestId}: for analyzer load issues {string.Join(";", errorMessages)}");
                 return new AnalyzerInconsistencyBuildResponse(new ReadOnlyCollection<string>(errorMessages));
             }
+#endif
 
             Logger.Log($"Begin {request.RequestId} {request.Language} compiler run");
             try
             {
+                bool utf8output = compiler.Arguments.Utf8Output;
                 TextWriter output = new StringWriter(CultureInfo.InvariantCulture);
                 int returnCode = compiler.Run(output, cancellationToken);
                 var outputString = output.ToString();

src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#if NETFRAMEWORK
 #nullable disable
 
 using System;
@@ -43,6 +44,39 @@ public AnalyzerConsistencyCheckerTests(ITestOutputHelper testOutputHelper, Assem
             TestFixture = testFixture;
         }
 
+        private TempFile CreateNetStandardDll(TempDirectory directory, string assemblyName, string version, ImmutableArray<byte> publicKey, string? extraSource = null)
+        {
+            var source = $$"""
+                using System;
+                using System.Reflection;
+
+                [assembly: AssemblyVersion("{{version}}")]
+                [assembly: AssemblyFileVersion("{{version}}")]
+                """;
+
+            var sources = extraSource is null 
+                ? new[] { CSharpTestSource.Parse(source) }
+                : new[] { CSharpTestSource.Parse(source), CSharpTestSource.Parse(extraSource) };
+
+            var options = new CSharpCompilationOptions(
+                OutputKind.DynamicallyLinkedLibrary,
+                warningLevel: Diagnostic.MaxWarningLevel,
+                cryptoPublicKey: publicKey,
+                deterministic: true,
+                publicSign: true);
+
+            var comp = CSharpCompilation.Create(
+                assemblyName,
+                sources,
+                references: NetStandard20.All,
+                options: options);
+
+            var file = directory.CreateFile($"{assemblyName}.dll");
+            var emitResult = comp.Emit(file.Path);
+            Assert.Empty(emitResult.Diagnostics.Where(x => x.Severity == DiagnosticSeverity.Error));
+            return file;
+        }
+
         [Fact]
         public void MissingReference()
         {
@@ -73,25 +107,66 @@ public void DifferingMvids()
         {
             var directory = Temp.CreateDirectory();
 
-            // Load Beta.dll from the future Alpha.dll path to prime the assembly loader
-            var alphaDll = directory.CopyFile(TestFixture.Beta.Path, name: "Alpha.dll");
+            var key = NetStandard20.netstandard.GetAssemblyIdentity().PublicKey;
+            var mvidAlpha1 = CreateNetStandardDll(directory.CreateDirectory("mvid1"), "MvidAlpha", "1.0.0.0", key, "class C { }");
+            var mvidAlpha2 = CreateNetStandardDll(directory.CreateDirectory("mvid2"), "MvidAlpha", "1.0.0.0", key, "class D { }");
+
+            // Can't use InMemoryAssemblyLoader because that uses the None context which fakes paths
+            // to always be the currently executing application. That makes it look like everything 
+            // is in the same directory
+            var assemblyLoader = new DefaultAnalyzerAssemblyLoader();
+            var analyzerReferences = ImmutableArray.Create(
+                new CommandLineAnalyzerReference(mvidAlpha1.Path),
+                new CommandLineAnalyzerReference(mvidAlpha2.Path));
 
-            var assemblyLoader = new InMemoryAssemblyLoader();
-            var betaAssembly = assemblyLoader.LoadFromPath(alphaDll.Path);
+            var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger);
+            Assert.False(result);
+        }
 
-            // now overwrite the {directory}/Alpha.dll file with the content from our Alpha.dll test resource
-            alphaDll.CopyContentFrom(TestFixture.Alpha.Path);
-            directory.CopyFile(TestFixture.Gamma.Path);
-            directory.CopyFile(TestFixture.Delta1.Path);
+        /// <summary>
+        /// A differing MVID is okay when it's loading a DLL from the compiler directory. That is 
+        /// considered an exchange type. For example if an analyzer has a reference to System.Memory
+        /// it will always load the copy the compiler used and that is not a consistency issue.
+        /// </summary>
+        [Fact]
+        [WorkItem(64826, "https://github.com/dotnet/roslyn/issues/64826")]
+        public void LoadingLibraryFromCompiler()
+        {
+            var directory = Temp.CreateDirectory();
+            var dllFile = CreateNetStandardDll(directory, "System.Memory", "2.0.0.0", NetStandard20.netstandard.GetAssemblyIdentity().PublicKey);
 
+            // This test must use the DefaultAnalyzerAssemblyLoader as we want assembly binding redirects
+            // to take affect here.
+            var assemblyLoader = new DefaultAnalyzerAssemblyLoader();
             var analyzerReferences = ImmutableArray.Create(
-                new CommandLineAnalyzerReference("Alpha.dll"),
-                new CommandLineAnalyzerReference("Gamma.dll"),
-                new CommandLineAnalyzerReference("Delta.dll"));
+                new CommandLineAnalyzerReference("System.Memory.dll"));
 
             var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger);
 
-            Assert.False(result);
+            Assert.True(result);
+        }
+
+        /// <summary>
+        /// A differing MVID is okay when it's loading a DLL from the GAC. There is no reason that 
+        /// falling back to csc would change the load result.
+        /// </summary>
+        [Fact]
+        [WorkItem(64826, "https://github.com/dotnet/roslyn/issues/64826")]
+        public void LoadingLibraryFromGAC()
+        {
+            var directory = Temp.CreateDirectory();
+            var dllFile = directory.CreateFile("System.Core.dll");
+            dllFile.WriteAllBytes(NetStandard20.Resources.SystemCore);
+
+            // This test must use the DefaultAnalyzerAssemblyLoader as we want assembly binding redirects
+            // to take affect here.
+            var assemblyLoader = new DefaultAnalyzerAssemblyLoader();
+            var analyzerReferences = ImmutableArray.Create(
+                new CommandLineAnalyzerReference("System.Core.dll"));
+
+            var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, assemblyLoader, Logger);
+
+            Assert.True(result);
         }
 
         [Fact]
@@ -109,22 +184,15 @@ public void AssemblyLoadException()
         }
 
         [Fact]
-        public void NetstandardIgnored()
+        public void LoadingSimpleLibrary()
         {
             var directory = Temp.CreateDirectory();
-            const string name = "netstandardRef";
-            var comp = CSharpCompilation.Create(
-                name,
-                new[] { CSharpTestSource.Parse("class C {}") },
-                references: new MetadataReference[] { NetStandard20.netstandard },
-                options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, warningLevel: Diagnostic.MaxWarningLevel));
-            var compFile = directory.CreateFile(name);
-            comp.Emit(compFile.Path);
+            var key = NetStandard20.netstandard.GetAssemblyIdentity().PublicKey;
+            var compFile = CreateNetStandardDll(directory, "netstandardRef", "1.0.0.0", key);
 
+            var analyzerReferences = ImmutableArray.Create(new CommandLineAnalyzerReference(compFile.Path));
 
-            var analyzerReferences = ImmutableArray.Create(new CommandLineAnalyzerReference(name));
-
-            var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, new InMemoryAssemblyLoader(), Logger);
+            var result = AnalyzerConsistencyChecker.Check(directory.Path, analyzerReferences, new DefaultAnalyzerAssemblyLoader(), Logger);
 
             Assert.True(result);
         }
@@ -152,3 +220,4 @@ public Assembly LoadFromPath(string fullPath)
         }
     }
 }
+#endif