Fix reporting rude edits (#48966)

tmat · web-flow · commit ca27b943f56c · 2025-05-19T16:31:18.000-07:00
diff --git a/src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs b/src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs
@@ -15,7 +15,6 @@ internal sealed class CompilationHandler : IDisposable
     {
         public readonly IncrementalMSBuildWorkspace Workspace;
         public readonly EnvironmentOptions EnvironmentOptions;
-        private readonly GlobalOptions _options;
         private readonly IReporter _reporter;
         private readonly WatchHotReloadService _hotReloadService;
 
@@ -41,11 +40,10 @@ internal sealed class CompilationHandler : IDisposable
 
         private bool _isDisposed;
 
-        public CompilationHandler(IReporter reporter, EnvironmentOptions environmentOptions, GlobalOptions options, CancellationToken shutdownCancellationToken)
+        public CompilationHandler(IReporter reporter, EnvironmentOptions environmentOptions, CancellationToken shutdownCancellationToken)
         {
             _reporter = reporter;
             EnvironmentOptions = environmentOptions;
-            _options = options;
             Workspace = new IncrementalMSBuildWorkspace(reporter);
             _hotReloadService = new WatchHotReloadService(Workspace.CurrentSolution.Services, () => ValueTask.FromResult(GetAggregateCapabilities()));
             _shutdownCancellationToken = shutdownCancellationToken;
@@ -265,6 +263,7 @@ private static void PrepareCompilations(Solution solution, string projectPath, C
         }
 
         public async ValueTask<(ImmutableDictionary<ProjectId, string> projectsToRebuild, ImmutableArray<RunningProject> terminatedProjects)> HandleManagedCodeChangesAsync(
+            bool autoRestart,
             Func<IEnumerable<string>, CancellationToken, Task> restartPrompt,
             CancellationToken cancellationToken)
         {
@@ -275,8 +274,8 @@ private static void PrepareCompilations(Solution solution, string projectPath, C
                (from project in currentSolution.Projects
                 let runningProject = GetCorrespondingRunningProject(project, runningProjects)
                 where runningProject != null
-                let autoRestart = _options.NonInteractive || runningProject.ProjectNode.IsAutoRestartEnabled()
-                select (project.Id, info: new WatchHotReloadService.RunningProjectInfo() { RestartWhenChangesHaveNoEffect = autoRestart }))
+                let autoRestartProject = autoRestart || runningProject.ProjectNode.IsAutoRestartEnabled()
+                select (project.Id, info: new WatchHotReloadService.RunningProjectInfo() { RestartWhenChangesHaveNoEffect = autoRestartProject }))
                 .ToImmutableDictionary(e => e.Id, e => e.info);
 
             var updates = await _hotReloadService.GetUpdatesAsync(currentSolution, runningProjectInfos, cancellationToken);
@@ -290,41 +289,43 @@ private static void PrepareCompilations(Solution solution, string projectPath, C
                 return (ImmutableDictionary<ProjectId, string>.Empty, []);
             }
 
-            ImmutableDictionary<string, ImmutableArray<RunningProject>> projectsToUpdate;
-            lock (_runningProjectsAndUpdatesGuard)
+            if (updates.ProjectUpdates.Any())
             {
-                // Adding the updates makes sure that all new processes receive them before they are added to running processes.
-                _previousUpdates = _previousUpdates.AddRange(updates.ProjectUpdates);
+                ImmutableDictionary<string, ImmutableArray<RunningProject>> projectsToUpdate;
+                lock (_runningProjectsAndUpdatesGuard)
+                {
+                    // Adding the updates makes sure that all new processes receive them before they are added to running processes.
+                    _previousUpdates = _previousUpdates.AddRange(updates.ProjectUpdates);
 
-                // Capture the set of processes that do not have the currently calculated deltas yet.
-                projectsToUpdate = _runningProjects;
-            }
+                    // Capture the set of processes that do not have the currently calculated deltas yet.
+                    projectsToUpdate = _runningProjects;
+                }
 
-            // Apply changes to all running projects, even if they do not have a static project dependency on any project that changed.
-            // The process may load any of the binaries using MEF or some other runtime dependency loader.
+                // Apply changes to all running projects, even if they do not have a static project dependency on any project that changed.
+                // The process may load any of the binaries using MEF or some other runtime dependency loader.
 
-            await ForEachProjectAsync(projectsToUpdate, async (runningProject, cancellationToken) =>
-            {
-                try
+                await ForEachProjectAsync(projectsToUpdate, async (runningProject, cancellationToken) =>
                 {
-                    using var processCommunicationCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(runningProject.ProcessExitedSource.Token, cancellationToken);
-                    var applySucceded = await runningProject.DeltaApplier.ApplyManagedCodeUpdates(updates.ProjectUpdates, processCommunicationCancellationSource.Token) != ApplyStatus.Failed;
-                    if (applySucceded)
+                    try
                     {
-                        runningProject.Reporter.Report(MessageDescriptor.HotReloadSucceeded);
-                        if (runningProject.BrowserRefreshServer is { } server)
+                        using var processCommunicationCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(runningProject.ProcessExitedSource.Token, cancellationToken);
+                        var applySucceded = await runningProject.DeltaApplier.ApplyManagedCodeUpdates(updates.ProjectUpdates, processCommunicationCancellationSource.Token) != ApplyStatus.Failed;
+                        if (applySucceded)
                         {
-                            runningProject.Reporter.Verbose("Refreshing browser.");
-                            await server.RefreshBrowserAsync(cancellationToken);
+                            runningProject.Reporter.Report(MessageDescriptor.HotReloadSucceeded);
+                            if (runningProject.BrowserRefreshServer is { } server)
+                            {
+                                runningProject.Reporter.Verbose("Refreshing browser.");
+                                await server.RefreshBrowserAsync(cancellationToken);
+                            }
                         }
                     }
-                }
-                catch (OperationCanceledException) when (runningProject.ProcessExitedSource.Token.IsCancellationRequested && !cancellationToken.IsCancellationRequested)
-                {
-                    runningProject.Reporter.Verbose("Hot reload canceled because the process exited.", emoji: "🔥");
-                }
-            }, cancellationToken);
-
+                    catch (OperationCanceledException) when (runningProject.ProcessExitedSource.Token.IsCancellationRequested && !cancellationToken.IsCancellationRequested)
+                    {
+                        runningProject.Reporter.Verbose("Hot reload canceled because the process exited.", emoji: "🔥");
+                    }
+                }, cancellationToken);
+            }
 
             if (updates.ProjectsToRestart.IsEmpty)
             {
@@ -430,11 +431,15 @@ void ReportRudeEdits()
                 // Rude edits in projects that caused restart of a project that can be restarted automatically
                 // will be reported only as verbose output.
                 var projectsRestartedDueToRudeEdits = updates.ProjectsToRestart
-                    .Where(e => runningProjectInfos.TryGetValue(e.Key, out var info) && info.RestartWhenChangesHaveNoEffect)
+                    .Where(e => IsAutoRestartEnabled(e.Key))
                     .SelectMany(e => e.Value)
                     .ToHashSet();
 
-                var projectsRebuiltDueToRudeEdits = updates.ProjectsToRebuild.ToHashSet();
+                // Project with rude edit that doesn't impact running project is only listed in ProjectsToRebuild.
+                // Such projects are always auto-rebuilt whether or not there is any project to be restarted that needs a confirmation.
+                var projectsRebuiltDueToRudeEdits = updates.ProjectsToRebuild
+                    .Where(p => !updates.ProjectsToRestart.ContainsKey(p))
+                    .ToHashSet();
 
                 foreach (var (projectId, diagnostics) in updates.RudeEdits)
                 {
@@ -451,6 +456,9 @@ void ReportRudeEdits()
                 }
             }
 
+            bool IsAutoRestartEnabled(ProjectId id)
+                => runningProjectInfos.TryGetValue(id, out var info) && info.RestartWhenChangesHaveNoEffect;
+
             void ReportDiagnostic(Diagnostic diagnostic, MessageDescriptor descriptor, string prefix = "")
             {
                 var display = CSharpDiagnosticFormatter.Instance.Format(diagnostic);
diff --git a/src/BuiltInTools/dotnet-watch/HotReload/RestartPrompt.cs b/src/BuiltInTools/dotnet-watch/HotReload/RestartPrompt.cs
@@ -5,14 +5,14 @@ namespace Microsoft.DotNet.Watch
 {
     internal sealed class RestartPrompt(IReporter reporter, ConsoleInputReader requester, bool? noPrompt)
     {
-        private bool? _restartImmediatelySessionPreference = noPrompt;
+        public bool? AutoRestartPreference { get; private set; } = noPrompt;
 
         public async ValueTask<bool> WaitForRestartConfirmationAsync(string question, CancellationToken cancellationToken)
         {
-            if (_restartImmediatelySessionPreference.HasValue)
+            if (AutoRestartPreference.HasValue)
             {
                 reporter.Output("Restarting");
-                return _restartImmediatelySessionPreference.Value;
+                return AutoRestartPreference.Value;
             }
 
             var key = await requester.GetKeyAsync(
@@ -30,11 +30,11 @@ public async ValueTask<bool> WaitForRestartConfirmationAsync(string question, Ca
                     return false;
 
                 case ConsoleKey.A:
-                    _restartImmediatelySessionPreference = true;
+                    AutoRestartPreference = true;
                     return true;
 
                 case ConsoleKey.V:
-                    _restartImmediatelySessionPreference = false;
+                    AutoRestartPreference = false;
                     return false;
             }
 
diff --git a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
@@ -100,7 +100,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
                     }
 
                     var projectMap = new ProjectNodeMap(evaluationResult.ProjectGraph, Context.Reporter);
-                    compilationHandler = new CompilationHandler(Context.Reporter, Context.EnvironmentOptions, Context.Options, shutdownCancellationToken);
+                    compilationHandler = new CompilationHandler(Context.Reporter, Context.EnvironmentOptions, shutdownCancellationToken);
                     var scopedCssFileHandler = new ScopedCssFileHandler(Context.Reporter, projectMap, browserConnector);
                     var projectLauncher = new ProjectLauncher(Context, projectMap, browserConnector, compilationHandler, iteration);
                     var outputDirectories = GetProjectOutputDirectories(evaluationResult.ProjectGraph);
@@ -265,46 +265,50 @@ void FileChangedCallback(ChangedPath change)
 
                         HotReloadEventSource.Log.HotReloadStart(HotReloadEventSource.StartType.CompilationHandler);
 
-                        var (projectsToRebuild, projectsToRestart) = await compilationHandler.HandleManagedCodeChangesAsync(restartPrompt: async (projectNames, cancellationToken) =>
-                        {
-                            if (_rudeEditRestartPrompt != null)
+                        var (projectsToRebuild, projectsToRestart) = await compilationHandler.HandleManagedCodeChangesAsync(
+                            autoRestart: Context.Options.NonInteractive || _rudeEditRestartPrompt?.AutoRestartPreference is true,
+                            restartPrompt: async (projectNames, cancellationToken) =>
                             {
-                                // stop before waiting for user input:
-                                stopwatch.Stop();
-
-                                string question;
-                                if (runtimeProcessLauncher == null)
-                                {
-                                    question = "Do you want to restart your app?";
-                                }
-                                else
+                                if (_rudeEditRestartPrompt != null)
                                 {
-                                    Context.Reporter.Output("Affected projects:");
+                                    // stop before waiting for user input:
+                                    stopwatch.Stop();
 
-                                    foreach (var projectName in projectNames.OrderBy(n => n))
+                                    string question;
+                                    if (runtimeProcessLauncher == null)
                                     {
-                                        Context.Reporter.Output("  " + projectName);
+                                        question = "Do you want to restart your app?";
                                     }
+                                    else
+                                    {
+                                        Context.Reporter.Output("Affected projects:");
 
-                                    question = "Do you want to restart these projects?";
-                                }
+                                        foreach (var projectName in projectNames.OrderBy(n => n))
+                                        {
+                                            Context.Reporter.Output("  " + projectName);
+                                        }
 
-                                if (!await _rudeEditRestartPrompt.WaitForRestartConfirmationAsync(question, cancellationToken))
-                                {
-                                    Context.Reporter.Output("Hot reload suspended. To continue hot reload, press \"Ctrl + R\".", emoji: "🔥");
-                                    await Task.Delay(-1, cancellationToken);
-                                }
-                            }
-                            else
-                            {
-                                Context.Reporter.Verbose("Restarting without prompt since dotnet-watch is running in non-interactive mode.");
+                                        question = "Do you want to restart these projects?";
+                                    }
 
-                                foreach (var projectName in projectNames)
+                                    if (!await _rudeEditRestartPrompt.WaitForRestartConfirmationAsync(question, cancellationToken))
+                                    {
+                                        Context.Reporter.Output("Hot reload suspended. To continue hot reload, press \"Ctrl + R\".", emoji: "🔥");
+                                        await Task.Delay(-1, cancellationToken);
+                                    }
+                                }
+                                else
                                 {
-                                    Context.Reporter.Verbose($"  Project to restart: '{projectName}'");
+                                    Context.Reporter.Verbose("Restarting without prompt since dotnet-watch is running in non-interactive mode.");
+
+                                    foreach (var projectName in projectNames)
+                                    {
+                                        Context.Reporter.Verbose($"  Project to restart: '{projectName}'");
+                                    }
                                 }
-                            }
-                        }, iterationCancellationToken);
+                            },
+                            iterationCancellationToken);
+
                         HotReloadEventSource.Log.HotReloadEnd(HotReloadEventSource.StartType.CompilationHandler);
 
                         stopwatch.Stop();
diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs
@@ -105,6 +105,47 @@ public async Task AutoRestartOnRudeEdit(bool nonInteractive)
             App.AssertOutputContains($"[WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Launched");
         }
 
+        [Fact]
+        public async Task AutoRestartOnRudeEditAfterRestartPrompt()
+        {
+            var testAsset = TestAssets.CopyTestAsset("WatchHotReloadApp")
+                .WithSource();
+
+            var programPath = Path.Combine(testAsset.Path, "Program.cs");
+
+            App.Start(testAsset, [], testFlags: TestFlags.ReadKeyFromStdin);
+
+            await App.AssertWaitingForChanges();
+            App.Process.ClearOutput();
+
+            // rude edit: adding virtual method
+            UpdateSourceFile(programPath, src => src.Replace("/* member placeholder */", "public virtual void F() {}"));
+
+            await App.AssertOutputLineStartsWith("  ❔ Do you want to restart your app? Yes (y) / No (n) / Always (a) / Never (v)", failure: _ => false);
+
+            App.AssertOutputContains("⌚ Restart is needed to apply the changes.");
+            App.AssertOutputContains($"❌ {programPath}(33,11): error ENC0023: Adding an abstract method or overriding an inherited method requires restarting the application.");
+            App.Process.ClearOutput();
+
+            App.SendKey('a');
+
+            await App.AssertOutputLineStartsWith(MessageDescriptor.WaitingForChanges, failure: _ => false);
+
+            App.AssertOutputContains($"[WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Exited");
+            App.AssertOutputContains($"[WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Launched");
+            App.Process.ClearOutput();
+
+            // rude edit: deleting virtual method
+            UpdateSourceFile(programPath, src => src.Replace("public virtual void F() {}", ""));
+
+            await App.AssertOutputLineStartsWith(MessageDescriptor.WaitingForChanges, failure: _ => false);
+
+            App.AssertOutputContains("⌚ Restart is needed to apply the changes");
+            App.AssertOutputContains($"⌚ [auto-restart] {programPath}(33,1): error ENC0033: Deleting method 'F()' requires restarting the application.");
+            App.AssertOutputContains($"[WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Exited");
+            App.AssertOutputContains($"[WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Launched");
+        }
+
         [Theory]
         [CombinatorialData]
         public async Task AutoRestartOnNoEffectEdit(bool nonInteractive)
@@ -760,7 +801,7 @@ public async Task Aspire()
             await App.AssertOutputLineStartsWith("  ❔ Do you want to restart these projects? Yes (y) / No (n) / Always (a) / Never (v)");
 
             App.AssertOutputContains("dotnet watch ⌚ Restart is needed to apply the changes.");
-            App.AssertOutputContains("error ENC0020: Renaming record 'WeatherForecast' requires restarting the application.");
+            App.AssertOutputContains($"dotnet watch ❌ {serviceSourcePath}(36,1): error ENC0020: Renaming record 'WeatherForecast' requires restarting the application.");
             App.AssertOutputContains("dotnet watch ⌚ Affected projects:");
             App.AssertOutputContains("dotnet watch ⌚   WatchAspire.ApiService");
             App.Process.ClearOutput();
diff --git a/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs b/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs
@@ -27,7 +27,7 @@ public async Task ReferenceOutputAssembly_False()
             reporter);
 
         var projectGraph = factory.TryLoadProjectGraph(projectGraphRequired: false);
-        var handler = new CompilationHandler(reporter, environmentOptions, new GlobalOptions(), CancellationToken.None);
+        var handler = new CompilationHandler(reporter, environmentOptions, CancellationToken.None);
 
         await handler.Workspace.UpdateProjectConeAsync(hostProject, CancellationToken.None);
 

Original file line number	Diff line number	Diff line change
`@@ -5,14 +5,14 @@ namespace Microsoft.DotNet.Watch`
`5`	`5`	`{`
`6`	`6`	`internal sealed class RestartPrompt(IReporter reporter, ConsoleInputReader requester, bool? noPrompt)`
`7`	`7`	`{`
`8`		`- private bool? _restartImmediatelySessionPreference = noPrompt;`
	`8`	`+ public bool? AutoRestartPreference { get; private set; } = noPrompt;`
`9`	`9`
`10`	`10`	`public async ValueTask<bool> WaitForRestartConfirmationAsync(string question, CancellationToken cancellationToken)`
`11`	`11`	`{`
`12`		`- if (_restartImmediatelySessionPreference.HasValue)`
	`12`	`+ if (AutoRestartPreference.HasValue)`
`13`	`13`	`{`
`14`	`14`	`reporter.Output("Restarting");`
`15`		`- return _restartImmediatelySessionPreference.Value;`
	`15`	`+ return AutoRestartPreference.Value;`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`var key = await requester.GetKeyAsync(`
`@@ -30,11 +30,11 @@ public async ValueTask<bool> WaitForRestartConfirmationAsync(string question, Ca`
`30`	`30`	`return false;`
`31`	`31`
`32`	`32`	`case ConsoleKey.A:`
`33`		`- _restartImmediatelySessionPreference = true;`
	`33`	`+ AutoRestartPreference = true;`
`34`	`34`	`return true;`
`35`	`35`
`36`	`36`	`case ConsoleKey.V:`
`37`		`- _restartImmediatelySessionPreference = false;`
	`37`	`+ AutoRestartPreference = false;`
`38`	`38`	`return false;`
`39`	`39`	`}`
`40`	`40`