From 6f2b3c416cadb75ec3db5a2697b2127a35d089dd Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Mon, 6 Jul 2026 17:19:52 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20O(N=C2=B2)=20blowup=20in=20character-leve?= =?UTF-8?q?l=20diff=20for=20large=20dissimilar=20strings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports https://github.com/mpizenberg/elm-test-runner/pull/16. Co-authored-by: Matthieu Pizenberg --- elm/src/Test/Reporter/Console/Format.elm | 28 ++++---- elm/src/Test/Reporter/Highlightable.elm | 29 ++++++++- elm/src/Test/Runner/Node/Vendor/Diff.elm | 81 ++++++++++++++++-------- 3 files changed, 99 insertions(+), 39 deletions(-) diff --git a/elm/src/Test/Reporter/Console/Format.elm b/elm/src/Test/Reporter/Console/Format.elm index fc633556..891a58f4 100644 --- a/elm/src/Test/Reporter/Console/Format.elm +++ b/elm/src/Test/Reporter/Console/Format.elm @@ -119,21 +119,27 @@ highlightEqual expected actual = highlightedActual = Highlightable.diffLists actualChars expectedChars |> List.map (Highlightable.map String.fromChar) - - plainCharCount = - highlightedExpected - |> List.filter (not << isHighlighted) - |> List.length in - if edgeCount highlightedActual > plainCharCount || edgeCount highlightedExpected > plainCharCount then - -- Large number of small highlighted blocks. Diff is too messy to be useful. + if List.length highlightedExpected /= String.length expected then + -- Diff bailed out (too many edits). Skip highlighting. Nothing else - Just - ( highlightedExpected - , highlightedActual - ) + let + plainCharCount = + highlightedExpected + |> List.filter (not << isHighlighted) + |> List.length + in + if edgeCount highlightedActual > plainCharCount || edgeCount highlightedExpected > plainCharCount then + -- Large number of small highlighted blocks. Diff is too messy to be useful. + Nothing + + else + Just + ( highlightedExpected + , highlightedActual + ) isFloat : String -> Bool diff --git a/elm/src/Test/Reporter/Highlightable.elm b/elm/src/Test/Reporter/Highlightable.elm index 3b0c9d8a..e5d4ad53 100644 --- a/elm/src/Test/Reporter/Highlightable.elm +++ b/elm/src/Test/Reporter/Highlightable.elm @@ -20,9 +20,32 @@ resolve { fromHighlighted, fromPlain } highlightable = diffLists : List a -> List a -> List (Highlightable a) diffLists expected actual = - -- TODO make sure this looks reasonable for multiline strings - Diff.diff expected actual - |> List.concatMap fromDiff + let + ( prefix, restExpected, restActual ) = + trimPrefix [] expected actual + + ( suffixRev, midExpectedRev, midActualRev ) = + trimPrefix [] (List.reverse restExpected) (List.reverse restActual) + + middle = + Diff.diff (List.reverse midExpectedRev) (List.reverse midActualRev) + |> List.concatMap fromDiff + in + List.map Plain prefix ++ middle ++ List.map Plain (List.reverse suffixRev) + + +trimPrefix : List a -> List a -> List a -> ( List a, List a, List a ) +trimPrefix acc a b = + case ( a, b ) of + ( x :: restA, y :: restB ) -> + if x == y then + trimPrefix (x :: acc) restA restB + + else + ( List.reverse acc, a, b ) + + _ -> + ( List.reverse acc, a, b ) map : (a -> b) -> Highlightable a -> Highlightable b diff --git a/elm/src/Test/Runner/Node/Vendor/Diff.elm b/elm/src/Test/Runner/Node/Vendor/Diff.elm index 6f59e0d0..919fc8a3 100644 --- a/elm/src/Test/Runner/Node/Vendor/Diff.elm +++ b/elm/src/Test/Runner/Node/Vendor/Diff.elm @@ -24,6 +24,7 @@ Each function internally uses Wu's [O(NP) algorithm](http://myerslab.mpi-cbg.de/ -- commit 19047f01d460739bfe7f16466bc60b41430a8f09 - because it assumes -- the end user has the correct elm-package on their PATH, which is not a -- safe assumption. +-- It also contains this fix: https://github.com/mpizenberg/elm-test-runner/pull/16 -- -- License: {- @@ -86,7 +87,20 @@ type BugReport -} diff : List a -> List a -> List (Change a) diff a b = - case testDiff a b of + diffWithMaxEdits maxEditsDefault a b + + +{-| Maximum number of edits (P in the O(NP) algorithm) before bailing out. +This prevents O(N²) blowups on large, very dissimilar inputs. +-} +maxEditsDefault : Int +maxEditsDefault = + 200 + + +diffWithMaxEdits : Int -> List a -> List a -> List (Change a) +diffWithMaxEdits maxEdits a b = + case testDiff maxEdits a b of Ok changes -> changes @@ -97,8 +111,8 @@ diff a b = {-| Test the algolithm itself. If it returns Err, it should be a bug. -} -testDiff : List a -> List a -> Result BugReport (List (Change a)) -testDiff a b = +testDiff : Int -> List a -> List a -> Result BugReport (List (Change a)) +testDiff maxEdits a b = let arrA = Array.fromList a @@ -121,9 +135,7 @@ testDiff a b = \y -> Array.get (y - 1) arrB path = - -- Is there any case ond is needed? - -- ond getA getB m n - onp getA getB m n + onp maxEdits getA getB m n in makeChanges getA getB path @@ -196,8 +208,8 @@ makeChangesHelp changes getA getB ( x, y ) path = -- Wu's O(NP) algorithm (http://myerslab.mpi-cbg.de/wp-content/uploads/2014/06/np_diff.pdf) -onp : (Int -> Maybe a) -> (Int -> Maybe a) -> Int -> Int -> List ( Int, Int ) -onp getA getB m n = +onp : Int -> (Int -> Maybe a) -> (Int -> Maybe a) -> Int -> Int -> List ( Int, Int ) +onp maxEdits getA getB m n = let v = Array.initialize (m + n + 1) (always []) @@ -205,33 +217,39 @@ onp getA getB m n = delta = n - m in - onpLoopP (snake getA getB) delta m 0 v + onpLoopP maxEdits (snake getA getB) delta m 0 v onpLoopP : - (Int -> Int -> List ( Int, Int ) -> ( List ( Int, Int ), Bool )) + Int + -> (Int -> Int -> List ( Int, Int ) -> ( List ( Int, Int ), Bool )) -> Int -> Int -> Int -> Array (List ( Int, Int )) -> List ( Int, Int ) -onpLoopP snake_ delta offset p v = - let - ks = - if delta > 0 then - List.reverse (List.range (delta + 1) (delta + p)) - ++ List.range -p delta +onpLoopP maxEdits snake_ delta offset p v = + if p > maxEdits then + -- Too many edits; bail out to avoid O(N²) blowup. + [] - else - List.reverse (List.range (delta + 1) p) - ++ List.range (-p + delta) delta - in - case onpLoopK snake_ offset ks v of - Found path -> - path + else + let + ks = + if delta > 0 then + reversePrepend (List.range (delta + 1) (delta + p)) + (List.range -p delta) - Continue v_ -> - onpLoopP snake_ delta offset (p + 1) v_ + else + reversePrepend (List.range (delta + 1) p) + (List.range (-p + delta) delta) + in + case onpLoopK snake_ offset ks v of + Found path -> + path + + Continue v_ -> + onpLoopP maxEdits snake_ delta offset (p + 1) v_ onpLoopK : @@ -324,3 +342,16 @@ snake getA getB nextX nextY path = _ -> ( path, False ) + + +{-| Prepend elements from the first list onto the second, in reverse order. +Equivalent to `List.reverse xs ++ ys` but in a single pass. +-} +reversePrepend : List a -> List a -> List a +reversePrepend xs ys = + case xs of + [] -> + ys + + x :: rest -> + reversePrepend rest (x :: ys)