diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index 1488fbd1a0..668da781cd 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -26,6 +26,14 @@ public sealed class PausePointCompiledLineMapWarningTests private const string GenericPatchedMethodsUseEditedFileSentence = "Methods currently patched by hot reload resolve against the edited file instead"; + private const string CompiledMethodSpanFixtureFile = + "Assets/Tests/Editor/SourcePausePointResolver/Fixtures/CompiledMethodSpanFixture.cs"; + + private const int CompiledMethodSpanFixtureBlankLine = 12; + + private const string MissingEditedLineFile = + "Assets/Tests/Editor/SourcePausePointResolver/Fixtures/DoesNotExistEditedLineRead.cs"; + [SetUp] public void SetUp() { @@ -140,11 +148,12 @@ public void BuildCompiledLineMapResolveFailureWarningOrEmpty_WhenPatchesAreInact [Test] public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsDiffer_ReturnsFormattedWarning() { - string warning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( " return 1; ", "return 2;", ForwardSlashFile, - 17); + 17, + true); Assert.That( warning, @@ -163,37 +172,332 @@ public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsDiffer_ReturnsFormatte [Test] public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsMatchAfterTrim_ReturnsEmpty() { - string warning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( " return 1; ", "return 1;", ForwardSlashFile, - 17); + 17, + true); Assert.That(warning, Is.EqualTo(string.Empty)); } /// - /// What: a missing compiled or edited line skips the comparison instead of warning. + /// What: a missing compiled line, or a failed edited-line read, skips the comparison. /// [Test] - public void BuildCompiledLineDriftWarningOrEmpty_WhenEitherSideIsEmpty_ReturnsEmpty() + public void BuildCompiledLineDriftWarningOrEmpty_WhenCompiledMissingOrEditedReadFails_ReturnsEmpty() { Assert.That( - PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( string.Empty, "return 1;", ForwardSlashFile, - 17), + 17, + true), Is.EqualTo(string.Empty)); Assert.That( - PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( "return 1;", string.Empty, ForwardSlashFile, - 17), + 17, + false), Is.EqualTo(string.Empty)); } + /// + /// What: a successfully read blank edited line at the resolved line is drift, not silence. + /// + [Test] + public void BuildCompiledLineDriftWarningOrEmpty_WhenEditedLineIsBlankAndReadSucceeded_ReturnsBlankDriftWarning() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( + " { ", + " ", + ForwardSlashFile, + 109, + true); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable.")); + } + + /// + /// What: a blank line that exists in an on-disk fixture is a successful read of empty text. + /// + [Test] + public void ReadEditedLineText_WhenLineIsBlank_ReturnsReadOkWithEmptyText() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + CompiledMethodSpanFixtureFile, + CompiledMethodSpanFixtureBlankLine); + + Assert.That(readOk, Is.True); + Assert.That(text, Is.EqualTo(string.Empty)); + } + + /// + /// What: a line number past the end of an on-disk fixture is a failed read, not a blank line. + /// + [Test] + public void ReadEditedLineText_WhenLineIsPastEndOfFile_ReturnsReadFailed() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + CompiledMethodSpanFixtureFile, + UnresolvableLine); + + Assert.That(readOk, Is.False); + Assert.That(text, Is.EqualTo(string.Empty)); + } + + /// + /// What: a missing file is a failed read, not a blank line. + /// + [Test] + public void ReadEditedLineText_WhenFileDoesNotExist_ReturnsReadFailed() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + MissingEditedLineFile, + 1); + + Assert.That(readOk, Is.False); + Assert.That(text, Is.EqualTo(string.Empty)); + } + + /// + /// What: a forward snap from the requested line names the requested edited text and the armed method. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineDiffers_ReturnsSnapDisclosure() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + true, + " LastRemainingBlocks = remainingBlocks; "); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: snap disclosure stays silent when the marker did not leave the requested line. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineEqualsRequestedLine_ReturnsEmpty() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 109, + 109, + "GameDirector.ComputeScoreTarget", + true, + "LastRemainingBlocks = remainingBlocks;"); + + Assert.That(warning, Is.EqualTo(string.Empty)); + } + + /// + /// What: a blank requested line still discloses the snap without quoting empty text. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenRequestedLineIsBlank_ReturnsBlankSnapDisclosure() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + true, + " "); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is blank in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: a failed requested-line read discloses the snap without claiming the line was blank. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenRequestedLineReadFails_OmitsEditedText() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + false, + string.Empty); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: snap disclosure precedes resolved-line drift when both apply, using the existing + /// non-blank drift sentence unchanged. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapAndNonBlankDrift_PutsSnapBeforeDrift() + { + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 10, + 17, + "Example.Run", + "return 1;", + true, + "return 3;", + true, + "return 2;", + 0, + 0, + Array.Empty()); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 10 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 17 in 'Example.Run'. " + + "'Assets/Scripts/Example.cs' line 17 is 'return 1;' in the last compiled source but 'return 3;' in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable.")); + } + + /// + /// What: a forward snap onto a blank edited resolved line keeps a drift warning, discloses + /// the snap, and lists the compiled line that matches the requested edited statement. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapAndBlankResolvedLine_DisclosesSnapBlankDriftAndRequestedCandidate() + { + string[] compiledSourceLines = new string[104]; + for (int index = 0; index < 103; index++) + { + compiledSourceLines[index] = "class Sample"; + } + + compiledSourceLines[103] = " LastRemainingBlocks = remainingBlocks;"; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + string.Empty, + true, + "LastRemainingBlocks = remainingBlocks;", + 100, + 120, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable. " + + "In the last compiled source, 'GameDirector.ComputeScoreTarget' spans lines 100-120. " + + "Candidate: the text at --line 107 in the edited file appears at line 104 in the last compiled source.")); + } + + /// + /// What: a snap-only warning still lists a compiled match for the requested --line text + /// and does not list the armed line as a candidate for its own text. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapOnly_OmitsResolvedSelfCandidate() + { + string[] compiledSourceLines = + { + "class Sample", + " {", + " LastRemainingBlocks = remainingBlocks;", + " {" + }; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + "{", + true, + "LastRemainingBlocks = remainingBlocks;", + 100, + 120, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "In the last compiled source, 'GameDirector.ComputeScoreTarget' spans lines 100-120. " + + "Candidate: the text at --line 107 in the edited file appears at line 3 in the last compiled source.")); + } + + /// + /// What: when resolved-line drift and a distinct requested-line match both exist, the + /// two candidate sentences name different searches. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenDriftAndRequestedLineBothMatch_DistinguishesCandidateSentences() + { + string[] compiledSourceLines = + { + "class Sample", + " return 3;", + " LastRemainingBlocks = remainingBlocks;" + }; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + "return 3;", + true, + "LastRemainingBlocks = remainingBlocks;", + 0, + 0, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but 'return 3;' in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable. " + + "Candidate: the edited line's text appears at line 2 in the last compiled source. " + + "Candidate: the text at --line 107 in the edited file appears at line 3 in the last compiled source.")); + } + /// /// What: enable on an unpatched method in a hot-reloaded file merges the exact drift /// warning and sets the drift next-action when compiled vs edited text differ. @@ -283,6 +587,85 @@ public void Enable_WhenCompiledLineDriftsFromEditedFile_AddsDriftWarningAndNextA } } + /// + /// What: enable on a comment line that rounds forward discloses the snap even when the + /// armed compiled and edited texts match, and still sets the drift next-action. + /// + [Test] + public void Enable_WhenRequestedLineSnapsForward_DisclosesSnapAndSetsNextAction() + { + Func previousLookup = + HotReloadPausePointCoordination.GetShimLookupForFile; + Func previousSnapshot = + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile; + HotReloadShimFileLookup stubLookup = new HotReloadShimFileLookup( + Array.Empty(), + Array.Empty(), + null, + Array.Empty()); + + string absolutePath = Path.Combine( + UnityCliLoopPathResolver.GetProjectRoot(), + ResolveFailureFile); + string diskSource = File.ReadAllText(absolutePath); + int requestedLine = FindLineNumberContaining( + diskSource, + "compiled-line-drift" + "-probe-unique"); + Assert.That(requestedLine, Is.GreaterThan(0)); + int compiledResolvedLine = requestedLine + 1; + + try + { + HotReloadPausePointCoordination.GetShimLookupForFile = _ => stubLookup; + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile = _ => diskSource; + + PausePointResponse response = new PausePointUseCase().Enable(new EnablePausePointSchema + { + File = ResolveFailureFile, + Line = requestedLine, + TimeoutSeconds = 30, + Mode = UloopPausePointCaptureMode.SingleShot + }); + + Assert.That( + response.Success, + Is.True, + response.ErrorCode + " / " + response.Message + " / " + response.RecommendedNextAction); + Assert.That(response.ResolvedLine, Is.EqualTo(compiledResolvedLine)); + SourcePausePointResolveResult spanResult = SourcePausePointResolver.Resolve( + ResolveFailureFile, + response.ResolvedLine); + Assert.That(spanResult.Success, Is.True, spanResult.ErrorMessage); + string requestedEditedText = "// compiled-line-drift" + "-probe-unique"; + string expectedSnap = + "'" + ResolveFailureFile + "' --line " + requestedLine + + " is '" + requestedEditedText + "' in the edited file, but the marker snapped forward to line " + + compiledResolvedLine + " in '" + response.ResolvedMethod + "'." + + " In the last compiled source, '" + response.ResolvedMethod + "' spans lines " + + spanResult.Resolution.CompiledMethodStartLine + "-" + + spanResult.Resolution.CompiledMethodEndLine + "." + + " Candidate: the text at --line " + requestedLine + + " in the edited file appears at line " + requestedLine + + " in the last compiled source."; + string expectedWarning = PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.CreateEnableWarning(), + PausePointEnableWarnings.BuildCompiledLineMapWarningOrEmpty(true, ResolveFailureFile)), + expectedSnap), + SourcePausePointConstants.SmallMethodInliningRiskWarning); + Assert.That(response.Warning, Is.EqualTo(expectedWarning)); + Assert.That( + response.RecommendedNextAction, + Is.EqualTo(SourcePausePointConstants.HotReloadCompiledLineMapLineDriftNextAction)); + } + finally + { + HotReloadPausePointCoordination.GetShimLookupForFile = previousLookup; + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile = previousSnapshot; + } + } + /// /// What: a known compiled span is appended to a non-empty drift warning. /// @@ -488,6 +871,67 @@ public void AppendCandidateCompiledLinesToDriftWarningOrUnchanged_WhenDriftIsEmp Assert.That(warning, Is.EqualTo(string.Empty)); } + /// + /// What: one compiled-source match for the requested --line text names that --line. + /// + [Test] + public void AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged_WhenOneLineMatches_NamesRequestedLine() + { + string drift = + "'Assets/Scripts/Example.cs' --line 107 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 109 in 'Example.Run'."; + string[] compiledLines = + { + "class Sample", + " return 2;", + " return 1;" + }; + + string warning = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + drift, + 107, + " return 2; ", + compiledLines); + + Assert.That( + warning, + Is.EqualTo( + drift + + " Candidate: the text at --line 107 in the edited file appears at line 2 in the last compiled source.")); + } + + /// + /// What: more than three compiled-source matches for the requested --line text cap at the + /// first three and name that --line. + /// + [Test] + public void AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged_WhenMoreThanThreeLinesMatch_CapsAtFirstThree() + { + string drift = + "'Assets/Scripts/Example.cs' --line 107 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 109 in 'Example.Run'."; + string[] compiledLines = + { + " return 2;", + " return 1;", + " return 2;", + " return 2;", + " return 2;" + }; + + string warning = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + drift, + 107, + "return 2;", + compiledLines); + + Assert.That( + warning, + Is.EqualTo( + drift + + " Candidate: the text at --line 107 in the edited file appears at lines 1, 3, 4 (first 3 matches) in the last compiled source.")); + } + /// /// What: retarget warning interpolates resolved method, requested line, and edited span. /// diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs new file mode 100644 index 0000000000..3a90a4e9bf --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs @@ -0,0 +1,196 @@ +using System; +using System.Collections.Generic; +using System.IO; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds compiled-vs-edited line-drift and requested-line snap warnings for pause-point enable. + /// + internal static class PausePointCompiledLineComparisonWarnings + { + // Why success-only: resolve failure leaves ResolvedMethod and ResolvedLineText empty, + // so this wording would point at fields that are not on the response. + // Why same resolvedLine on both sides: the resolver rounds empty/comment lines forward, + // so comparing the requested line to the resolved line is a false drift. + // Why readOk is distinct from empty text: a blank edited line is a real mismatch; + // a failed read is not evidence of drift. + internal static string BuildCompiledLineDriftWarningOrEmpty( + string compiledLineText, + string editedLineText, + string file, + int resolvedLine, + bool editedLineReadOk) + { + if (string.IsNullOrEmpty(compiledLineText) || !editedLineReadOk) + { + return string.Empty; + } + + string compiledTrimmed = compiledLineText.Trim(); + string editedTrimmed = editedLineText == null ? string.Empty : editedLineText.Trim(); + if (editedTrimmed.Length == 0) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineMapBlankEditedLineDriftWarningFormat, + SourcePausePointPathNormalizer.ToForwardSlashes(file), + resolvedLine, + compiledTrimmed); + } + + if (string.Equals(compiledTrimmed, editedTrimmed, StringComparison.Ordinal)) + { + return string.Empty; + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineMapLineDriftWarningFormat, + SourcePausePointPathNormalizer.ToForwardSlashes(file), + resolvedLine, + compiledTrimmed, + editedTrimmed); + } + + // Why resolvedLine != requestedLine only: the compiled resolver rounds empty and comment + // lines forward, so this inequality is the snap and has no false positive on this path. + internal static string BuildLineSnapDisclosureWarningOrEmpty( + string file, + int requestedLine, + int resolvedLine, + string resolvedMethod, + bool requestedLineReadOk, + string requestedLineEditedText) + { + if (requestedLine <= 0 || resolvedLine <= 0 || resolvedLine == requestedLine) + { + return string.Empty; + } + + string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(file); + string methodDisplay = resolvedMethod ?? string.Empty; + if (!requestedLineReadOk) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureWithoutEditedTextFormat, + normalizedFile, + requestedLine, + resolvedLine, + methodDisplay); + } + + string requestedTrimmed = requestedLineEditedText == null + ? string.Empty + : requestedLineEditedText.Trim(); + if (requestedTrimmed.Length == 0) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureBlankRequestedLineFormat, + normalizedFile, + requestedLine, + resolvedLine, + methodDisplay); + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureFormat, + normalizedFile, + requestedLine, + requestedTrimmed, + resolvedLine, + methodDisplay); + } + + // Why snap before resolved-line drift: the requested line is what the agent passed; + // the armed line is what actually paused. + // Why resolved-text candidates only with a drift sentence: a snap-only warning already + // named the armed line, so searching that same text finds the armed line itself. + // Why still search requested-line text on a snap-only warning: the intended statement is + // on --line, including when braces at the armed line happen to match. + // Why skip the requested-line search when texts match: the suffix would duplicate. + internal static string ComposeCompiledLineDriftAndSnapWarningOrEmpty( + string file, + int requestedLine, + int resolvedLine, + string resolvedMethod, + string compiledResolvedLineText, + bool resolvedEditedLineReadOk, + string resolvedEditedLineText, + bool requestedEditedLineReadOk, + string requestedEditedLineText, + int compiledMethodStartLine, + int compiledMethodEndLine, + IReadOnlyList compiledSourceLines) + { + string snapWarning = BuildLineSnapDisclosureWarningOrEmpty( + file, + requestedLine, + resolvedLine, + resolvedMethod, + requestedEditedLineReadOk, + requestedEditedLineText); + string driftWarning = BuildCompiledLineDriftWarningOrEmpty( + compiledResolvedLineText, + resolvedEditedLineText, + file, + resolvedLine, + resolvedEditedLineReadOk); + string combined = PausePointEnableWarnings.MergeWarnings(snapWarning, driftWarning); + combined = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( + combined, + resolvedMethod, + compiledMethodStartLine, + compiledMethodEndLine); + if (driftWarning.Length > 0) + { + combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + combined, + resolvedEditedLineText, + compiledSourceLines); + } + + string resolvedTrimmed = resolvedEditedLineText == null + ? string.Empty + : resolvedEditedLineText.Trim(); + string requestedTrimmed = requestedEditedLineText == null + ? string.Empty + : requestedEditedLineText.Trim(); + if (!string.Equals(resolvedTrimmed, requestedTrimmed, StringComparison.Ordinal)) + { + combined = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + combined, + requestedLine, + requestedEditedLineText, + compiledSourceLines); + } + + return combined; + } + + // Why not ReadLineTextFromSource: that helper returns empty for both a missing line + // and a blank line, which used to suppress a real blank-vs-compiled mismatch. + internal static (bool readOk, string text) ReadEditedLineText(string requestedFile, int lineNumber) + { + if (string.IsNullOrEmpty(requestedFile) || lineNumber <= 0) + { + return (false, string.Empty); + } + + string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(requestedFile); + string absoluteFilePath = Path.Combine(UnityCliLoopPathResolver.GetProjectRoot(), normalizedFile); + if (!File.Exists(absoluteFilePath)) + { + return (false, string.Empty); + } + + string[] lines = SourcePausePointSourceLineReader.SplitSourceLines(File.ReadAllText(absoluteFilePath)); + if (lineNumber > lines.Length) + { + return (false, string.Empty); + } + + return (true, lines[lineNumber - 1].Trim()); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta new file mode 100644 index 0000000000..4e28426d80 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4832e38c6a4704af9abc041a6f182305 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs index 51b5d24308..d93bcd2e62 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.IO; using UnityEditor; using UnityEngine; @@ -139,36 +138,6 @@ private static string FormatTypeMethodDisplay(string resolvedMethod, string simp return typeName + "." + simpleName; } - // Why success-only: resolve failure leaves ResolvedMethod and ResolvedLineText empty, - // so this wording would point at fields that are not on the response. - // Why same resolvedLine on both sides: the resolver rounds empty/comment lines forward, - // so comparing the requested line to the resolved line is a false drift. - internal static string BuildCompiledLineDriftWarningOrEmpty( - string compiledLineText, - string editedLineText, - string file, - int resolvedLine) - { - if (string.IsNullOrEmpty(compiledLineText) || string.IsNullOrEmpty(editedLineText)) - { - return string.Empty; - } - - string compiledTrimmed = compiledLineText.Trim(); - string editedTrimmed = editedLineText.Trim(); - if (string.Equals(compiledTrimmed, editedTrimmed, StringComparison.Ordinal)) - { - return string.Empty; - } - - return string.Format( - SourcePausePointConstants.HotReloadCompiledLineMapLineDriftWarningFormat, - SourcePausePointPathNormalizer.ToForwardSlashes(file), - resolvedLine, - compiledTrimmed, - editedTrimmed); - } - internal static string AppendCompiledMethodSpanToDriftWarningOrUnchanged( string driftWarning, string resolvedMethod, @@ -190,7 +159,8 @@ internal static string AppendCompiledMethodSpanToDriftWarningOrUnchanged( } // Why only after a non-empty drift warning: a candidate list without drift would look - // like a second resolution, and empty edited text never produces drift in the first place. + // like a second resolution. + // Why skip empty edited text: a blank line has no statement to locate in compiled source. internal static string AppendCandidateCompiledLinesToDriftWarningOrUnchanged( string driftWarning, string editedLineText, @@ -223,6 +193,44 @@ internal static string AppendCandidateCompiledLinesToDriftWarningOrUnchanged( return driftWarning + FormatCandidateCompiledLinesSuffix(matches, truncated); } + // Why a distinct sentence: the resolved-line candidate does not name --line, so two + // identical "edited line" suffixes would not say which search produced which hit. + internal static string AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + string driftWarning, + int requestedLine, + string requestedLineEditedText, + IReadOnlyList compiledSourceLines) + { + if (string.IsNullOrEmpty(driftWarning) || requestedLine <= 0) + { + return driftWarning ?? string.Empty; + } + + if (string.IsNullOrEmpty(requestedLineEditedText) || compiledSourceLines == null) + { + return driftWarning; + } + + string editedTrimmed = requestedLineEditedText.Trim(); + if (editedTrimmed.Length == 0) + { + return driftWarning; + } + + (List matches, bool truncated) = CollectCandidateCompiledLineNumbers( + editedTrimmed, + compiledSourceLines); + if (matches.Count == 0) + { + return driftWarning; + } + + return driftWarning + FormatRequestedLineCandidateCompiledLinesSuffix( + requestedLine, + matches, + truncated); + } + private static (List matches, bool truncated) CollectCandidateCompiledLineNumbers( string editedTrimmed, IReadOnlyList compiledSourceLines) @@ -277,6 +285,33 @@ private static string FormatCandidateCompiledLinesSuffix(List matches, bool listed); } + private static string FormatRequestedLineCandidateCompiledLinesSuffix( + int requestedLine, + List matches, + bool truncated) + { + if (matches.Count == 1 && !truncated) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftRequestedLineCandidateSingleFormat, + requestedLine, + matches[0]); + } + + string listed = string.Join(", ", matches); + if (truncated) + { + listed += string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftCandidateTruncatedMatchesSuffixFormat, + SourcePausePointConstants.CompiledLineDriftCandidateMatchLimit); + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftRequestedLineCandidateMultipleFormat, + requestedLine, + listed); + } + internal static string BuildRetargetedToHotReloadPatchWarningOrEmpty( bool retargetedToHotReloadPatch, string resolvedMethod, @@ -323,25 +358,6 @@ internal static string AppendNearbyCompiledMethodsSuffix( + "."; } - internal static string ReadEditedLineTextOrEmpty(string requestedFile, int resolvedLine) - { - if (string.IsNullOrEmpty(requestedFile) || resolvedLine <= 0) - { - return string.Empty; - } - - string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(requestedFile); - string absoluteFilePath = Path.Combine(UnityCliLoopPathResolver.GetProjectRoot(), normalizedFile); - if (!File.Exists(absoluteFilePath)) - { - return string.Empty; - } - - return SourcePausePointSourceLineReader.ReadLineTextFromSource( - File.ReadAllText(absoluteFilePath), - resolvedLine); - } - internal static string BuildPatchedMethodPdbUnavailableWarningOrEmpty( bool patchedMethodPdbUnavailable, string methodDisplayName, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs index f795bae18c..92d52535b1 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs @@ -387,21 +387,23 @@ private static PausePointResponse FinishEnableBySourceLocation( enableWarning = PausePointEnableWarnings.MergeWarnings(enableWarning, compiledLineMapWarning); if (compareCompiledLineDrift) { - string editedLineText = PausePointEnableWarnings.ReadEditedLineTextOrEmpty(parameters.File, resolvedLine); - string driftWarning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( - resolvedLineText, - editedLineText, + (bool resolvedEditedReadOk, string resolvedEditedLineText) = + PausePointCompiledLineComparisonWarnings.ReadEditedLineText(parameters.File, resolvedLine); + (bool requestedEditedReadOk, string requestedEditedLineText) = + PausePointCompiledLineComparisonWarnings.ReadEditedLineText(parameters.File, parameters.Line); + string[] compiledSourceLines = SourcePausePointSourceLineReader.SplitSourceLines(compiledSnapshotSource); + string driftWarning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( parameters.File, - resolvedLine); - driftWarning = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( - driftWarning, + parameters.Line, + resolvedLine, resolvedMethod, + resolvedLineText, + resolvedEditedReadOk, + resolvedEditedLineText, + requestedEditedReadOk, + requestedEditedLineText, compiledMethodStartLine, - compiledMethodEndLine); - string[] compiledSourceLines = SourcePausePointSourceLineReader.SplitSourceLines(compiledSnapshotSource); - driftWarning = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( - driftWarning, - editedLineText, + compiledMethodEndLine, compiledSourceLines); enableWarning = PausePointEnableWarnings.MergeWarnings(enableWarning, driftWarning); if (driftWarning.Length > 0) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 1086c77dc0..4fb3fd9333 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -240,6 +240,26 @@ internal static class SourcePausePointConstants + "The marker is armed on the compiled statement. If that is not the statement you meant, " + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable."; + // Format: file, resolved line, compiled line text. + // Why a distinct sentence: quoting an empty edited line as '' looks like a missing field. + public const string HotReloadCompiledLineMapBlankEditedLineDriftWarningFormat = + "'{0}' line {1} is '{2}' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable."; + + // Format: file, requested line, requested edited text, resolved line, resolved method. + public const string HotReloadCompiledLineSnapDisclosureFormat = + "'{0}' --line {1} is '{2}' in the edited file, but the marker snapped forward to line {3} in '{4}'."; + + // Format: file, requested line, resolved line, resolved method. + public const string HotReloadCompiledLineSnapDisclosureBlankRequestedLineFormat = + "'{0}' --line {1} is blank in the edited file, but the marker snapped forward to line {2} in '{3}'."; + + // Format: file, requested line, resolved line, resolved method. + // Why omit edited text: a failed read is not the same as a blank line. + public const string HotReloadCompiledLineSnapDisclosureWithoutEditedTextFormat = + "'{0}' --line {1} snapped forward to line {2} in '{3}'."; + public const string HotReloadCompiledLineMapLineDriftNextAction = "Verify ResolvedLineText is the statement you intended. If it is not, run 'uloop compile' " + "and re-enable the pause point."; @@ -276,6 +296,15 @@ internal static class SourcePausePointConstants public const string HotReloadCompiledLineDriftCandidateMultipleFormat = " Candidate: the edited line's text appears at lines {0} in the last compiled source."; + // Format: requested --line, 1-based compiled line number. + public const string HotReloadCompiledLineDriftRequestedLineCandidateSingleFormat = + " Candidate: the text at --line {0} in the edited file appears at line {1} in the last compiled source."; + + // Format: requested --line, comma-separated 1-based compiled line numbers, with an + // optional truncation note. + public const string HotReloadCompiledLineDriftRequestedLineCandidateMultipleFormat = + " Candidate: the text at --line {0} in the edited file appears at lines {1} in the last compiled source."; + // Why format from CompiledLineDriftCandidateMatchLimit: a hard-coded "3" would lie // if the cap changed. public const string HotReloadCompiledLineDriftCandidateTruncatedMatchesSuffixFormat =