diff --git a/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs b/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs index c27d27b..bc47e44 100644 --- a/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs +++ b/WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs @@ -342,7 +342,20 @@ static string ResolveUnit( } if (mergeResult.HasValue && !mergeResult.Value.HasConflicts) - return mergeResult.Value.MergedText; + { + // DiffPlex's silent bug operates at line granularity too: a clean-looking + // per-function splice can duplicate a local declaration (observed live - + // see LocalVarDeclRegex's comment). A duplicated local is invalid + // WitcherScript, and the whole-function tiebreak below is always a valid + // alternative - one side's intact function, never a splice. + if (!HasDuplicatedLocalVarDecls(mergeResult.Value.MergedText, baseText, oldText, newText, out var dupLocal)) + return mergeResult.Value.MergedText; + + decisions.Add( + $"{baseUnit.DescribeKind()} {baseUnit.Name}: the fine-grained 3-way merge silently duplicated " + + $"local variable '{dupLocal}' (a known DiffPlex failure mode) - used the whole-function " + + "tiebreak below instead of the spliced result."); + } var oldDistinctness = ComputeDistinctness(baseText, oldText); var newDistinctness = ComputeDistinctness(baseText, newText); @@ -599,6 +612,74 @@ static bool GetGapEligibilityOneSide(UnitAlignment alignment, int slot, int vani #region Output sanity gate + // Local `var` declarations inside a unit's text, counted by name. The invariant + // this feeds exists because DiffPlex's silent duplication operates at LINE + // granularity: a splice inside a function body can duplicate a local + // declaration (observed live: `var mCSMCR : CCSMCR;` emitted twice inside + // combat.ws's OnUpdate locals -> "Variable 'mCSMCR' is already defined" at + // compile), which no unit-level count can see. Counted over comment-stripped + // text so a commented-out declaration can't skew the tally; multi-declarator + // lines count each name. + static readonly Regex LocalVarDeclRegex = new Regex( + @"^\s*var\s+(?\w+(?:\s*,\s*\w+)*)\s*:", RegexOptions.Compiled | RegexOptions.Multiline); + + static Dictionary CountLocalVarDecls(string unitText) + { + var counts = new Dictionary(StringComparer.Ordinal); + string stripped; + try + { + stripped = ScriptUnitExtractor.StripComments(unitText); + } + catch (ScriptUnitExtractor.ExtractionException) + { + stripped = unitText; + } + foreach (Match m in LocalVarDeclRegex.Matches(stripped)) + { + foreach (var raw in m.Groups["names"].Value.Split(',')) + { + var name = raw.Trim(); + if (name.Length == 0) + continue; + counts.TryGetValue(name, out var n); + counts[name] = n + 1; + } + } + return counts; + } + + // True when mergedUnitText declares some local var name more often than ANY of + // the input versions of the same unit do - the line-level silent-duplication + // signature. Inputs that don't contain this unit pass null. + public static bool HasDuplicatedLocalVarDecls( + string mergedUnitText, string baseUnitText, string oldUnitText, string newUnitText, out string duplicatedName) + { + duplicatedName = null; + var mergedCounts = CountLocalVarDecls(mergedUnitText); + if (mergedCounts.Count == 0) + return false; + var baseCounts = baseUnitText == null ? null : CountLocalVarDecls(baseUnitText); + var oldCounts = oldUnitText == null ? null : CountLocalVarDecls(oldUnitText); + var newCounts = newUnitText == null ? null : CountLocalVarDecls(newUnitText); + + int At(Dictionary counts, string name) => + counts != null && counts.TryGetValue(name, out var n) ? n : 0; + + foreach (var (name, count) in mergedCounts) + { + if (count <= 1) + continue; + var maxInput = Math.Max(At(baseCounts, name), Math.Max(At(oldCounts, name), At(newCounts, name))); + if (count > maxInput) + { + duplicatedName = name; + return true; + } + } + return false; + } + // Validates a whole-file "clean" merge's output against the three inputs it was // built from - the guard for DiffPlex's SILENT ThreeWayDiffer failure mode // (no exception, no conflict block, but content lost or duplicated; see @@ -634,23 +715,27 @@ public static bool ValidateWholeFileMergeOutput( return false; } - Dictionary Counts(string text) + (Dictionary counts, Dictionary firstTexts) Index(string text) { var counts = new Dictionary(StringComparer.Ordinal); + var firstTexts = new Dictionary(StringComparer.Ordinal); foreach (var unit in ScriptUnitExtractor.Extract(text).Units) { counts.TryGetValue(unit.ScopedName, out var n); counts[unit.ScopedName] = n + 1; + if (n == 0) + firstTexts[unit.ScopedName] = unit.FullText; } - return counts; + return (counts, firstTexts); } Dictionary baseCounts, oldCounts, newCounts, mergedCounts; + Dictionary baseTexts, oldTexts, newTexts, mergedTexts; try { - baseCounts = Counts(baseText); - oldCounts = Counts(oldText); - newCounts = Counts(newText); + (baseCounts, baseTexts) = Index(baseText); + (oldCounts, oldTexts) = Index(oldText); + (newCounts, newTexts) = Index(newText); } catch (ScriptUnitExtractor.ExtractionException) { @@ -658,7 +743,7 @@ Dictionary Counts(string text) } try { - mergedCounts = Counts(mergedText); + (mergedCounts, mergedTexts) = Index(mergedText); } catch (ScriptUnitExtractor.ExtractionException ex) { @@ -667,6 +752,7 @@ Dictionary Counts(string text) } int At(Dictionary counts, string name) => counts.TryGetValue(name, out var n) ? n : 0; + string TextAt(Dictionary texts, string name) => texts.TryGetValue(name, out var t) ? t : null; foreach (var (name, mergedCount) in mergedCounts) { @@ -691,6 +777,20 @@ Dictionary Counts(string text) } } + // Line-level: DiffPlex's silent duplication can strike INSIDE a function + // body too, duplicating a local declaration a unit-count invariant can't + // see (observed live - see LocalVarDeclRegex's comment). Each merged unit's + // local-var-declaration counts must not exceed every input version's. + foreach (var (name, mergedUnitText) in mergedTexts) + { + if (HasDuplicatedLocalVarDecls( + mergedUnitText, TextAt(baseTexts, name), TextAt(oldTexts, name), TextAt(newTexts, name), out var dupLocal)) + { + violation = $"local variable '{dupLocal}' is declared more than once inside '{name}' in the merged output but not in any input (duplicated splice)"; + return false; + } + } + return true; } diff --git a/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs b/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs index efc3909..84245ef 100644 --- a/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs +++ b/WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs @@ -571,6 +571,41 @@ public void ValidateWholeFileMergeOutput_NonScriptFiles_AlwaysTrusted() Assert.True(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput("", "", "", "", "x.xml", out _)); } + // The combat.ws shape: a clean-looking splice duplicated a LOCAL declaration + // inside a function body - invisible to unit-level counts, fatal at compile + // ("Variable 'mCSMCR' is already defined"). + [Fact] + public void ValidateWholeFileMergeOutput_DetectsDuplicatedLocalVarInsideAFunction() + { + var baseText = Fn("A", "\tvar x : int;\r\n\tx = 1;\r\n"); + var oldText = Fn("A", "\tvar x : int;\r\n\tvar mCS : CCS;\r\n\tx = 1;\r\n"); + var corrupted = Fn("A", "\tvar x : int;\r\n\tvar mCS : CCS;\r\n\tvar mCS : CCS;\r\n\tx = 1;\r\n"); + + Assert.False(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(baseText, oldText, baseText, corrupted, "x.ws", out var violation)); + Assert.Contains("mCS", violation); + } + + // An input that itself declares a local twice is tolerated when the merge + // carries it through unchanged - the invariant only flags duplication the + // merge INTRODUCED. + [Fact] + public void ValidateWholeFileMergeOutput_ToleratesPreexistingDuplicateLocalFromAnInput() + { + var baseText = Fn("A", "\tvar x : int;\r\n"); + var oldText = Fn("A", "\tvar x : int;\r\n\tvar mCS : CCS;\r\n\tvar mCS : CCS;\r\n"); + + Assert.True(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(baseText, oldText, baseText, oldText, "x.ws", out _)); + } + + [Fact] + public void HasDuplicatedLocalVarDecls_IgnoresCommentedOutDeclarations() + { + var merged = Fn("A", "\tvar x : int;\r\n\t// var x : int;\r\n"); + var input = Fn("A", "\tvar x : int;\r\n"); + + Assert.False(FunctionLevelMergeEngine.HasDuplicatedLocalVarDecls(merged, input, input, input, out _)); + } + #endregion } }