diff --git a/CHANGELOG.md b/CHANGELOG.md index 19dd3f7209..4511f5fb28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [8.0.0-alpha-025] - 2026-08-28 + +### Fixed + +- A `!` line of a `.fantomasignore` takes a path back out again even when a line above it matched the folder holding that path, so `sub/*` followed by `!sub/keep` formats and checks the files under `sub/keep`. Since `8.0.0-alpha-016` a folder an ignore file names is never opened, which is what a folder pattern should mean and is not what it can mean here: closing `sub` decides that `sub/keep` is not there, and the line that would have taken it back out is never reached. The files were not reported as ignored, they were never found, so `--check` passed over a scope smaller than the ignore file describes and `doctor`, which asks about one file and answers correctly, disagreed with the run about that same file. An ignore file that negates anything now leaves every folder open and asks about the files inside one at a time, which is what every version up to `8.0.0-alpha-015` did. An ignore file with no `!` line in it still closes the folders it names. [#3447](https://github.com/fsprojects/fantomas/issues/3447) + ## [8.0.0-alpha-024] - 2026-08-28 ### Fixed diff --git a/src/Fantomas.Tests/IgnoreFileTests.fs b/src/Fantomas.Tests/IgnoreFileTests.fs index 2fe11ce4e7..3c19f7a00e 100644 --- a/src/Fantomas.Tests/IgnoreFileTests.fs +++ b/src/Fantomas.Tests/IgnoreFileTests.fs @@ -322,6 +322,24 @@ let ``the last line that matches is the one the ignore file itself decided by`` byTheIgnoreFile |> shouldEqual expected byTheLastMatch |> shouldEqual expected +[] +let ``an ignore file is asked whether any line takes a path back out`` () = + // What a walk consults before it closes a folder whole, so the cases that matter are the ones + // that decide it: a `!` line is one, and a line that only looks like one is not. + let cases: (string list * bool) list = + [ + [ "*.fs" ], false + [ "*.fs"; "!A.fs" ], true + [ "# !A.fs" ], false + [ "" ], false + // A backslash takes the `!` literally, so this names a file called `!A.fs`. + [ "\\!A.fs" ], false + ] + + for patterns, expected in cases do + let ignoreFile, _ = governing patterns "A.fs" + IgnoreFile.hasNegatedPattern ignoreFile |> shouldEqual expected + [] let ``a pattern the ignore library will not compile fails the whole ignore file`` () = // Worth pinning rather than assuming, because it decides what every caller can be told. The diff --git a/src/Fantomas.Tests/PlanTests.fs b/src/Fantomas.Tests/PlanTests.fs index a966f07ef5..0b6925f6fd 100644 --- a/src/Fantomas.Tests/PlanTests.fs +++ b/src/Fantomas.Tests/PlanTests.fs @@ -311,3 +311,23 @@ let ``the nearest ignore file wins rather than every one above it`` () = // The root would ignore it; the nearer one says nothing about it, and the nearer one is asked. planIgnoringUnder fs sub "other.fs" (InputPath.Folder root) OutputPath.NotKnown |> shouldPlan [ WorkItem.Format(inside, inside) ] + +[] +let ``a negated pattern takes a folder back out of one an earlier pattern matched, 3447`` () = + // `sub/*` followed by `!sub/keep` is how `.gitignore` spells "all of it but that one", and it + // only works if `sub` is opened: closing it decides that `sub/keep` is not there, and the line + // that would have taken it back out is never reached. + let fs: IFileSystem = MockFileSystem() + let root: string = mockRoot fs + let top: string = fs.Path.Combine(root, "Top.fs") + let kept: string = fs.Path.Combine(root, "sub", "keep", "A.fs") + let skipped: string = fs.Path.Combine(root, "sub", "drop", "B.fs") + [ top; kept; skipped ] |> makeFileHierarchy fs + + planIgnoring fs "sub/*\n!sub/keep\n" (InputPath.Folder root) OutputPath.NotKnown + |> shouldPlan + [ + WorkItem.Format(top, top) + WorkItem.Format(kept, kept) + WorkItem.Ignored skipped + ] diff --git a/src/Fantomas/IgnoreFile.fs b/src/Fantomas/IgnoreFile.fs index 98e6a319a0..d64fa7a4a0 100644 --- a/src/Fantomas/IgnoreFile.fs +++ b/src/Fantomas/IgnoreFile.fs @@ -170,6 +170,21 @@ module IgnoreFile = log.Debug $"%A{ex}" false + let hasNegatedPattern (ignoreFile: IgnoreFile) : bool = + let fs: IFileSystem = ignoreFile.Location.FileSystem + + fs.File.ReadAllLines ignoreFile.Location.FullName + |> Array.exists (fun (line: string) -> + // The library reads the line the way it reads it anywhere else, so a comment, a blank + // line and a `\!` that means a literal exclamation mark are all told apart here the + // same way they are told apart when the pattern is matched. A line it will not compile + // is not a negation; whatever it is, it is not this function's to report. + try + IgnoreRule(line).Negate + with _ -> + false + ) + let matchingLines (ignoreFile: IgnoreFile) (file: string) : IgnoreMatch list = let fs: IFileSystem = ignoreFile.Location.FileSystem let ignoreRoot: string = ignoreFile.Location.Directory.FullName diff --git a/src/Fantomas/IgnoreFile.fsi b/src/Fantomas/IgnoreFile.fsi index 851c63642b..96a2541cd7 100644 --- a/src/Fantomas/IgnoreFile.fsi +++ b/src/Fantomas/IgnoreFile.fsi @@ -87,6 +87,16 @@ module IgnoreFile = /// if it does, the failure is reported through the sink and the file counts as not ignored. val isIgnoredFile: log: ILogger -> ignoreFile: IgnoreFile option -> file: string -> bool + /// Does any line of the ignore file take a path back out of what a line above it matched? + /// + /// What this decides is whether a folder may be closed whole. `sub/*` followed by `!sub/keep` + /// is how `.gitignore` spells "all of it but that one", and the second line is only ever + /// reached for a path the walk turns up: closing `sub` decides that `sub/keep` is not there. + /// So an ignore file that negates anything keeps every folder open, and the files inside are + /// asked about one at a time, which costs a walk over a folder nothing will come out of and is + /// the only way the answer can come out right. + val hasNegatedPattern: ignoreFile: IgnoreFile -> bool + /// Every line of the ignore file whose pattern matches the path, in the order they are /// written. The last of them is the one that decided: a pattern overrules every pattern above /// it, so a `!` line that comes last un-ignores what an earlier line matched and an ordinary diff --git a/src/Fantomas/Plan.fs b/src/Fantomas/Plan.fs index 4cadf5a2d2..58d001afcb 100644 --- a/src/Fantomas/Plan.fs +++ b/src/Fantomas/Plan.fs @@ -1,6 +1,7 @@ module Fantomas.Plan open System +open System.Collections.Concurrent open System.IO.Abstractions open Fantomas open Fantomas.Arguments @@ -27,18 +28,44 @@ let plan else WorkItem.Format(inputFile, outputFile) + // Asked once per ignore file rather than once per folder, because the answer is about the file + // as a whole and the walk puts the question at every directory it meets. + let negations: ConcurrentDictionary = + ConcurrentDictionary() + // A folder the ignore file names is never opened, so nothing inside it is planned, counted or // reported. `isIgnoredFile` reads a path rather than a file, so a directory is asked the same // question a file is; what makes this the parent's answer is that `findIgnoreFile` walks up // from the directory it is given, which for a folder is the one above it. + // + // Unless the ignore file negates something. A `!` line takes a path back out of what a line + // above it matched, and the path it takes back out can be one inside a folder an earlier line + // matched: `sub/*` followed by `!sub/keep` is how `.gitignore` spells "all of it but that + // one". Closing `sub` would decide that `sub/keep` is not there and the line that takes it + // back out would never be reached, so such an ignore file leaves every folder open and every + // file inside is asked about one at a time. let isIgnoredDirectory (directory: string) : bool = + match findIgnoreFile directory with + | None -> false + | Some ignoreFile -> + + let negates: bool = + negations.GetOrAdd( + ignoreFile.Location.FullName, + Func(fun _ -> IgnoreFile.hasNegatedPattern ignoreFile) + ) + + if negates then + false + else + let asDirectory: string = String.Concat( directory.TrimEnd(fs.Path.DirectorySeparatorChar), string fs.Path.DirectorySeparatorChar ) - IgnoreFile.isIgnoredFile log (findIgnoreFile directory) asDirectory + IgnoreFile.isIgnoredFile log (Some ignoreFile) asDirectory let folder (inputFolder: string) (outputFolder: string) : WorkItem list = let inPlace: bool = isSamePath fs inputFolder outputFolder