diff --git a/src/Fable.Cli/CHANGELOG.md b/src/Fable.Cli/CHANGELOG.md index d260861d9..c29f54096 100644 --- a/src/Fable.Cli/CHANGELOG.md +++ b/src/Fable.Cli/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* [Dart] Fix `Array.compareWith` comparing lengths before elements, producing wrong results for arrays with common prefixes (fixes #2961) * [Python] Fix unsafe option unwrapping in `DateTimeOffset.get_Offset` and regex replacements (by @dbrattli) * [All] Replace unsafe option `.Value` unwrapping with safe alternatives in Python/Replacements.fs and Rust/Fable2Rust.fs (code scanning alerts IONIDE-006) * [All] Add `[]` to partial active patterns in Dart and Rust targets to reduce allocations (code scanning alerts IONIDE-009) diff --git a/src/Fable.Compiler/CHANGELOG.md b/src/Fable.Compiler/CHANGELOG.md index 80965d86b..a819f5710 100644 --- a/src/Fable.Compiler/CHANGELOG.md +++ b/src/Fable.Compiler/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* [Dart] Fix `Array.compareWith` comparing lengths before elements, producing wrong results for arrays with common prefixes (fixes #2961) * [Python] Fix unsafe option unwrapping in `DateTimeOffset.get_Offset` and regex replacements (by @dbrattli) * [All] Replace unsafe option `.Value` unwrapping with safe alternatives in Python/Replacements.fs and Rust/Fable2Rust.fs (code scanning alerts IONIDE-006) * [All] Add `[]` to partial active patterns in Dart and Rust targets to reduce allocations (code scanning alerts IONIDE-009) diff --git a/src/fable-library-dart/Array.fs b/src/fable-library-dart/Array.fs index 9ed6054a5..55425c3d5 100644 --- a/src/fable-library-dart/Array.fs +++ b/src/fable-library-dart/Array.fs @@ -713,28 +713,33 @@ let splitAt (index: int) (array: 'T[]) : 'T[] * 'T[] = Native.sublist (array, 0, index), Native.sublist (array, index) +// Note that, unlike the `compare` operator, Array.compareWith compares elements first, +// then falls back to length comparison. See #2961. let compareWith (comparer: 'T -> 'T -> int) (array1: 'T[]) (array2: 'T[]) : int = // Null checks not necessary because Dart provides null safety - // if isNull array1 then - // if isNull array2 then 0 else -1 - // elif isNull array2 then - // 1 - // else let mutable i = 0 let mutable result = 0 let length1 = array1.Length let length2 = array2.Length - if length1 > length2 then + let minLength = + if length1 < length2 then + length1 + else + length2 + + while i < minLength && result = 0 do + result <- comparer array1[i] array2[i] + i <- i + 1 + + if result <> 0 then + result + elif length1 > length2 then 1 elif length1 < length2 then -1 else - while i < length1 && result = 0 do - result <- comparer array1[i] array2[i] - i <- i + 1 - - result + 0 let equalsWith (equals: 'T -> 'T -> bool) (array1: 'T[]) (array2: 'T[]) : bool = // Null checks not necessary because Dart provides null safety diff --git a/tests/Dart/src/ArrayTests.fs b/tests/Dart/src/ArrayTests.fs index 3a20dea33..38407633d 100644 --- a/tests/Dart/src/ArrayTests.fs +++ b/tests/Dart/src/ArrayTests.fs @@ -1102,17 +1102,17 @@ let tests () = throwsAnyError (fun () -> Array.removeManyAt -1 2 [|1|] |> ignore) throwsAnyError (fun () -> Array.removeManyAt 2 2 [|1|] |> ignore) - // testCase "Array.compareWith works" <| fun () -> // See #2961 - // let a = [|1;3|] - // let b = [|1;2;3|] - // // compares lengths first, then elements - // let c1 = a < b - // let c2 = compare a b - // // should compare elements first, then lengths - // let c3 = Array.compareWith compare a b - // equal c1 true - // equal c2 -1 - // equal c3 1 + testCase "Array.compareWith works" <| fun () -> // See #2961 + let a = [|1;3|] + let b = [|1;2;3|] + // compares lengths first, then elements + let c1 = a < b + let c2 = compare a b + // should compare elements first, then lengths + let c3 = Array.compareWith compare a b + equal c1 true + equal c2 -1 + equal c3 1 // testCase "System.Array.Resize works" <| fun () -> // let mutable xs = [|1; 2; 3; 4; 5|]