EnC: A couple of small fixes (#62454)

* Use NoCompilationLanguageService instead of a custom one in EnC tests

* SupportsEditAndContinue asserts

* Do not cache failures to read MVID

* Feedback

src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs

@@ -340,7 +340,7 @@ public async Task TestOpenFileOnlyAnalyzerDiagnostics()
         [Fact]
         public async Task TestSynchronizeWithBuild()
         {
-            using var workspace = CreateWorkspace(new[] { typeof(NoCompilationLanguageServiceFactory) });
+            using var workspace = CreateWorkspace(new[] { typeof(NoCompilationLanguageService) });
 
             var analyzerReference = new AnalyzerImageReference(ImmutableArray.Create<DiagnosticAnalyzer>(new NoNameAnalyzer()));
             workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { analyzerReference }));

src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs

@@ -486,7 +486,7 @@ public async Task StartDebuggingSession_CapturingDocuments(bool captureAllDocume
             sourceFileB.WriteAllText(sourceB2, encodingB);
 
             // prepare workspace as if it was loaded from project files:
-            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
+            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });
 
             var projectP = solution.AddProject("P", "P", LanguageNames.CSharp);
             solution = projectP.Solution;
@@ -531,7 +531,7 @@ public async Task StartDebuggingSession_CapturingDocuments(bool captureAllDocume
                 AddDocument(CreateDesignTimeOnlyDocument(projectP.Id, name: "dt2.cs", path: "dt2.cs"));
 
             // project that does not support EnC - the contents of documents in this project shouldn't be loaded:
-            var projectQ = solution.AddProject("Q", "Q", DummyLanguageService.LanguageName);
+            var projectQ = solution.AddProject("Q", "Q", NoCompilationConstants.LanguageName);
             solution = projectQ.Solution;
 
             solution = solution.AddDocument(DocumentInfo.Create(
@@ -576,7 +576,7 @@ public async Task ProjectNotBuilt()
 
             _mockCompilationOutputsProvider = _ => new MockCompilationOutputs(Guid.Empty);
 
-            await StartDebuggingSessionAsync(service, solution);
+            var debuggingSession = await StartDebuggingSessionAsync(service, solution);
 
             // no changes:
             var diagnostics = await service.GetDocumentDiagnosticsAsync(document1, s_noActiveSpans, CancellationToken.None);
@@ -588,6 +588,14 @@ public async Task ProjectNotBuilt()
 
             diagnostics = await service.GetDocumentDiagnosticsAsync(document2, s_noActiveSpans, CancellationToken.None);
             Assert.Empty(diagnostics);
+
+            // changes in the project are ignored:
+            var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
+            Assert.Equal(ManagedModuleUpdateStatus.None, updates.Status);
+            Assert.Empty(updates.Updates);
+            Assert.Empty(emitDiagnostics);
+
+            EndDebuggingSession(debuggingSession);
         }
 
         [Fact]
@@ -631,8 +639,8 @@ public async Task DifferentDocumentWithSameContent()
         [CombinatorialData]
         public async Task ProjectThatDoesNotSupportEnC(bool breakMode)
         {
-            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
-            var project = solution.AddProject("dummy_proj", "dummy_proj", DummyLanguageService.LanguageName);
+            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });
+            var project = solution.AddProject("dummy_proj", "dummy_proj", NoCompilationConstants.LanguageName);
             var document = project.AddDocument("test", SourceText.From("dummy1"));
             solution = document.Project.Solution;
 
@@ -823,8 +831,11 @@ public async Task ErrorReadingModuleFile(bool breakMode)
                 expectedErrorMessage = e.Message;
             }
 
+            var source1 = "class C { void M() { System.Console.WriteLine(1); } }";
+            var source2 = "class C { void M() { System.Console.WriteLine(2); } }";
+
             using var _w = CreateWorkspace(out var solution, out var service);
-            (solution, var document1) = AddDefaultTestProject(solution, "class C1 { void M() { System.Console.WriteLine(1); } }");
+            (solution, var document1) = AddDefaultTestProject(solution, source1);
 
             _mockCompilationOutputsProvider = _ => new CompilationOutputFiles(moduleFile.Path);
 
@@ -836,7 +847,7 @@ public async Task ErrorReadingModuleFile(bool breakMode)
             }
 
             // change the source:
-            solution = solution.WithDocumentText(document1.Id, SourceText.From("class C1 { void M() { System.Console.WriteLine(2); } }"));
+            solution = solution.WithDocumentText(document1.Id, SourceText.From(source2, encoding: Encoding.UTF8));
             var document2 = solution.GetDocument(document1.Id);
 
             // error not reported here since it might be intermittent and will be reported if the issue persist when applying the update:
@@ -848,6 +859,15 @@ public async Task ErrorReadingModuleFile(bool breakMode)
             Assert.Empty(updates.Updates);
             AssertEx.Equal(new[] { $"{document2.Project.Id}: Error ENC1001: {string.Format(FeaturesResources.ErrorReadingFile, moduleFile.Path, expectedErrorMessage)}" }, InspectDiagnostics(emitDiagnostics));
 
+            // correct the error:
+            EmitLibrary(source2);
+
+            var (updates2, emitDiagnostics2) = await EmitSolutionUpdateAsync(debuggingSession, solution);
+            Assert.Equal(ManagedModuleUpdateStatus.Ready, updates2.Status);
+            Assert.Empty(emitDiagnostics2);
+
+            CommitSolutionUpdate(debuggingSession);
+
             if (breakMode)
             {
                 ExitBreakState(debuggingSession);
@@ -859,17 +879,17 @@ public async Task ErrorReadingModuleFile(bool breakMode)
             {
                 AssertEx.Equal(new[]
                 {
-                    "Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=1|EmptySessionCount=0|HotReloadSessionCount=0|EmptyHotReloadSessionCount=2",
-                    "Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=True|Capabilities=31|ProjectIdsWithAppliedChanges=",
+                    "Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=1|EmptySessionCount=0|HotReloadSessionCount=0|EmptyHotReloadSessionCount=3",
+                    "Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=True|Capabilities=31|ProjectIdsWithAppliedChanges={00000000-AAAA-AAAA-AAAA-111111111111}",
                     "Debugging_EncSession_EditSession_EmitDeltaErrorId: SessionId=1|EditSessionId=2|ErrorId=ENC1001"
                 }, _telemetryLog);
             }
             else
             {
                 AssertEx.Equal(new[]
                 {
-                    "Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=0|EmptySessionCount=0|HotReloadSessionCount=1|EmptyHotReloadSessionCount=0",
-                    "Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=False|Capabilities=31|ProjectIdsWithAppliedChanges=",
+                    "Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=0|EmptySessionCount=0|HotReloadSessionCount=1|EmptyHotReloadSessionCount=1",
+                    "Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=False|Capabilities=31|ProjectIdsWithAppliedChanges={00000000-AAAA-AAAA-AAAA-111111111111}",
                     "Debugging_EncSession_EditSession_EmitDeltaErrorId: SessionId=1|EditSessionId=2|ErrorId=ENC1001"
                 }, _telemetryLog);
             }
@@ -3454,11 +3474,11 @@ public async Task ActiveStatements_SyntaxErrorOrOutOfSyncDocument(bool isOutOfSy
         [CombinatorialData]
         public async Task ActiveStatements_ForeignDocument(bool withPath, bool designTimeOnly)
         {
-            var composition = FeaturesTestCompositions.Features.AddParts(typeof(DummyLanguageService));
+            var composition = FeaturesTestCompositions.Features.AddParts(typeof(NoCompilationLanguageService));
 
-            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
+            using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });
 
-            var project = solution.AddProject("dummy_proj", "dummy_proj", designTimeOnly ? LanguageNames.CSharp : DummyLanguageService.LanguageName);
+            var project = solution.AddProject("dummy_proj", "dummy_proj", designTimeOnly ? LanguageNames.CSharp : NoCompilationConstants.LanguageName);
             var filePath = withPath ? Path.Combine(TempRoot.Root, "test.cs") : null;
 
             var documentInfo = DocumentInfo.Create(

src/EditorFeatures/Test/EditAndContinue/EditSessionActiveStatementsTests.cs

@@ -17,6 +17,7 @@
 using Microsoft.CodeAnalysis.Emit;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Text;
+using Microsoft.CodeAnalysis.UnitTests;
 using Moq;
 using Roslyn.Test.Utilities;
 using Roslyn.Utilities;
@@ -31,7 +32,7 @@ namespace Microsoft.CodeAnalysis.EditAndContinue.UnitTests
     [UseExportProvider]
     public class EditSessionActiveStatementsTests : TestBase
     {
-        private static readonly TestComposition s_composition = EditorTestCompositions.EditorFeatures.AddParts(typeof(DummyLanguageService));
+        private static readonly TestComposition s_composition = EditorTestCompositions.EditorFeatures.AddParts(typeof(NoCompilationLanguageService));
 
         private static EditSession CreateEditSession(
             Solution solution,
@@ -174,8 +175,8 @@ static void Main()
 
             var solution = AddDefaultTestSolution(workspace, markedSources);
             var projectId = solution.ProjectIds.Single();
-            var dummyProject = solution.AddProject("dummy_proj", "dummy_proj", DummyLanguageService.LanguageName);
-            solution = dummyProject.Solution.AddDocument(DocumentId.CreateNewId(dummyProject.Id, DummyLanguageService.LanguageName), "a.dummy", "");
+            var dummyProject = solution.AddProject("dummy_proj", "dummy_proj", NoCompilationConstants.LanguageName);
+            solution = dummyProject.Solution.AddDocument(DocumentId.CreateNewId(dummyProject.Id, NoCompilationConstants.LanguageName), "a.dummy", "");
             var project = solution.GetProject(projectId);
             var document1 = project.Documents.Single(d => d.Name == "test1.cs");
             var document2 = project.Documents.Single(d => d.Name == "test2.cs");

src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs

@@ -335,7 +335,7 @@ public async Task<(Document? Document, DocumentState State)> GetDocumentAndState
                 var (project, documentStates) = projectDocumentStates;
 
                 // Skip projects that do not support Roslyn EnC (e.g. F#, etc).
-                // Source files of these do not even need to be captured in the solution snapshot.
+                // Source files of these may not even be captured in the solution snapshot.
                 if (!project.SupportsEditAndContinue())
                 {
                     return Array.Empty<DocumentId?>();

src/Features/Core/Portable/EditAndContinue/DebuggingSession.cs

@@ -34,6 +34,7 @@ internal sealed class DebuggingSession : IDisposable
 
         /// <summary>
         /// MVIDs read from the assembly built for given project id.
+        /// Only contains ids for projects that support EnC.
         /// </summary>
         private readonly Dictionary<ProjectId, (Guid Mvid, Diagnostic Error)> _projectModuleIds = new();
         private readonly Dictionary<Guid, ProjectId> _moduleIds = new();
@@ -241,10 +242,12 @@ private bool AddModulePreparedForUpdate(Guid mvid)
         /// </summary>
         /// <returns>
         /// An MVID and an error message to report, in case an IO exception occurred while reading the binary.
-        /// The MVID is default if either project not built, or an it can't be read from the module binary.
+        /// The MVID is <see cref="Guid.Empty"/> if either the project is not built, or the MVID can't be read from the module binary.
         /// </returns>
         internal async Task<(Guid Mvid, Diagnostic? Error)> GetProjectModuleIdAsync(Project project, CancellationToken cancellationToken)
         {
+            Debug.Assert(project.SupportsEditAndContinue());
+
             lock (_projectModuleIdsGuard)
             {
                 if (_projectModuleIds.TryGetValue(project.Id, out var id))
@@ -281,8 +284,14 @@ internal async Task<(Guid Mvid, Diagnostic? Error)> GetProjectModuleIdAsync(Proj
                     return id;
                 }
 
-                _moduleIds[newId.Mvid] = project.Id;
-                return _projectModuleIds[project.Id] = newId;
+                // Do not cache failures. The error might be intermittent and might be corrected next time we attempt to read the MVID.
+                if (newId.Mvid != Guid.Empty)
+                {
+                    _moduleIds[newId.Mvid] = project.Id;
+                    _projectModuleIds[project.Id] = newId;
+                }
+
+                return newId;
             }
         }
 
@@ -650,6 +659,10 @@ public async ValueTask<ImmutableArray<ImmutableArray<ActiveStatementSpan>>> GetB
                     }
 
                     var newProject = solution.GetRequiredProject(projectId);
+
+                    Debug.Assert(oldProject.SupportsEditAndContinue());
+                    Debug.Assert(newProject.SupportsEditAndContinue());
+
                     var analyzer = newProject.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();
 
                     await foreach (var documentId in EditSession.GetChangedDocumentsAsync(oldProject, newProject, cancellationToken).ConfigureAwait(false))
@@ -745,7 +758,7 @@ public async ValueTask<ImmutableArray<ActiveStatementSpan>> GetAdjustedActiveSta
         {
             try
             {
-                if (_isDisposed || !EditSession.InBreakState || !mappedDocument.State.SupportsEditAndContinue())
+                if (_isDisposed || !EditSession.InBreakState || !mappedDocument.State.SupportsEditAndContinue() || !mappedDocument.Project.SupportsEditAndContinue())
                 {
                     return ImmutableArray<ActiveStatementSpan>.Empty;
                 }
@@ -965,6 +978,10 @@ await foreach (var unmappedDocumentId in EditSession.GetChangedDocumentsAsync(ol
                         return null;
                     }
 
+                    // We only maintain module ids for projects that support EnC:
+                    Debug.Assert(oldProject.SupportsEditAndContinue());
+                    Debug.Assert(newProject.SupportsEditAndContinue());
+
                     documentId = await GetChangedDocumentContainingUnmappedActiveStatementAsync(activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, cancellationToken).ConfigureAwait(false);
                 }
                 else
@@ -982,8 +999,10 @@ async Task GetTaskAsync(ProjectId projectId)
                         // TODO: https://github.com/dotnet/roslyn/issues/1204
                         // oldProject == null ==> project has been added - it may have active statements if the project was unloaded when debugging session started but the sources 
                         // correspond to the PDB.
-                        var id = (oldProject != null) ? await GetChangedDocumentContainingUnmappedActiveStatementAsync(
-                        activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, linkedTokenSource.Token).ConfigureAwait(false) : null;
+                        var id = (oldProject?.SupportsEditAndContinue() == true) ?
+                            await GetChangedDocumentContainingUnmappedActiveStatementAsync(
+                                activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, linkedTokenSource.Token).ConfigureAwait(false) :
+                            null;
 
                         Interlocked.CompareExchange(ref documentId, id, null);
                         if (id != null)
@@ -1017,6 +1036,9 @@ async Task GetTaskAsync(ProjectId projectId)
         private static async ValueTask<DocumentId?> GetChangedDocumentContainingUnmappedActiveStatementAsync(ActiveStatementsMap baseActiveStatements, CommittedSolution oldSolution, Project oldProject, Project newProject, ActiveStatement activeStatement, CancellationToken cancellationToken)
         {
             Debug.Assert(oldProject.Id == newProject.Id);
+            Debug.Assert(oldProject.SupportsEditAndContinue());
+            Debug.Assert(newProject.SupportsEditAndContinue());
+
             var analyzer = newProject.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();
 
             await foreach (var documentId in EditSession.GetChangedDocumentsAsync(oldProject, newProject, cancellationToken).ConfigureAwait(false))

src/Features/Core/Portable/EditAndContinue/Extensions.cs

@@ -68,6 +68,10 @@ public static UnmappedActiveStatement GetStatement(this ImmutableArray<UnmappedA
             throw ExceptionUtilities.UnexpectedValue(ordinal);
         }
 
+        /// <summary>
+        /// True if the project supports Edit and Continue.
+        /// Only depends on the language of the project and never changes.
+        /// </summary>
         public static bool SupportsEditAndContinue(this Project project)
             => project.LanguageServices.GetService<IEditAndContinueAnalyzer>() != null;
 

src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/CSharpReferencesTests.cs

@@ -115,7 +115,7 @@ public async Task RemoveProjectConvertsProjectReferencesBack()
         [Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
         public async Task AddingMetadataReferenceToProjectThatCannotCompileInTheIdeKeepsMetadataReference()
         {
-            using var environment = new TestEnvironment(typeof(NoCompilationLanguageServiceFactory));
+            using var environment = new TestEnvironment(typeof(NoCompilationLanguageService));
             var project1 = await CreateCSharpCPSProjectAsync(environment, "project1", commandLineArguments: @"/out:c:\project1.dll");
             var project2 = await CreateNonCompilableProjectAsync(environment, "project2", @"C:\project2.fsproj");
             project2.BinOutputPath = "c:\\project2.dll";