Skip to content

Commit 38f239f

Browse files
authored
Fix updating committed solution with insignificant changes (#77648)
1 parent c98aef0 commit 38f239f

File tree

8 files changed

+139
-42
lines changed

8 files changed

+139
-42
lines changed

src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs

+12-3
Original file line numberDiff line numberDiff line change
@@ -378,10 +378,19 @@ public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(ImmutableArray<s
378378
var runningProjectIds = solution.Projects.Where(p => p.FilePath != null && runningProjectPaths.Contains(p.FilePath)).Select(static p => p.Id).ToImmutableHashSet();
379379
var result = await GetDebuggingSession().EmitSolutionUpdateAsync(solution, runningProjectIds, activeStatementSpanProvider, cancellationToken).ConfigureAwait(false);
380380

381-
// Only store the solution if we have any changes to apply, otherwise CommitUpdatesAsync/DiscardUpdatesAsync won't be called.
382-
if (result.ModuleUpdates.Status == ModuleUpdateStatus.Ready)
381+
switch (result.ModuleUpdates.Status)
383382
{
384-
_pendingUpdatedDesignTimeSolution = designTimeSolution;
383+
case ModuleUpdateStatus.Ready:
384+
// We have updates to be applied. The debugger will call Commit/Discard on the solution
385+
// based on whether the updates will be applied successfully or not.
386+
_pendingUpdatedDesignTimeSolution = designTimeSolution;
387+
break;
388+
389+
case ModuleUpdateStatus.None:
390+
// No significant changes have been made.
391+
// Commit the solution to apply any changes in comments that do not generate updates.
392+
_committedDesignTimeSolution = designTimeSolution;
393+
break;
385394
}
386395

387396
UpdateApplyChangesDiagnostics(result.Diagnostics);

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

+20-6
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,27 @@ public async ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(
511511
solutionUpdate.Log(SessionLog, updateId);
512512
_lastModuleUpdatesLog = solutionUpdate.ModuleUpdates.Updates;
513513

514-
if (solutionUpdate.ModuleUpdates.Status == ModuleUpdateStatus.Ready)
514+
switch (solutionUpdate.ModuleUpdates.Status)
515515
{
516-
StorePendingUpdate(new PendingSolutionUpdate(
517-
solution,
518-
solutionUpdate.ProjectBaselines,
519-
solutionUpdate.ModuleUpdates.Updates,
520-
solutionUpdate.NonRemappableRegions));
516+
case ModuleUpdateStatus.Ready:
517+
// We have updates to be applied. The debugger will call Commit/Discard on the solution
518+
// based on whether the updates will be applied successfully or not.
519+
StorePendingUpdate(new PendingSolutionUpdate(
520+
solution,
521+
solutionUpdate.ProjectBaselines,
522+
solutionUpdate.ModuleUpdates.Updates,
523+
solutionUpdate.NonRemappableRegions));
524+
525+
break;
526+
527+
case ModuleUpdateStatus.None:
528+
Contract.ThrowIfFalse(solutionUpdate.ModuleUpdates.Updates.IsEmpty);
529+
Contract.ThrowIfFalse(solutionUpdate.NonRemappableRegions.IsEmpty);
530+
531+
// No significant changes have been made.
532+
// Commit the solution to apply any changes in comments that do not generate updates.
533+
LastCommittedSolution.CommitSolution(solution);
534+
break;
521535
}
522536

523537
using var _ = ArrayBuilder<ProjectDiagnostics>.GetInstance(out var rudeEditDiagnostics);

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ void AddGeneralDiagnostic(EditAndContinueErrorCode code, string resourceName, Di
166166
AddGeneralDiagnostic(EditAndContinueErrorCode.ErrorReadingFile, nameof(FeaturesResources.ErrorReadingFile));
167167
AddGeneralDiagnostic(EditAndContinueErrorCode.CannotApplyChangesUnexpectedError, nameof(FeaturesResources.CannotApplyChangesUnexpectedError));
168168
AddGeneralDiagnostic(EditAndContinueErrorCode.ChangesDisallowedWhileStoppedAtException, nameof(FeaturesResources.ChangesDisallowedWhileStoppedAtException));
169-
AddGeneralDiagnostic(EditAndContinueErrorCode.DocumentIsOutOfSyncWithDebuggee, nameof(FeaturesResources.DocumentIsOutOfSyncWithDebuggee), DiagnosticSeverity.Warning);
170-
AddGeneralDiagnostic(EditAndContinueErrorCode.UnableToReadSourceFileOrPdb, nameof(FeaturesResources.UnableToReadSourceFileOrPdb), DiagnosticSeverity.Warning);
169+
AddGeneralDiagnostic(EditAndContinueErrorCode.DocumentIsOutOfSyncWithDebuggee, nameof(FeaturesResources.DocumentIsOutOfSyncWithDebuggee));
170+
AddGeneralDiagnostic(EditAndContinueErrorCode.UnableToReadSourceFileOrPdb, nameof(FeaturesResources.UnableToReadSourceFileOrPdb));
171171
AddGeneralDiagnostic(EditAndContinueErrorCode.AddingTypeRuntimeCapabilityRequired, nameof(FeaturesResources.ChangesRequiredSynthesizedType));
172172

173173
s_descriptors = builder.ToImmutable();

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

+20-10
Original file line numberDiff line numberDiff line change
@@ -821,8 +821,9 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
821821

822822
var oldSolution = DebuggingSession.LastCommittedSolution;
823823

824-
var isBlocked = false;
824+
var blockUpdates = false;
825825
var hasEmitErrors = false;
826+
var hadDocumentReadError = false;
826827
foreach (var newProject in solution.Projects)
827828
{
828829
if (!newProject.SupportsEditAndContinue(Log))
@@ -866,7 +867,7 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
866867
diagnostics.Add(new(newProject.Id, [mvidReadError]));
867868

868869
Telemetry.LogProjectAnalysisSummary(ProjectAnalysisSummary.ValidChanges, newProject.State.ProjectInfo.Attributes.TelemetryId, ImmutableArray.Create(mvidReadError.Descriptor.Id));
869-
isBlocked = true;
870+
blockUpdates = true;
870871
continue;
871872
}
872873

@@ -899,6 +900,11 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
899900
// If in future the file is updated so that its content matches the PDB checksum, the document transitions to a matching state,
900901
// and we consider any further changes to it for application.
901902
diagnostics.Add(new(newProject.Id, documentDiagnostics));
903+
904+
if (documentDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error))
905+
{
906+
blockUpdates = hadDocumentReadError = true;
907+
}
902908
}
903909

904910
foreach (var changedDocumentAnalysis in changedDocumentAnalyses)
@@ -935,12 +941,12 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
935941
if (isModuleEncBlocked)
936942
{
937943
diagnostics.Add(new(newProject.Id, moduleDiagnostics));
938-
isBlocked = true;
944+
blockUpdates = true;
939945
}
940946

941947
if (projectSummary is ProjectAnalysisSummary.SyntaxErrors or ProjectAnalysisSummary.RudeEdits)
942948
{
943-
isBlocked = true;
949+
blockUpdates = true;
944950
}
945951

946952
// Report rude edit diagnostics - these can be blocking (errors) or non-blocking (warnings):
@@ -972,7 +978,7 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
972978
diagnostics.Add(new(newProject.Id, createBaselineErrors));
973979
Telemetry.LogProjectAnalysisSummary(projectSummary, newProject.State.ProjectInfo.Attributes.TelemetryId, createBaselineErrors);
974980

975-
isBlocked = true;
981+
blockUpdates = true;
976982
await LogDocumentChangesAsync(generation: null, cancellationToken).ConfigureAwait(false);
977983
continue;
978984
}
@@ -1050,7 +1056,7 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
10501056
if (!emitResult.Success)
10511057
{
10521058
// error
1053-
isBlocked = hasEmitErrors = true;
1059+
blockUpdates = hasEmitErrors = true;
10541060
emitDiagnostics = emitResult.Diagnostics;
10551061
break;
10561062
}
@@ -1062,7 +1068,7 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
10621068
{
10631069
emitDiagnostics = [unsupportedChangesDiagnostic];
10641070
diagnostics.Add(new(newProject.Id, emitDiagnostics));
1065-
isBlocked = true;
1071+
blockUpdates = true;
10661072
break;
10671073
}
10681074

@@ -1127,13 +1133,17 @@ async ValueTask LogDocumentChangesAsync(int? generation, CancellationToken cance
11271133
}
11281134

11291135
// log capabilities for edit sessions with changes or reported errors:
1130-
if (isBlocked || deltas.Count > 0)
1136+
if (blockUpdates || deltas.Count > 0)
11311137
{
11321138
Telemetry.LogRuntimeCapabilities(await Capabilities.GetValueAsync(cancellationToken).ConfigureAwait(false));
11331139
}
11341140

1135-
var update = isBlocked
1136-
? SolutionUpdate.Blocked(diagnostics.ToImmutable(), documentsWithRudeEdits.ToImmutable(), syntaxError, hasEmitErrors)
1141+
var update = blockUpdates
1142+
? SolutionUpdate.Empty(
1143+
diagnostics.ToImmutable(),
1144+
documentsWithRudeEdits.ToImmutable(),
1145+
syntaxError,
1146+
syntaxError != null || hasEmitErrors || hadDocumentReadError ? ModuleUpdateStatus.Blocked : ModuleUpdateStatus.RestartRequired)
11371147
: new SolutionUpdate(
11381148
new ModuleUpdates(
11391149
(deltas.Count > 0) ? ModuleUpdateStatus.Ready : ModuleUpdateStatus.None,

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ internal readonly struct SolutionUpdate(
2424
public readonly ImmutableArray<(DocumentId DocumentId, ImmutableArray<RudeEditDiagnostic> Diagnostics)> DocumentsWithRudeEdits = documentsWithRudeEdits;
2525
public readonly Diagnostic? SyntaxError = syntaxError;
2626

27-
public static SolutionUpdate Blocked(
27+
public static SolutionUpdate Empty(
2828
ImmutableArray<ProjectDiagnostics> diagnostics,
2929
ImmutableArray<(DocumentId, ImmutableArray<RudeEditDiagnostic>)> documentsWithRudeEdits,
3030
Diagnostic? syntaxError,
31-
bool hasEmitErrors)
31+
ModuleUpdateStatus status)
3232
=> new(
33-
new(syntaxError != null || hasEmitErrors ? ModuleUpdateStatus.Blocked : ModuleUpdateStatus.RestartRequired, []),
33+
new(status, Updates: []),
3434
nonRemappableRegions: [],
3535
projectBaselines: [],
3636
diagnostics,

src/Features/Core/Portable/ExternalAccess/Watch/Api/WatchHotReloadService.cs

+6-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ public async Task<Updates> GetUpdatesAsync(Solution solution, IImmutableSet<Proj
190190

191191
var results = await _encService.EmitSolutionUpdateAsync(sessionId, solution, runningProjects, s_solutionActiveStatementSpanProvider, cancellationToken).ConfigureAwait(false);
192192

193-
if (results.ModuleUpdates.Status == ModuleUpdateStatus.Ready)
193+
// If the changes fail to apply dotnet-watch fails.
194+
// We don't support discarding the changes and letting the user retry.
195+
if (!results.ModuleUpdates.Updates.IsEmpty)
194196
{
195197
_encService.CommitSolutionUpdate(sessionId);
196198
}
@@ -241,6 +243,9 @@ internal readonly struct TestAccessor(WatchHotReloadService instance)
241243
{
242244
public DebuggingSessionId SessionId
243245
=> instance._sessionId;
246+
247+
public IEditAndContinueService EncService
248+
=> instance._encService;
244249
}
245250
}
246251
#endif

src/Features/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs

+48-10
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ public async Task StartDebuggingSession_CapturingDocuments(bool captureAllDocume
153153
EnterBreakState(debuggingSession);
154154

155155
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
156-
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
156+
Assert.Equal(ModuleUpdateStatus.Blocked, updates.Status);
157157
Assert.Empty(updates.Updates);
158-
AssertEx.Equal([$"P.csproj: (0,0)-(0,0): Warning ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFileB.Path)}"], InspectDiagnostics(emitDiagnostics));
158+
AssertEx.Equal([$"P.csproj: (0,0)-(0,0): Error ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFileB.Path)}"], InspectDiagnostics(emitDiagnostics));
159159

160160
EndDebuggingSession(debuggingSession);
161161
}
@@ -595,9 +595,9 @@ public async Task ErrorReadingPdbFile()
595595

596596
// an error occurred so we need to call update to determine whether we have changes to apply or not:
597597
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
598-
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
598+
Assert.Equal(ModuleUpdateStatus.Blocked, updates.Status);
599599
Assert.Empty(updates.Updates);
600-
AssertEx.Equal([$"proj.csproj: (0,0)-(0,0): Warning ENC1006: {string.Format(FeaturesResources.UnableToReadSourceFileOrPdb, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
600+
AssertEx.Equal([$"proj.csproj: (0,0)-(0,0): Error ENC1006: {string.Format(FeaturesResources.UnableToReadSourceFileOrPdb, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
601601

602602
EndDebuggingSession(debuggingSession);
603603

@@ -642,9 +642,9 @@ public async Task ErrorReadingSourceFile()
642642

643643
// an error occurred so we need to call update to determine whether we have changes to apply or not:
644644
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
645-
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
645+
Assert.Equal(ModuleUpdateStatus.Blocked, updates.Status);
646646
Assert.Empty(updates.Updates);
647-
AssertEx.Equal([$"test.csproj: (0,0)-(0,0): Warning ENC1006: {string.Format(FeaturesResources.UnableToReadSourceFileOrPdb, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
647+
AssertEx.Equal([$"test.csproj: (0,0)-(0,0): Error ENC1006: {string.Format(FeaturesResources.UnableToReadSourceFileOrPdb, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
648648

649649
fileLock.Dispose();
650650

@@ -1168,9 +1168,9 @@ public async Task RudeEdits_DocumentOutOfSync(bool breakMode)
11681168

11691169
// since the document is out-of-sync we need to call update to determine whether we have changes to apply or not:
11701170
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
1171-
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
1171+
Assert.Equal(ModuleUpdateStatus.Blocked, updates.Status);
11721172
Assert.Empty(updates.Updates);
1173-
AssertEx.Equal([$"proj.csproj: (0,0)-(0,0): Warning ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
1173+
AssertEx.Equal([$"proj.csproj: (0,0)-(0,0): Error ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
11741174

11751175
// update the file to match the build:
11761176
sourceFile.WriteAllText(source0, Encoding.UTF8);
@@ -2312,8 +2312,8 @@ public async Task ValidSignificantChange_FileUpdateNotObservedBeforeDebuggingSes
23122312

23132313
// since the document is out-of-sync we need to call update to determine whether we have changes to apply or not:
23142314
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
2315-
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
2316-
AssertEx.Equal([$"test.csproj: (0,0)-(0,0): Warning ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
2315+
Assert.Equal(ModuleUpdateStatus.Blocked, updates.Status);
2316+
AssertEx.Equal([$"test.csproj: (0,0)-(0,0): Error ENC1005: {string.Format(FeaturesResources.DocumentIsOutOfSyncWithDebuggee, sourceFile.Path)}"], InspectDiagnostics(emitDiagnostics));
23172317

23182318
// undo:
23192319
solution = solution.WithDocumentText(documentId, CreateText(source1));
@@ -3111,6 +3111,44 @@ public async Task ValidSignificantChange_SourceGenerators_DocumentRemove()
31113111
EndDebuggingSession(debuggingSession);
31123112
}
31133113

3114+
[Fact]
3115+
public async Task ValidInsignificantChange()
3116+
{
3117+
var sourceV1 = "class C1 { void M() { /* System.Console.WriteLine(1); */ } }";
3118+
var sourceV2 = "class C1 { void M() { /* System.Console.WriteLine(2); */ } }";
3119+
3120+
using var _ = CreateWorkspace(out var solution, out var service);
3121+
(solution, var document1) = AddDefaultTestProject(solution, sourceV1);
3122+
3123+
var moduleId = EmitAndLoadLibraryToDebuggee(sourceV1);
3124+
3125+
var debuggingSession = await StartDebuggingSessionAsync(service, solution);
3126+
3127+
// change the source (valid edit):
3128+
solution = solution.WithDocumentText(document1.Id, CreateText(sourceV2));
3129+
var document2 = solution.GetDocument(document1.Id);
3130+
3131+
var diagnostics1 = await service.GetDocumentDiagnosticsAsync(document2, s_noActiveSpans, CancellationToken.None);
3132+
AssertEx.Empty(diagnostics1);
3133+
3134+
// validate solution update status and emit:
3135+
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
3136+
Assert.Empty(emitDiagnostics);
3137+
Assert.Equal(ModuleUpdateStatus.None, updates.Status);
3138+
3139+
// solution has been updated:
3140+
var text = await debuggingSession.LastCommittedSolution.GetRequiredProject(document1.Project.Id).GetDocument(document1.Id).GetTextAsync();
3141+
Assert.Equal(sourceV2, text.ToString());
3142+
3143+
EndDebuggingSession(debuggingSession);
3144+
3145+
AssertEx.SequenceEqual(
3146+
[
3147+
"Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=0|EmptySessionCount=0|HotReloadSessionCount=1|EmptyHotReloadSessionCount=0",
3148+
"Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=False|HadValidInsignificantChanges=True|RudeEditsCount=0|EmitDeltaErrorIdCount=0|InBreakState=False|Capabilities=0|ProjectIdsWithAppliedChanges=|ProjectIdsWithUpdatedBaselines="
3149+
], _telemetryLog);
3150+
}
3151+
31143152
[Fact]
31153153
public async Task RudeEdit()
31163154
{

0 commit comments

Comments
 (0)