From c3174d7c3f353b8a4468a8017c996c018342ab24 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Mon, 10 Aug 2026 15:09:40 -0400 Subject: [PATCH] Add function-level merge fallback for whole-file merge failures DiffPlexMergeEngine.MergeHeadless merges whole files line-by-line, which means a handful of real, incidental whitespace/comment differences (or a confirmed upstream DiffPlex bug that gets worse as edit density rises) can block an entire file from auto-solving even when the actual overlapping logic changes are confined to one or two functions. Adds a fallback, activated only at the two points where the whole-file merge has already failed for a given pairwise step: split vanilla and both sides into function/field units (ScriptUnitExtractor, a brace/paren-matching tokenizer - WitcherScript has no nested functions, so this doesn't need a full parser), align each side against vanilla by name (UnitAligner, LCS-based, handling both insertions and deletions), and resolve each function independently (FunctionLevelMergeEngine) - cheap shortcuts for untouched or single-sided edits, a real 3-way merge for non-overlapping changes, and a most-distinct-from-vanilla tiebreak for genuine collisions. An edit always wins over a competing deletion. Every non-mechanical resolution is recorded in an audit trail (DiffPlexMergeEngine.LastFunctionLevelDecisions -> FileMerger.HeadlessMergeSummary.FunctionLevelDecisions -> both hosts' CLI output and the MCP merge_conflicts tool's functionLevelDecisions field), never applied silently. Validated empirically before and after building: a real live install's actor.ws conflict (6 real overhaul mods) showed only 6 of 395 functions were genuine two-mod collisions, confirming per-function decomposition was worth building. A chain-step replay against that same install's 5 real-world unresolved conflicts found an insertion-only alignment would rescue only 2 of 5 (one mod deletes several vanilla functions outright, and that deletion persists through the merge chain even at steps where the deleting mod isn't a direct input) - UnitAligner's symmetric insert/delete handling was added specifically to cover this. Final end-to-end validation against an isolated copy of the same 5 real files got all 5 to merge successfully, including actor.ws itself, with well-formed output and no conflict markers. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah --- WitcherScriptMerger.Core/CLAUDE.md | 85 +++- .../Inventory/FileMerger.cs | 43 +- WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs | 9 +- .../Tools/DiffPlexMergeEngine.cs | 98 +++- .../Tools/FunctionLevelMergeEngine.cs | 333 +++++++++++++ .../Tools/ScriptUnitExtractor.cs | 453 ++++++++++++++++++ WitcherScriptMerger.Core/Tools/UnitAligner.cs | 106 ++++ WitcherScriptMerger.Headless/Program.cs | 2 + WitcherScriptMerger.Tests/CLAUDE.md | 13 + .../Tools/FunctionLevelMergeEngineTests.cs | 258 ++++++++++ .../Tools/ScriptUnitExtractorTests.cs | 241 ++++++++++ .../Tools/UnitAlignerTests.cs | 140 ++++++ WitcherScriptMerger/Program.cs | 2 + 13 files changed, 1774 insertions(+), 9 deletions(-) create mode 100644 WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs create mode 100644 WitcherScriptMerger.Core/Tools/ScriptUnitExtractor.cs create mode 100644 WitcherScriptMerger.Core/Tools/UnitAligner.cs create mode 100644 WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs create mode 100644 WitcherScriptMerger.Tests/Tools/ScriptUnitExtractorTests.cs create mode 100644 WitcherScriptMerger.Tests/Tools/UnitAlignerTests.cs diff --git a/WitcherScriptMerger.Core/CLAUDE.md b/WitcherScriptMerger.Core/CLAUDE.md index 52045c3..e53305d 100644 --- a/WitcherScriptMerger.Core/CLAUDE.md +++ b/WitcherScriptMerger.Core/CLAUDE.md @@ -27,7 +27,9 @@ still external dependencies rather than an in-process replacement. (UTF-16LE+BOM normalization — see "Text-merge input encoding" below), `DiffPlexMergeEngine.cs` (see below), `FileOpener.cs` (portable "open in the OS's default associated app" helper; the only call site is - `DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar). + `DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar), + `ScriptUnitExtractor.cs`/`UnitAligner.cs`/`FunctionLevelMergeEngine.cs` (the + function-level merge fallback — see "Function-level merge engine" below). - `Cli/` — `MergeOperations.cs`: the scan-then-merge sequence shared by both hosts' `merge` CLI verb and by the MCP tools (see "CLI & MCP orchestration" below). - `Mcp/` — `WsmMcpTools.cs`: the MCP server's tool implementations (see below and @@ -375,6 +377,87 @@ switching chunkers** — the bug reproduces under DiffPlex's own default/tested `LineChunker` too, just at a somewhat lower rate, so switching would trade a real, working byte-for-byte line-ending-preservation property for no actual safety gain. +## Function-level merge engine + +`Tools/ScriptUnitExtractor.cs`, `Tools/UnitAligner.cs`, and +`Tools/FunctionLevelMergeEngine.cs` are a **fallback**, activated only from inside +`DiffPlexMergeEngine.MergeHeadless` at the two points where the whole-file merge has +already failed for a given pairwise chain step (the `DiffAlgorithmException` catch and +the `HasConflicts` branch) — never a parallel code path, so every conflict that already +auto-solves via the whole-file engine is unaffected. The idea: split vanilla and both +sides of a pairwise merge into individual function/field units, resolve each +independently, then reassemble — most of a real `.ws` file's line-level "conflict" +surface comes from whitespace/comment noise around a handful of actually-edited +functions, not from genuine overlapping logic changes, and per-function merging sidesteps +that noise entirely (and, as a side effect, mitigates `DiffAlgorithmException`'s +worse-at-small-inputs failure rate above by attempting DiffPlex's inline 3-way merge at +function granularity only when both sides changed a given function differently, not on +the whole file at once). + +**Validated empirically before being built, not just unit-tested.** A throwaway +measurement against a real, live Witcher 3 install's `actor.ws` conflict (vanilla + +6 real overhaul mods) found only 6 of 395 functions were genuine two-mod collisions +(both sides edit the same function differently) — confirming per-function decomposition +was worth building rather than just relocating the same conflict into a smaller, +statistically more `DiffAlgorithmException`-prone box. A follow-up chain-step replay +against the same real install's 5 currently-unresolved conflicts found that a +naively insertion-only alignment (tolerating a mod adding new functions, but declining +outright the moment any mod deleted a vanilla function) would rescue only 2 of the 5 — +one real mod in that install deletes several vanilla functions outright, and that +deletion was found to persist through the merge chain into later steps even where the +deleting mod isn't a direct input, since an earlier *clean* whole-file merge step +faithfully propagates a one-sided deletion into the accumulated text. `UnitAligner` +handles insertions and deletions symmetrically (an LCS alignment of each side's unit +names against vanilla's) for this reason; final validation against the same 5 real +files, run against an isolated copy (never the live install directly), got all 5 to +merge successfully, including `actor.ws` itself. + +**Function identity is name-only** (`(scope-free) name`, no parameter signature) — +confirmed empirically against several large real vanilla files (`actor.ws`, `player.ws`, +`npc.ws`) that WitcherScript function names don't collide within a file in practice, so +overload-aware identity wasn't needed. + +**Extraction (`ScriptUnitExtractor`)** is a brace/paren-matching tokenizer, not a full +parser or a binding to the third-party `tree-sitter-witcherscript` grammar — confirmed +via direct research into WitcherScript's grammar that class/state/struct/enum +declarations are top-level only (never nested) and the language has no nested +function-like constructs at all (no lambdas, local functions, or closures), so a +function body only ever gains brace depth from control flow, never another function +declaration. That structural simplicity is what makes plain brace/paren counting +sufficient, as long as it's string/comment-aware (a single masking pass shared by both +the brace-safe extraction path and the public `StripComments` helper) so a brace or +paren inside a string literal or comment can never be mistaken for real syntax. Reuses +`Tools/FileEncoding.cs` for all file I/O — mod files are inconsistently encoded even +though vanilla is always UTF-16LE+BOM (see "Text-merge input encoding" below), the exact +same hazard this class's own callers already have to account for. + +**Per-function resolution (`FunctionLevelMergeEngine.TryMerge`)** tries cheap one-sided +shortcuts (unchanged, only-one-side-edited, both-sides-made-the-identical-edit) before +ever calling `DiffPlexMergeEngine.BuildMerge` — only a function genuinely edited +differently on both sides reaches a real per-function 3-way merge attempt, falling back +to a whole-function tiebreak (**most distinct from vanilla wins**, scored via +`DiffPlex.Differ`'s plain 2-way line diff over comment-stripped, whitespace-ignored text +— deliberately not the buggy `ThreeWayDiffer`) if that merge attempt itself conflicts or +throws `DiffAlgorithmException`. A vanilla function deleted on one side and edited on the +other resolves as **edit wins** (a deletion never silently overrides a surviving edit — +losing code silently is unrecoverable if the deletion was wrong, keeping an unwanted +edit is not) — every non-mechanical resolution (a tiebreak, an edit surviving a +competing deletion, a mod's gap comment not making it into the output) is recorded in a +`Decisions` audit trail, never applied silently. That trail is threaded all the way out: +`DiffPlexMergeEngine.LastFunctionLevelDecisions` → `FileMerger`'s +`HeadlessMergeSummary.FunctionLevelDecisions` → both hosts' CLI output and the MCP +`merge_conflicts` tool's `functionLevelDecisions` field. Not yet surfaced in the WinForms +GUI's `MergeReportForm` — a deliberate deferral, not an oversight, since that's UI work +on the host side rather than engine work here. + +**Scope note on non-function content ("gaps"):** a gap is only ever compared, and only +ever produces a `Decisions` note (never a decline), when it sits between two vanilla +units both sides kept and neither side inserted anything at that slot — comparison uses +the same whitespace-tolerant spirit as `IsWhitespaceOnlyDifference` above, deliberately +NOT comment-stripped (unlike the extraction masking pass) since the whole point is to +detect when comment *content* differs, not just formatting. Reassembly always keeps +vanilla's own gap text verbatim regardless. + ## Text-merge input encoding Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain diff --git a/WitcherScriptMerger.Core/Inventory/FileMerger.cs b/WitcherScriptMerger.Core/Inventory/FileMerger.cs index 0306b55..d1bceaa 100644 --- a/WitcherScriptMerger.Core/Inventory/FileMerger.cs +++ b/WitcherScriptMerger.Core/Inventory/FileMerger.cs @@ -61,6 +61,11 @@ public class HeadlessMergeSummary { public List Merged { get; } = new List(); public List Skipped { get; } = new List(); + // Audit notes from FunctionLevelMergeEngine (see DiffPlexMergeEngine. + // TryFunctionLevelRescue), each prefixed with the file it belongs to - + // empty whenever no conflict in this run needed the function-level + // fallback at all, which is the common case. + public List FunctionLevelDecisions { get; } = new List(); } // One file's interactive merge request, extracted by the host project's @@ -123,6 +128,15 @@ public class MergeReportData bool _bundleChanged; List _pendingBundleMerges = new List(); + // Drained into HeadlessMergeSummary.FunctionLevelDecisions at the end of + // MergeConflictsHeadless. Appended to (not overwritten) right after every + // MergeTextHeadless call, since _mergeEngine.LastFunctionLevelDecisions only + // reflects the single most recent pairwise MergeHeadless call - a multi-mod + // chain can trigger the function-level rescue at more than one step, and each + // one's decisions would otherwise be lost the moment the next chain step's + // MergeHeadless call resets that property back to empty. + List _functionLevelDecisions = new List(); + // Anchored at BOTH ends ("^...$") - see IsVanillaDlcBundleFolder's own comment // below for why this matters: it's matched against just the extracted folder-name // segment, not the full path, so a full-string match is required, not merely a @@ -534,6 +548,8 @@ public HeadlessMergeSummary MergeConflictsHeadless( } } + summary.FunctionLevelDecisions.AddRange(_functionLevelDecisions); + CleanUpTempFiles(); CleanUpEmptyDirectories(); @@ -571,15 +587,28 @@ bool MergeFlatConflictHeadless(ModFile conflict, Merge merge, string mergedModNa _vanillaFile = new FileInfo(conflict.GetVanillaFile()); + // Tracked locally, independent of merge.Mods (which can carry stale entries + // from a previous run when merge is a re-merge pulled from _inventory.Merges + // rather than freshly created) - this is only ever the real mod names folded + // into source1 so far within THIS chain, for FunctionLevelMergeEngine's + // Decisions[] audit text (see DiffPlexMergeEngine.TryFunctionLevelRescue's + // own comment on why source1.Name alone is misleading past the first step). + var accumulatedModNames = new List { orderedNames[0] }; + for (int i = 1; i < orderedNames.Length; ++i) { var hash = conflict.Mods.First(h => h.Name.EqualsIgnoreCase(orderedNames[i])); var source2 = MergeSource.FromFlatFile(new FileInfo(conflict.GetModFile(orderedNames[i])), hash); - var mergedFile = MergeTextHeadless(merge, source1, source2, dryRun); + var oldDescription = accumulatedModNames.Count > 1 + ? "accumulated merge (" + string.Join(", ", accumulatedModNames) + ")" + : accumulatedModNames[0]; + + var mergedFile = MergeTextHeadless(merge, source1, source2, dryRun, oldDescription, orderedNames[i]); if (mergedFile == null) return false; source1 = MergeSource.FromFlatFile(mergedFile, null); + accumulatedModNames.Add(orderedNames[i]); } return true; } @@ -700,7 +729,7 @@ string[] ResolveMergeOrder(ModFile conflict, string mergedModName, IReadOnlyDict .ToArray(); } - FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2, bool dryRun) + FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2, bool dryRun, string oldDescription = null, string newDescription = null) { ProgressInfo.CurrentAction = $"Merging {source1.Name} && {source2.Name}"; @@ -711,7 +740,15 @@ FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2 // this, MergeConflictsHeadless(dryRun: true) against a mods folder with N // genuine conflicts would pop open N editor windows - a real bug caught in // review before it shipped (see docs/decisions/kdiff3-retirement.md). - var result = _mergeEngine.MergeHeadless(source1, source2, _vanillaFile, _outputPath, openConflictMarkers: !dryRun); + var result = _mergeEngine.MergeHeadless( + source1, source2, _vanillaFile, _outputPath, openConflictMarkers: !dryRun, + oldDescription: oldDescription, newDescription: newDescription); + + if (_mergeEngine.LastFunctionLevelDecisions.Count > 0) + { + foreach (var decision in _mergeEngine.LastFunctionLevelDecisions) + _functionLevelDecisions.Add(merge.RelativePath + ": " + decision); + } if (result != MergeEngineResult.AutoSolved) return null; diff --git a/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs b/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs index 390cce1..a25b0ea 100644 --- a/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs +++ b/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs @@ -50,7 +50,12 @@ public static object ScanConflicts() [McpServerTool(Name = "merge_conflicts"), Description( "Merges detected conflicts headlessly; conflicts that can't be auto-solved are skipped " + "and reported, not merged - a conflict-marker sidecar file is written and opened in the " + - "default editor for manual review instead. Restrict to specific files with " + + "default editor for manual review instead. A conflict a whole-file merge can't resolve " + + "may still auto-solve via a function-level fallback (splits vanilla/mod .ws files into " + + "individual functions and resolves each independently); any such case is reported in " + + "functionLevelDecisions, one line per resolved function explaining which side's version " + + "was kept and why - always worth surfacing to the user, not just the merged/skipped " + + "counts. Restrict to specific files with " + "relativePaths (default: every detected conflict); override a file's mod merge order " + "with orderOverrides (default merge order otherwise matches the game's own load order). " + "Set dryRun to preview which conflicts would auto-solve without writing any merged " + @@ -143,7 +148,7 @@ public static object MergeConflicts( if (!dryRun) AppState.Inventory.Save(); - return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun }; + return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun, functionLevelDecisions = summary.FunctionLevelDecisions }; } } diff --git a/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs b/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs index 1fc5168..7d7bc70 100644 --- a/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs +++ b/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs @@ -116,13 +116,22 @@ public DiffAlgorithmException(string message) : base(message) { } #endregion + // Set (or reset to empty) at the start of every MergeHeadless call - see that + // method's own comment. A single instance of this class is reused for a whole + // headless run (FileMerger constructs one field), so this must never be left + // holding a previous file's decisions when the current file never reaches the + // function-level engine at all. + public IReadOnlyList LastFunctionLevelDecisions { get; private set; } = Array.Empty(); + public MergeEngineResult Merge( FileMerger.MergeSource source1, FileMerger.MergeSource source2, FileInfo vanillaFile, - string outputPath) + string outputPath, + string oldDescription = null, + string newDescription = null) { - var result = MergeHeadless(source1, source2, vanillaFile, outputPath); + var result = MergeHeadless(source1, source2, vanillaFile, outputPath, oldDescription: oldDescription, newDescription: newDescription); return result == MergeEngineResult.NeedsManualResolution ? MergeEngineResult.Failed : result; } @@ -147,8 +156,12 @@ public MergeEngineResult MergeHeadless( FileMerger.MergeSource source2, FileInfo vanillaFile, string outputPath, - bool openConflictMarkers = true) + bool openConflictMarkers = true, + string oldDescription = null, + string newDescription = null) { + LastFunctionLevelDecisions = Array.Empty(); + var hasVanillaVersion = vanillaFile != null && vanillaFile.Exists; // A 3-way merge is meaningless without a base to diff against - confirmed @@ -213,6 +226,15 @@ public MergeEngineResult MergeHeadless( } catch (DiffAlgorithmException ex) { + // Before giving up on the whole file: the function-level engine catches + // this same exception per-function (see FunctionLevelMergeEngine) and + // falls back to a whole-function tiebreak instead, so a file can still + // merge even when the whole-file 3-way diff hits this bug. Only + // attempted for .ws files - the extractor is WitcherScript-specific and + // has no notion of XML structure. + if (TryFunctionLevelRescue(baseText, oldText, newText, source1, source2, outputPath, oldDescription, newDescription)) + return MergeEngineResult.AutoSolved; + // DiffPlex's own diff algorithm produced output it isn't safe to trust // (see BuildMerge's comment) - don't write anything, including a sidecar: // the "conflict marker" content itself would have been built from the @@ -242,6 +264,15 @@ public MergeEngineResult MergeHeadless( return MergeEngineResult.AutoSolved; } + // Before falling back to conflict markers: same function-level rescue + // attempt as the DiffAlgorithmException catch above, for the "produced + // output, but with a real conflict block" case. See + // FunctionLevelMergeEngine's own comment for why this is a fallback that + // only ever activates where the whole-file merge has already failed, never + // a parallel code path for merges that would have succeeded anyway. + if (TryFunctionLevelRescue(baseText, oldText, newText, source1, source2, outputPath, oldDescription, newDescription)) + return MergeEngineResult.AutoSolved; + // Never write conflict markers to outputPath itself: FileMerger's headless // callers (MergeFlatConflictHeadless/MergeBundleConflictHeadless) check // `File.Exists(_outputPath)` BEFORE attempting a merge and, if it exists, @@ -366,6 +397,67 @@ static void DeleteIfExists(string path) try { if (File.Exists(path)) File.Delete(path); } catch { } } + // The function-level fallback (see FunctionLevelMergeEngine's own header + // comment). Called from both of MergeHeadless's give-up points, never from + // anywhere else - this method's whole job is "try to do better than the + // failure that's already about to happen," so every early return here means + // "the caller proceeds exactly as if this method didn't exist," never a worse + // outcome than today's baseline. + // + // oldDescription/newDescription default to source1.Name/source2.Name (the + // pre-existing marker-label convention) when not supplied - accurate for a + // merge chain's first pairwise step, but source1.Name resolves to the + // accumulated-merge output's own mod name (e.g. the configured merged-mod + // folder) from the second step onward, since FileMerger.MergeFlatConflictHeadless + // reassigns source1 to the prior step's output file. FileMerger passes a + // richer "accumulated merge (modA, modB)" description once it has more than + // one real mod recorded for this chain (see MergeTextHeadless/ + // MergeTextInteractive) so a Decisions[] note reads sensibly past the first + // step; this method doesn't know or care which case it's in. + bool TryFunctionLevelRescue( + string baseText, string oldText, string newText, + FileMerger.MergeSource source1, FileMerger.MergeSource source2, string outputPath, + string oldDescription, string newDescription) + { + if (!Path.GetExtension(outputPath).EqualsIgnoreCase(".ws")) + return false; + + FunctionLevelMergeResult result; + try + { + result = FunctionLevelMergeEngine.TryMerge( + baseText, oldText, newText, + source1.Name, source2.Name, + oldDescription ?? source1.Name, newDescription ?? source2.Name); + } + catch + { + // A latent bug in the new engine must never regress this method below + // its pre-existing behavior - the caller falls through to whatever it + // was already about to do (write a sidecar, or report the + // DiffAlgorithmException as-is) exactly as if this rescue attempt had + // declined outright. + return false; + } + + if (!result.Applied) + return false; + + DeleteIfExists(GetConflictMarkerPath(outputPath)); + FileEncoding.WriteUtf16(outputPath, result.MergedText); + LastFunctionLevelDecisions = result.Decisions; + + if (result.Decisions.Count > 0) + { + AppState.Notifier.ShowMessage( + $"Merged {source1.Name} + {source2.Name} at the function level after the whole-file merge " + + $"couldn't auto-solve it:\n\n" + string.Join("\n", result.Decisions), + "Merged (function-level)", NotifyButtons.OK, DialogIcon.Information); + } + + return true; + } + #region Merge algorithm // The actual 3-way merge, factored out as a public static method (independent of diff --git a/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs b/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs new file mode 100644 index 0000000..76f161e --- /dev/null +++ b/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs @@ -0,0 +1,333 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; +using DiffPlex; + +namespace WitcherScriptMerger.Tools +{ + public readonly struct FunctionLevelMergeResult + { + // False means every other field is meaningless - the caller keeps whatever it + // was about to do before considering this engine (write a conflict-marker + // sidecar, or report the DiffAlgorithmException as-is). This engine never makes + // a file-level outcome worse than that baseline - see TryMerge's own comment. + public bool Applied { get; } + public string MergedText { get; } + // Human-readable audit notes for every place this engine picked one side over + // another - a tiebreak, an edit surviving a competing deletion, or a mod's gap + // comment not making it into the merged output. Never empty content silently: + // every non-mechanical decision (i.e. every one that isn't "both sides agree" + // or "only one side touched this at all") gets a note. + public IReadOnlyList Decisions { get; } + + public FunctionLevelMergeResult(bool applied, string mergedText, IReadOnlyList decisions) + { + Applied = applied; + MergedText = mergedText; + Decisions = decisions; + } + + public static readonly FunctionLevelMergeResult Declined = new FunctionLevelMergeResult(false, null, null); + } + + // The function-level merge fallback (see WitcherScriptMerger.Core/CLAUDE.md's + // "Function-level merge engine" section for the full design rationale). Only ever + // called from DiffPlexMergeEngine.MergeHeadless at the two points where a whole-file merge + // has already failed for a given pairwise chain step - this class's whole contract + // is "try to do better than that specific failure, never worse." Every early-exit + // path here (extraction failure, a genuine new-functionality naming collision) + // returns FunctionLevelMergeResult.Declined, which the caller treats identically to + // this class not existing at all. + // + // Splits vanilla/old/new into ScriptUnitExtractor.Extract's function/field units, + // aligns old and new each independently against vanilla via UnitAligner (handling + // both insertions and deletions, not just insertions - see the plan's Step 0 + // addendum: a real, common WitcherScript modding pattern, one mod outright + // removing several vanilla functions, showed up on 3 of the 5 real files this + // feature was built to help with, and that removal was found to persist through + // the merge chain even at steps where the removing mod isn't a direct input - + // treating deletions as an automatic decline would have left this engine unable to + // help with most of what motivated it), then resolves each vanilla function + // independently before reassembling. + public static class FunctionLevelMergeEngine + { + static readonly Regex WhitespaceRun = new Regex(@"[ \t\r\n\f\v]+", RegexOptions.Compiled); + + public static FunctionLevelMergeResult TryMerge( + string baseText, string oldText, string newText, + string oldMarkerLabel, string newMarkerLabel, + string oldDescription, string newDescription) + { + ScriptDocument baseDoc, oldDoc, newDoc; + try + { + baseDoc = ScriptUnitExtractor.Extract(baseText); + oldDoc = ScriptUnitExtractor.Extract(oldText); + newDoc = ScriptUnitExtractor.Extract(newText); + } + catch (ScriptUnitExtractor.ExtractionException) + { + return FunctionLevelMergeResult.Declined; + } + + var vanillaCount = baseDoc.Units.Count; + var oldAlignment = UnitAligner.Align(baseDoc.Units, oldDoc.Units); + var newAlignment = UnitAligner.Align(baseDoc.Units, newDoc.Units); + + var decisions = new List(); + + // Reconcile insertions (units on either side with no vanilla counterpart at + // all) before resolving vanilla units - a same-named, differently-bodied + // insertion on both sides is a genuine new-functionality collision, a + // different problem shape from "vanilla function edited two ways", and out + // of scope for this engine (declines the whole file, same as if this + // engine didn't exist for this particular pair). + var insertionsPerSlot = new List[vanillaCount + 1]; + for (var slot = 0; slot <= vanillaCount; ++slot) + { + var resolved = ReconcileInsertions(oldDoc, oldAlignment, newDoc, newAlignment, slot); + if (resolved == null) + return FunctionLevelMergeResult.Declined; + insertionsPerSlot[slot] = resolved; + } + + var resolvedUnits = new string[vanillaCount]; + for (var i = 0; i < vanillaCount; ++i) + { + resolvedUnits[i] = ResolveUnit( + baseDoc.Units[i], + oldAlignment.MatchedSideIndex[i].HasValue ? oldDoc.Units[oldAlignment.MatchedSideIndex[i].Value].FullText : null, + newAlignment.MatchedSideIndex[i].HasValue ? newDoc.Units[newAlignment.MatchedSideIndex[i].Value].FullText : null, + oldMarkerLabel, newMarkerLabel, oldDescription, newDescription, decisions); + } + + var merged = new StringBuilder(); + for (var slot = 0; slot <= vanillaCount; ++slot) + { + if (IsGapComparisonEligible(oldAlignment, newAlignment, slot, vanillaCount)) + { + NoteGapMismatchIfAny( + baseDoc.Gaps[slot], + oldDoc.Gaps[GetSideGapIndex(oldAlignment, slot, vanillaCount)], + newDoc.Gaps[GetSideGapIndex(newAlignment, slot, vanillaCount)], + oldDescription, newDescription, decisions); + } + + merged.Append(baseDoc.Gaps[slot]); + foreach (var insertion in insertionsPerSlot[slot]) + merged.Append(insertion); + if (slot < vanillaCount) + merged.Append(resolvedUnits[slot]); + } + + return new FunctionLevelMergeResult(true, merged.ToString(), decisions); + } + + #region Per-unit resolution + + // null oldText/newText means that side deleted this vanilla unit outright + // (UnitAligner.MatchedSideIndex was null for it). Ten-case resolution table: + // both-deleted, both-unchanged, only-one-side-touched-it (edited OR deleted), + // both-made-the-same-edit, an edit surviving a competing deletion (deletions + // never silently win over a surviving edit - deleting code is unrecoverable if + // wrong, keeping it is - always noted either way), and a genuine edit-vs-edit + // collision (real 3-way merge first, tiebreak on distinctness-from-vanilla if + // that fails or still conflicts). + static string ResolveUnit( + ScriptUnit baseUnit, string oldText, string newText, + string oldMarkerLabel, string newMarkerLabel, string oldDescription, string newDescription, + List decisions) + { + var baseText = baseUnit.FullText; + + if (oldText == null && newText == null) + return string.Empty; + + if (oldText == null) + { + if (newText == baseText) + return string.Empty; // new side never touched it either - the deletion propagates + decisions.Add( + $"function {baseUnit.Name}: kept {newDescription}'s edit; {oldDescription} had deleted this " + + "function (a deletion never silently overrides a surviving edit)."); + return newText; + } + + if (newText == null) + { + if (oldText == baseText) + return string.Empty; + decisions.Add( + $"function {baseUnit.Name}: kept {oldDescription}'s edit; {newDescription} had deleted this " + + "function (a deletion never silently overrides a surviving edit)."); + return oldText; + } + + if (oldText == baseText && newText == baseText) + return baseText; + if (oldText == baseText) + return newText; + if (newText == baseText) + return oldText; + if (oldText == newText) + return oldText; + + // Both sides changed this function, differently - try a real per-function + // 3-way merge before falling back to the whole-function tiebreak. Catching + // DiffAlgorithmException here (rather than letting it propagate) is a real + // improvement over today's whole-file behavior, not just defensive + // symmetry: today, this exception means "give up on the entire file"; + // here, it means "skip DiffPlex's fine-grained inline merge for just this + // one function," and the file still gets merged via the tiebreak below. + DiffPlexMergeEngine.MergeTextResult? mergeResult; + try + { + mergeResult = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, oldMarkerLabel, newMarkerLabel); + } + catch (DiffPlexMergeEngine.DiffAlgorithmException) + { + mergeResult = null; + } + + if (mergeResult.HasValue && !mergeResult.Value.HasConflicts) + return mergeResult.Value.MergedText; + + var oldDistinctness = ComputeDistinctness(baseText, oldText); + var newDistinctness = ComputeDistinctness(baseText, newText); + + if (newDistinctness > oldDistinctness) + { + decisions.Add( + $"function {baseUnit.Name}: kept {newDescription}'s version ({newDistinctness} changed diff " + + $"blocks vs. vanilla, more distinct than {oldDescription}'s {oldDistinctness}), discarded " + + $"{oldDescription}'s conflicting change to this function."); + return newText; + } + + // A tie (including the newDistinctness == oldDistinctness case) falls back + // to oldText, mirroring DiffPlexMergeEngine's own whitespace-tiebreak + // convention of always picking one deterministic side rather than guessing + // further - see its IsWhitespaceOnlyDifference comment. + decisions.Add( + $"function {baseUnit.Name}: kept {oldDescription}'s version ({oldDistinctness} changed diff " + + $"blocks vs. vanilla" + (newDistinctness == oldDistinctness ? ", tied with" : ", more distinct than") + + $" {newDescription}'s {newDistinctness}), discarded {newDescription}'s conflicting change to this function."); + return oldText; + } + + static int ComputeDistinctness(string baseText, string sideText) + { + var baseStripped = ScriptUnitExtractor.StripComments(baseText); + var sideStripped = ScriptUnitExtractor.StripComments(sideText); + var diff = Differ.Instance.CreateLineDiffs(baseStripped, sideStripped, ignoreWhitespace: true); + + var score = 0; + foreach (var block in diff.DiffBlocks) + score += block.DeleteCountA + block.InsertCountB; + return score; + } + + #endregion + + #region Insertion reconciliation + + // Returns the resolved, ordered list of FullText to emit at this slot, or null + // if a same-named insertion on both sides has different content (a genuine + // new-functionality collision - declines the whole file, the one case this + // method can't resolve on its own). + static List ReconcileInsertions( + ScriptDocument oldDoc, UnitAlignment oldAlignment, ScriptDocument newDoc, UnitAlignment newAlignment, int slot) + { + var oldInsertions = oldAlignment.InsertionsAtSlot[slot].Select(i => oldDoc.Units[i]).ToList(); + var newInsertions = newAlignment.InsertionsAtSlot[slot].Select(i => newDoc.Units[i]).ToList(); + + if (oldInsertions.Count == 0 && newInsertions.Count == 0) + return new List(); + + var newByName = newInsertions.ToDictionary(u => u.Name); + var consumedNewNames = new HashSet(); + var result = new List(); + + foreach (var oldUnit in oldInsertions) + { + if (newByName.TryGetValue(oldUnit.Name, out var newUnit)) + { + consumedNewNames.Add(oldUnit.Name); + if (oldUnit.FullText != newUnit.FullText) + return null; + result.Add(oldUnit.FullText); + } + else + { + result.Add(oldUnit.FullText); + } + } + foreach (var newUnit in newInsertions) + if (!consumedNewNames.Contains(newUnit.Name)) + result.Add(newUnit.FullText); + + return result; + } + + #endregion + + #region Gap comparison + + // A slot is only compared when both its neighboring vanilla units (if any) are + // present, unmatched-to-nothing, on both sides, and neither side inserted + // anything at this slot - i.e. the simple, overwhelmingly common case (per this + // feature's own real-data measurement: the large majority of a file's gaps sit + // between two functions neither mod touched structurally). Once an insertion or + // deletion touches a slot's boundary, "the equivalent gap on each side" stops + // being a single well-defined span to compare - deferred rather than guessed at. + static bool IsGapComparisonEligible(UnitAlignment oldAlignment, UnitAlignment newAlignment, int slot, int vanillaCount) + { + if (oldAlignment.InsertionsAtSlot[slot].Count > 0 || newAlignment.InsertionsAtSlot[slot].Count > 0) + return false; + if (slot > 0 && (!oldAlignment.MatchedSideIndex[slot - 1].HasValue || !newAlignment.MatchedSideIndex[slot - 1].HasValue)) + return false; + if (slot < vanillaCount && (!oldAlignment.MatchedSideIndex[slot].HasValue || !newAlignment.MatchedSideIndex[slot].HasValue)) + return false; + return true; + } + + // Only valid when IsGapComparisonEligible(slot) is true, which guarantees + // MatchedSideIndex[slot] (or [slot - 1], for the final slot) has a value. + static int GetSideGapIndex(UnitAlignment alignment, int slot, int vanillaCount) + { + if (slot < vanillaCount && alignment.MatchedSideIndex[slot].HasValue) + return alignment.MatchedSideIndex[slot].Value; + return alignment.MatchedSideIndex[slot - 1].Value + 1; + } + + // Reassembly always keeps vanilla's own gap text verbatim (deterministic, + // matches DiffPlexMergeEngine's own "take one side" precedent elsewhere) - this + // only ever adds an audit note when a side's gap content differs from vanilla's + // by more than whitespace/comments, since that's real, non-mechanical content + // (typically a mod author's own comment) silently not making it into the merged + // output. A purely whitespace/comment difference is never noted - that's exactly + // the class of noise this whole engine exists to stop treating as meaningful. + static void NoteGapMismatchIfAny(string baseGap, string oldGap, string newGap, string oldDescription, string newDescription, List decisions) + { + var baseNorm = NormalizeGap(baseGap); + var oldDiffers = NormalizeGap(oldGap) != baseNorm; + var newDiffers = NormalizeGap(newGap) != baseNorm; + + if (oldDiffers) + decisions.Add($"a comment from {oldDescription} near this position was not preserved (vanilla formatting/comments kept)."); + if (newDiffers) + decisions.Add($"a comment from {newDescription} near this position was not preserved (vanilla formatting/comments kept)."); + } + + // Deliberately whitespace-collapse only, NOT comment-stripped: this feeds the + // note above, whose whole point is to detect when comment CONTENT differs, not + // just formatting - stripping comments first would blank away the very thing + // being compared, silently defeating the check (caught by + // TryMerge_GapCommentDifference_NotedButVanillaGapTextKept). Matches + // DiffPlexMergeEngine.NormalizeWhitespace's own whitespace-only spirit. + static string NormalizeGap(string text) => WhitespaceRun.Replace(text, " ").Trim(); + + #endregion + } +} diff --git a/WitcherScriptMerger.Core/Tools/ScriptUnitExtractor.cs b/WitcherScriptMerger.Core/Tools/ScriptUnitExtractor.cs new file mode 100644 index 0000000..e9340bf --- /dev/null +++ b/WitcherScriptMerger.Core/Tools/ScriptUnitExtractor.cs @@ -0,0 +1,453 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Text.RegularExpressions; + +namespace WitcherScriptMerger.Tools +{ + public enum ScriptUnitKind + { + Function, + Field, + } + + // One function/event declaration (with its body, or just a signature for a + // body-less forward/interface declaration) or one @addField-decorated field + // declaration, as a verbatim slice of the original file text. FullText always + // includes any immediately-preceding annotation lines (@wrapMethod/@addMethod/ + // @replaceMethod/@addField) - see ScriptUnitExtractor.Extract. + public readonly struct ScriptUnit + { + public string Name { get; } + public ScriptUnitKind Kind { get; } + public bool HasBody { get; } + public int StartOffset { get; } + public int EndOffset { get; } + public string FullText { get; } + + public ScriptUnit(string name, ScriptUnitKind kind, bool hasBody, int startOffset, int endOffset, string fullText) + { + Name = name; + Kind = kind; + HasBody = hasBody; + StartOffset = startOffset; + EndOffset = endOffset; + FullText = fullText; + } + } + + // A file split into alternating gap/unit segments: Gaps[0] + Units[0].FullText + + // Gaps[1] + ... + Units[N-1].FullText + Gaps[N] reassembles byte-for-byte to the + // original input (see ScriptUnitExtractor.Reassemble) - Gaps always has exactly + // one more entry than Units, by construction. + public sealed class ScriptDocument + { + public IReadOnlyList Gaps { get; } + public IReadOnlyList Units { get; } + + public ScriptDocument(IReadOnlyList gaps, IReadOnlyList units) + { + Gaps = gaps; + Units = units; + } + } + + // Splits a .ws file's text into function/event/@addField-field units for the + // function-level merge engine (see WitcherScriptMerger.Core/CLAUDE.md's + // "Function-level merge engine" section for the full design rationale). + // Deliberately a brace-matching tokenizer, not a full parser or a bound to the third-party + // tree-sitter-witcherscript grammar: confirmed via direct research into + // WitcherScript's grammar that class/state/struct/enum declarations are top-level + // only (never nested) and the language has no nested function-like constructs at + // all (no lambdas, local functions, or closures) - a function body only ever gains + // brace depth from control flow, never from another function declaration. That + // structural simplicity is what makes plain brace/paren counting sufficient here, + // as long as it's string/comment-aware (see ClassifySpans) so a brace or paren + // inside a string literal or comment can never be mistaken for real syntax. + public static class ScriptUnitExtractor + { + // Thrown when the input doesn't parse cleanly - unbalanced braces/parens, or an + // unterminated string/block comment. Mirrors DiffPlexMergeEngine. + // DiffAlgorithmException's shape: a distinct exception type so callers can catch + // specifically "this input isn't safe to extract from" and fall back to + // whatever they did before this class existed, rather than crashing. + public sealed class ExtractionException : Exception + { + public ExtractionException(string message) : base(message) { } + } + + // Every specifier/flavour keyword that can precede "function"/"event" in a real + // declaration (confirmed against WitcherScript's grammar during this feature's + // design research) plus "override", included defensively even though it wasn't + // independently confirmed - an unrecognized word here just means the line + // doesn't match and its content stays in gap territory, never a crash, so + // including an extra plausible keyword costs nothing if it turns out to be + // wrong. + const string SpecifierAlternation = + "public|private|protected|final|latent|exec|entry|timer|storyscene|quest|reward|" + + "cleanup|import|editable|const|out|optional|inlined|statemachine|saved|abstract|override"; + + static readonly Regex DeclarationRegex = new Regex( + @"^[ \t]*(?:(?:" + SpecifierAlternation + @")\s+)*(?function|event)\s+(?\w+)\s*\(", + RegexOptions.Compiled | RegexOptions.Multiline); + + // A single, simple annotation argument (or none) - e.g. "@wrapMethod(CR4Player)". + // Deliberately doesn't support nested parens or a multi-line argument list: real + // WitcherScript modding annotations are single-line with at most one identifier + // argument, and an annotation shaped some other way just fails to match here, + // falling into gap territory (attached to nothing) rather than crashing - a safe + // degradation, not silent corruption. + static readonly Regex AnnotationLineRegex = new Regex(@"^@\w+\s*(\([^()\r\n]*\))?\s*$", RegexOptions.Compiled); + static readonly Regex AddFieldAnnotationRegex = new Regex(@"^@addField\s*\([^()\r\n]*\)\s*$", RegexOptions.Compiled); + static readonly Regex FieldNameRegex = new Regex(@"\bvar\s+(?\w+)\s*:", RegexOptions.Compiled); + + enum SpanKind : byte + { + None, + String, + LineComment, + BlockComment, + } + + #region Public API + + public static ScriptDocument Extract(string text) + { + var kinds = ClassifySpans(text); + var mask = BuildMask(text, kinds, blankStringsToo: true); + var lineStarts = ComputeLineStarts(text); + + var gaps = new List(); + var units = new List(); + + var cursor = 0; + var pos = 0; + while (pos <= text.Length) + { + var funcMatch = DeclarationRegex.Match(mask, pos); + var fieldLineStart = FindNextAddFieldAnnotationLineStart(mask, lineStarts, pos); + + var funcPos = funcMatch.Success ? funcMatch.Index : int.MaxValue; + var fieldPos = fieldLineStart ?? int.MaxValue; + + if (funcPos == int.MaxValue && fieldPos == int.MaxValue) + break; + + ScriptUnit unit; + if (fieldPos < funcPos) + unit = ExtractField(text, mask, lineStarts, fieldPos, cursor); + else + unit = ExtractFunction(text, mask, lineStarts, funcMatch, cursor); + + gaps.Add(text.Substring(cursor, unit.StartOffset - cursor)); + units.Add(unit); + cursor = unit.EndOffset; + pos = unit.EndOffset; + } + + gaps.Add(text.Substring(cursor)); + return new ScriptDocument(gaps, units); + } + + public static string Reassemble(ScriptDocument document) + { + var sb = new StringBuilder(); + for (var i = 0; i < document.Units.Count; ++i) + { + sb.Append(document.Gaps[i]); + sb.Append(document.Units[i].FullText); + } + sb.Append(document.Gaps[document.Gaps.Count - 1]); + return sb.ToString(); + } + + // Blanks comment spans only (line and block), preserving string-literal content + // and everything else verbatim - unlike the brace-safe mask Extract uses + // internally, this is meant to produce comparable, still-readable text for the + // function-level merge engine's distinctness metric (see the plan), not a + // scratch buffer for structural matching. + public static string StripComments(string text) + { + var kinds = ClassifySpans(text); + return BuildMask(text, kinds, blankStringsToo: false); + } + + #endregion + + #region Unit extraction + + static ScriptUnit ExtractFunction(string text, string mask, List lineStarts, Match declMatch, int cursor) + { + var unitStart = Math.Max(cursor, ExtendStartBackwardOverAnnotations(mask, lineStarts, declMatch.Index)); + + var openParen = declMatch.Index + declMatch.Length - 1; + if (mask[openParen] != '(') + throw new ExtractionException( + "Internal error: declaration match for '" + declMatch.Groups["name"].Value + + "' did not end on its own opening parenthesis."); + + var closeParen = FindMatchingDelimiter(mask, openParen, '(', ')'); + if (closeParen < 0) + throw new ExtractionException( + "Unbalanced parameter-list parentheses in the declaration of '" + + declMatch.Groups["name"].Value + "' starting at offset " + declMatch.Index + "."); + + var terminator = FindNextSemicolonOrBrace(mask, closeParen + 1); + if (terminator < 0) + throw new ExtractionException( + "Reached end of file looking for ';' or '{' after the declaration of '" + + declMatch.Groups["name"].Value + "' starting at offset " + declMatch.Index + "."); + + int unitEnd; + bool hasBody; + if (mask[terminator] == ';') + { + hasBody = false; + unitEnd = terminator + 1; + } + else + { + var closeBrace = FindMatchingDelimiter(mask, terminator, '{', '}'); + if (closeBrace < 0) + throw new ExtractionException( + "Unbalanced braces in the body of '" + declMatch.Groups["name"].Value + + "' starting at offset " + declMatch.Index + "."); + hasBody = true; + unitEnd = closeBrace + 1; + } + + return new ScriptUnit( + declMatch.Groups["name"].Value, ScriptUnitKind.Function, hasBody, + unitStart, unitEnd, text.Substring(unitStart, unitEnd - unitStart)); + } + + static ScriptUnit ExtractField(string text, string mask, List lineStarts, int annotationLineStart, int cursor) + { + var unitStart = Math.Max(cursor, ExtendStartBackwardOverAnnotations(mask, lineStarts, annotationLineStart)); + + var annotationLineEnd = GetLineEnd(mask, lineStarts, annotationLineStart); + var terminator = FindNextChar(mask, annotationLineEnd, ';'); + if (terminator < 0) + throw new ExtractionException( + "Reached end of file looking for the ';' terminating the @addField declaration " + + "starting at offset " + annotationLineStart + "."); + + var unitEnd = terminator + 1; + var fullText = text.Substring(unitStart, unitEnd - unitStart); + var nameMatch = FieldNameRegex.Match(mask, annotationLineEnd, terminator - annotationLineEnd); + var name = nameMatch.Success ? nameMatch.Groups["name"].Value : "@addField#" + unitStart; + + return new ScriptUnit(name, ScriptUnitKind.Field, hasBody: false, unitStart, unitEnd, fullText); + } + + // Walks backward over any immediately preceding @-annotation lines (tolerating + // blank lines between them, and between the last annotation and the unit + // itself), so @wrapMethod/@addMethod/@replaceMethod/@addField stay glued to + // what they decorate. Clamped to never walk back past cursor (the end of the + // previously extracted unit/gap), so a pathological run of stacked declarations + // with no blank line between them can never make one unit's annotation walk + // swallow part of the previous unit. + static int ExtendStartBackwardOverAnnotations(string mask, List lineStarts, int unitStart) + { + var resultStart = unitStart; + var lineIndex = GetLineIndex(lineStarts, unitStart) - 1; + + while (lineIndex >= 0) + { + var lineStart = lineStarts[lineIndex]; + var lineEnd = GetLineEnd(mask, lineStarts, lineStart); + var content = mask.Substring(lineStart, lineEnd - lineStart).Trim(); + + if (content.Length == 0) + { + --lineIndex; + continue; + } + + if (AnnotationLineRegex.IsMatch(content)) + { + resultStart = lineStart; + --lineIndex; + continue; + } + + break; + } + + return resultStart; + } + + static int? FindNextAddFieldAnnotationLineStart(string mask, List lineStarts, int pos) + { + var lineIndex = GetLineIndex(lineStarts, pos); + if (lineStarts[lineIndex] < pos) + ++lineIndex; + + for (; lineIndex < lineStarts.Count; ++lineIndex) + { + var lineStart = lineStarts[lineIndex]; + var lineEnd = GetLineEnd(mask, lineStarts, lineStart); + var content = mask.Substring(lineStart, lineEnd - lineStart).Trim(); + if (content.Length == 0) + continue; + if (AddFieldAnnotationRegex.IsMatch(content)) + return lineStart; + } + + return null; + } + + #endregion + + #region Masking (string/comment-safe scanning) + + // One left-to-right scan classifying every character as inside a "//" comment, + // a "/* */" comment (non-nesting), a "\"...\"" string literal (backslash-escape + // aware - "\\\"" doesn't terminate the string, and "\\\\" consumes as one + // escaped-backslash unit rather than misreading the following character as an + // escape target), or none of those. Single-quote literals aren't handled - not + // part of the confirmed WitcherScript syntax facts this class was built from. + // Reused by both the brace-safe mask (BuildMask, blankStringsToo: true) and + // StripComments (blankStringsToo: false) so the actual comment/string detection + // logic exists exactly once. + static SpanKind[] ClassifySpans(string text) + { + var kinds = new SpanKind[text.Length]; + var i = 0; + while (i < text.Length) + { + var c = text[i]; + var next = i + 1 < text.Length ? text[i + 1] : '\0'; + + if (c == '/' && next == '/') + { + var start = i; + while (i < text.Length && text[i] != '\n' && text[i] != '\r') + ++i; + for (var k = start; k < i; ++k) + kinds[k] = SpanKind.LineComment; + continue; + } + + if (c == '/' && next == '*') + { + var start = i; + i += 2; + while (i + 1 < text.Length && !(text[i] == '*' && text[i + 1] == '/')) + ++i; + if (i + 1 >= text.Length) + throw new ExtractionException("Unterminated block comment starting at offset " + start + "."); + i += 2; + for (var k = start; k < i; ++k) + kinds[k] = SpanKind.BlockComment; + continue; + } + + if (c == '"') + { + var start = i; + ++i; + while (i < text.Length && text[i] != '"') + { + if (text[i] == '\\' && i + 1 < text.Length) + i += 2; + else + ++i; + } + if (i >= text.Length) + throw new ExtractionException("Unterminated string literal starting at offset " + start + "."); + ++i; + for (var k = start; k < i; ++k) + kinds[k] = SpanKind.String; + continue; + } + + ++i; + } + return kinds; + } + + // Blanks masked characters to a literal space, EXCEPT '\n'/'\r' - a multi-line + // block comment or string must keep its own line breaks intact, or line-start + // offsets (ComputeLineStarts) and the '^'-anchored DeclarationRegex would both + // silently desync from the real text. + static string BuildMask(string text, SpanKind[] kinds, bool blankStringsToo) + { + var chars = text.ToCharArray(); + for (var i = 0; i < chars.Length; ++i) + { + var blank = kinds[i] == SpanKind.LineComment || kinds[i] == SpanKind.BlockComment + || (blankStringsToo && kinds[i] == SpanKind.String); + if (blank && chars[i] != '\n' && chars[i] != '\r') + chars[i] = ' '; + } + return new string(chars); + } + + static int FindMatchingDelimiter(string mask, int openIndex, char open, char close) + { + var depth = 0; + for (var i = openIndex; i < mask.Length; ++i) + { + if (mask[i] == open) ++depth; + else if (mask[i] == close) + { + --depth; + if (depth == 0) + return i; + } + } + return -1; + } + + static int FindNextSemicolonOrBrace(string mask, int start) + { + for (var i = start; i < mask.Length; ++i) + if (mask[i] == ';' || mask[i] == '{') + return i; + return -1; + } + + static int FindNextChar(string mask, int start, char target) + { + for (var i = start; i < mask.Length; ++i) + if (mask[i] == target) + return i; + return -1; + } + + #endregion + + #region Line bookkeeping + + static List ComputeLineStarts(string text) + { + var starts = new List { 0 }; + for (var i = 0; i < text.Length; ++i) + if (text[i] == '\n') + starts.Add(i + 1); + return starts; + } + + static int GetLineIndex(List lineStarts, int offset) + { + var lo = 0; + var hi = lineStarts.Count - 1; + while (lo < hi) + { + var mid = (lo + hi + 1) / 2; + if (lineStarts[mid] <= offset) lo = mid; + else hi = mid - 1; + } + return lo; + } + + static int GetLineEnd(string mask, List lineStarts, int lineStart) + { + var lineIndex = GetLineIndex(lineStarts, lineStart); + return lineIndex + 1 < lineStarts.Count ? lineStarts[lineIndex + 1] : mask.Length; + } + + #endregion + } +} diff --git a/WitcherScriptMerger.Core/Tools/UnitAligner.cs b/WitcherScriptMerger.Core/Tools/UnitAligner.cs new file mode 100644 index 0000000..e844775 --- /dev/null +++ b/WitcherScriptMerger.Core/Tools/UnitAligner.cs @@ -0,0 +1,106 @@ +using System; +using System.Collections.Generic; + +namespace WitcherScriptMerger.Tools +{ + // The result of aligning one side's (old's or new's) ScriptUnit sequence against + // vanilla's, by name, for the function-level merge engine (see + // FunctionLevelMergeEngine's own header comment). + public sealed class UnitAlignment + { + // One entry per vanilla unit, in vanilla order: the matching index into this + // side's own unit list, or null if this side deleted that vanilla unit + // entirely (present in vanilla, absent here). + public IReadOnlyList MatchedSideIndex { get; } + + // vanillaUnitCount + 1 entries, one per gap slot (0 = before vanilla's first + // unit, i = between vanilla unit i-1 and i, last = after vanilla's last unit). + // Each entry lists this side's own unit indices that don't correspond to any + // vanilla unit at all (this side's insertions), in their original relative + // order, attributed to the slot they fall between. + public IReadOnlyList> InsertionsAtSlot { get; } + + public UnitAlignment(IReadOnlyList matchedSideIndex, IReadOnlyList> insertionsAtSlot) + { + MatchedSideIndex = matchedSideIndex; + InsertionsAtSlot = insertionsAtSlot; + } + } + + // Aligns one side's extracted units against vanilla's, by name, so + // FunctionLevelMergeEngine can tell - per vanilla function - whether each side kept + // it unchanged, edited it, or deleted it outright, and which units on each side are + // wholly new (not in vanilla at all). Confirmed empirically (this feature's own + // design research, against real vanilla actor.ws/player.ws/npc.ws) that function + // names don't collide within a file, so this is a longest-common-subsequence + // alignment on plain name tokens - standard LCS DP, not a heuristic - with no + // ambiguity from duplicate names to worry about in practice. A file that somehow did + // have duplicate names still produces *a* valid LCS, just not necessarily the one a + // human would consider "obviously correct" - a reasonable degradation, not a crash. + public static class UnitAligner + { + public static UnitAlignment Align(IReadOnlyList vanillaUnits, IReadOnlyList sideUnits) + { + var n = vanillaUnits.Count; + var m = sideUnits.Count; + + // dp[i, j] = length of the LCS of vanillaUnits[i..n) and sideUnits[j..m). + // Sized (n+1) x (m+1) so dp[n, *] and dp[*, m] (the "nothing left" base + // case) are always in bounds without a separate edge check. + var dp = new int[n + 1, m + 1]; + for (var i = n - 1; i >= 0; --i) + { + for (var j = m - 1; j >= 0; --j) + { + dp[i, j] = vanillaUnits[i].Name == sideUnits[j].Name + ? dp[i + 1, j + 1] + 1 + : Math.Max(dp[i + 1, j], dp[i, j + 1]); + } + } + + var matched = new int?[n]; + var insertionsAtSlot = new List[n + 1]; + for (var s = 0; s <= n; ++s) + insertionsAtSlot[s] = new List(); + + // Standard LCS backtrack, walking forward (not the more common + // backward-from-(0,0) direction) since dp was built with the "suffix LCS + // length" convention above - equivalent, just avoids reversing the result + // afterward. + var vi = 0; + var si = 0; + while (vi < n && si < m) + { + if (vanillaUnits[vi].Name == sideUnits[si].Name) + { + matched[vi] = si; + ++vi; + ++si; + } + else if (dp[vi + 1, si] >= dp[vi, si + 1]) + { + // vanillaUnits[vi] contributes nothing further to the LCS from here - + // this side deleted it. matched[vi] stays null. + ++vi; + } + else + { + // sideUnits[si] contributes nothing further to the LCS from here - + // it's this side's own insertion, attributed to the gap slot right + // before whichever vanilla unit is still unresolved (vi). + insertionsAtSlot[vi].Add(si); + ++si; + } + } + // Any side units left once vanilla is exhausted are trailing insertions, + // attributed to the final slot (after vanilla's last unit). + while (si < m) + { + insertionsAtSlot[n].Add(si); + ++si; + } + + return new UnitAlignment(matched, insertionsAtSlot); + } + } +} diff --git a/WitcherScriptMerger.Headless/Program.cs b/WitcherScriptMerger.Headless/Program.cs index 19a1268..3f9a141 100644 --- a/WitcherScriptMerger.Headless/Program.cs +++ b/WitcherScriptMerger.Headless/Program.cs @@ -157,6 +157,8 @@ static int RunMerge(string[] args) Console.WriteLine($"Merged {summary.Merged.Count} file(s), skipped {summary.Skipped.Count}."); foreach (var path in summary.Skipped) Console.WriteLine($" skipped: {path}"); + foreach (var decision in summary.FunctionLevelDecisions) + Console.WriteLine($" function-level: {decision}"); return summary.Skipped.Count == 0 ? 0 : 2; } diff --git a/WitcherScriptMerger.Tests/CLAUDE.md b/WitcherScriptMerger.Tests/CLAUDE.md index f39a6e7..dd7fe05 100644 --- a/WitcherScriptMerger.Tests/CLAUDE.md +++ b/WitcherScriptMerger.Tests/CLAUDE.md @@ -20,6 +20,19 @@ does. `MergeHeadless_EncodingMismatch_...` fixture reproducing the `baseEffect.ws`-style false conflict that motivated it. - `Tools/HasherTests.cs` — `Hasher`'s xxHash32 output, including synthetic edge cases. +- `Tools/ScriptUnitExtractorTests.cs` — `ScriptUnitExtractor`'s function-level merge + splitter: round-trip fidelity (`Reassemble(Extract(x)) == x`) across annotations, + forward declarations, nested control-flow braces, string/comment-embedded braces, and + `ExtractionException` on malformed input — see Core's `CLAUDE.md`'s "Function-level + merge engine" section. +- `Tools/UnitAlignerTests.cs` — `UnitAligner`'s vanilla-vs-one-side LCS alignment: + matches, insertions, deletions, and both at once. +- `Tools/FunctionLevelMergeEngineTests.cs` — `FunctionLevelMergeEngine.TryMerge`: every + one-sided shortcut, a genuine collision resolved by `BuildMerge`, the + most-distinct-from-vanilla tiebreak (including its deterministic tie-break and a + scaled-down `DiffAlgorithmException` case), edit-survives-competing-deletion, insertion + reconciliation (including the same-name-different-body decline case), and gap-comment + detection. - `Inventory/FileMergerTests.cs` — `FileMerger.IsVanillaDlcBundleFolder`: known vanilla DLC-folder names, case-insensitivity, and non-matches (including anchoring) — see Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section. Also covers the two-arg diff --git a/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs b/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs new file mode 100644 index 0000000..49f796a --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs @@ -0,0 +1,258 @@ +using System.Linq; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Regression coverage for FunctionLevelMergeEngine (see + // WitcherScriptMerger.Core/CLAUDE.md once this lands there, and the plan this + // class was built from). Fixtures use small synthetic WitcherScript-shaped + // excerpts, never literal game script text (copyrighted) - only structurally + // similar stand-ins for the real collision shapes this engine was built to + // handle. + public class FunctionLevelMergeEngineTests + { + const string OldLabel = "modA"; + const string NewLabel = "modB"; + const string OldDesc = "modA"; + const string NewDesc = "modB"; + + static string Fn(string name, string body) => + "function " + name + "()\r\n{\r\n" + body + "}\r\n"; + + static FunctionLevelMergeResult Merge(string baseText, string oldText, string newText) => + FunctionLevelMergeEngine.TryMerge(baseText, oldText, newText, OldLabel, NewLabel, OldDesc, NewDesc); + + [Fact] + public void TryMerge_BothSidesUnchanged_KeepsVanillaTextForEveryFunction() + { + var baseText = Fn("A", "\tx = 1;\r\n") + Fn("B", "\ty = 2;\r\n"); + + var result = Merge(baseText, baseText, baseText); + + Assert.True(result.Applied); + Assert.Equal(baseText, result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_OnlyOldSideChangedOneFunction_KeepsOldVersionNoBuildMergeNeeded() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var oldText = Fn("A", "\tx = 2;\r\n"); + + var result = Merge(baseText, oldText, baseText); + + Assert.True(result.Applied); + Assert.Equal(oldText, result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_OnlyNewSideChangedOneFunction_KeepsNewVersion() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var newText = Fn("A", "\tx = 3;\r\n"); + + var result = Merge(baseText, baseText, newText); + + Assert.True(result.Applied); + Assert.Equal(newText, result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_BothSidesMadeIdenticalEdit_KeepsItOnceNoDecisionNote() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var sameEdit = Fn("A", "\tx = 9;\r\n"); + + var result = Merge(baseText, sameEdit, sameEdit); + + Assert.True(result.Applied); + Assert.Equal(sameEdit, result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_NonOverlappingEditsWithinFunction_CleanlyMergedByBuildMerge() + { + var baseText = "function A()\r\n{\r\n\ta();\r\n\tb();\r\n\tc();\r\n}\r\n"; + var oldText = "function A()\r\n{\r\n\ta();\r\n\tMOD1();\r\n\tb();\r\n\tc();\r\n}\r\n"; + var newText = "function A()\r\n{\r\n\ta();\r\n\tb();\r\n\tc();\r\n\tMOD2();\r\n}\r\n"; + + var result = Merge(baseText, oldText, newText); + + Assert.True(result.Applied); + Assert.Contains("MOD1();", result.MergedText); + Assert.Contains("MOD2();", result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_GenuineCollision_TiebreakPicksMoreDistinctSideAndRecordsDecision() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var oldText = Fn("A", "\tx = 2;\r\n"); // 1 changed line vs. vanilla + var newText = Fn("A", "\tx = 2;\r\n\ty = 3;\r\n\tz = 4;\r\n"); // 3 changed lines vs. vanilla + + var result = Merge(baseText, oldText, newText); + + Assert.True(result.Applied); + Assert.Equal(newText, result.MergedText); + var note = Assert.Single(result.Decisions); + Assert.Contains("modB", note); + Assert.Contains("more distinct", note); + } + + [Fact] + public void TryMerge_GenuineCollisionExactTie_FallsBackDeterministicallyToOldSide() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var oldText = Fn("A", "\tx = 2;\r\n"); + var newText = Fn("A", "\tx = 3;\r\n"); + + var result = Merge(baseText, oldText, newText); + + Assert.True(result.Applied); + Assert.Equal(oldText, result.MergedText); + var note = Assert.Single(result.Decisions); + Assert.Contains("modA", note); + } + + [Fact] + public void TryMerge_DeletedOnOldSideOnly_NewSideAlsoUnchanged_FunctionDroppedNoDecisionNote() + { + var baseText = Fn("A", "\tx = 1;\r\n") + Fn("B", "\ty = 2;\r\n"); + var oldText = Fn("B", "\ty = 2;\r\n"); // deleted A entirely, didn't touch B + + var result = Merge(baseText, oldText, baseText); + + Assert.True(result.Applied); + Assert.DoesNotContain("function A", result.MergedText); + Assert.Contains("function B", result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_EditSurvivesCompetingDeletion_KeepsEditAndRecordsDecision() + { + var baseText = Fn("A", "\tx = 1;\r\n") + Fn("B", "\ty = 2;\r\n"); + var oldText = Fn("B", "\ty = 2;\r\n"); // old deleted A + var newText = Fn("A", "\tx = 99;\r\n") + Fn("B", "\ty = 2;\r\n"); // new edited A + + var result = Merge(baseText, oldText, newText); + + Assert.True(result.Applied); + Assert.Contains("x = 99;", result.MergedText); + var note = Assert.Single(result.Decisions); + Assert.Contains("modB", note); + Assert.Contains("deleted", note); + } + + [Fact] + public void TryMerge_DeletedOnBothSides_FunctionDropped() + { + var baseText = Fn("A", "\tx = 1;\r\n") + Fn("B", "\ty = 2;\r\n"); + var bothDeleteA = Fn("B", "\ty = 2;\r\n"); + + var result = Merge(baseText, bothDeleteA, bothDeleteA); + + Assert.True(result.Applied); + Assert.DoesNotContain("function A", result.MergedText); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_InsertionOnOldSideOnly_IsKept() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var oldText = Fn("A", "\tx = 1;\r\n") + Fn("NewFunc", "\tz = 1;\r\n"); + + var result = Merge(baseText, oldText, baseText); + + Assert.True(result.Applied); + Assert.Contains("function NewFunc", result.MergedText); + } + + [Fact] + public void TryMerge_IdenticalInsertionOnBothSides_KeptOnce() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var withNewFunc = Fn("A", "\tx = 1;\r\n") + Fn("NewFunc", "\tz = 1;\r\n"); + + var result = Merge(baseText, withNewFunc, withNewFunc); + + Assert.True(result.Applied); + var occurrences = result.MergedText.Split(new[] { "function NewFunc" }, System.StringSplitOptions.None).Length - 1; + Assert.Equal(1, occurrences); + } + + [Fact] + public void TryMerge_SameNameDifferentBodyInsertionOnBothSides_DeclinesWholeFile() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var oldText = Fn("A", "\tx = 1;\r\n") + Fn("NewFunc", "\tz = 1;\r\n"); + var newText = Fn("A", "\tx = 1;\r\n") + Fn("NewFunc", "\tz = 2;\r\n"); // same name, different body + + var result = Merge(baseText, oldText, newText); + + Assert.False(result.Applied); + } + + [Fact] + public void TryMerge_ExtractionFailsOnAnySide_Declines() + { + var baseText = Fn("A", "\tx = 1;\r\n"); + var unbalanced = "function A()\r\n{\r\n\tx = 1;\r\n"; // never closes + + var result = Merge(baseText, unbalanced, baseText); + + Assert.False(result.Applied); + } + + [Fact] + public void TryMerge_GapCommentDifference_NotedButVanillaGapTextKept() + { + var baseText = Fn("A", "\treturn;\r\n") + "\r\n// vanilla comment\r\n\r\n" + Fn("B", "\treturn;\r\n"); + var oldText = Fn("A", "\treturn;\r\n") + "\r\n// modA's own comment here\r\n\r\n" + Fn("B", "\treturn;\r\n"); + + var result = Merge(baseText, oldText, baseText); + + Assert.True(result.Applied); + Assert.Contains("vanilla comment", result.MergedText); + Assert.DoesNotContain("modA's own comment", result.MergedText); + var note = Assert.Single(result.Decisions); + Assert.Contains("modA", note); + } + + [Fact] + public void TryMerge_GapWhitespaceOnlyDifference_NoDecisionNote() + { + var baseText = Fn("A", "\treturn;\r\n") + "\r\n\r\n" + Fn("B", "\treturn;\r\n"); + var oldText = Fn("A", "\treturn;\r\n") + "\r\n\r\n\r\n" + Fn("B", "\treturn;\r\n"); // extra blank line only + + var result = Merge(baseText, oldText, baseText); + + Assert.True(result.Applied); + Assert.Empty(result.Decisions); + } + + [Fact] + public void TryMerge_FunctionLevelDiffAlgorithmException_FallsBackToTiebreakRatherThanDeclining() + { + // Same interleaved-edit shape DiffPlexMergeEngineTests uses to trigger the + // confirmed upstream DiffPlex bug, scaled down to function size: one side + // inserts a line right after "a();", the other independently changes + // "b()" to "B()". + var baseText = Fn("A", "\ta();\r\n\tb();\r\n\tc();\r\n"); + var oldText = Fn("A", "\ta();\r\n\tnewline();\r\n\tb();\r\n\tc();\r\n"); + var newText = Fn("A", "\ta();\r\n\tB();\r\n\tc();\r\n"); + + var result = Merge(baseText, oldText, newText); + + Assert.True(result.Applied); + Assert.Single(result.Decisions); + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/ScriptUnitExtractorTests.cs b/WitcherScriptMerger.Tests/Tools/ScriptUnitExtractorTests.cs new file mode 100644 index 0000000..86f6a14 --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/ScriptUnitExtractorTests.cs @@ -0,0 +1,241 @@ +using System.Linq; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Regression coverage for ScriptUnitExtractor - the function-level merge engine's + // splitter (see WitcherScriptMerger.Core/CLAUDE.md once this lands there, and + // the plan this class was built from). Every fixture asserts the round-trip + // property (Reassemble(Extract(x)) == x) AND the specific units found, since + // round-tripping alone doesn't prove extraction found the right boundaries - a + // document with zero units detected still round-trips trivially as one big gap. + public class ScriptUnitExtractorTests + { + [Fact] + public void Extract_PlainFunction_FindsOneUnitAndRoundTrips() + { + var text = "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\tx = 1;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + Assert.Equal(ScriptUnitKind.Function, unit.Kind); + Assert.True(unit.HasBody); + } + + [Fact] + public void Extract_EventDeclaration_FindsOneUnitAndRoundTrips() + { + var text = "class Foo\r\n{\r\n\tevent OnSpawned( SEntitySpawnData data )\r\n\t{\r\n\t\treturn true;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("OnSpawned", unit.Name); + } + + [Fact] + public void Extract_ForwardDeclarationTerminatedBySemicolon_HasBodyIsFalse() + { + var text = "class Foo\r\n{\r\n\timport function Bar();\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + Assert.False(unit.HasBody); + Assert.EndsWith(";", unit.FullText); + } + + [Fact] + public void Extract_AnnotationImmediatelyPrecedingFunction_IsIncludedInUnit() + { + var text = "class Foo\r\n{\r\n\t@wrapMethod(Foo)\r\n\tfunction Bar()\r\n\t{\r\n\t\tx = 1;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.StartsWith("\t@wrapMethod(Foo)", unit.FullText); + } + + [Fact] + public void Extract_AnnotationSeparatedFromFunctionByBlankLine_IsStillIncludedInUnit() + { + var text = "class Foo\r\n{\r\n\t@wrapMethod(Foo)\r\n\r\n\tfunction Bar()\r\n\t{\r\n\t\tx = 1;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.StartsWith("\t@wrapMethod(Foo)", unit.FullText); + } + + [Fact] + public void Extract_AddFieldAnnotatedField_IsFoundAsFieldUnit() + { + var text = "class Foo\r\n{\r\n\t@addField(Foo)\r\n\tvar myNewField : bool;\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("myNewField", unit.Name); + Assert.Equal(ScriptUnitKind.Field, unit.Kind); + Assert.False(unit.HasBody); + Assert.StartsWith("\t@addField(Foo)", unit.FullText); + } + + [Fact] + public void Extract_NestedControlFlowBraces_DoNotConfuseBodyEnd() + { + var text = + "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n" + + "\t\tif (x > 0)\r\n\t\t{\r\n\t\t\twhile (y < 10)\r\n\t\t\t{\r\n\t\t\t\ty += 1;\r\n\t\t\t}\r\n\t\t}\r\n" + + "\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + Assert.EndsWith("\t}", unit.FullText); + } + + [Fact] + public void Extract_BraceInsideStringLiteral_DoesNotConfuseBodyEnd() + { + var text = "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\ts = \"{ not a real brace }\";\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + Assert.Contains("not a real brace", unit.FullText); + } + + [Fact] + public void Extract_BraceInsideBlockComment_DoesNotConfuseBodyEnd() + { + var text = "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\t/* if (x) { y = 1; } */\r\n\t\tz = 2;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + } + + [Fact] + public void Extract_MultiLineBlockComment_PreservesLineStructureForLaterUnits() + { + var text = + "/* a multi-line\r\n block comment\r\n { with braces } inside */\r\n" + + "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\tx = 1;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Bar", unit.Name); + } + + [Fact] + public void Extract_LineCommentContainingWhatLooksLikeADeclaration_IsNotTreatedAsAUnit() + { + var text = + "class Foo\r\n{\r\n\t// function FakeOne() { return 1; }\r\n" + + "\tfunction Real()\r\n\t{\r\n\t\tx = 1;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + var unit = Assert.Single(doc.Units); + Assert.Equal("Real", unit.Name); + } + + [Fact] + public void Extract_MultipleFunctionsWithGapsBetween_FindsAllInOrderAndRoundTrips() + { + var text = + "class Foo\r\n{\r\n\tvar x : int;\r\n\r\n\tfunction A()\r\n\t{\r\n\t\treturn;\r\n\t}\r\n\r\n" + + "\t// a comment gap\r\n\r\n\tfunction B()\r\n\t{\r\n\t\treturn;\r\n\t}\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + Assert.Equal(new[] { "A", "B" }, doc.Units.Select(u => u.Name)); + Assert.Equal(3, doc.Gaps.Count); + } + + [Fact] + public void Extract_NoFunctionsAtAll_ReturnsSingleGapAndRoundTrips() + { + var text = "class Foo\r\n{\r\n\tvar x : int;\r\n\tvar y : int;\r\n}\r\n"; + + var doc = ScriptUnitExtractor.Extract(text); + + Assert.Equal(text, ScriptUnitExtractor.Reassemble(doc)); + Assert.Empty(doc.Units); + Assert.Single(doc.Gaps); + } + + [Fact] + public void Extract_EmptyString_ReturnsSingleEmptyGap() + { + var doc = ScriptUnitExtractor.Extract(string.Empty); + + Assert.Equal(string.Empty, ScriptUnitExtractor.Reassemble(doc)); + Assert.Empty(doc.Units); + Assert.Single(doc.Gaps); + } + + [Fact] + public void Extract_UnbalancedBraceInBody_ThrowsExtractionException() + { + // No closing brace anywhere after the function's own opening brace - the + // extractor only ever brace-matches within a function body, never validates + // enclosing class/state braces, so the "missing" brace must be the + // function's own for this to actually exercise the unbalanced-body path. + var text = "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\tx = 1;\r\n"; + + var ex = Assert.Throws(() => ScriptUnitExtractor.Extract(text)); + Assert.NotNull(ex.Message); + } + + [Fact] + public void Extract_UnterminatedStringLiteral_ThrowsExtractionException() + { + var text = "class Foo\r\n{\r\n\tfunction Bar()\r\n\t{\r\n\t\ts = \"never closed;\r\n\t}\r\n}\r\n"; + + Assert.Throws(() => ScriptUnitExtractor.Extract(text)); + } + + [Fact] + public void Extract_UnterminatedBlockComment_ThrowsExtractionException() + { + var text = "class Foo\r\n{\r\n\t/* never closed\r\n\tfunction Bar() { x = 1; }\r\n}\r\n"; + + Assert.Throws(() => ScriptUnitExtractor.Extract(text)); + } + + [Fact] + public void StripComments_BlanksCommentsButPreservesStringContentAndLineStructure() + { + var text = "x = 1; // trailing comment\r\ny = \"keep { this }\"; /* block */\r\nz = 3;\r\n"; + + var stripped = ScriptUnitExtractor.StripComments(text); + + Assert.DoesNotContain("trailing comment", stripped); + Assert.DoesNotContain("block", stripped); + Assert.Contains("keep { this }", stripped); + Assert.Equal(text.Split('\n').Length, stripped.Split('\n').Length); + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/UnitAlignerTests.cs b/WitcherScriptMerger.Tests/Tools/UnitAlignerTests.cs new file mode 100644 index 0000000..72fce21 --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/UnitAlignerTests.cs @@ -0,0 +1,140 @@ +using System.Collections.Generic; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Regression coverage for UnitAligner - the function-level merge engine's + // vanilla-vs-one-side alignment (see WitcherScriptMerger.Core/CLAUDE.md once this + // lands there). Builds ScriptUnit fixtures directly via its public constructor + // (offsets/FullText are irrelevant to alignment, which only reads Name) rather than + // running the real extractor - keeps these fixtures focused purely on the + // alignment algorithm. + public class UnitAlignerTests + { + static ScriptUnit Unit(string name) => + new ScriptUnit(name, ScriptUnitKind.Function, hasBody: true, startOffset: 0, endOffset: 0, fullText: name); + + static List Units(params string[] names) + { + var list = new List(); + foreach (var name in names) + list.Add(Unit(name)); + return list; + } + + [Fact] + public void Align_IdenticalSequences_EveryUnitMatchedNoInsertionsOrDeletions() + { + var vanilla = Units("A", "B", "C"); + var side = Units("A", "B", "C"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(new int?[] { 0, 1, 2 }, alignment.MatchedSideIndex); + foreach (var slot in alignment.InsertionsAtSlot) + Assert.Empty(slot); + } + + [Fact] + public void Align_SideDeletesOneVanillaUnit_ThatUnitIsUnmatched() + { + var vanilla = Units("A", "B", "C"); + var side = Units("A", "C"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(0, alignment.MatchedSideIndex[0]); + Assert.Null(alignment.MatchedSideIndex[1]); + Assert.Equal(1, alignment.MatchedSideIndex[2]); + } + + [Fact] + public void Align_SideDeletesAllVanillaUnits_EveryUnitIsUnmatched() + { + var vanilla = Units("A", "B", "C"); + var side = Units(); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.All(alignment.MatchedSideIndex, m => Assert.Null(m)); + } + + [Fact] + public void Align_SideInsertsOneNewUnit_AttributedToCorrectSlot() + { + var vanilla = Units("A", "B", "C"); + var side = Units("A", "NEW", "B", "C"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(new int?[] { 0, 2, 3 }, alignment.MatchedSideIndex); + Assert.Equal(new[] { 1 }, alignment.InsertionsAtSlot[1]); // slot 1 = between vanilla[0] and vanilla[1] + Assert.Empty(alignment.InsertionsAtSlot[0]); + Assert.Empty(alignment.InsertionsAtSlot[2]); + Assert.Empty(alignment.InsertionsAtSlot[3]); + } + + [Fact] + public void Align_SideInsertsAtStartAndEnd_AttributedToOuterSlots() + { + var vanilla = Units("A", "B"); + var side = Units("BEFORE", "A", "B", "AFTER"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(new[] { 0 }, alignment.InsertionsAtSlot[0]); + Assert.Equal(new[] { 3 }, alignment.InsertionsAtSlot[2]); + } + + [Fact] + public void Align_MultipleInsertionsAtSameSlot_PreserveRelativeOrder() + { + var vanilla = Units("A", "B"); + var side = Units("A", "X", "Y", "Z", "B"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(new[] { 1, 2, 3 }, alignment.InsertionsAtSlot[1]); + } + + [Fact] + public void Align_SimultaneousInsertionAndDeletion_BothHandledIndependently() + { + var vanilla = Units("A", "B", "C"); + var side = Units("A", "NEW", "C"); // deletes B, inserts NEW before C + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Equal(0, alignment.MatchedSideIndex[0]); + Assert.Null(alignment.MatchedSideIndex[1]); // B deleted + Assert.Equal(2, alignment.MatchedSideIndex[2]); + Assert.Equal(new[] { 1 }, alignment.InsertionsAtSlot[2]); // slot before vanilla[2] = "C" + } + + [Fact] + public void Align_EmptyVanilla_EverySideUnitIsAnInsertionAtSlotZero() + { + var vanilla = Units(); + var side = Units("A", "B"); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.Empty(alignment.MatchedSideIndex); + Assert.Equal(new[] { 0, 1 }, alignment.InsertionsAtSlot[0]); + } + + [Fact] + public void Align_EmptySide_EveryVanillaUnitIsDeletedNoInsertions() + { + var vanilla = Units("A", "B"); + var side = Units(); + + var alignment = UnitAligner.Align(vanilla, side); + + Assert.All(alignment.MatchedSideIndex, m => Assert.Null(m)); + foreach (var slot in alignment.InsertionsAtSlot) + Assert.Empty(slot); + } + } +} diff --git a/WitcherScriptMerger/Program.cs b/WitcherScriptMerger/Program.cs index f7b6c20..04e3d95 100644 --- a/WitcherScriptMerger/Program.cs +++ b/WitcherScriptMerger/Program.cs @@ -226,6 +226,8 @@ static int RunCli(string[] args) Console.WriteLine($"Merged {summary.Merged.Count} file(s), skipped {summary.Skipped.Count}."); foreach (var path in summary.Skipped) Console.WriteLine($" skipped: {path}"); + foreach (var decision in summary.FunctionLevelDecisions) + Console.WriteLine($" function-level: {decision}"); return summary.Skipped.Count == 0 ? 0 : 2; }