diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fd28f65e..aa1f8551 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,61 +10,135 @@ ## 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). + +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 +│ └── 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 -│ ├── ComputationExpressions.fs +│ ├── Helper.fs – argument checks and exceptions shared by the modules │ ├── FlatList/ – ImmutableArray API surface and builders -│ │ ├── FlatList.fs / FlatList.fsi -│ │ ├── FlatListBuilder.fs -│ │ └── FlatListCE.fs +│ │ ├── 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 / FlatListBuilder.fsi +│ │ └── FlatListCE.fs / FlatListCE.fsi │ ├── IndexedList/ – ImmutableList API surface and builders -│ │ ├── IIndexedList.fs / IIndexedList.fsi +│ │ ├── IndexedListTypes.fs │ │ ├── IndexedList.fs / IndexedList.fsi -│ │ ├── IndexedListBuilder.fs -│ │ └── IndexedListCE.fs +│ │ ├── 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 -│ │ ├── IQueue.fs / IQueue.fsi +│ │ ├── QueueTypes.fs – the Queue and IQueue abbreviations │ │ ├── Queue.fs / Queue.fsi -│ │ └── QueueCE.fs +│ │ ├── 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 -│ │ ├── IStack.fs / IStack.fsi +│ │ ├── StackTypes.fs – the Stack and IStack abbreviations │ │ ├── Stack.fs / Stack.fsi -│ │ └── StackCE.fs +│ │ ├── 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 -│ │ ├── IHashMap.fs / IHashMap.fsi -│ │ ├── HashMap.fs / HashMap.fsi -│ │ └── HashMapCE.fs +│ │ ├── HashMapTypes.fs – the HashMap and IMap abbreviations +│ │ ├── 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 -│ │ ├── ISortedMap.fs / ISortedMap.fsi +│ │ ├── SortedMapTypes.fs │ │ ├── SortedMap.fs / SortedMap.fsi -│ │ └── SortedMapCE.fs +│ │ │ ├── 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 -│ │ ├── IHashSet.fs / IHashSet.fsi +│ │ ├── HashSetTypes.fs │ │ ├── HashSet.fs / HashSet.fsi -│ │ └── HashSetCE.fs -│ └── SortedSet/ – ImmutableSortedSet API surface and CE -│ ├── ISortedSet.fs / ISortedSet.fsi -│ ├── SortedSet.fs / SortedSet.fsi -│ └── SortedSetCE.fs +│ │ │ └── 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 +│ │ │ └── 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 +│ │ │ ├── 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 +│ │ │ └── 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 - ├── 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/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 + `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. The categories are attributes on + the test classes; test methods carry none. + ## 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..3660945f 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -9,7 +9,7 @@ - + @@ -24,6 +24,7 @@ + diff --git a/README.md b/README.md index 093ef25e..58e776c9 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` - 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..cb006e4a --- /dev/null +++ b/build/CheckOrder.fs @@ -0,0 +1,423 @@ +/// +/// 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 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 signatureFiles = + Directory.EnumerateFiles (sourceDirectory, "*.fsi", SearchOption.AllDirectories) + |> Seq.filter (isGenerated >> not) + |> Seq.sort + |> Seq.toArray + + // The views of the target frameworks mostly agree, so the same violation is usually found once per framework + let violations = + signatureFiles + |> Array.Parallel.collect checkPair + |> Array.distinct + |> Array.sortBy (fun violation -> struct (violation.Path, violation.Line)) + + struct (signatureFiles.Length, violations) diff --git a/build/build.fs b/build/build.fs index 77346b84..004a55b3 100644 --- a/build/build.fs +++ b/build/build.fs @@ -588,6 +588,18 @@ 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 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 () @@ -651,6 +663,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 +711,7 @@ let initTargets (ctx : Context.FakeExecutionContext) = "DotnetRestore" =?> ("CheckFormatCode", isCI.Value) + ==> "CheckOrder" ==> "DotnetBuild" ==> "DotnetTest" ==> "DotnetPack" @@ -708,6 +722,7 @@ let initTargets (ctx : Context.FakeExecutionContext) = "DotnetRestore" =?> ("CheckFormatCode", isCI.Value) =?> ("GenerateAssemblyInfo", isPublishToGitHub) + ==> "CheckOrder" ==> "DotnetBuild" ==> "DotnetTest" ==> "DotnetPack" 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/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..d1e3b892 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/ExceptionAssert.fs @@ -0,0 +1,98 @@ +namespace FSharp.Collections.Immutable.Tests + +open System +open System.Globalization +open Microsoft.VisualStudio.TestTools.UnitTesting + +/// 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}')" + + 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. + /// + 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")) + + 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 + + Assert.AreEqual(expectedName, thrown.ParamName, message) + | _ -> () + + /// + /// 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 + + ExceptionAssert.Same (paramName, oracle, actual, ?message = 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..bc0809f0 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/Inputs.fs @@ -0,0 +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). +/// 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/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 new file mode 100644 index 00000000..6b8d0c46 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/Contracts/OverrideAssert.fs @@ -0,0 +1,40 @@ +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). +[] +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 |> 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) + = + Assert.AreEqual<'T1 * 'T2>( + (expectedFirst, expectedSecond), + actual, + HashIdentity.Structural, + 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 + ) + = + Assert.AreEqual<'T1 * 'T2 * 'T3>( + (expectedFirst, expectedSecond, expectedThird), + actual, + HashIdentity.Structural, + message |> ValueOption.defaultValue "" + ) 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..547aa7af 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,12 @@ - - - - - - - + + + + + + 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..49796ff7 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategories.fs @@ -0,0 +1,141 @@ +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 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 () = + 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..2c1958f7 --- /dev/null +++ b/tests/FSharp.Collections.Immutable.Tests/TestCategoriesTests.fs @@ -0,0 +1,92 @@ +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 ADR 0001, D8. +[] +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 _.``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) + |> Array.filter (fun testMethod -> testMethod.IsDefined (typeof, true)) + + for testMethod in testMethods do + let categories = testMethod.GetCustomAttributes(true) + Assert.IsEmpty (categories, $"%s{testClass.Name}.%s{testMethod.Name} must not carry a category")