Some PR feedback

Author: chsienki

src/Compilers/Test/Core/SourceGeneration/TestGenerators.cs

@@ -202,15 +202,16 @@ public IncrementalAndSourceCallbackGenerator(Action<GeneratorInitializationConte
 
 namespace Microsoft.NET.Sdk.Razor.SourceGenerators
 {
-    internal sealed class RazorSourceGenerator : ISourceGenerator
+    /// <summary>
+    /// We check for the presence of the razor SG by full name
+    /// so we have to make sure this is the right name in the right namespace.
+    /// </summary>
+    internal sealed class RazorSourceGenerator(Action<GeneratorExecutionContext> execute) : ISourceGenerator
     {
         private int _callCount = 0;
 
         public void Initialize(GeneratorInitializationContext context) { }
 
-        public void Execute(GeneratorExecutionContext context)
-        {
-            context.AddSource("file.cs", $"// callCount: {_callCount++}");
-        }
+        public void Execute(GeneratorExecutionContext context) => execute(context);
     }
 }

src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs

@@ -1488,9 +1488,11 @@ internal async Task TestSourceGenerationExecution_RazorGeneratorAlwaysRuns_Other
         var globalOptionService = workspace.ExportProvider.GetExportedValue<IGlobalOptionService>();
         globalOptionService.SetGlobalOption(WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, executionPreference);
 
-        var callCount = 0;
-        var generator1 = new CallbackGenerator(() => ("hintName.cs", "// callCount: " + callCount++));
-        var generator2 = new Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator();
+        var callBackCallCount = 0;
+        var generator1 = new CallbackGenerator(() => ("hintName.cs", "// callCount: " + callBackCallCount++));
+
+        var razorCallCount = 0;
+        var generator2 = new Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator((c) => c.AddSource("file.cs", "// callCount: " + razorCallCount++));
 
         var projectId = ProjectId.CreateNewId();
         var project = workspace.CurrentSolution
@@ -1506,25 +1508,29 @@ internal async Task TestSourceGenerationExecution_RazorGeneratorAlwaysRuns_Other
         Assert.True(workspace.SetCurrentSolution(_ => tempDoc.Project.Solution, WorkspaceChangeKind.SolutionChanged));
 
         // Initial: all generators run
-        await ValidateSourceGeneratorDocuments(expectedCallback: 0,
-                                               expectedRazor: 0);
+        await ValidateSourceGeneratorDocuments(
+            expectedCallback: 0,
+            expectedRazor: 0);
 
         // Now, make a simple edit to the main document.
         Contract.ThrowIfFalse(workspace.TryApplyChanges(workspace.CurrentSolution.WithDocumentText(tempDoc.Id, SourceText.From("// new text"))));
 
-        await ValidateSourceGeneratorDocuments(expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 1 : 0,
-                                               expectedRazor: 1);
+        await ValidateSourceGeneratorDocuments(
+            expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 1 : 0,
+            expectedRazor: 1);
 
         // Get the documents again and ensure nothing ran
-        await ValidateSourceGeneratorDocuments(expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 1 : 0,
-                                               expectedRazor: 1);
+        await ValidateSourceGeneratorDocuments(
+            expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 1 : 0,
+            expectedRazor: 1);
 
         // Make another change, but this time enqueue an update too
         Contract.ThrowIfFalse(workspace.TryApplyChanges(workspace.CurrentSolution.WithDocumentText(tempDoc.Id, SourceText.From("// more new text"))));
         workspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration: false);
 
-        await ValidateSourceGeneratorDocuments(expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 2 : 1,
-                                               expectedRazor: 2);
+        await ValidateSourceGeneratorDocuments(
+            expectedCallback: executionPreference == SourceGeneratorExecutionPreference.Automatic ? 2 : 1,
+            expectedRazor: 2);
 
         async Task ValidateSourceGeneratorDocuments(int expectedCallback, int expectedRazor)
         {

src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.RegularCompilationTracker_Generators.cs

@@ -211,7 +211,7 @@ private sealed partial class RegularCompilationTracker : ICompilationTracker
             if (creationPolicy == GeneratedDocumentCreationPolicy.CreateRequired)
             {
                 // the documents we got back are only for the required generators, meaning any documents from other generators remain the same. 
-                var generatorsThatRan = infos.Select(di => di.DocumentIdentity.Generator).Distinct().ToImmutableHashSet();
+                var generatorsThatRan = infos.Select(di => di.DocumentIdentity.Generator).Distinct().ToArray();
 
                 // go through the old docs and add any that aren't in the new set
                 foreach (var (_, oldDocumentState) in oldGeneratedDocuments.States)
@@ -300,6 +300,8 @@ await newGeneratedDocuments.States.Values.SelectAsArrayAsync(
             var generationDateTime = DateTime.Now;
             foreach (var generatorResult in runResult.Results)
             {
+                // When we only run required generators, there may be generators that didn't run at all, and so didn't produce any sources.
+                // Note that this is different from running and producing zero documents, in which case we still need to consider it.
                 if (generatorResult.GeneratedSources.IsDefault)
                     continue;