From c73b9c814f59650901e3eb4533c0d512ad2f2380 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 03:20:23 +0200 Subject: [PATCH 01/10] chore: lay the foundation for the per-collection modules Prepares the chain of ADR 0001, one pull request per collection: - `Helper.fs` replaces `ImmutableCollectionUtil.fs` and adds `checkNotDefault`, which treats a default `ImmutableArray` as FSharp.Core treats a `null` array. - `build/check-order.fsx` checks that every signature file groups its functions under category banners in the canonical order, sorts them alphabetically inside a category and that the implementation file repeats that order. The FAKE target `CheckOrder` runs it before `DotnetBuild`. - The library and the tests target `net8.0` and `net10.0`; FSharp.Core 10.1.301 is the first release with `partitionWith`. - The placeholder tests are replaced by the category attributes of every collection, tests that enforce the category rules, and the shared helpers of the contract tests: inputs, the comparison of an exception with the FSharp.Core oracle, and the comparison of the `Option` and `Tuple` overrides with the default functions. - The README and the documentation pages list the collections in a table that each following pull request updates. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 71 ++-- CHANGELOG.md | 7 + Directory.Packages.props | 2 +- README.md | 21 +- build/build.fs | 11 + build/check-order.fsx | 307 ++++++++++++++++++ docsSrc/Explanations/Background.md | 20 +- docsSrc/index.md | 20 +- .../FSharp.Collections.Immutable.fsproj | 4 +- src/FSharp.Collections.Immutable/Helper.fs | 37 +++ .../ImmutableCollectionUtil.fs | 19 -- .../Contracts/ExceptionAssert.fs | 96 ++++++ .../Contracts/Inputs.fs | 38 +++ .../Contracts/OverrideAssert.fs | 23 ++ .../FSharp.Collections.Immutable.Tests.fsproj | 14 +- .../FlatList.fs | 10 - .../ImmutableList.fs | 10 - .../IndexedSeq.fs | 10 - .../Maps.fs | 16 - .../Queue.fs | 10 - .../Sets.fs | 16 - .../Stack.fs | 10 - .../TestCategories.fs | 139 ++++++++ .../TestCategoriesTests.fs | 83 +++++ 24 files changed, 827 insertions(+), 167 deletions(-) create mode 100644 build/check-order.fsx create mode 100644 src/FSharp.Collections.Immutable/Helper.fs delete mode 100644 src/FSharp.Collections.Immutable/ImmutableCollectionUtil.fs create mode 100644 tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs create mode 100644 tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs create mode 100644 tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/FlatList.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/ImmutableList.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/IndexedSeq.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/Maps.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/Queue.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/Sets.fs delete mode 100644 tests/FSharp.Collections.Immutable.Tests/Stack.fs create mode 100644 tests/FSharp.Collections.Immutable.Tests/TestCategories.fs create mode 100644 tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fd28f65e..40e0e7a1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -15,56 +15,71 @@ ```text / ├── build/ – FAKE build scripts and release automation +│ └── check-order.fsx – checks the category order of the signature files ├── docsSrc/ – FSharp.Formatting documentation source ├── src/FSharp.Collections.Immutable/ – main library -│ ├── Helper.fs -│ ├── ComputationExpressions.fs +│ ├── Helper.fs – argument checks and exceptions shared by the modules │ ├── FlatList/ – ImmutableArray API surface and builders -│ │ ├── FlatList.fs / FlatList.fsi +│ │ ├── FlatList.fs / FlatList.fsi – includes the nested Parallel module +│ │ ├── FlatListOption.fs / FlatListOption.fsi – Option namespace overrides +│ │ ├── FlatListTuple.fs / FlatListTuple.fsi – Tuple namespace overrides │ │ ├── FlatListBuilder.fs │ │ └── FlatListCE.fs │ ├── IndexedList/ – ImmutableList API surface and builders -│ │ ├── IIndexedList.fs / IIndexedList.fsi -│ │ ├── IndexedList.fs / IndexedList.fsi +│ │ ├── IndexedList.fs / IndexedList.fsi (+ Option / Tuple) +│ │ ├── IIndexedList.fs / IIndexedList.fsi (+ Option / Tuple) │ │ ├── IndexedListBuilder.fs │ │ └── IndexedListCE.fs │ ├── Queue/ – ImmutableQueue API surface and CE -│ │ ├── IQueue.fs / IQueue.fsi -│ │ ├── Queue.fs / Queue.fsi +│ │ ├── Queue.fs / Queue.fsi (+ Option / Tuple) +│ │ ├── IQueue.fs / IQueue.fsi (+ Option / Tuple) │ │ └── QueueCE.fs │ ├── Stack/ – ImmutableStack API surface and CE -│ │ ├── IStack.fs / IStack.fsi -│ │ ├── Stack.fs / Stack.fsi +│ │ ├── Stack.fs / Stack.fsi (+ Option / Tuple) +│ │ ├── IStack.fs / IStack.fsi (+ Option / Tuple) │ │ └── StackCE.fs │ ├── HashMap/ – ImmutableDictionary API surface and CE -│ │ ├── IHashMap.fs / IHashMap.fsi -│ │ ├── HashMap.fs / HashMap.fsi +│ │ ├── HashMap.fs / HashMap.fsi (+ Option / Tuple) +│ │ ├── IMap.fs / IMap.fsi (+ Option / Tuple) │ │ └── HashMapCE.fs │ ├── SortedMap/ – ImmutableSortedDictionary API surface and CE -│ │ ├── ISortedMap.fs / ISortedMap.fsi -│ │ ├── SortedMap.fs / SortedMap.fsi +│ │ ├── SortedMap.fs / SortedMap.fsi (+ Option / Tuple) │ │ └── SortedMapCE.fs │ ├── HashSet/ – ImmutableHashSet API surface and CE -│ │ ├── IHashSet.fs / IHashSet.fsi -│ │ ├── HashSet.fs / HashSet.fsi +│ │ ├── HashSet.fs / HashSet.fsi (+ Option / Tuple) +│ │ ├── ISet.fs / ISet.fsi (+ Option / Tuple) │ │ └── HashSetCE.fs -│ └── SortedSet/ – ImmutableSortedSet API surface and CE -│ ├── ISortedSet.fs / ISortedSet.fsi -│ ├── SortedSet.fs / SortedSet.fsi -│ └── SortedSetCE.fs +│ ├── SortedSet/ – ImmutableSortedSet API surface and CE +│ │ ├── SortedSet.fs / SortedSet.fsi (+ Option / Tuple) +│ │ └── SortedSetCE.fs +│ ├── FrozenMap/ – FrozenDictionary API surface and CE +│ │ ├── FrozenMap.fs / FrozenMap.fsi (+ Option / Tuple) +│ │ └── FrozenMapCE.fs +│ ├── FrozenSet/ – FrozenSet API surface and CE +│ │ ├── FrozenSet.fs / FrozenSet.fsi (+ Option / Tuple) +│ │ └── FrozenSetCE.fs +│ └── ComputationExpressions.fs – the computation expression instances └── tests/FSharp.Collections.Immutable.Tests/ – MSTest test project - ├── HelperTests.fs ├── TestCategories.fs – centralized custom test category attributes - ├── FlatList/ – focused test files per operation group - ├── IndexedList/ – focused test files per operation group - ├── HashMap/ – focused test files per operation group - ├── SortedMap/ – focused test files per operation group - ├── HashSet/ – focused test files per operation group - ├── SortedSet/ – focused test files per operation group - ├── Queue/ – focused test files per operation group - └── Stack/ – focused test files per operation group + ├── TestCategoriesTests.fs – enforces the test category rules + ├── Contracts/ – shared contract bases, inputs and assertion helpers + └── / – ops records, contract classes, collection-specific, + builder, computation expression, Option and Tuple tests ``` +The modules follow ADR 0001 – Per-collection modules (`docsSrc/Explanations/ADR_0001_Collection_Modules.md`): + +* Each module groups its functions under banner comments `// ---- Category ----` in the order `Creation`, `Builder`, + `Properties`, `Access`, `Search`, `Transformation`, `Folding`, `Partitioning and grouping`, `Sorting`, `Random`, + `Conversion`, `Type-specific`, and sorts the functions of a category alphabetically (ordinal), active patterns last. + The `.fs` file repeats the order of its `.fsi` file. `build/check-order.fsx` (FAKE target `CheckOrder`) enforces it. +* `try*` functions return `voption`, tuples are struct tuples. The namespaces `FSharp.Collections.Immutable.Option` and + `FSharp.Collections.Immutable.Tuple` repeat these functions with `option` results and reference tuples. +* A function throws what the FSharp.Core function of the mirrored module throws for the same input: `Array` for + `FlatList`, `List` for the other sequence-like modules, `Map` and `Set` for maps and sets. +* Contract test bases in `Contracts/` take a record of the module's functions; a collection derives one test class per + capability that it supports. Expected values and exceptions come from FSharp.Core. + ## Libraries in Use * [`System.Collections.Immutable`](https://www.nuget.org/packages/System.Collections.Immutable/) – immutable collections foundation diff --git a/CHANGELOG.md b/CHANGELOG.md index e3f2456e..ae631ade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +* The package targets `net8.0` and `net10.0`. +* The package requires FSharp.Core 10.1.301 or later, the first release with `partitionWith`. + ## [1.0.0] - 2020-02-23 First release diff --git a/Directory.Packages.props b/Directory.Packages.props index 44cc9d01..583ed282 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -9,7 +9,7 @@ - + diff --git a/README.md b/README.md index 093ef25e..5f298fa4 100644 --- a/README.md +++ b/README.md @@ -10,15 +10,17 @@ Please also join the [F# Open Source Group](http://fsharp.github.com) ## Features -* `FlatList` (`ImmutableArray`) -* `ImmutableList` -* `Stack` (`ImmutableStack`) -* `Queue` (`ImmutableQueue`) -* `HashMap` (`ImmutableDictionary`) -* `SortedMap` (`ImmutableSortedDictionary`) -* `HashSet` (`ImmutableHashSet`) -* `SortedSet` (`ImmutableSortedSet`) -* `IIndexedSeq` (`IReadOnlyList`) +| Module | .NET type | Computation expression | +|---|---|---| +| `FlatList` | `ImmutableArray<'T>` | – | +| `ImmutableList` | `ImmutableList<'T>` | – | +| `Stack` | `ImmutableStack<'T>` | – | +| `Queue` | `ImmutableQueue<'T>` | – | +| `HashMap` | `ImmutableDictionary<'Key, 'Value>` | – | +| `SortedMap` | `ImmutableSortedDictionary<'Key, 'Value>` | – | +| `HashSet` | `ImmutableHashSet<'T>` | – | +| `SortedSet` | `ImmutableSortedSet<'T>` | – | +| `IIndexedSeq` | `IReadOnlyList<'T>` | – | [Documentation](https://fsprojects.github.io/FSharp.Collections.Immutable/) @@ -85,6 +87,7 @@ or - `GitHubRelease` - Publishes a [GitHub Release](https://help.github.com/en/articles/creating-releases) with the Release Notes and any NuGet packages. Runs only from `Github Actions`. - `FormatCode` - Runs [Fantomas](https://github.com/fsprojects/fantomas) on the solution file. - `CheckFormatCode` - Runs [Fantomas --check](https://fsprojects.github.io/fantomas/docs/end-users/FormattingCheck.html) on the solution file. +- `CheckOrder` - Runs `build/check-order.fsx`, which checks that every signature file groups its functions by category in the order of ADR 0001 and sorts them alphabetically inside a category. Runs before `DotnetBuild`. - `BuildDocs` - Generates [Documentation](https://fsprojects.github.io/FSharp.Formatting) from `docsSrc` and the [XML Documentation Comments](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/) from your libraries in `src`. - `WatchDocs` - Generates documentation and starts a webserver locally. It will rebuild and hot reload if it detects any changes made to `docsSrc` files, or libraries in `src`. diff --git a/build/build.fs b/build/build.fs index 77346b84..aad2b703 100644 --- a/build/build.fs +++ b/build/build.fs @@ -163,6 +163,8 @@ module dotnet = let fantomas args = DotNet.exec id "fantomas" args + let fsi args = tool id "fsi" args + module FSharpAnalyzers = type Arguments = | Project of string @@ -588,6 +590,12 @@ let checkFormatCode ctx = else Trace.logf "Errors while formatting: %A" result.Errors +/// Checks the category banners and the alphabetical order of the functions in the signature files (ADR 0001, D4) +let checkOrder _ = + let script = rootDirectory "build" "check-order.fsx" + let sourceDirectory = rootDirectory "src" + dotnet.fsi $"\"%s{script}\" \"%s{sourceDirectory}\"" + let cleanDocsCache _ = DocsTool.cleanDocsCache () @@ -651,6 +659,7 @@ let initTargets (ctx : Context.FakeExecutionContext) = Target.create "GitHubRelease" githubRelease Target.create "FormatCode" formatCode Target.create "CheckFormatCode" checkFormatCode + Target.create "CheckOrder" checkOrder Target.create "Release" ignore // For local Target.create "Publish" ignore //For CI Target.create "CleanDocsCache" cleanDocsCache @@ -698,6 +707,7 @@ let initTargets (ctx : Context.FakeExecutionContext) = "DotnetRestore" =?> ("CheckFormatCode", isCI.Value) + ==> "CheckOrder" ==> "DotnetBuild" ==> "DotnetTest" ==> "DotnetPack" @@ -708,6 +718,7 @@ let initTargets (ctx : Context.FakeExecutionContext) = "DotnetRestore" =?> ("CheckFormatCode", isCI.Value) =?> ("GenerateAssemblyInfo", isPublishToGitHub) + ==> "CheckOrder" ==> "DotnetBuild" ==> "DotnetTest" ==> "DotnetPack" diff --git a/build/check-order.fsx b/build/check-order.fsx new file mode 100644 index 00000000..fdb1ca1a --- /dev/null +++ b/build/check-order.fsx @@ -0,0 +1,307 @@ +// Checks the order of the functions in the signature files of the library (ADR 0001, D4). +// +// dotnet fsi build/check-order.fsx [source directory] +// +// Every module of a signature file groups its `val`s under banner comments `// ---- Category ----`. The banners follow +// the canonical category order, and the `val`s of a category are sorted alphabetically (ordinal), active patterns last. +// The sibling implementation file has the same banners and defines its public functions in the same order. +// A violation is printed as `path(line): message`, and the script exits with code 1. +// +// The script needs no packages, so that the FAKE target `CheckOrder` and a contributor can run it before a build. + +open System +open System.IO +open System.Text.RegularExpressions + +/// The canonical category sequence of ADR 0001, D4. +let categories = [| + "Creation" + "Builder" + "Properties" + "Access" + "Search" + "Transformation" + "Folding" + "Partitioning and grouping" + "Sorting" + "Random" + "Conversion" + "Type-specific" +|] + +/// Maps and sets only partition, so their banner may say "Partitioning"; it takes the place of "Partitioning and grouping". +let categoryIndex (category : string) = + let canonical = + if String.Equals (category, "Partitioning", StringComparison.Ordinal) then + "Partitioning and grouping" + else + category + + Array.IndexOf (categories, canonical) + +let bannerRegex = Regex (@"^\s*// ---- (?.+?) ----\s*$", RegexOptions.CultureInvariant) + +let moduleRegex = + Regex ( + @"^(? *)module\s+(?(?:private|internal)\s+)?(?[A-Za-z_][A-Za-z0-9_]*)\s*=", + RegexOptions.CultureInvariant + ) + +// An identifier or an active pattern such as `(|Cons|_|)`, optionally preceded by attributes on the same line +let private nameGroup = @"(?\(\|[^)]*\|\)|[A-Za-z_][A-Za-z0-9_']*)" + +let valRegex = + Regex ( + @"^(? *)(?:\[<.*>\]\s*)?val\s+(?:inline\s+)?" + + nameGroup, + RegexOptions.CultureInvariant + ) + +let letRegex = + Regex ( + @"^(? *)(?:\[<.*>\]\s*)?let\s+(?:rec\s+)?(?:inline\s+)?(?(?:private|internal)\s+)?(?:inline\s+)?" + + nameGroup, + RegexOptions.CultureInvariant + ) + +[] +type Banner = { Line : int; Category : string } + +[] +type Member = { Line : int; Category : string; Name : string } + +/// A module of a source file: its banners and its public members in source order. +type Scope = { + Path : string + Indent : int + IsPublic : bool + Banners : ResizeArray + Members : ResizeArray +} + +let indentOf (line : string) = line.Length - line.AsSpan().TrimStart(' ').Length + +/// Reads the modules of a file. `memberRegex` recognises a member declaration (`val` or `let`); +/// members at the content level of a public module count, nested bindings and non-public members do not. +let parse (path : string) (memberRegex : Regex) = + let scopes = ResizeArray() + let stack = Collections.Generic.Stack() + let lines = File.ReadAllLines path + + for index in 0 .. lines.Length - 1 do + let line = lines[index] + let lineNumber = index + 1 + let trimmed = line.AsSpan().Trim() + + // Preprocessor directives start at column 0 and must not close the enclosing modules + if + not trimmed.IsEmpty + && not (trimmed.StartsWith ("#", StringComparison.Ordinal)) + then + let indent = indentOf line + + while stack.Count > 0 && indent <= stack.Peek().Indent do + stack.Pop () |> ignore + + let moduleMatch = moduleRegex.Match line + + if moduleMatch.Success then + let name = moduleMatch.Groups["name"].Value + let parent = + if stack.Count > 0 then + ValueSome (stack.Peek ()) + else + ValueNone + + let scope = { + Path = + match parent with + | ValueSome parent -> $"%s{parent.Path}.%s{name}" + | ValueNone -> name + Indent = indent + IsPublic = + not moduleMatch.Groups["access"].Success + && (parent |> ValueOption.forall _.IsPublic) + Banners = ResizeArray () + Members = ResizeArray () + } + + scopes.Add scope + stack.Push scope + elif stack.Count > 0 then + let scope = stack.Peek () + let bannerMatch = bannerRegex.Match line + + if bannerMatch.Success then + scope.Banners.Add { Line = lineNumber; Category = bannerMatch.Groups["category"].Value } + else + let memberMatch = memberRegex.Match line + + let isContentLevel = + memberMatch.Success + && memberMatch.Groups["indent"].Length = scope.Indent + 4 + let isPublic = isContentLevel && not memberMatch.Groups["access"].Success + + if scope.IsPublic && isPublic then + let category = + if scope.Banners.Count = 0 then + "" + else + scope.Banners[scope.Banners.Count - 1].Category + + scope.Members.Add { + Line = lineNumber + Category = category + Name = memberMatch.Groups["name"].Value + } + + scopes |> Seq.filter _.IsPublic |> Seq.toArray + +let isActivePattern (name : string) = name.StartsWith ("(|", StringComparison.Ordinal) + +/// Identifiers sort ordinally; active patterns come after them, also ordinally. +let compareNames (left : string) (right : string) = + match isActivePattern left, isActivePattern right with + | false, true -> -1 + | true, false -> 1 + | _ -> String.CompareOrdinal (left, right) + +let errors = ResizeArray() + +let report path line message = errors.Add (struct (path, line, message)) + +let checkSignature (path : string) (scope : Scope) = + let mutable previousIndex = -1 + + for banner in scope.Banners do + let index = categoryIndex banner.Category + + if index < 0 then + report path banner.Line $"Unknown category '%s{banner.Category}' in module %s{scope.Path}" + elif index <= previousIndex then + report path banner.Line $"Category '%s{banner.Category}' is out of the canonical order in module %s{scope.Path}" + else + previousIndex <- index + + let hasMembers = + scope.Members + |> Seq.exists (fun item -> String.Equals (item.Category, banner.Category, StringComparison.Ordinal)) + + if not hasMembers then + report path banner.Line $"Category '%s{banner.Category}' is empty in module %s{scope.Path}; omit its banner" + + for index in 0 .. scope.Members.Count - 1 do + let item = scope.Members[index] + + if item.Category.Length = 0 then + report path item.Line $"`%s{item.Name}` comes before the first category banner of module %s{scope.Path}" + elif index > 0 then + let previous = scope.Members[index - 1] + + if String.Equals (previous.Category, item.Category, StringComparison.Ordinal) then + let order = compareNames previous.Name item.Name + + if order = 0 then + report path item.Line $"`%s{item.Name}` is declared twice in module %s{scope.Path}" + elif order > 0 then + report path item.Line $"`%s{item.Name}` must come before `%s{previous.Name}` in category '%s{item.Category}'" + +let checkImplementation (signaturePath : string) (signatureScopes : Scope array) = + let implementationPath = Path.ChangeExtension (signaturePath, ".fs") + + if not (File.Exists implementationPath) then + report signaturePath 1 "The signature file has no implementation file next to it" + else + let implementationScopes = parse implementationPath letRegex + + let findScope (scopes : Scope array) path = + scopes + |> Array.tryFind (fun scope -> String.Equals (scope.Path, path, StringComparison.Ordinal)) + |> Option.toValueOption + + for signature in signatureScopes do + match findScope implementationScopes signature.Path with + | ValueNone -> report implementationPath 1 $"Module %s{signature.Path} of the signature file is missing" + | ValueSome implementation -> + let signatureBanners = signature.Banners |> Seq.map _.Category |> Seq.toArray + let implementationBanners = implementation.Banners |> Seq.map _.Category |> Seq.toArray + + if signatureBanners <> implementationBanners then + let line = + if implementation.Banners.Count > 0 then + implementation.Banners[0].Line + else + 1 + + let expected = String.Join (", ", signatureBanners) + let actual = String.Join (", ", implementationBanners) + report + implementationPath + line + $"Module %s{signature.Path} has the banners [%s{actual}]; the signature has [%s{expected}]" + + let count = min signature.Members.Count implementation.Members.Count + + let firstDifference = + seq { 0 .. count - 1 } + |> Seq.tryFind (fun index -> + not ( + String.Equals ( + signature.Members[index].Name, + implementation.Members[index].Name, + StringComparison.Ordinal + ) + ) + ) + |> Option.toValueOption + + match firstDifference with + | ValueSome index -> + let expected = signature.Members[index] + let actual = implementation.Members[index] + report + implementationPath + actual.Line + $"`%s{actual.Name}` is defined where the signature declares `%s{expected.Name}` (line %d{expected.Line})" + | ValueNone when signature.Members.Count <> implementation.Members.Count -> + report + implementationPath + 1 + $"Module %s{signature.Path} defines %d{implementation.Members.Count} public functions; the signature declares %d{signature.Members.Count}" + | ValueNone -> () + +let sourceDirectory = + match fsi.CommandLineArgs |> Array.skip 1 with + | [| directory |] -> Path.GetFullPath directory + | [||] -> Path.GetFullPath (Path.Combine (__SOURCE_DIRECTORY__, "..", "src")) + | _ -> failwith "Usage: dotnet fsi build/check-order.fsx [source directory]" + +let isGenerated (path : string) = + let separator = Path.DirectorySeparatorChar + path.Contains ($"%c{separator}obj%c{separator}", StringComparison.Ordinal) + || path.Contains ($"%c{separator}bin%c{separator}", StringComparison.Ordinal) + +let signatureFiles = + Directory.EnumerateFiles (sourceDirectory, "*.fsi", SearchOption.AllDirectories) + |> Seq.filter (isGenerated >> not) + |> Seq.sort + |> Seq.toArray + +for signaturePath in signatureFiles do + let scopes = parse signaturePath valRegex + + for scope in scopes do + checkSignature signaturePath scope + + checkImplementation signaturePath scopes + +if errors.Count = 0 then + printfn $"check-order: %d{signatureFiles.Length} signature files are in order" +else + for struct (path, line, message) in + errors + |> Seq.sortBy (fun struct (path, line, _) -> struct (path, line)) do + eprintfn $"%s{path}(%d{line}): %s{message}" + + eprintfn $"check-order: %d{errors.Count} violations" + exit 1 diff --git a/docsSrc/Explanations/Background.md b/docsSrc/Explanations/Background.md index a1525b3a..c568bf59 100644 --- a/docsSrc/Explanations/Background.md +++ b/docsSrc/Explanations/Background.md @@ -24,15 +24,17 @@ While F# has its own built-in immutable collections, System.Collections.Immutabl ## Available Collections -- `FlatList` (`ImmutableArray`) -- `ImmutableList` -- `Stack` (`ImmutableStack`) -- `Queue` (`ImmutableQueue`) -- `HashMap` (`ImmutableDictionary`) -- `SortedMap` (`ImmutableSortedDictionary`) -- `HashSet` (`ImmutableHashSet`) -- `SortedSet` (`ImmutableSortedSet`) -- `IIndexedSeq` (`IReadOnlyList`) +| Module | .NET type | Computation expression | +|---|---|---| +| `FlatList` | `ImmutableArray<'T>` | – | +| `ImmutableList` | `ImmutableList<'T>` | – | +| `Stack` | `ImmutableStack<'T>` | – | +| `Queue` | `ImmutableQueue<'T>` | – | +| `HashMap` | `ImmutableDictionary<'Key, 'Value>` | – | +| `SortedMap` | `ImmutableSortedDictionary<'Key, 'Value>` | – | +| `HashSet` | `ImmutableHashSet<'T>` | – | +| `SortedSet` | `ImmutableSortedSet<'T>` | – | +| `IIndexedSeq` | `IReadOnlyList<'T>` | – | --- diff --git a/docsSrc/index.md b/docsSrc/index.md index a58115a2..39c16874 100644 --- a/docsSrc/index.md +++ b/docsSrc/index.md @@ -6,15 +6,17 @@ F# bindings for [System.Collections.Immutable](https://learn.microsoft.com/en-us ## Key Features -- `FlatList` (`ImmutableArray`) -- `ImmutableList` -- `Stack` (`ImmutableStack`) -- `Queue` (`ImmutableQueue`) -- `HashMap` (`ImmutableDictionary`) -- `SortedMap` (`ImmutableSortedDictionary`) -- `HashSet` (`ImmutableHashSet`) -- `SortedSet` (`ImmutableSortedSet`) -- `IIndexedSeq` (`IReadOnlyList`) +| Module | .NET type | Computation expression | +|---|---|---| +| `FlatList` | `ImmutableArray<'T>` | – | +| `ImmutableList` | `ImmutableList<'T>` | – | +| `Stack` | `ImmutableStack<'T>` | – | +| `Queue` | `ImmutableQueue<'T>` | – | +| `HashMap` | `ImmutableDictionary<'Key, 'Value>` | – | +| `SortedMap` | `ImmutableSortedDictionary<'Key, 'Value>` | – | +| `HashSet` | `ImmutableHashSet<'T>` | – | +| `SortedSet` | `ImmutableSortedSet<'T>` | – | +| `IIndexedSeq` | `IReadOnlyList<'T>` | – | --- diff --git a/src/FSharp.Collections.Immutable/FSharp.Collections.Immutable.fsproj b/src/FSharp.Collections.Immutable/FSharp.Collections.Immutable.fsproj index 6a4bc983..f460764a 100644 --- a/src/FSharp.Collections.Immutable/FSharp.Collections.Immutable.fsproj +++ b/src/FSharp.Collections.Immutable/FSharp.Collections.Immutable.fsproj @@ -1,7 +1,7 @@  - net8.0 + net8.0;net10.0 $(AssemblyBaseName) true true @@ -20,7 +20,7 @@ - + diff --git a/src/FSharp.Collections.Immutable/Helper.fs b/src/FSharp.Collections.Immutable/Helper.fs new file mode 100644 index 00000000..b4e365b7 --- /dev/null +++ b/src/FSharp.Collections.Immutable/Helper.fs @@ -0,0 +1,37 @@ +#if INTERACTIVE +namespace global +#else +namespace FSharp.Collections.Immutable +#endif + +open System.Collections.Immutable + +/// Argument checks shared by the collection modules. +/// The module keeps its old name, so that the modules written before ADR 0001 compile unchanged until they are replaced. +[] +module internal ImmutableCollectionUtil = + + /// Throws `ArgumentNullException` for a `null` argument, as FSharp.Core does for a `null` array or list. + let inline checkNotNull name (arg : _ | null) = + match arg with + | null -> nullArg name + | _ -> () + + /// + /// Throws for a default . + /// + /// + /// A default immutable array has no underlying array. The collection modules treat it as FSharp.Core treats a + /// array (ADR 0001, D6). + /// + let inline checkNotDefault name (array : ImmutableArray<'T>) = + if array.IsDefault then + nullArg name + +/// Exception messages of the modules written before ADR 0001. +module internal ErrorStrings = + [] + let InputMustBeNonNegative = "The input must be non-negative." + + [] + let ListsHaveDifferentLengths = "The lists have different lengths." diff --git a/src/FSharp.Collections.Immutable/ImmutableCollectionUtil.fs b/src/FSharp.Collections.Immutable/ImmutableCollectionUtil.fs deleted file mode 100644 index 608fe4fe..00000000 --- a/src/FSharp.Collections.Immutable/ImmutableCollectionUtil.fs +++ /dev/null @@ -1,19 +0,0 @@ -#if INTERACTIVE -namespace global -#else -namespace FSharp.Collections.Immutable -#endif - -[] -module internal ImmutableCollectionUtil = - let inline checkNotNull name (arg : _ | null) = - match arg with - | null -> nullArg name - | _ -> () - -module internal ErrorStrings = - [] - let InputMustBeNonNegative = "The input must be non-negative." - - [] - let ListsHaveDifferentLengths = "The lists have different lengths." diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs new file mode 100644 index 00000000..b64db207 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -0,0 +1,96 @@ +namespace FSharp.Collections.Immutable.Tests + +open System +open System.Globalization +open Microsoft.VisualStudio.TestTools.UnitTesting + +/// The FSharp.Core module whose argument checks a collection module mirrors (ADR 0001, D6). +type Mirror = + /// `FlatList` mirrors `Array`. + | ArrayMirror + /// `IndexedList`, `Queue`, `Stack` and their interface modules mirror `List`. + | ListMirror + +/// Compares the exception of a function with the exception of the FSharp.Core function that it mirrors (ADR 0001, D8). +[] +module ExceptionAssert = + + /// Runs an action under the invariant culture. + /// FSharp.Core ships localized messages and this library does not, so the messages are compared in English. + let private invariant (action : unit -> 'T) = + let culture = CultureInfo.CurrentCulture + let uiCulture = CultureInfo.CurrentUICulture + CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture + CultureInfo.CurrentUICulture <- CultureInfo.InvariantCulture + + try + action () + finally + CultureInfo.CurrentCulture <- culture + CultureInfo.CurrentUICulture <- uiCulture + + /// The first line of a message, without the parameter suffix of . + /// + /// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), + /// and this library writes the names of its own parameters there, so only the first line is compared. + /// + let headline (ex : exn) = + let message = + match ex with + | :? ArgumentException as argumentException -> + match argumentException.ParamName with + | null -> ex.Message + | paramName -> + let suffix = $" (Parameter '%s{paramName}')" + + if ex.Message.EndsWith (suffix, StringComparison.Ordinal) then + ex.Message.Substring (0, ex.Message.Length - suffix.Length) + else + ex.Message + | _ -> ex.Message + + match message.IndexOf ('\n') with + | -1 -> message + | newline -> message.AsSpan(0, newline).TrimEnd('\r').ToString() + + /// + /// Asserts that throws what throws: an exception of the same type + /// with the same headline and, for an argument exception, the parameter name that + /// maps the parameter name of the oracle to. + /// + let same (paramName : string -> string) (oracle : unit -> unit) (actual : unit -> unit) (message : string) = + let expected = + invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) + let thrown = invariant (fun () -> Assert.Throws(Action actual, message)) + Assert.AreEqual(expected.GetType (), thrown.GetType (), message) + Assert.AreEqual(headline expected, headline thrown, message) + + match expected, thrown with + | (:? ArgumentException as expected), (:? ArgumentException as thrown) -> + let expectedName = + match expected.ParamName with + | null -> null + | name -> paramName name + + Assert.AreEqual(expectedName, thrown.ParamName, message) + | _ -> () + + /// + /// Asserts that throws what the FSharp.Core module that the collection mirrors throws: + /// for , + /// for . + /// + let mirrored + (mirror : Mirror) + (paramName : string -> string) + (arrayOracle : unit -> unit) + (listOracle : unit -> unit) + (actual : unit -> unit) + (message : string) + = + let oracle = + match mirror with + | ArrayMirror -> arrayOracle + | ListMirror -> listOracle + + same paramName oracle actual message diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs new file mode 100644 index 00000000..5820fc03 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs @@ -0,0 +1,38 @@ +namespace FSharp.Collections.Immutable.Tests + +/// The inputs that the contract tests run the functions on. +/// Expectations are computed with FSharp.Core over the order in which the collection enumerates its elements (ADR 0001, D8). +[] +module Inputs = + + /// No elements. + let empty : int list = [] + + /// One element. + let singleton = [ 42 ] + + /// Unsorted, with duplicates, negative numbers, zero and an even length. + let several = [ 5; -3; 8; 5; 0; -3; 12; 7 ] + + /// Every input. + let all = [ empty; singleton; several ] + + /// Every input with at least one element. + let nonEmpty = [ singleton; several ] + + /// A second input for the functions of two collections, as long as `several`. + let other = [ 2; 4; 6; 8; 10; 12; 14; 16 ] + + /// A third input for the functions of three collections, as long as `several`. + let third = [ -1; -2; -3; -4; -5; -6; -7; -8 ] + + /// Elements whose projection collides, for the stability of sorting and the order of grouping: + /// the pairs share a key in their first component and are told apart by the second. + let pairs = [ + struct (2, 'a') + struct (1, 'b') + struct (2, 'c') + struct (1, 'd') + struct (0, 'e') + struct (2, 'f') + ] diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs new file mode 100644 index 00000000..d283eb1f --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -0,0 +1,23 @@ +namespace FSharp.Collections.Immutable.Tests + +open Microsoft.VisualStudio.TestTools.UnitTesting + +/// Compares a function of the `Option` or `Tuple` namespace with the default function after conversion (ADR 0001, D3). +[] +module OverrideAssert = + + /// The `Option` namespace returns an `option` where the default module returns a `voption`. + let optionMatches (expected : 'T voption) (actual : 'T option) (message : string) = + Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message) + + /// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. + let pairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2)) (actual : 'T1 * 'T2) (message : string) = + Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, message) + + /// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. + let tripleMatches + (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3)) + (actual : 'T1 * 'T2 * 'T3) + (message : string) + = + Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, message) diff --git a/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj b/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj index 93335254..7ddd1712 100644 --- a/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj +++ b/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj @@ -1,7 +1,7 @@  - net8.0 + net8.0;net10.0 $(AssemblyBaseName).Tests Exe @@ -17,13 +17,11 @@ - - - - - - - + + + + + diff --git a/tests/FSharp.Collections.Immutable.Tests/FlatList.fs b/tests/FSharp.Collections.Immutable.Tests/FlatList.fs deleted file mode 100644 index 351bac90..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/FlatList.fs +++ /dev/null @@ -1,10 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type FlatListTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/ImmutableList.fs b/tests/FSharp.Collections.Immutable.Tests/ImmutableList.fs deleted file mode 100644 index 452167fe..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/ImmutableList.fs +++ /dev/null @@ -1,10 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type ImmutableListTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/IndexedSeq.fs b/tests/FSharp.Collections.Immutable.Tests/IndexedSeq.fs deleted file mode 100644 index 17294a9a..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/IndexedSeq.fs +++ /dev/null @@ -1,10 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type IndexedSeqTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/Maps.fs b/tests/FSharp.Collections.Immutable.Tests/Maps.fs deleted file mode 100644 index 84f2fde8..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/Maps.fs +++ /dev/null @@ -1,16 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type HashMapTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) - -[] -type SortedMapTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/Queue.fs b/tests/FSharp.Collections.Immutable.Tests/Queue.fs deleted file mode 100644 index 826a1cbc..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/Queue.fs +++ /dev/null @@ -1,10 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type QueueTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/Sets.fs b/tests/FSharp.Collections.Immutable.Tests/Sets.fs deleted file mode 100644 index b1704205..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/Sets.fs +++ /dev/null @@ -1,16 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type HashSetTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) - -[] -type SortedSetTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/Stack.fs b/tests/FSharp.Collections.Immutable.Tests/Stack.fs deleted file mode 100644 index 2674f682..00000000 --- a/tests/FSharp.Collections.Immutable.Tests/Stack.fs +++ /dev/null @@ -1,10 +0,0 @@ -namespace FSharp.Collections.Immutable.Tests - -open System -open Microsoft.VisualStudio.TestTools.UnitTesting - -[] -type StackTests () = - - [] - member this.TestMethodPassing () = Assert.IsTrue (true) diff --git a/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs b/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs new file mode 100644 index 00000000..9c327b94 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs @@ -0,0 +1,139 @@ +namespace FSharp.Collections.Immutable.Tests + +open System.Collections.Generic +open Microsoft.VisualStudio.TestTools.UnitTesting + +// Each attribute gives one category: its type name without the `TestCategoryAttribute` suffix, written as separate words. +// The attributes are sorted by type name (ordinal); a new attribute goes to its sorted position. + +/// The builder functions and the builder modules. +type BuilderTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Builder" |] + override _.TestCategories = categories + +/// The computation expressions. +type ComputationExpressionTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Computation expression" |] + override _.TestCategories = categories + +/// The conventions of the test project itself. +type ConventionTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Convention" |] + override _.TestCategories = categories + +/// The `FlatList` module (`ImmutableArray`). +type FlatListTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "FlatList" |] + override _.TestCategories = categories + +/// The `FrozenMap` module (`FrozenDictionary`). +type FrozenMapTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "FrozenMap" |] + override _.TestCategories = categories + +/// The `FrozenSet` module (`FrozenSet`). +type FrozenSetTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "FrozenSet" |] + override _.TestCategories = categories + +/// The `HashMap` module (`ImmutableDictionary`). +type HashMapTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "HashMap" |] + override _.TestCategories = categories + +/// The `HashSet` module (`ImmutableHashSet`). +type HashSetTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "HashSet" |] + override _.TestCategories = categories + +/// The `IIndexedList` module (`IImmutableList`). +type IIndexedListTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "IIndexedList" |] + override _.TestCategories = categories + +/// The `IMap` module (`IImmutableDictionary`). +type IMapTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "IMap" |] + override _.TestCategories = categories + +/// The `IQueue` module (`IImmutableQueue`). +type IQueueTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "IQueue" |] + override _.TestCategories = categories + +/// The `ISet` module (`IImmutableSet`). +type ISetTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "ISet" |] + override _.TestCategories = categories + +/// The `IStack` module (`IImmutableStack`). +type IStackTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "IStack" |] + override _.TestCategories = categories + +/// The `IndexedList` module (`ImmutableList`). +type IndexedListTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "IndexedList" |] + override _.TestCategories = categories + +/// The modules of the `FSharp.Collections.Immutable.Option` namespace. +type OptionNamespaceTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Option namespace" |] + override _.TestCategories = categories + +/// The nested `Parallel` modules. +type ParallelTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Parallel" |] + override _.TestCategories = categories + +/// The active patterns. +type PatternMatchingTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Pattern matching" |] + override _.TestCategories = categories + +/// The `Queue` module (`ImmutableQueue`). +type QueueTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Queue" |] + override _.TestCategories = categories + +/// The `SortedMap` module (`ImmutableSortedDictionary`). +type SortedMapTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "SortedMap" |] + override _.TestCategories = categories + +/// The `SortedSet` module (`ImmutableSortedSet`). +type SortedSetTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "SortedSet" |] + override _.TestCategories = categories + +/// The `Stack` module (`ImmutableStack`). +type StackTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Stack" |] + override _.TestCategories = categories + +/// The modules of the `FSharp.Collections.Immutable.Tuple` namespace. +type TupleNamespaceTestCategoryAttribute () = + inherit TestCategoryBaseAttribute () + static let categories : IList = [| "Tuple namespace" |] + override _.TestCategories = categories diff --git a/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs b/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs new file mode 100644 index 00000000..dccfd8f4 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs @@ -0,0 +1,83 @@ +namespace FSharp.Collections.Immutable.Tests + +open System +open System.Reflection +open Microsoft.VisualStudio.TestTools.UnitTesting + +/// Enforces the category rules of `.github/instructions/test-categories.instructions.md` and of the copilot instructions. +[] +type TestCategoriesTests () = + + static let suffix = "TestCategoryAttribute" + + static let assembly = typeof.Assembly + + static let categoryAttributeTypes = + assembly.GetTypes () + |> Array.filter (fun candidate -> + candidate.IsSubclassOf typeof + && not candidate.IsAbstract + ) + + static let testClasses = + assembly.GetTypes () + |> Array.filter (fun candidate -> candidate.IsDefined (typeof, false)) + + /// A category attribute of this project, as opposed to the `TestCategory` attribute of MSTest. + static let isProjectCategory (attribute : TestCategoryBaseAttribute) = attribute.GetType().Assembly = assembly + + [] + member _.``Every category attribute gives one category named after its type`` () = + Assert.IsNotEmpty (categoryAttributeTypes, "TestCategories.fs must define category attributes") + + for attributeType in categoryAttributeTypes do + let name = attributeType.Name + StringAssert.EndsWith ( + name, + suffix, + StringComparison.Ordinal, + "A category attribute must end with the TestCategoryAttribute suffix" + ) + + let attribute = nonNull (Activator.CreateInstance attributeType) :?> TestCategoryBaseAttribute + let category = + Assert.ContainsSingle (attribute.TestCategories, $"%s{name} must give exactly one category") + let expected = name.Substring (0, name.Length - suffix.Length) + + Assert.AreEqual( + expected.ToUpperInvariant (), + category.Replace(" ", "").ToUpperInvariant(), + $"The category of %s{name} must be its type name without the suffix, written as separate words" + ) + + Assert.IsTrue (Char.IsUpper category[0], $"The category of %s{name} must start with a capital letter") + + [] + member _.``Category attributes are sorted by type name`` () = + let names = categoryAttributeTypes |> Array.map _.Name + let sorted = + names + |> Array.sortWith (fun left right -> String.CompareOrdinal (left, right)) + CollectionAssert.AreEqual (sorted, names, "TestCategories.fs must keep its attributes sorted by type name") + + [] + member _.``Every test class has a category attribute of this project`` () = + Assert.IsNotEmpty (testClasses, "The assembly must contain test classes") + + for testClass in testClasses do + let categories = + testClass.GetCustomAttributes(true) + |> Seq.filter isProjectCategory + + Assert.IsNotEmpty (categories, $"%s{testClass.Name} must carry the category attribute of what it examines") + + [] + member _.``Every test method has an operation category`` () = + for testClass in testClasses do + let testMethods = + testClass.GetMethods (BindingFlags.Public ||| BindingFlags.Instance) + |> Array.filter (fun testMethod -> testMethod.IsDefined (typeof, true)) + + for testMethod in testMethods do + let categories = testMethod.GetCustomAttributes(true) + Assert.IsNotEmpty (categories, $"%s{testClass.Name}.%s{testMethod.Name} must carry an operation category") From 52a4ab8456edb2c68cad314c64de9cfbb8389039 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 12:53:19 +0200 Subject: [PATCH 02/10] fixup! chore: lay the foundation for the per-collection modules `build/check-order.fsx` (review of #25): - understands file-scoped modules (`module A.B` without `=`), whose members start at column 0, and `module rec`; - reports a `val` at column 0 outside every module that it understands, instead of passing an unknown header unchecked; - compares the category of each function of the implementation with the signature, so that a moved banner is reported. Co-Authored-By: Claude Opus 5.5 --- build/check-order.fsx | 86 +++++++++++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 20 deletions(-) diff --git a/build/check-order.fsx b/build/check-order.fsx index fdb1ca1a..00fac478 100644 --- a/build/check-order.fsx +++ b/build/check-order.fsx @@ -43,7 +43,14 @@ let bannerRegex = Regex (@"^\s*// ---- (?.+?) ----\s*$", RegexOptions. let moduleRegex = Regex ( - @"^(? *)module\s+(?(?:private|internal)\s+)?(?[A-Za-z_][A-Za-z0-9_]*)\s*=", + @"^(? *)module\s+(?(?:private|internal)\s+)?(?:rec\s+)?(?[A-Za-z_][A-Za-z0-9_]*)\s*=", + RegexOptions.CultureInvariant + ) + +/// A file-scoped module, `module A.B` without `=`, whose members start at column 0. +let fileModuleRegex = + Regex ( + @"^module\s+(?(?:private|internal)\s+)?(?:rec\s+)?(?[A-Za-z_][A-Za-z0-9_.]*)\s*$", RegexOptions.CultureInvariant ) @@ -81,10 +88,15 @@ type Scope = { let indentOf (line : string) = line.Length - line.AsSpan().TrimStart(' ').Length -/// Reads the modules of a file. `memberRegex` recognises a member declaration (`val` or `let`); -/// members at the content level of a public module count, nested bindings and non-public members do not. +/// Reads the modules of a file. +/// +/// memberRegex recognises a member declaration (val or let); members at the content level of a public +/// module count, nested bindings and non-public members do not. The second result lists the members at column 0 outside +/// any module: they belong to a module header that the script does not understand, so they must not pass unchecked. +/// let parse (path : string) (memberRegex : Regex) = let scopes = ResizeArray() + let orphans = ResizeArray() let stack = Collections.Generic.Stack() let lines = File.ReadAllLines path @@ -104,8 +116,21 @@ let parse (path : string) (memberRegex : Regex) = stack.Pop () |> ignore let moduleMatch = moduleRegex.Match line + let fileModuleMatch = fileModuleRegex.Match line - if moduleMatch.Success then + if fileModuleMatch.Success then + // The members of a file-scoped module start at column 0, the content level of an indent of -4 + let scope = { + Path = fileModuleMatch.Groups["name"].Value + Indent = -4 + IsPublic = not fileModuleMatch.Groups["access"].Success + Banners = ResizeArray () + Members = ResizeArray () + } + + scopes.Add scope + stack.Push scope + elif moduleMatch.Success then let name = moduleMatch.Groups["name"].Value let parent = if stack.Count > 0 then @@ -154,8 +179,16 @@ let parse (path : string) (memberRegex : Regex) = Category = category Name = memberMatch.Groups["name"].Value } + else + let memberMatch = memberRegex.Match line - scopes |> Seq.filter _.IsPublic |> Seq.toArray + if + memberMatch.Success + && memberMatch.Groups["indent"].Length = 0 + then + orphans.Add { Line = lineNumber; Category = ""; Name = memberMatch.Groups["name"].Value } + + struct (scopes |> Seq.filter _.IsPublic |> Seq.toArray, orphans |> Seq.toArray) let isActivePattern (name : string) = name.StartsWith ("(|", StringComparison.Ordinal) @@ -212,7 +245,7 @@ let checkImplementation (signaturePath : string) (signatureScopes : Scope array) if not (File.Exists implementationPath) then report signaturePath 1 "The signature file has no implementation file next to it" else - let implementationScopes = parse implementationPath letRegex + let struct (implementationScopes, _) = parse implementationPath letRegex let findScope (scopes : Scope array) path = scopes @@ -242,27 +275,34 @@ let checkImplementation (signaturePath : string) (signatureScopes : Scope array) let count = min signature.Members.Count implementation.Members.Count + // A member must have the same name and the same category, so that a moved banner cannot go unnoticed + let differs index = + let expected = signature.Members[index] + let actual = implementation.Members[index] + + not (String.Equals (expected.Name, actual.Name, StringComparison.Ordinal)) + || not (String.Equals (expected.Category, actual.Category, StringComparison.Ordinal)) + let firstDifference = seq { 0 .. count - 1 } - |> Seq.tryFind (fun index -> - not ( - String.Equals ( - signature.Members[index].Name, - implementation.Members[index].Name, - StringComparison.Ordinal - ) - ) - ) + |> Seq.tryFind differs |> Option.toValueOption match firstDifference with | ValueSome index -> let expected = signature.Members[index] let actual = implementation.Members[index] - report - implementationPath - actual.Line - $"`%s{actual.Name}` is defined where the signature declares `%s{expected.Name}` (line %d{expected.Line})" + + if String.Equals (expected.Name, actual.Name, StringComparison.Ordinal) then + report + implementationPath + actual.Line + $"`%s{actual.Name}` is in category '%s{actual.Category}'; the signature puts it in '%s{expected.Category}' (line %d{expected.Line})" + else + report + implementationPath + actual.Line + $"`%s{actual.Name}` is defined where the signature declares `%s{expected.Name}` (line %d{expected.Line})" | ValueNone when signature.Members.Count <> implementation.Members.Count -> report implementationPath @@ -288,7 +328,13 @@ let signatureFiles = |> Seq.toArray for signaturePath in signatureFiles do - let scopes = parse signaturePath valRegex + let struct (scopes, orphans) = parse signaturePath valRegex + + for orphan in orphans do + report + signaturePath + orphan.Line + $"`%s{orphan.Name}` is outside every module header that the script understands (`module X =` or `module A.B`)" for scope in scopes do checkSignature signaturePath scope From 54d6df3cf313c55b74069f2ee365b8e2d25cc7ed Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 15:23:08 +0200 Subject: [PATCH 03/10] fixup! chore: lay the foundation for the per-collection modules Follows the reworked ADR 0001: - Test categories go on the test classes only (D8). `TestCategoriesTests` checks that no test class has a category written as a string and that test methods carry no categories, instead of an operation category on every method; its own methods lose their categories. `TestCategories.fs` states the rule. - Root modules (D2): `ExceptionAssert`, `Inputs` and `OverrideAssert` are the root modules of their files. The `Mirror` union moves to `Contracts/Mirror.fs`, so it stays in the namespace. - `check-order.fsx` reads the access modifier of `val internal`/`val private`, so an internal value no longer counts as a public function named `internal`. - The copilot instructions list the types files, the comparer structs, the extension modules and the rules of the reworked ADR. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 36 +++- build/check-order.fsx | 3 +- .../Contracts/ExceptionAssert.fs | 155 +++++++++--------- .../Contracts/Inputs.fs | 52 +++--- .../Contracts/Mirror.fs | 8 + .../Contracts/OverrideAssert.fs | 34 ++-- .../FSharp.Collections.Immutable.Tests.fsproj | 1 + .../TestCategories.fs | 2 + .../TestCategoriesTests.fs | 23 ++- 9 files changed, 175 insertions(+), 139 deletions(-) create mode 100644 tests/FSharp.Collections.Immutable.Tests/Contracts/Mirror.fs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 40e0e7a1..e7655dc1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,7 +10,9 @@ ## Solution Structure -**IMPORTANT: Every collection binding implementation file must have a corresponding `.fsi` signature file.** +**IMPORTANT: Every collection binding implementation file must have a corresponding `.fsi` signature file.** The one +exception is the `XTypes.fs` file of a folder, which holds only type abbreviations and is its own signature: a signature +file without an implementation file does not compile (FS0240). ```text / @@ -20,43 +22,61 @@ ├── src/FSharp.Collections.Immutable/ – main library │ ├── Helper.fs – argument checks and exceptions shared by the modules │ ├── FlatList/ – ImmutableArray API surface and builders +│ │ ├── FlatListTypes.fs – the FlatList abbreviation │ │ ├── FlatList.fs / FlatList.fsi – includes the nested Parallel module │ │ ├── FlatListOption.fs / FlatListOption.fsi – Option namespace overrides │ │ ├── FlatListTuple.fs / FlatListTuple.fsi – Tuple namespace overrides │ │ ├── FlatListBuilder.fs │ │ └── FlatListCE.fs │ ├── IndexedList/ – ImmutableList API surface and builders +│ │ ├── IndexedListTypes.fs │ │ ├── IndexedList.fs / IndexedList.fsi (+ Option / Tuple) │ │ ├── IIndexedList.fs / IIndexedList.fsi (+ Option / Tuple) │ │ ├── IndexedListBuilder.fs │ │ └── IndexedListCE.fs │ ├── Queue/ – ImmutableQueue API surface and CE +│ │ ├── QueueTypes.fs – the Queue and IQueue abbreviations │ │ ├── Queue.fs / Queue.fsi (+ Option / Tuple) │ │ ├── IQueue.fs / IQueue.fsi (+ Option / Tuple) │ │ └── QueueCE.fs │ ├── Stack/ – ImmutableStack API surface and CE +│ │ ├── StackTypes.fs – the Stack and IStack abbreviations │ │ ├── Stack.fs / Stack.fsi (+ Option / Tuple) │ │ ├── IStack.fs / IStack.fsi (+ Option / Tuple) │ │ └── StackCE.fs │ ├── HashMap/ – ImmutableDictionary API surface and CE -│ │ ├── HashMap.fs / HashMap.fsi (+ Option / Tuple) +│ │ ├── HashMapTypes.fs – the HashMap and IMap abbreviations +│ │ ├── HashMap.fs / HashMap.fsi (+ Option / Tuple) – nested Tuple / ValueTuple submodules +│ │ ├── CustomComparersHashMap.fs / .fsi – creates hash maps with comparers +│ │ ├── HashMapExtensions.fs / .fsi – ofArray / ofList / ofSeq for every binding shape │ │ ├── IMap.fs / IMap.fsi (+ Option / Tuple) │ │ └── HashMapCE.fs │ ├── SortedMap/ – ImmutableSortedDictionary API surface and CE +│ │ ├── SortedMapTypes.fs │ │ ├── SortedMap.fs / SortedMap.fsi (+ Option / Tuple) +│ │ ├── CustomComparersSortedMap.fs / .fsi +│ │ ├── SortedMapExtensions.fs / .fsi │ │ └── SortedMapCE.fs │ ├── HashSet/ – ImmutableHashSet API surface and CE +│ │ ├── HashSetTypes.fs │ │ ├── HashSet.fs / HashSet.fsi (+ Option / Tuple) +│ │ ├── CustomComparerHashSet.fs / .fsi – creates hash sets with a comparer │ │ ├── ISet.fs / ISet.fsi (+ Option / Tuple) │ │ └── HashSetCE.fs │ ├── SortedSet/ – ImmutableSortedSet API surface and CE +│ │ ├── SortedSetTypes.fs │ │ ├── SortedSet.fs / SortedSet.fsi (+ Option / Tuple) +│ │ ├── CustomComparerSortedSet.fs / .fsi │ │ └── SortedSetCE.fs │ ├── FrozenMap/ – FrozenDictionary API surface and CE +│ │ ├── FrozenMapTypes.fs │ │ ├── FrozenMap.fs / FrozenMap.fsi (+ Option / Tuple) +│ │ ├── CustomComparersFrozenMap.fs / .fsi +│ │ ├── FrozenMapExtensions.fs / .fsi │ │ └── FrozenMapCE.fs -│ ├── FrozenSet/ – FrozenSet API surface and CE +│ ├── FrozenSet/ – FrozenSet API surface and CE (the BCL type keeps its name) │ │ ├── FrozenSet.fs / FrozenSet.fsi (+ Option / Tuple) +│ │ ├── CustomComparerFrozenSet.fs / .fsi │ │ └── FrozenSetCE.fs │ └── ComputationExpressions.fs – the computation expression instances └── tests/FSharp.Collections.Immutable.Tests/ – MSTest test project @@ -73,12 +93,20 @@ The modules follow ADR 0001 – Per-collection modules (`docsSrc/Explanations/AD `Properties`, `Access`, `Search`, `Transformation`, `Folding`, `Partitioning and grouping`, `Sorting`, `Random`, `Conversion`, `Type-specific`, and sorts the functions of a category alphabetically (ordinal), active patterns last. The `.fs` file repeats the order of its `.fsi` file. `build/check-order.fsx` (FAKE target `CheckOrder`) enforces it. +* A file with one module declares it as the root module of the file (`module FSharp.Collections.Immutable.FlatList`); + the type abbreviations of a folder live in its `XTypes.fs`. No module carries `[]`. * `try*` functions return `voption`, tuples are struct tuples. The namespaces `FSharp.Collections.Immutable.Option` and `FSharp.Collections.Immutable.Tuple` repeat these functions with `option` results and reference tuples. +* No module function creates a collection with comparers: the `CustomComparers*` (maps) and `CustomComparer*` (sets) + structs do, and the modules create the structs with `forComparers`, `forKeyComparer` and `forComparer`. +* Maps take bindings in every shape: `ofArray`, `ofList` and `ofSeq` are static extension members of `XExtensions`, + overloaded for `KeyValuePair`, reference tuples and struct tuples; `addRange` and `setItems` take `KeyValuePair`s, and + the nested `Tuple` and `ValueTuple` submodules of the module take the tuple shapes. * A function throws what the FSharp.Core function of the mirrored module throws for the same input: `Array` for `FlatList`, `List` for the other sequence-like modules, `Map` and `Set` for maps and sets. * Contract test bases in `Contracts/` take a record of the module's functions; a collection derives one test class per - capability that it supports. Expected values and exceptions come from FSharp.Core. + capability that it supports. Expected values and exceptions come from FSharp.Core. The categories are attributes on + the test classes; test methods carry none. ## Libraries in Use diff --git a/build/check-order.fsx b/build/check-order.fsx index 00fac478..7c204276 100644 --- a/build/check-order.fsx +++ b/build/check-order.fsx @@ -57,9 +57,10 @@ let fileModuleRegex = // An identifier or an active pattern such as `(|Cons|_|)`, optionally preceded by attributes on the same line let private nameGroup = @"(?\(\|[^)]*\|\)|[A-Za-z_][A-Za-z0-9_']*)" +// The access modifier is its own group, so that `val internal f` is a non-public `f`, not a value named `internal` let valRegex = Regex ( - @"^(? *)(?:\[<.*>\]\s*)?val\s+(?:inline\s+)?" + @"^(? *)(?:\[<.*>\]\s*)?val\s+(?:inline\s+)?(?(?:private|internal)\s+)?(?:inline\s+)?" + nameGroup, RegexOptions.CultureInvariant ) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs index b64db207..21f5e11c 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -1,96 +1,87 @@ -namespace FSharp.Collections.Immutable.Tests +/// Compares the exception of a function with the exception of the FSharp.Core function that it mirrors (ADR 0001, D8). +[] +module FSharp.Collections.Immutable.Tests.ExceptionAssert open System open System.Globalization open Microsoft.VisualStudio.TestTools.UnitTesting -/// The FSharp.Core module whose argument checks a collection module mirrors (ADR 0001, D6). -type Mirror = - /// `FlatList` mirrors `Array`. - | ArrayMirror - /// `IndexedList`, `Queue`, `Stack` and their interface modules mirror `List`. - | ListMirror - -/// Compares the exception of a function with the exception of the FSharp.Core function that it mirrors (ADR 0001, D8). -[] -module ExceptionAssert = - - /// Runs an action under the invariant culture. - /// FSharp.Core ships localized messages and this library does not, so the messages are compared in English. - let private invariant (action : unit -> 'T) = - let culture = CultureInfo.CurrentCulture - let uiCulture = CultureInfo.CurrentUICulture - CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture - CultureInfo.CurrentUICulture <- CultureInfo.InvariantCulture +/// Runs an action under the invariant culture. +/// FSharp.Core ships localized messages and this library does not, so the messages are compared in English. +let private invariant (action : unit -> 'T) = + let culture = CultureInfo.CurrentCulture + let uiCulture = CultureInfo.CurrentUICulture + CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture + CultureInfo.CurrentUICulture <- CultureInfo.InvariantCulture - try - action () - finally - CultureInfo.CurrentCulture <- culture - CultureInfo.CurrentUICulture <- uiCulture + try + action () + finally + CultureInfo.CurrentCulture <- culture + CultureInfo.CurrentUICulture <- uiCulture - /// The first line of a message, without the parameter suffix of . - /// - /// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), - /// and this library writes the names of its own parameters there, so only the first line is compared. - /// - let headline (ex : exn) = - let message = - match ex with - | :? ArgumentException as argumentException -> - match argumentException.ParamName with - | null -> ex.Message - | paramName -> - let suffix = $" (Parameter '%s{paramName}')" +/// The first line of a message, without the parameter suffix of . +/// +/// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), +/// and this library writes the names of its own parameters there, so only the first line is compared. +/// +let headline (ex : exn) = + let message = + match ex with + | :? ArgumentException as argumentException -> + match argumentException.ParamName with + | null -> ex.Message + | paramName -> + let suffix = $" (Parameter '%s{paramName}')" - if ex.Message.EndsWith (suffix, StringComparison.Ordinal) then - ex.Message.Substring (0, ex.Message.Length - suffix.Length) - else - ex.Message - | _ -> ex.Message + if ex.Message.EndsWith (suffix, StringComparison.Ordinal) then + ex.Message.Substring (0, ex.Message.Length - suffix.Length) + else + ex.Message + | _ -> ex.Message - match message.IndexOf ('\n') with - | -1 -> message - | newline -> message.AsSpan(0, newline).TrimEnd('\r').ToString() + match message.IndexOf ('\n') with + | -1 -> message + | newline -> message.AsSpan(0, newline).TrimEnd('\r').ToString() - /// - /// Asserts that throws what throws: an exception of the same type - /// with the same headline and, for an argument exception, the parameter name that - /// maps the parameter name of the oracle to. - /// - let same (paramName : string -> string) (oracle : unit -> unit) (actual : unit -> unit) (message : string) = - let expected = - invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) - let thrown = invariant (fun () -> Assert.Throws(Action actual, message)) - Assert.AreEqual(expected.GetType (), thrown.GetType (), message) - Assert.AreEqual(headline expected, headline thrown, message) +/// +/// Asserts that throws what throws: an exception of the same type +/// with the same headline and, for an argument exception, the parameter name that +/// maps the parameter name of the oracle to. +/// +let same (paramName : string -> string) (oracle : unit -> unit) (actual : unit -> unit) (message : string) = + let expected = + invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) + let thrown = invariant (fun () -> Assert.Throws(Action actual, message)) + Assert.AreEqual(expected.GetType (), thrown.GetType (), message) + Assert.AreEqual(headline expected, headline thrown, message) - match expected, thrown with - | (:? ArgumentException as expected), (:? ArgumentException as thrown) -> - let expectedName = - match expected.ParamName with - | null -> null - | name -> paramName name + match expected, thrown with + | (:? ArgumentException as expected), (:? ArgumentException as thrown) -> + let expectedName = + match expected.ParamName with + | null -> null + | name -> paramName name - Assert.AreEqual(expectedName, thrown.ParamName, message) - | _ -> () + Assert.AreEqual(expectedName, thrown.ParamName, message) + | _ -> () - /// - /// Asserts that throws what the FSharp.Core module that the collection mirrors throws: - /// for , - /// for . - /// - let mirrored - (mirror : Mirror) - (paramName : string -> string) - (arrayOracle : unit -> unit) - (listOracle : unit -> unit) - (actual : unit -> unit) - (message : string) - = - let oracle = - match mirror with - | ArrayMirror -> arrayOracle - | ListMirror -> listOracle +/// +/// Asserts that throws what the FSharp.Core module that the collection mirrors throws: +/// for , +/// for . +/// +let mirrored + (mirror : Mirror) + (paramName : string -> string) + (arrayOracle : unit -> unit) + (listOracle : unit -> unit) + (actual : unit -> unit) + (message : string) + = + let oracle = + match mirror with + | ArrayMirror -> arrayOracle + | ListMirror -> listOracle - same paramName oracle actual message + same paramName oracle actual message diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs index 5820fc03..1de0b72a 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs @@ -1,38 +1,36 @@ -namespace FSharp.Collections.Immutable.Tests - /// The inputs that the contract tests run the functions on. /// Expectations are computed with FSharp.Core over the order in which the collection enumerates its elements (ADR 0001, D8). [] -module Inputs = +module FSharp.Collections.Immutable.Tests.Inputs - /// No elements. - let empty : int list = [] +/// No elements. +let empty : int list = [] - /// One element. - let singleton = [ 42 ] +/// One element. +let singleton = [ 42 ] - /// Unsorted, with duplicates, negative numbers, zero and an even length. - let several = [ 5; -3; 8; 5; 0; -3; 12; 7 ] +/// Unsorted, with duplicates, negative numbers, zero and an even length. +let several = [ 5; -3; 8; 5; 0; -3; 12; 7 ] - /// Every input. - let all = [ empty; singleton; several ] +/// Every input. +let all = [ empty; singleton; several ] - /// Every input with at least one element. - let nonEmpty = [ singleton; several ] +/// Every input with at least one element. +let nonEmpty = [ singleton; several ] - /// A second input for the functions of two collections, as long as `several`. - let other = [ 2; 4; 6; 8; 10; 12; 14; 16 ] +/// A second input for the functions of two collections, as long as `several`. +let other = [ 2; 4; 6; 8; 10; 12; 14; 16 ] - /// A third input for the functions of three collections, as long as `several`. - let third = [ -1; -2; -3; -4; -5; -6; -7; -8 ] +/// A third input for the functions of three collections, as long as `several`. +let third = [ -1; -2; -3; -4; -5; -6; -7; -8 ] - /// Elements whose projection collides, for the stability of sorting and the order of grouping: - /// the pairs share a key in their first component and are told apart by the second. - let pairs = [ - struct (2, 'a') - struct (1, 'b') - struct (2, 'c') - struct (1, 'd') - struct (0, 'e') - struct (2, 'f') - ] +/// Elements whose projection collides, for the stability of sorting and the order of grouping: +/// the pairs share a key in their first component and are told apart by the second. +let pairs = [ + struct (2, 'a') + struct (1, 'b') + struct (2, 'c') + struct (1, 'd') + struct (0, 'e') + struct (2, 'f') +] diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/Mirror.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/Mirror.fs new file mode 100644 index 00000000..4933a733 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/Mirror.fs @@ -0,0 +1,8 @@ +namespace FSharp.Collections.Immutable.Tests + +/// The FSharp.Core module whose argument checks a collection module mirrors (ADR 0001, D6). +type Mirror = + /// `FlatList` mirrors `Array`. + | ArrayMirror + /// `IndexedList`, `Queue`, `Stack` and their interface modules mirror `List`. + | ListMirror diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs index d283eb1f..41415e50 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -1,23 +1,21 @@ -namespace FSharp.Collections.Immutable.Tests - -open Microsoft.VisualStudio.TestTools.UnitTesting - /// Compares a function of the `Option` or `Tuple` namespace with the default function after conversion (ADR 0001, D3). [] -module OverrideAssert = +module FSharp.Collections.Immutable.Tests.OverrideAssert + +open Microsoft.VisualStudio.TestTools.UnitTesting - /// The `Option` namespace returns an `option` where the default module returns a `voption`. - let optionMatches (expected : 'T voption) (actual : 'T option) (message : string) = - Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message) +/// The `Option` namespace returns an `option` where the default module returns a `voption`. +let optionMatches (expected : 'T voption) (actual : 'T option) (message : string) = + Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message) - /// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. - let pairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2)) (actual : 'T1 * 'T2) (message : string) = - Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, message) +/// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. +let pairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2)) (actual : 'T1 * 'T2) (message : string) = + Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, message) - /// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. - let tripleMatches - (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3)) - (actual : 'T1 * 'T2 * 'T3) - (message : string) - = - Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, message) +/// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. +let tripleMatches + (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3)) + (actual : 'T1 * 'T2 * 'T3) + (message : string) + = + Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, message) diff --git a/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj b/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj index 7ddd1712..547aa7af 100644 --- a/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj +++ b/tests/FSharp.Collections.Immutable.Tests/FSharp.Collections.Immutable.Tests.fsproj @@ -18,6 +18,7 @@ + diff --git a/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs b/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs index 9c327b94..49796ff7 100644 --- a/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs @@ -5,6 +5,8 @@ open Microsoft.VisualStudio.TestTools.UnitTesting // Each attribute gives one category: its type name without the `TestCategoryAttribute` suffix, written as separate words. // The attributes are sorted by type name (ordinal); a new attribute goes to its sorted position. +// The attributes go on the test classes only, for the collection and the area that a class examines; test methods carry +// no categories (ADR 0001, D8). /// The builder functions and the builder modules. type BuilderTestCategoryAttribute () = diff --git a/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs b/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs index dccfd8f4..2c1958f7 100644 --- a/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs @@ -4,7 +4,7 @@ open System open System.Reflection open Microsoft.VisualStudio.TestTools.UnitTesting -/// Enforces the category rules of `.github/instructions/test-categories.instructions.md` and of the copilot instructions. +/// Enforces the category rules of `.github/instructions/test-categories.instructions.md` and of ADR 0001, D8. [] type TestCategoriesTests () = @@ -26,7 +26,7 @@ type TestCategoriesTests () = /// A category attribute of this project, as opposed to the `TestCategory` attribute of MSTest. static let isProjectCategory (attribute : TestCategoryBaseAttribute) = attribute.GetType().Assembly = assembly - [] + [] member _.``Every category attribute gives one category named after its type`` () = Assert.IsNotEmpty (categoryAttributeTypes, "TestCategories.fs must define category attributes") @@ -52,7 +52,7 @@ type TestCategoriesTests () = Assert.IsTrue (Char.IsUpper category[0], $"The category of %s{name} must start with a capital letter") - [] + [] member _.``Category attributes are sorted by type name`` () = let names = categoryAttributeTypes |> Array.map _.Name let sorted = @@ -60,7 +60,7 @@ type TestCategoriesTests () = |> Array.sortWith (fun left right -> String.CompareOrdinal (left, right)) CollectionAssert.AreEqual (sorted, names, "TestCategories.fs must keep its attributes sorted by type name") - [] + [] member _.``Every test class has a category attribute of this project`` () = Assert.IsNotEmpty (testClasses, "The assembly must contain test classes") @@ -71,8 +71,17 @@ type TestCategoriesTests () = Assert.IsNotEmpty (categories, $"%s{testClass.Name} must carry the category attribute of what it examines") - [] - member _.``Every test method has an operation category`` () = + [] + member _.``No test class has a category written as a string`` () = + for testClass in testClasses do + Assert.IsFalse ( + testClass.IsDefined (typeof, true), + $"%s{testClass.Name} must take its categories from the attributes of TestCategories.fs, not from TestCategory" + ) + + [] + member _.``Test methods carry no categories`` () = + // The categories belong to the test classes, so every test of a class shares them for testClass in testClasses do let testMethods = testClass.GetMethods (BindingFlags.Public ||| BindingFlags.Instance) @@ -80,4 +89,4 @@ type TestCategoriesTests () = for testMethod in testMethods do let categories = testMethod.GetCustomAttributes(true) - Assert.IsNotEmpty (categories, $"%s{testClass.Name}.%s{testMethod.Name} must carry an operation category") + Assert.IsEmpty (categories, $"%s{testClass.Name}.%s{testMethod.Name} must not carry a category") From e2e2652294c3238614dac0c517dcbcf52eaac987 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 16:40:25 +0200 Subject: [PATCH 04/10] fixup! chore: lay the foundation for the per-collection modules Tests compare arrays and take their cases as data rows: - `Inputs` is a static class of arrays, so that a test can name its properties in `DynamicData (nameof Inputs.All, typeof)`; `typeof` cannot name a module. `All` and `NonEmpty` are the row sources of the tests that run on every input. Every property builds new arrays on each read, so that no test sees the rows of another. - `OverrideAssert.pairMatches` and `tripleMatches` compare with F# structural equality: the equality of a tuple compares arrays by reference. Co-Authored-By: Claude Opus 5.5 --- .../Contracts/Inputs.fs | 79 +++++++++++-------- .../Contracts/OverrideAssert.fs | 7 +- 2 files changed, 50 insertions(+), 36 deletions(-) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs index 1de0b72a..bc0809f0 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs @@ -1,36 +1,47 @@ +namespace FSharp.Collections.Immutable.Tests + /// The inputs that the contract tests run the functions on. /// Expectations are computed with FSharp.Core over the order in which the collection enumerates its elements (ADR 0001, D8). -[] -module FSharp.Collections.Immutable.Tests.Inputs - -/// No elements. -let empty : int list = [] - -/// One element. -let singleton = [ 42 ] - -/// Unsorted, with duplicates, negative numbers, zero and an even length. -let several = [ 5; -3; 8; 5; 0; -3; 12; 7 ] - -/// Every input. -let all = [ empty; singleton; several ] - -/// Every input with at least one element. -let nonEmpty = [ singleton; several ] - -/// A second input for the functions of two collections, as long as `several`. -let other = [ 2; 4; 6; 8; 10; 12; 14; 16 ] - -/// A third input for the functions of three collections, as long as `several`. -let third = [ -1; -2; -3; -4; -5; -6; -7; -8 ] - -/// Elements whose projection collides, for the stability of sorting and the order of grouping: -/// the pairs share a key in their first component and are told apart by the second. -let pairs = [ - struct (2, 'a') - struct (1, 'b') - struct (2, 'c') - struct (1, 'd') - struct (0, 'e') - struct (2, 'f') -] +/// Every property builds new arrays on each read: MSTest hands the arrays of a row source to the tests as they are, so an +/// array that the tests shared would carry the change that one test made into the others. +[] +type Inputs = + + /// No elements. + static member Empty : int array = [||] + + /// One element. + static member Singleton = [| 42 |] + + /// Unsorted, with duplicates, negative numbers, zero and an even length. + static member Several = [| 5; -3; 8; 5; 0; -3; 12; 7 |] + + /// Every input, as the rows of the tests that run on each one. + static member All = seq { + Inputs.Empty + Inputs.Singleton + Inputs.Several + } + + /// Every input with at least one element, as the rows of the tests that need one. + static member NonEmpty = seq { + Inputs.Singleton + Inputs.Several + } + + /// A second input for the functions of two collections, as long as `Several`. + static member Other = [| 2; 4; 6; 8; 10; 12; 14; 16 |] + + /// A third input for the functions of three collections, as long as `Several`. + static member Third = [| -1; -2; -3; -4; -5; -6; -7; -8 |] + + /// Elements whose projection collides, for the stability of sorting and the order of grouping: + /// the pairs share a key in their first component and are told apart by the second. + static member Pairs = [| + struct (2, 'a') + struct (1, 'b') + struct (2, 'c') + struct (1, 'd') + struct (0, 'e') + struct (2, 'f') + |] diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs index 41415e50..1854d7b0 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -5,17 +5,20 @@ module FSharp.Collections.Immutable.Tests.OverrideAssert open Microsoft.VisualStudio.TestTools.UnitTesting /// The `Option` namespace returns an `option` where the default module returns a `voption`. +/// The equality of an F# option is structural, so an option of an array compares the elements of the array. let optionMatches (expected : 'T voption) (actual : 'T option) (message : string) = Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message) /// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. +/// The equality of a tuple compares arrays by reference, so the pairs are compared with F# structural equality. let pairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2)) (actual : 'T1 * 'T2) (message : string) = - Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, message) + Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, HashIdentity.Structural, message) /// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. +/// The equality of a tuple compares arrays by reference, so the triples are compared with F# structural equality. let tripleMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3)) (actual : 'T1 * 'T2 * 'T3) (message : string) = - Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, message) + Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, HashIdentity.Structural, message) From 2932758f9cc97a38dd5aec4847e1334a47552f57 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 17:21:04 +0200 Subject: [PATCH 05/10] fixup! chore: lay the foundation for the per-collection modules The `` of `ExceptionAssert.headline` spans several lines, as its `` does: a summary and a remarks stay on one line each only when both are short. Co-Authored-By: Claude Opus 5.5 --- .../Contracts/ExceptionAssert.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs index 21f5e11c..9233b288 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -20,7 +20,9 @@ let private invariant (action : unit -> 'T) = CultureInfo.CurrentCulture <- culture CultureInfo.CurrentUICulture <- uiCulture -/// The first line of a message, without the parameter suffix of . +/// +/// The first line of a message, without the parameter suffix of . +/// /// /// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), /// and this library writes the names of its own parameters there, so only the first line is compared. From 026cfcd851116faec4e216eb59042b81c13e3ef8 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 18:08:17 +0200 Subject: [PATCH 06/10] fixup! chore: lay the foundation for the per-collection modules The solution structure lists the files that every folder has: - Every `XOption` and `XTuple` file has a line of its own, as the `FlatList` files do. - The comparer struct and the extension files take their dotted names (`HashMap.CustomComparers`, `HashMap.Extensions`, `HashSet.CustomComparer`, ...) and nest under the line of their module; they carry ``, so that Visual Studio shows them as a tree. - The builder and computation expression files and `ComputationExpressions.fs` have their signature files, as the rule above the tree asks. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 103 +++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 36 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e7655dc1..bc8cac94 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -14,6 +14,9 @@ exception is the `XTypes.fs` file of a folder, which holds only type abbreviations and is its own signature: a signature file without an implementation file does not compile (FS0240). +The comparer struct and the extension files of a module carry `` in the project file, the `.fsi` on the +module's `.fsi` and the `.fs` on its `.fs`, so that Visual Studio shows them as a tree under the module. + ```text / ├── build/ – FAKE build scripts and release automation @@ -26,59 +29,87 @@ file without an implementation file does not compile (FS0240). │ │ ├── FlatList.fs / FlatList.fsi – includes the nested Parallel module │ │ ├── FlatListOption.fs / FlatListOption.fsi – Option namespace overrides │ │ ├── FlatListTuple.fs / FlatListTuple.fsi – Tuple namespace overrides -│ │ ├── FlatListBuilder.fs -│ │ └── FlatListCE.fs +│ │ ├── FlatListBuilder.fs / FlatListBuilder.fsi +│ │ └── FlatListCE.fs / FlatListCE.fsi │ ├── IndexedList/ – ImmutableList API surface and builders │ │ ├── IndexedListTypes.fs -│ │ ├── IndexedList.fs / IndexedList.fsi (+ Option / Tuple) -│ │ ├── IIndexedList.fs / IIndexedList.fsi (+ Option / Tuple) -│ │ ├── IndexedListBuilder.fs -│ │ └── IndexedListCE.fs +│ │ ├── IndexedList.fs / IndexedList.fsi +│ │ ├── IndexedListOption.fs / IndexedListOption.fsi +│ │ ├── IndexedListTuple.fs / IndexedListTuple.fsi +│ │ ├── IIndexedList.fs / IIndexedList.fsi +│ │ ├── IIndexedListOption.fs / IIndexedListOption.fsi +│ │ ├── IIndexedListTuple.fs / IIndexedListTuple.fsi +│ │ ├── IndexedListBuilder.fs / IndexedListBuilder.fsi +│ │ └── IndexedListCE.fs / IndexedListCE.fsi │ ├── Queue/ – ImmutableQueue API surface and CE │ │ ├── QueueTypes.fs – the Queue and IQueue abbreviations -│ │ ├── Queue.fs / Queue.fsi (+ Option / Tuple) -│ │ ├── IQueue.fs / IQueue.fsi (+ Option / Tuple) -│ │ └── QueueCE.fs +│ │ ├── Queue.fs / Queue.fsi +│ │ ├── QueueOption.fs / QueueOption.fsi +│ │ ├── QueueTuple.fs / QueueTuple.fsi +│ │ ├── IQueue.fs / IQueue.fsi +│ │ ├── IQueueOption.fs / IQueueOption.fsi +│ │ ├── IQueueTuple.fs / IQueueTuple.fsi +│ │ └── QueueCE.fs / QueueCE.fsi │ ├── Stack/ – ImmutableStack API surface and CE │ │ ├── StackTypes.fs – the Stack and IStack abbreviations -│ │ ├── Stack.fs / Stack.fsi (+ Option / Tuple) -│ │ ├── IStack.fs / IStack.fsi (+ Option / Tuple) -│ │ └── StackCE.fs +│ │ ├── Stack.fs / Stack.fsi +│ │ ├── StackOption.fs / StackOption.fsi +│ │ ├── StackTuple.fs / StackTuple.fsi +│ │ ├── IStack.fs / IStack.fsi +│ │ ├── IStackOption.fs / IStackOption.fsi +│ │ ├── IStackTuple.fs / IStackTuple.fsi +│ │ └── StackCE.fs / StackCE.fsi │ ├── HashMap/ – ImmutableDictionary API surface and CE │ │ ├── HashMapTypes.fs – the HashMap and IMap abbreviations -│ │ ├── HashMap.fs / HashMap.fsi (+ Option / Tuple) – nested Tuple / ValueTuple submodules -│ │ ├── CustomComparersHashMap.fs / .fsi – creates hash maps with comparers -│ │ ├── HashMapExtensions.fs / .fsi – ofArray / ofList / ofSeq for every binding shape -│ │ ├── IMap.fs / IMap.fsi (+ Option / Tuple) -│ │ └── HashMapCE.fs +│ │ ├── HashMap.fs / HashMap.fsi – nested Tuple / ValueTuple submodules +│ │ │ ├── HashMap.CustomComparers.fs / .fsi – creates hash maps with comparers +│ │ │ └── HashMap.Extensions.fs / .fsi – ofArray / ofList / ofSeq for every binding shape +│ │ ├── HashMapOption.fs / HashMapOption.fsi +│ │ ├── HashMapTuple.fs / HashMapTuple.fsi +│ │ ├── IMap.fs / IMap.fsi +│ │ ├── IMapOption.fs / IMapOption.fsi +│ │ ├── IMapTuple.fs / IMapTuple.fsi +│ │ └── HashMapCE.fs / HashMapCE.fsi │ ├── SortedMap/ – ImmutableSortedDictionary API surface and CE │ │ ├── SortedMapTypes.fs -│ │ ├── SortedMap.fs / SortedMap.fsi (+ Option / Tuple) -│ │ ├── CustomComparersSortedMap.fs / .fsi -│ │ ├── SortedMapExtensions.fs / .fsi -│ │ └── SortedMapCE.fs +│ │ ├── SortedMap.fs / SortedMap.fsi +│ │ │ ├── SortedMap.CustomComparers.fs / .fsi +│ │ │ └── SortedMap.Extensions.fs / .fsi +│ │ ├── SortedMapOption.fs / SortedMapOption.fsi +│ │ ├── SortedMapTuple.fs / SortedMapTuple.fsi +│ │ └── SortedMapCE.fs / SortedMapCE.fsi │ ├── HashSet/ – ImmutableHashSet API surface and CE │ │ ├── HashSetTypes.fs -│ │ ├── HashSet.fs / HashSet.fsi (+ Option / Tuple) -│ │ ├── CustomComparerHashSet.fs / .fsi – creates hash sets with a comparer -│ │ ├── ISet.fs / ISet.fsi (+ Option / Tuple) -│ │ └── HashSetCE.fs +│ │ ├── HashSet.fs / HashSet.fsi +│ │ │ └── HashSet.CustomComparer.fs / .fsi – creates hash sets with a comparer +│ │ ├── HashSetOption.fs / HashSetOption.fsi +│ │ ├── HashSetTuple.fs / HashSetTuple.fsi +│ │ ├── ISet.fs / ISet.fsi +│ │ ├── ISetOption.fs / ISetOption.fsi +│ │ ├── ISetTuple.fs / ISetTuple.fsi +│ │ └── HashSetCE.fs / HashSetCE.fsi │ ├── SortedSet/ – ImmutableSortedSet API surface and CE │ │ ├── SortedSetTypes.fs -│ │ ├── SortedSet.fs / SortedSet.fsi (+ Option / Tuple) -│ │ ├── CustomComparerSortedSet.fs / .fsi -│ │ └── SortedSetCE.fs +│ │ ├── SortedSet.fs / SortedSet.fsi +│ │ │ └── SortedSet.CustomComparer.fs / .fsi +│ │ ├── SortedSetOption.fs / SortedSetOption.fsi +│ │ ├── SortedSetTuple.fs / SortedSetTuple.fsi +│ │ └── SortedSetCE.fs / SortedSetCE.fsi │ ├── FrozenMap/ – FrozenDictionary API surface and CE │ │ ├── FrozenMapTypes.fs -│ │ ├── FrozenMap.fs / FrozenMap.fsi (+ Option / Tuple) -│ │ ├── CustomComparersFrozenMap.fs / .fsi -│ │ ├── FrozenMapExtensions.fs / .fsi -│ │ └── FrozenMapCE.fs +│ │ ├── FrozenMap.fs / FrozenMap.fsi +│ │ │ ├── FrozenMap.CustomComparers.fs / .fsi +│ │ │ └── FrozenMap.Extensions.fs / .fsi +│ │ ├── FrozenMapOption.fs / FrozenMapOption.fsi +│ │ ├── FrozenMapTuple.fs / FrozenMapTuple.fsi +│ │ └── FrozenMapCE.fs / FrozenMapCE.fsi │ ├── FrozenSet/ – FrozenSet API surface and CE (the BCL type keeps its name) -│ │ ├── FrozenSet.fs / FrozenSet.fsi (+ Option / Tuple) -│ │ ├── CustomComparerFrozenSet.fs / .fsi -│ │ └── FrozenSetCE.fs -│ └── ComputationExpressions.fs – the computation expression instances +│ │ ├── FrozenSet.fs / FrozenSet.fsi +│ │ │ └── FrozenSet.CustomComparer.fs / .fsi +│ │ ├── FrozenSetOption.fs / FrozenSetOption.fsi +│ │ ├── FrozenSetTuple.fs / FrozenSetTuple.fsi +│ │ └── FrozenSetCE.fs / FrozenSetCE.fsi +│ └── ComputationExpressions.fs / .fsi – the computation expression instances └── tests/FSharp.Collections.Immutable.Tests/ – MSTest test project ├── TestCategories.fs – centralized custom test category attributes ├── TestCategoriesTests.fs – enforces the test category rules From e524b23c51be1689272de6e922e3fc330b981ab5 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 19:22:11 +0200 Subject: [PATCH 07/10] fixup! chore: lay the foundation for the per-collection modules The order check parses the files with Fantomas.FCS instead of regular expressions and runs inside the build project as `CheckOrder.fs`; the signature decides which implementation bindings take part in the order. Co-Authored-By: Claude Fable 5.1 --- .github/copilot-instructions.md | 4 +- Directory.Packages.props | 1 + README.md | 2 +- build/CheckOrder.fs | 421 ++++++++++++++++++++++++++++++++ build/build.fs | 14 +- build/build.fsproj | 2 + build/check-order.fsx | 354 --------------------------- 7 files changed, 436 insertions(+), 362 deletions(-) create mode 100644 build/CheckOrder.fs delete mode 100644 build/check-order.fsx diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bc8cac94..aa1f8551 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -20,7 +20,7 @@ module's `.fsi` and the `.fs` on its `.fs`, so that Visual Studio shows them as ```text / ├── build/ – FAKE build scripts and release automation -│ └── check-order.fsx – checks the category order of the signature files +│ └── CheckOrder.fs – checks the category order of the signature files with the Fantomas parser ├── docsSrc/ – FSharp.Formatting documentation source ├── src/FSharp.Collections.Immutable/ – main library │ ├── Helper.fs – argument checks and exceptions shared by the modules @@ -123,7 +123,7 @@ The modules follow ADR 0001 – Per-collection modules (`docsSrc/Explanations/AD * Each module groups its functions under banner comments `// ---- Category ----` in the order `Creation`, `Builder`, `Properties`, `Access`, `Search`, `Transformation`, `Folding`, `Partitioning and grouping`, `Sorting`, `Random`, `Conversion`, `Type-specific`, and sorts the functions of a category alphabetically (ordinal), active patterns last. - The `.fs` file repeats the order of its `.fsi` file. `build/check-order.fsx` (FAKE target `CheckOrder`) enforces it. + The `.fs` file repeats the order of its `.fsi` file. `build/CheckOrder.fs` (FAKE target `CheckOrder`) enforces it. * A file with one module declares it as the root module of the file (`module FSharp.Collections.Immutable.FlatList`); the type abbreviations of a folder live in its `XTypes.fs`. No module carries `[]`. * `try*` functions return `voption`, tuples are struct tuples. The namespaces `FSharp.Collections.Immutable.Option` and diff --git a/Directory.Packages.props b/Directory.Packages.props index 583ed282..3660945f 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -24,6 +24,7 @@ + diff --git a/README.md b/README.md index 5f298fa4..58e776c9 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ or - `GitHubRelease` - Publishes a [GitHub Release](https://help.github.com/en/articles/creating-releases) with the Release Notes and any NuGet packages. Runs only from `Github Actions`. - `FormatCode` - Runs [Fantomas](https://github.com/fsprojects/fantomas) on the solution file. - `CheckFormatCode` - Runs [Fantomas --check](https://fsprojects.github.io/fantomas/docs/end-users/FormattingCheck.html) on the solution file. -- `CheckOrder` - Runs `build/check-order.fsx`, which checks that every signature file groups its functions by category in the order of ADR 0001 and sorts them alphabetically inside a category. Runs before `DotnetBuild`. +- `CheckOrder` - Checks, with the parser of [Fantomas](https://github.com/fsprojects/fantomas), that every signature file groups its functions by category in the order of ADR 0001 and sorts them alphabetically inside a category, and that the implementation file repeats the order. Runs before `DotnetBuild`. - `BuildDocs` - Generates [Documentation](https://fsprojects.github.io/FSharp.Formatting) from `docsSrc` and the [XML Documentation Comments](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/) from your libraries in `src`. - `WatchDocs` - Generates documentation and starts a webserver locally. It will rebuild and hot reload if it detects any changes made to `docsSrc` files, or libraries in `src`. diff --git a/build/CheckOrder.fs b/build/CheckOrder.fs new file mode 100644 index 00000000..b0be63f0 --- /dev/null +++ b/build/CheckOrder.fs @@ -0,0 +1,421 @@ +/// +/// Checks the order of the functions in the signature files of the library and in their implementation files +/// (ADR 0001, D4). +/// +/// +/// +/// Every module of a signature file groups its vals under banner comments // ---- Category ----. The +/// banners follow the canonical category order, and the vals of a category are sorted alphabetically (ordinal), +/// active patterns last. The sibling implementation file has the same banners and defines the functions of the +/// signature in the same order. +/// +/// +/// The files are read through the parser of Fantomas (Fantomas.FCS): the modules and their values come from the +/// syntax tree, the banners from the comment trivia. The parser only sees the code of one set of defines at a time, so +/// every file is parsed once per target framework of the library and the views are checked one by one. +/// +/// +module CheckOrder + +open System +open System.Collections.Generic +open System.IO +open System.Text.RegularExpressions +open Fantomas.FCS +open Fantomas.FCS.Syntax +open Fantomas.FCS.SyntaxTrivia +open Fantomas.FCS.Text + +/// A violation of the order rules: the file, the line and the message. +[] +type Violation = { Path : string; Line : int; Message : string } + +/// The canonical category sequence of ADR 0001, D4. +let private categories = [| + "Creation" + "Builder" + "Properties" + "Access" + "Search" + "Transformation" + "Folding" + "Partitioning and grouping" + "Sorting" + "Random" + "Conversion" + "Type-specific" +|] + +/// Maps and sets only partition, so their banner may say "Partitioning"; it takes the place of "Partitioning and grouping". +let private categoryIndex (category : string) = + let canonical = + if String.Equals (category, "Partitioning", StringComparison.Ordinal) then + "Partitioning and grouping" + else + category + + Array.IndexOf (categories, canonical) + +// The major versions of the target frameworks of the library (`TargetFrameworks` in Directory.Build.props). A `#if` +// hides one of its arms from the parser, so every file is parsed with the defines of each framework in turn. +let private targetFrameworks = [| 8; 10 |] + +/// The preprocessor symbols that the SDK defines for `net.0`. +let private definesOf (major : int) = [ + "NET" + "NETCOREAPP" + $"NET%d{major}_0" + for version in 5..major do + $"NET%d{version}_0_OR_GREATER" +] + +let private bannerRegex = + Regex (@"^\s*// ---- (?.+?) ----\s*$", RegexOptions.CultureInvariant) + +[] +type private Banner = { Line : int; Category : string } + +[] +type private Member = { Line : int; Category : string; Name : string } + +/// A public module of a source file while it is collected: its range and its public values, before the banners are known. +type private Scope = { + Path : string + Range : range + Values : ResizeArray + Banners : ResizeArray +} + +/// A public module of a source file: its banners and its public members in source order. +type private Module = { Path : string; Banners : Banner array; Members : Member array } + +let private isPublic (access : SynAccess option) = + match access with + | None + | Some (SynAccess.Public _) -> true + | Some _ -> false + +/// The name of a value as the source writes it: the identifier of an active pattern is `|Cons|_|`, the source says `(|Cons|_|)`. +let private displayName (ident : Ident) = + if ident.idText.StartsWith ("|", StringComparison.Ordinal) then + $"(%s{ident.idText})" + else + ident.idText + +let private pathOf (longId : LongIdent) = String.Join (".", longId |> List.map _.idText) + +let private childPath (parent : string) (longId : LongIdent) = + if parent.Length = 0 then + pathOf longId + else + $"%s{parent}.%s{pathOf longId}" + +let private newScope (scopes : ResizeArray) (path : string) (range : range) = + let scope = { + Path = path + Range = range + Values = ResizeArray () + Banners = ResizeArray () + } + + scopes.Add scope + scope + +let private addValue (scope : Scope voption) (ident : Ident) = + match scope with + | ValueSome scope -> scope.Values.Add (struct (ident.idRange.StartLine, displayName ident)) + | ValueNone -> () + +/// The identifier that a top-level binding defines, when the binding is a public function or value. +let rec private boundIdent (pattern : SynPat) = + match pattern with + | SynPat.Typed (pat = pattern) -> boundIdent pattern + | SynPat.Named (ident = SynIdent (ident, _); accessibility = access) when isPublic access -> ValueSome ident + | SynPat.LongIdent (longDotId = SynLongIdent ([ ident ], _, _); accessibility = access) when isPublic access -> + ValueSome ident + | _ -> ValueNone + +let rec private walkSignatureDecls (scopes : ResizeArray) (parent : string) (scope : Scope voption) decls = + for decl in decls do + match decl with + | SynModuleSigDecl.Val (SynValSig (ident = SynIdent (ident, _); accessibility = SynValSigAccess.Single access), _) when + isPublic access + -> + addValue scope ident + | SynModuleSigDecl.NestedModule ( + moduleInfo = (SynComponentInfo (accessibility = access) as info); moduleDecls = decls; range = range) when + isPublic access + -> + let path = childPath parent info.LongIdent + walkSignatureDecls scopes path (ValueSome (newScope scopes path range)) decls + | SynModuleSigDecl.NamespaceFragment fragment -> walkSignatureModule scopes fragment + | _ -> () + +and private walkSignatureModule (scopes : ResizeArray) fragment = + match fragment with + | SynModuleOrNamespaceSig (longId = longId; kind = kind; decls = decls; accessibility = access; range = range) -> + if kind.IsModule then + if isPublic access then + let path = pathOf longId + walkSignatureDecls scopes path (ValueSome (newScope scopes path range)) decls + else + // A namespace is not a scope: its values do not exist, its modules are the scopes + let path = if kind.IsGlobalNamespace then "" else pathOf longId + walkSignatureDecls scopes path ValueNone decls + +let rec private walkImplementationDecls (scopes : ResizeArray) (parent : string) (scope : Scope voption) decls = + for decl in decls do + match decl with + | SynModuleDecl.Let (bindings = bindings) -> + for SynBinding (headPat = pattern) in bindings do + match boundIdent pattern with + | ValueSome ident -> addValue scope ident + | ValueNone -> () + | SynModuleDecl.NestedModule ( + moduleInfo = (SynComponentInfo (accessibility = access) as info); decls = decls; range = range) when isPublic access -> + let path = childPath parent info.LongIdent + walkImplementationDecls scopes path (ValueSome (newScope scopes path range)) decls + | SynModuleDecl.NamespaceFragment fragment -> walkImplementationModule scopes fragment + | _ -> () + +and private walkImplementationModule (scopes : ResizeArray) fragment = + match fragment with + | SynModuleOrNamespace (longId = longId; kind = kind; decls = decls; accessibility = access; range = range) -> + if kind.IsModule then + if isPublic access then + let path = pathOf longId + walkImplementationDecls scopes path (ValueSome (newScope scopes path range)) decls + else + let path = if kind.IsGlobalNamespace then "" else pathOf longId + walkImplementationDecls scopes path ValueNone decls + +/// Banners are comments, so they come from the trivia; each one belongs to the innermost module that spans its line. +let private collectBanners (lines : string array) (trivia : ParsedInputTrivia) (scopes : ResizeArray) = + for comment in trivia.CodeComments do + match comment with + | CommentTrivia.LineComment range -> + let line = range.StartLine + let bannerMatch = bannerRegex.Match lines[line - 1] + + if bannerMatch.Success then + let owner = + scopes + |> Seq.filter (fun scope -> scope.Range.StartLine <= line && line <= scope.Range.EndLine) + |> Seq.sortByDescending _.Range.StartLine + |> Seq.tryHead + + match owner with + | Some owner -> owner.Banners.Add { Line = line; Category = bannerMatch.Groups["category"].Value } + | None -> () + | CommentTrivia.BlockComment _ -> () + +/// Orders the banners and the values of a scope by line and gives every value the category of the banner above it. +let private resolve (scope : Scope) = + let banners = scope.Banners |> Seq.sortBy _.Line |> Seq.toArray + + let members = + scope.Values + |> Seq.sortBy (fun struct (line, _) -> line) + |> Seq.map (fun struct (line, name) -> + let category = + banners + |> Array.tryFindBack (fun banner -> banner.Line < line) + |> Option.map _.Category + |> Option.defaultValue "" + + { Line = line; Category = category; Name = name } + ) + |> Seq.toArray + + { Path = scope.Path; Banners = banners; Members = members } + +/// Parses a file with the given defines and returns its public modules; a parse error is a violation. +let private parseModules (errors : ResizeArray) (path : string) (defines : string list) = + let lines = File.ReadAllLines path + let source = SourceText.ofString (String.Join (Environment.NewLine, lines)) + let isSignature = path.EndsWith (".fsi", StringComparison.OrdinalIgnoreCase) + let tree, diagnostics = Parse.parseFile isSignature source defines + + for diagnostic in diagnostics do + if diagnostic.Severity.IsError then + let line = + diagnostic.Range + |> Option.map _.StartLine + |> Option.defaultValue 1 + + errors.Add { Path = path; Line = line; Message = $"Parse error: %s{diagnostic.Message}" } + + let scopes = ResizeArray() + + match tree with + | ParsedInput.SigFile (ParsedSigFileInput (contents = contents; trivia = trivia)) -> + for fragment in contents do + walkSignatureModule scopes fragment + + collectBanners lines trivia scopes + | ParsedInput.ImplFile (ParsedImplFileInput (contents = contents; trivia = trivia)) -> + for fragment in contents do + walkImplementationModule scopes fragment + + collectBanners lines trivia scopes + + scopes |> Seq.map resolve |> Seq.toArray + +let private isActivePattern (name : string) = name.StartsWith ("(|", StringComparison.Ordinal) + +/// Identifiers sort ordinally; active patterns come after them, also ordinally. +let private compareNames (left : string) (right : string) = + match isActivePattern left, isActivePattern right with + | false, true -> -1 + | true, false -> 1 + | _ -> String.CompareOrdinal (left, right) + +let private report (errors : ResizeArray) path line message = + errors.Add { Path = path; Line = line; Message = message } + +let private checkSignature (errors : ResizeArray) (path : string) (scope : Module) = + let report = report errors path + let mutable previousIndex = -1 + + for banner in scope.Banners do + let index = categoryIndex banner.Category + + if index < 0 then + report banner.Line $"Unknown category '%s{banner.Category}' in module %s{scope.Path}" + elif index <= previousIndex then + report banner.Line $"Category '%s{banner.Category}' is out of the canonical order in module %s{scope.Path}" + else + previousIndex <- index + + let hasMembers = + scope.Members + |> Array.exists (fun item -> String.Equals (item.Category, banner.Category, StringComparison.Ordinal)) + + if not hasMembers then + report banner.Line $"Category '%s{banner.Category}' is empty in module %s{scope.Path}; omit its banner" + + for index in 0 .. scope.Members.Length - 1 do + let item = scope.Members[index] + + if item.Category.Length = 0 then + report item.Line $"`%s{item.Name}` comes before the first category banner of module %s{scope.Path}" + elif index > 0 then + let previous = scope.Members[index - 1] + + if String.Equals (previous.Category, item.Category, StringComparison.Ordinal) then + let order = compareNames previous.Name item.Name + + if order = 0 then + report item.Line $"`%s{item.Name}` is declared twice in module %s{scope.Path}" + elif order > 0 then + report item.Line $"`%s{item.Name}` must come before `%s{previous.Name}` in category '%s{item.Category}'" + +let private checkImplementation + (errors : ResizeArray) + (implementationPath : string) + (signatureModules : Module array) + (implementationModules : Module array) + = + let report = report errors implementationPath + + for signature in signatureModules do + let implementation = + implementationModules + |> Array.tryFind (fun scope -> String.Equals (scope.Path, signature.Path, StringComparison.Ordinal)) + + match implementation with + | None -> report 1 $"Module %s{signature.Path} of the signature file is missing" + | Some implementation -> + let signatureBanners = signature.Banners |> Array.map _.Category + let implementationBanners = implementation.Banners |> Array.map _.Category + + if signatureBanners <> implementationBanners then + let line = + if implementation.Banners.Length > 0 then + implementation.Banners[0].Line + else + 1 + + let expected = String.Join (", ", signatureBanners) + let actual = String.Join (", ", implementationBanners) + report line $"Module %s{signature.Path} has the banners [%s{actual}]; the signature has [%s{expected}]" + + // The signature decides what is public: a binding that the signature omits is a private helper, whatever + // its access modifier says, so it takes no place in the order. + let declared = HashSet(signature.Members |> Seq.map _.Name, StringComparer.Ordinal) + let members = + implementation.Members + |> Array.filter (fun item -> declared.Contains item.Name) + let count = min signature.Members.Length members.Length + + // A member must have the same name and the same category, so that a moved banner cannot go unnoticed + let differs index = + let expected = signature.Members[index] + let actual = members[index] + + not (String.Equals (expected.Name, actual.Name, StringComparison.Ordinal)) + || not (String.Equals (expected.Category, actual.Category, StringComparison.Ordinal)) + + match seq { 0 .. count - 1 } |> Seq.tryFind differs with + | Some index -> + let expected = signature.Members[index] + let actual = members[index] + + if String.Equals (expected.Name, actual.Name, StringComparison.Ordinal) then + report + actual.Line + $"`%s{actual.Name}` is in category '%s{actual.Category}'; the signature puts it in '%s{expected.Category}' (line %d{expected.Line})" + else + report + actual.Line + $"`%s{actual.Name}` is defined where the signature declares `%s{expected.Name}` (line %d{expected.Line})" + | None when signature.Members.Length <> members.Length -> + report + 1 + $"Module %s{signature.Path} defines %d{members.Length} of the %d{signature.Members.Length} functions that the signature declares" + | None -> () + +let private isGenerated (path : string) = + let separator = Path.DirectorySeparatorChar + + path.Contains ($"%c{separator}obj%c{separator}", StringComparison.Ordinal) + || path.Contains ($"%c{separator}bin%c{separator}", StringComparison.Ordinal) + +/// Checks every signature file under a directory and its sibling implementation file. +/// The directory of the sources; generated files under bin and obj are skipped. +/// The number of signature files checked and the violations, sorted by path and line. +let check (sourceDirectory : string) = + let errors = ResizeArray() + + let signatureFiles = + Directory.EnumerateFiles (sourceDirectory, "*.fsi", SearchOption.AllDirectories) + |> Seq.filter (isGenerated >> not) + |> Seq.sort + |> Seq.toArray + + for signaturePath in signatureFiles do + let implementationPath = Path.ChangeExtension (signaturePath, ".fs") |> nonNull + let hasImplementation = File.Exists implementationPath + + if not hasImplementation then + report errors signaturePath 1 "The signature file has no implementation file next to it" + + for major in targetFrameworks do + let defines = definesOf major + let signatureModules = parseModules errors signaturePath defines + + for scope in signatureModules do + checkSignature errors signaturePath scope + + if hasImplementation then + let implementationModules = parseModules errors implementationPath defines + checkImplementation errors implementationPath signatureModules implementationModules + + // The views of the target frameworks mostly agree, so the same violation is usually found once per framework + let violations = + errors + |> Seq.distinct + |> Seq.sortBy (fun violation -> struct (violation.Path, violation.Line)) + |> Seq.toArray + + struct (signatureFiles.Length, violations) diff --git a/build/build.fs b/build/build.fs index aad2b703..004a55b3 100644 --- a/build/build.fs +++ b/build/build.fs @@ -163,8 +163,6 @@ module dotnet = let fantomas args = DotNet.exec id "fantomas" args - let fsi args = tool id "fsi" args - module FSharpAnalyzers = type Arguments = | Project of string @@ -592,9 +590,15 @@ let checkFormatCode ctx = /// Checks the category banners and the alphabetical order of the functions in the signature files (ADR 0001, D4) let checkOrder _ = - let script = rootDirectory "build" "check-order.fsx" - let sourceDirectory = rootDirectory "src" - dotnet.fsi $"\"%s{script}\" \"%s{sourceDirectory}\"" + let struct (fileCount, violations) = CheckOrder.check (rootDirectory "src") + + if violations.Length = 0 then + Trace.log $"check-order: %d{fileCount} signature files are in order" + else + for violation in violations do + Trace.traceError $"%s{violation.Path}(%d{violation.Line}): %s{violation.Message}" + + failwith $"check-order: %d{violations.Length} violations" let cleanDocsCache _ = DocsTool.cleanDocsCache () diff --git a/build/build.fsproj b/build/build.fsproj index bf9aeda5..95ccdb3c 100644 --- a/build/build.fsproj +++ b/build/build.fsproj @@ -13,6 +13,7 @@ + @@ -29,6 +30,7 @@ + diff --git a/build/check-order.fsx b/build/check-order.fsx deleted file mode 100644 index 7c204276..00000000 --- a/build/check-order.fsx +++ /dev/null @@ -1,354 +0,0 @@ -// Checks the order of the functions in the signature files of the library (ADR 0001, D4). -// -// dotnet fsi build/check-order.fsx [source directory] -// -// Every module of a signature file groups its `val`s under banner comments `// ---- Category ----`. The banners follow -// the canonical category order, and the `val`s of a category are sorted alphabetically (ordinal), active patterns last. -// The sibling implementation file has the same banners and defines its public functions in the same order. -// A violation is printed as `path(line): message`, and the script exits with code 1. -// -// The script needs no packages, so that the FAKE target `CheckOrder` and a contributor can run it before a build. - -open System -open System.IO -open System.Text.RegularExpressions - -/// The canonical category sequence of ADR 0001, D4. -let categories = [| - "Creation" - "Builder" - "Properties" - "Access" - "Search" - "Transformation" - "Folding" - "Partitioning and grouping" - "Sorting" - "Random" - "Conversion" - "Type-specific" -|] - -/// Maps and sets only partition, so their banner may say "Partitioning"; it takes the place of "Partitioning and grouping". -let categoryIndex (category : string) = - let canonical = - if String.Equals (category, "Partitioning", StringComparison.Ordinal) then - "Partitioning and grouping" - else - category - - Array.IndexOf (categories, canonical) - -let bannerRegex = Regex (@"^\s*// ---- (?.+?) ----\s*$", RegexOptions.CultureInvariant) - -let moduleRegex = - Regex ( - @"^(? *)module\s+(?(?:private|internal)\s+)?(?:rec\s+)?(?[A-Za-z_][A-Za-z0-9_]*)\s*=", - RegexOptions.CultureInvariant - ) - -/// A file-scoped module, `module A.B` without `=`, whose members start at column 0. -let fileModuleRegex = - Regex ( - @"^module\s+(?(?:private|internal)\s+)?(?:rec\s+)?(?[A-Za-z_][A-Za-z0-9_.]*)\s*$", - RegexOptions.CultureInvariant - ) - -// An identifier or an active pattern such as `(|Cons|_|)`, optionally preceded by attributes on the same line -let private nameGroup = @"(?\(\|[^)]*\|\)|[A-Za-z_][A-Za-z0-9_']*)" - -// The access modifier is its own group, so that `val internal f` is a non-public `f`, not a value named `internal` -let valRegex = - Regex ( - @"^(? *)(?:\[<.*>\]\s*)?val\s+(?:inline\s+)?(?(?:private|internal)\s+)?(?:inline\s+)?" - + nameGroup, - RegexOptions.CultureInvariant - ) - -let letRegex = - Regex ( - @"^(? *)(?:\[<.*>\]\s*)?let\s+(?:rec\s+)?(?:inline\s+)?(?(?:private|internal)\s+)?(?:inline\s+)?" - + nameGroup, - RegexOptions.CultureInvariant - ) - -[] -type Banner = { Line : int; Category : string } - -[] -type Member = { Line : int; Category : string; Name : string } - -/// A module of a source file: its banners and its public members in source order. -type Scope = { - Path : string - Indent : int - IsPublic : bool - Banners : ResizeArray - Members : ResizeArray -} - -let indentOf (line : string) = line.Length - line.AsSpan().TrimStart(' ').Length - -/// Reads the modules of a file. -/// -/// memberRegex recognises a member declaration (val or let); members at the content level of a public -/// module count, nested bindings and non-public members do not. The second result lists the members at column 0 outside -/// any module: they belong to a module header that the script does not understand, so they must not pass unchecked. -/// -let parse (path : string) (memberRegex : Regex) = - let scopes = ResizeArray() - let orphans = ResizeArray() - let stack = Collections.Generic.Stack() - let lines = File.ReadAllLines path - - for index in 0 .. lines.Length - 1 do - let line = lines[index] - let lineNumber = index + 1 - let trimmed = line.AsSpan().Trim() - - // Preprocessor directives start at column 0 and must not close the enclosing modules - if - not trimmed.IsEmpty - && not (trimmed.StartsWith ("#", StringComparison.Ordinal)) - then - let indent = indentOf line - - while stack.Count > 0 && indent <= stack.Peek().Indent do - stack.Pop () |> ignore - - let moduleMatch = moduleRegex.Match line - let fileModuleMatch = fileModuleRegex.Match line - - if fileModuleMatch.Success then - // The members of a file-scoped module start at column 0, the content level of an indent of -4 - let scope = { - Path = fileModuleMatch.Groups["name"].Value - Indent = -4 - IsPublic = not fileModuleMatch.Groups["access"].Success - Banners = ResizeArray () - Members = ResizeArray () - } - - scopes.Add scope - stack.Push scope - elif moduleMatch.Success then - let name = moduleMatch.Groups["name"].Value - let parent = - if stack.Count > 0 then - ValueSome (stack.Peek ()) - else - ValueNone - - let scope = { - Path = - match parent with - | ValueSome parent -> $"%s{parent.Path}.%s{name}" - | ValueNone -> name - Indent = indent - IsPublic = - not moduleMatch.Groups["access"].Success - && (parent |> ValueOption.forall _.IsPublic) - Banners = ResizeArray () - Members = ResizeArray () - } - - scopes.Add scope - stack.Push scope - elif stack.Count > 0 then - let scope = stack.Peek () - let bannerMatch = bannerRegex.Match line - - if bannerMatch.Success then - scope.Banners.Add { Line = lineNumber; Category = bannerMatch.Groups["category"].Value } - else - let memberMatch = memberRegex.Match line - - let isContentLevel = - memberMatch.Success - && memberMatch.Groups["indent"].Length = scope.Indent + 4 - let isPublic = isContentLevel && not memberMatch.Groups["access"].Success - - if scope.IsPublic && isPublic then - let category = - if scope.Banners.Count = 0 then - "" - else - scope.Banners[scope.Banners.Count - 1].Category - - scope.Members.Add { - Line = lineNumber - Category = category - Name = memberMatch.Groups["name"].Value - } - else - let memberMatch = memberRegex.Match line - - if - memberMatch.Success - && memberMatch.Groups["indent"].Length = 0 - then - orphans.Add { Line = lineNumber; Category = ""; Name = memberMatch.Groups["name"].Value } - - struct (scopes |> Seq.filter _.IsPublic |> Seq.toArray, orphans |> Seq.toArray) - -let isActivePattern (name : string) = name.StartsWith ("(|", StringComparison.Ordinal) - -/// Identifiers sort ordinally; active patterns come after them, also ordinally. -let compareNames (left : string) (right : string) = - match isActivePattern left, isActivePattern right with - | false, true -> -1 - | true, false -> 1 - | _ -> String.CompareOrdinal (left, right) - -let errors = ResizeArray() - -let report path line message = errors.Add (struct (path, line, message)) - -let checkSignature (path : string) (scope : Scope) = - let mutable previousIndex = -1 - - for banner in scope.Banners do - let index = categoryIndex banner.Category - - if index < 0 then - report path banner.Line $"Unknown category '%s{banner.Category}' in module %s{scope.Path}" - elif index <= previousIndex then - report path banner.Line $"Category '%s{banner.Category}' is out of the canonical order in module %s{scope.Path}" - else - previousIndex <- index - - let hasMembers = - scope.Members - |> Seq.exists (fun item -> String.Equals (item.Category, banner.Category, StringComparison.Ordinal)) - - if not hasMembers then - report path banner.Line $"Category '%s{banner.Category}' is empty in module %s{scope.Path}; omit its banner" - - for index in 0 .. scope.Members.Count - 1 do - let item = scope.Members[index] - - if item.Category.Length = 0 then - report path item.Line $"`%s{item.Name}` comes before the first category banner of module %s{scope.Path}" - elif index > 0 then - let previous = scope.Members[index - 1] - - if String.Equals (previous.Category, item.Category, StringComparison.Ordinal) then - let order = compareNames previous.Name item.Name - - if order = 0 then - report path item.Line $"`%s{item.Name}` is declared twice in module %s{scope.Path}" - elif order > 0 then - report path item.Line $"`%s{item.Name}` must come before `%s{previous.Name}` in category '%s{item.Category}'" - -let checkImplementation (signaturePath : string) (signatureScopes : Scope array) = - let implementationPath = Path.ChangeExtension (signaturePath, ".fs") - - if not (File.Exists implementationPath) then - report signaturePath 1 "The signature file has no implementation file next to it" - else - let struct (implementationScopes, _) = parse implementationPath letRegex - - let findScope (scopes : Scope array) path = - scopes - |> Array.tryFind (fun scope -> String.Equals (scope.Path, path, StringComparison.Ordinal)) - |> Option.toValueOption - - for signature in signatureScopes do - match findScope implementationScopes signature.Path with - | ValueNone -> report implementationPath 1 $"Module %s{signature.Path} of the signature file is missing" - | ValueSome implementation -> - let signatureBanners = signature.Banners |> Seq.map _.Category |> Seq.toArray - let implementationBanners = implementation.Banners |> Seq.map _.Category |> Seq.toArray - - if signatureBanners <> implementationBanners then - let line = - if implementation.Banners.Count > 0 then - implementation.Banners[0].Line - else - 1 - - let expected = String.Join (", ", signatureBanners) - let actual = String.Join (", ", implementationBanners) - report - implementationPath - line - $"Module %s{signature.Path} has the banners [%s{actual}]; the signature has [%s{expected}]" - - let count = min signature.Members.Count implementation.Members.Count - - // A member must have the same name and the same category, so that a moved banner cannot go unnoticed - let differs index = - let expected = signature.Members[index] - let actual = implementation.Members[index] - - not (String.Equals (expected.Name, actual.Name, StringComparison.Ordinal)) - || not (String.Equals (expected.Category, actual.Category, StringComparison.Ordinal)) - - let firstDifference = - seq { 0 .. count - 1 } - |> Seq.tryFind differs - |> Option.toValueOption - - match firstDifference with - | ValueSome index -> - let expected = signature.Members[index] - let actual = implementation.Members[index] - - if String.Equals (expected.Name, actual.Name, StringComparison.Ordinal) then - report - implementationPath - actual.Line - $"`%s{actual.Name}` is in category '%s{actual.Category}'; the signature puts it in '%s{expected.Category}' (line %d{expected.Line})" - else - report - implementationPath - actual.Line - $"`%s{actual.Name}` is defined where the signature declares `%s{expected.Name}` (line %d{expected.Line})" - | ValueNone when signature.Members.Count <> implementation.Members.Count -> - report - implementationPath - 1 - $"Module %s{signature.Path} defines %d{implementation.Members.Count} public functions; the signature declares %d{signature.Members.Count}" - | ValueNone -> () - -let sourceDirectory = - match fsi.CommandLineArgs |> Array.skip 1 with - | [| directory |] -> Path.GetFullPath directory - | [||] -> Path.GetFullPath (Path.Combine (__SOURCE_DIRECTORY__, "..", "src")) - | _ -> failwith "Usage: dotnet fsi build/check-order.fsx [source directory]" - -let isGenerated (path : string) = - let separator = Path.DirectorySeparatorChar - path.Contains ($"%c{separator}obj%c{separator}", StringComparison.Ordinal) - || path.Contains ($"%c{separator}bin%c{separator}", StringComparison.Ordinal) - -let signatureFiles = - Directory.EnumerateFiles (sourceDirectory, "*.fsi", SearchOption.AllDirectories) - |> Seq.filter (isGenerated >> not) - |> Seq.sort - |> Seq.toArray - -for signaturePath in signatureFiles do - let struct (scopes, orphans) = parse signaturePath valRegex - - for orphan in orphans do - report - signaturePath - orphan.Line - $"`%s{orphan.Name}` is outside every module header that the script understands (`module X =` or `module A.B`)" - - for scope in scopes do - checkSignature signaturePath scope - - checkImplementation signaturePath scopes - -if errors.Count = 0 then - printfn $"check-order: %d{signatureFiles.Length} signature files are in order" -else - for struct (path, line, message) in - errors - |> Seq.sortBy (fun struct (path, line, _) -> struct (path, line)) do - eprintfn $"%s{path}(%d{line}): %s{message}" - - eprintfn $"check-order: %d{errors.Count} violations" - exit 1 From cd34eb0617f0cb3111172156d70ad38f1f9b8a00 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 20:04:22 +0200 Subject: [PATCH 08/10] fixup! chore: lay the foundation for the per-collection modules The order check runs the signature files in parallel: each pair is independent, so `Array.Parallel.collect` keeps the violations in file order without a shared list. Co-Authored-By: Claude Fable 5.1 --- build/CheckOrder.fs | 50 +++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/build/CheckOrder.fs b/build/CheckOrder.fs index b0be63f0..cb006e4a 100644 --- a/build/CheckOrder.fs +++ b/build/CheckOrder.fs @@ -381,41 +381,43 @@ let private isGenerated (path : string) = path.Contains ($"%c{separator}obj%c{separator}", StringComparison.Ordinal) || path.Contains ($"%c{separator}bin%c{separator}", StringComparison.Ordinal) +/// Checks one signature file and its sibling implementation file; the files are independent, so the pairs run in parallel. +let private checkPair (signaturePath : string) = + let errors = ResizeArray() + let implementationPath = Path.ChangeExtension (signaturePath, ".fs") |> nonNull + let hasImplementation = File.Exists implementationPath + + if not hasImplementation then + report errors signaturePath 1 "The signature file has no implementation file next to it" + + for major in targetFrameworks do + let defines = definesOf major + let signatureModules = parseModules errors signaturePath defines + + for scope in signatureModules do + checkSignature errors signaturePath scope + + if hasImplementation then + let implementationModules = parseModules errors implementationPath defines + checkImplementation errors implementationPath signatureModules implementationModules + + errors.ToArray () + /// Checks every signature file under a directory and its sibling implementation file. /// The directory of the sources; generated files under bin and obj are skipped. /// The number of signature files checked and the violations, sorted by path and line. let check (sourceDirectory : string) = - let errors = ResizeArray() - let signatureFiles = Directory.EnumerateFiles (sourceDirectory, "*.fsi", SearchOption.AllDirectories) |> Seq.filter (isGenerated >> not) |> Seq.sort |> Seq.toArray - for signaturePath in signatureFiles do - let implementationPath = Path.ChangeExtension (signaturePath, ".fs") |> nonNull - let hasImplementation = File.Exists implementationPath - - if not hasImplementation then - report errors signaturePath 1 "The signature file has no implementation file next to it" - - for major in targetFrameworks do - let defines = definesOf major - let signatureModules = parseModules errors signaturePath defines - - for scope in signatureModules do - checkSignature errors signaturePath scope - - if hasImplementation then - let implementationModules = parseModules errors implementationPath defines - checkImplementation errors implementationPath signatureModules implementationModules - // The views of the target frameworks mostly agree, so the same violation is usually found once per framework let violations = - errors - |> Seq.distinct - |> Seq.sortBy (fun violation -> struct (violation.Path, violation.Line)) - |> Seq.toArray + signatureFiles + |> Array.Parallel.collect checkPair + |> Array.distinct + |> Array.sortBy (fun violation -> struct (violation.Path, violation.Line)) struct (signatureFiles.Length, violations) From 617167e3cfb952f33a1305c486246d9a085f80db Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 20:14:34 +0200 Subject: [PATCH 09/10] fixup! chore: lay the foundation for the per-collection modules The contract assert helpers are static classes with methods and an optional `message`, like the `Assert` class of MSTest: `ExceptionAssert.Same` / `Mirrored` / `Headline` and `OverrideAssert.OptionMatches` / `PairMatches` / `TripleMatches`. Co-Authored-By: Claude Sonnet 5.5 --- .../Contracts/ExceptionAssert.fs | 157 +++++++++--------- .../Contracts/OverrideAssert.fs | 48 +++--- 2 files changed, 111 insertions(+), 94 deletions(-) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs index 9233b288..bbd9ecf6 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -1,89 +1,96 @@ -/// Compares the exception of a function with the exception of the FSharp.Core function that it mirrors (ADR 0001, D8). -[] -module FSharp.Collections.Immutable.Tests.ExceptionAssert +namespace FSharp.Collections.Immutable.Tests open System open System.Globalization open Microsoft.VisualStudio.TestTools.UnitTesting -/// Runs an action under the invariant culture. -/// FSharp.Core ships localized messages and this library does not, so the messages are compared in English. -let private invariant (action : unit -> 'T) = - let culture = CultureInfo.CurrentCulture - let uiCulture = CultureInfo.CurrentUICulture - CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture - CultureInfo.CurrentUICulture <- CultureInfo.InvariantCulture +/// Compares the exception of a function with the exception of the FSharp.Core function that it mirrors (ADR 0001, D8). +[] +type ExceptionAssert = + + /// Runs an action under the invariant culture. + /// FSharp.Core ships localized messages and this library does not, so the messages are compared in English. + static member private Invariant (action : unit -> 'T) = + let culture = CultureInfo.CurrentCulture + let uiCulture = CultureInfo.CurrentUICulture + CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture + CultureInfo.CurrentUICulture <- CultureInfo.InvariantCulture + + try + action () + finally + CultureInfo.CurrentCulture <- culture + CultureInfo.CurrentUICulture <- uiCulture + + /// + /// The first line of a message, without the parameter suffix of . + /// + /// + /// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), + /// and this library writes the names of its own parameters there, so only the first line is compared. + /// + static member Headline (ex : exn) = + let message = + match ex with + | :? ArgumentException as argumentException -> + match argumentException.ParamName with + | null -> ex.Message + | paramName -> + let suffix = $" (Parameter '%s{paramName}')" - try - action () - finally - CultureInfo.CurrentCulture <- culture - CultureInfo.CurrentUICulture <- uiCulture + if ex.Message.EndsWith (suffix, StringComparison.Ordinal) then + ex.Message.Substring (0, ex.Message.Length - suffix.Length) + else + ex.Message + | _ -> ex.Message -/// -/// The first line of a message, without the parameter suffix of . -/// -/// -/// FSharp.Core writes parameter names into the lines after the first one ("list1 is 2 elements shorter than list2"), -/// and this library writes the names of its own parameters there, so only the first line is compared. -/// -let headline (ex : exn) = - let message = - match ex with - | :? ArgumentException as argumentException -> - match argumentException.ParamName with - | null -> ex.Message - | paramName -> - let suffix = $" (Parameter '%s{paramName}')" + match message.IndexOf ('\n') with + | -1 -> message + | newline -> message.AsSpan(0, newline).TrimEnd('\r').ToString() - if ex.Message.EndsWith (suffix, StringComparison.Ordinal) then - ex.Message.Substring (0, ex.Message.Length - suffix.Length) - else - ex.Message - | _ -> ex.Message + /// + /// Asserts that throws what throws: an exception of the same type + /// with the same headline and, for an argument exception, the parameter name that + /// maps the parameter name of the oracle to. + /// + static member Same (paramName : string -> string, oracle : unit -> unit, actual : unit -> unit, ?message : string) = + let message = message |> Option.defaultValue "" - match message.IndexOf ('\n') with - | -1 -> message - | newline -> message.AsSpan(0, newline).TrimEnd('\r').ToString() + let expected = + ExceptionAssert.Invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) -/// -/// Asserts that throws what throws: an exception of the same type -/// with the same headline and, for an argument exception, the parameter name that -/// maps the parameter name of the oracle to. -/// -let same (paramName : string -> string) (oracle : unit -> unit) (actual : unit -> unit) (message : string) = - let expected = - invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) - let thrown = invariant (fun () -> Assert.Throws(Action actual, message)) - Assert.AreEqual(expected.GetType (), thrown.GetType (), message) - Assert.AreEqual(headline expected, headline thrown, message) + let thrown = ExceptionAssert.Invariant (fun () -> Assert.Throws(Action actual, message)) + Assert.AreEqual(expected.GetType (), thrown.GetType (), message) + Assert.AreEqual(ExceptionAssert.Headline expected, ExceptionAssert.Headline thrown, message) - match expected, thrown with - | (:? ArgumentException as expected), (:? ArgumentException as thrown) -> - let expectedName = - match expected.ParamName with - | null -> null - | name -> paramName name + match expected, thrown with + | (:? ArgumentException as expected), (:? ArgumentException as thrown) -> + let expectedName = + match expected.ParamName with + | null -> null + | name -> paramName name - Assert.AreEqual(expectedName, thrown.ParamName, message) - | _ -> () + Assert.AreEqual(expectedName, thrown.ParamName, message) + | _ -> () -/// -/// Asserts that throws what the FSharp.Core module that the collection mirrors throws: -/// for , -/// for . -/// -let mirrored - (mirror : Mirror) - (paramName : string -> string) - (arrayOracle : unit -> unit) - (listOracle : unit -> unit) - (actual : unit -> unit) - (message : string) - = - let oracle = - match mirror with - | ArrayMirror -> arrayOracle - | ListMirror -> listOracle + /// + /// Asserts that throws what the FSharp.Core module that the collection mirrors throws: + /// for , + /// for . + /// + static member Mirrored + ( + mirror : Mirror, + paramName : string -> string, + arrayOracle : unit -> unit, + listOracle : unit -> unit, + actual : unit -> unit, + ?message : string + ) + = + let oracle = + match mirror with + | ArrayMirror -> arrayOracle + | ListMirror -> listOracle - same paramName oracle actual message + ExceptionAssert.Same (paramName, oracle, actual, ?message = message) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs index 1854d7b0..7da76298 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -1,24 +1,34 @@ -/// Compares a function of the `Option` or `Tuple` namespace with the default function after conversion (ADR 0001, D3). -[] -module FSharp.Collections.Immutable.Tests.OverrideAssert +namespace FSharp.Collections.Immutable.Tests open Microsoft.VisualStudio.TestTools.UnitTesting -/// The `Option` namespace returns an `option` where the default module returns a `voption`. -/// The equality of an F# option is structural, so an option of an array compares the elements of the array. -let optionMatches (expected : 'T voption) (actual : 'T option) (message : string) = - Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message) +/// Compares a function of the `Option` or `Tuple` namespace with the default function after conversion (ADR 0001, D3). +[] +type OverrideAssert = + + /// The `Option` namespace returns an `option` where the default module returns a `voption`. + /// The equality of an F# option is structural, so an option of an array compares the elements of the array. + static member OptionMatches (expected : 'T voption, actual : 'T option, ?message : string) = + Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message |> Option.defaultValue "") -/// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. -/// The equality of a tuple compares arrays by reference, so the pairs are compared with F# structural equality. -let pairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2)) (actual : 'T1 * 'T2) (message : string) = - Assert.AreEqual<'T1 * 'T2>((expectedFirst, expectedSecond), actual, HashIdentity.Structural, message) + /// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. + /// The equality of a tuple compares arrays by reference, so the pairs are compared with F# structural equality. + static member PairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2), actual : 'T1 * 'T2, ?message : string) = + Assert.AreEqual<'T1 * 'T2>( + (expectedFirst, expectedSecond), + actual, + HashIdentity.Structural, + message |> Option.defaultValue "" + ) -/// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. -/// The equality of a tuple compares arrays by reference, so the triples are compared with F# structural equality. -let tripleMatches - (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3)) - (actual : 'T1 * 'T2 * 'T3) - (message : string) - = - Assert.AreEqual<'T1 * 'T2 * 'T3>((expectedFirst, expectedSecond, expectedThird), actual, HashIdentity.Structural, message) + /// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. + /// The equality of a tuple compares arrays by reference, so the triples are compared with F# structural equality. + static member TripleMatches + (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3), actual : 'T1 * 'T2 * 'T3, ?message : string) + = + Assert.AreEqual<'T1 * 'T2 * 'T3>( + (expectedFirst, expectedSecond, expectedThird), + actual, + HashIdentity.Structural, + message |> Option.defaultValue "" + ) From 6f2eceb5847dd72ffd9d35a862ea79410d030360 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 20:20:55 +0200 Subject: [PATCH 10/10] fixup! chore: lay the foundation for the per-collection modules The optional `message` of the contract assert helpers is a struct optional parameter, so it is a `voption`. Co-Authored-By: Claude Sonnet 5.5 --- .../Contracts/ExceptionAssert.fs | 8 +++++--- .../Contracts/OverrideAssert.fs | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs index bbd9ecf6..d1e3b892 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -53,8 +53,10 @@ type ExceptionAssert = /// with the same headline and, for an argument exception, the parameter name that /// maps the parameter name of the oracle to. /// - static member Same (paramName : string -> string, oracle : unit -> unit, actual : unit -> unit, ?message : string) = - let message = message |> Option.defaultValue "" + static member Same + (paramName : string -> string, oracle : unit -> unit, actual : unit -> unit, [] ?message : string) + = + let message = message |> ValueOption.defaultValue "" let expected = ExceptionAssert.Invariant (fun () -> Assert.Throws(Action oracle, "The FSharp.Core oracle must throw")) @@ -85,7 +87,7 @@ type ExceptionAssert = arrayOracle : unit -> unit, listOracle : unit -> unit, actual : unit -> unit, - ?message : string + [] ?message : string ) = let oracle = diff --git a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs index 7da76298..6b8d0c46 100644 --- a/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -8,27 +8,33 @@ type OverrideAssert = /// The `Option` namespace returns an `option` where the default module returns a `voption`. /// The equality of an F# option is structural, so an option of an array compares the elements of the array. - static member OptionMatches (expected : 'T voption, actual : 'T option, ?message : string) = - Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message |> Option.defaultValue "") + static member OptionMatches (expected : 'T voption, actual : 'T option, [] ?message : string) = + Assert.AreEqual<'T option>(ValueOption.toOption expected, actual, message |> ValueOption.defaultValue "") /// The `Tuple` namespace returns a reference pair where the default module returns a struct pair. /// The equality of a tuple compares arrays by reference, so the pairs are compared with F# structural equality. - static member PairMatches (struct (expectedFirst : 'T1, expectedSecond : 'T2), actual : 'T1 * 'T2, ?message : string) = + static member PairMatches + (struct (expectedFirst : 'T1, expectedSecond : 'T2), actual : 'T1 * 'T2, [] ?message : string) + = Assert.AreEqual<'T1 * 'T2>( (expectedFirst, expectedSecond), actual, HashIdentity.Structural, - message |> Option.defaultValue "" + message |> ValueOption.defaultValue "" ) /// The `Tuple` namespace returns a reference triple where the default module returns a struct triple. /// The equality of a tuple compares arrays by reference, so the triples are compared with F# structural equality. static member TripleMatches - (struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3), actual : 'T1 * 'T2 * 'T3, ?message : string) + ( + struct (expectedFirst : 'T1, expectedSecond : 'T2, expectedThird : 'T3), + actual : 'T1 * 'T2 * 'T3, + [] ?message : string + ) = Assert.AreEqual<'T1 * 'T2 * 'T3>( (expectedFirst, expectedSecond, expectedThird), actual, HashIdentity.Structural, - message |> Option.defaultValue "" + message |> ValueOption.defaultValue "" )