Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call ignoreFile.IsIgnored with absolute path. #2656

Merged
merged 3 commits into from
Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## [5.1.5] - 2022-12-22

### Fixed
* Call ignoreFile.IsIgnored with absolute path. [#2656](https://github.com/fsprojects/fantomas/pull/2656)

## [5.1.4] - 2022-11-30

### Fixed
Expand Down
29 changes: 29 additions & 0 deletions src/Fantomas.Tests/Integration/DaemonTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,32 @@ let add (a : int) (b : int) = //
"
| otherResponse -> Assert.Fail $"Unexpected response %A{otherResponse}"
})

[<Test>]
let ``format nested ignored file`` () =
runWithDaemon (fun client ->
async {
let sourceCode = "let foo = 4"

use codeFile =
new TemporaryFileCodeSample(
sourceCode,
fileName = "NicePrint",
subFolders = [| "src"; "Compiler"; "Checking" |]
)

use _ignoreFixture = new FantomasIgnoreFile("src/Compiler/Checking/NicePrint.fs")

let request =
{ SourceCode = sourceCode
FilePath = codeFile.Filename
Config = None }

let! response =
client.InvokeAsync<FormatDocumentResponse>(Methods.FormatDocument, request)
|> Async.AwaitTask

match response with
| FormatDocumentResponse.IgnoredFile _ -> Assert.Pass()
| otherResponse -> Assert.Fail $"Unexpected response %A{otherResponse}"
})
30 changes: 16 additions & 14 deletions src/Fantomas/IgnoreFile.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,20 @@ namespace Fantomas
open System.IO.Abstractions
open Ignore

type AbsoluteFilePath =
private
| AbsoluteFilePath of string

member x.Path =
let (AbsoluteFilePath(path)) = x
path

static member Create (fs: IFileSystem) (filePath: string) =
fs.Path.GetFullPath filePath |> AbsoluteFilePath

/// The string argument is taken relative to the location
/// of the ignore-file.
type IsPathIgnored = string -> bool
type IsPathIgnored = AbsoluteFilePath -> bool

type IgnoreFile =
{ Location: IFileInfo
Expand Down Expand Up @@ -46,15 +57,15 @@ module IgnoreFile =
(Ignore(), lines)
||> Array.fold (fun (ig: Ignore) (line: string) -> ig.Add(line))

fun path ->
fun (absoluteFilePath: AbsoluteFilePath) ->
// See https://git-scm.com/docs/gitignore
// We transform the incoming path relative to the .ignoreFilePath folder.
// In a cli scenario that is the current directory, for the daemon it is the first found ignore file.
// .gitignore uses forward slashes to path separators
let relativePath =
fs
.Path
.GetRelativePath(fs.Directory.GetParent(ignoreFilePath).FullName, path)
.GetRelativePath(fs.Directory.GetParent(ignoreFilePath).FullName, absoluteFilePath.Path)
.Replace("\\", "/")

fantomasIgnore.IsIgnored(relativePath)
Expand All @@ -74,19 +85,10 @@ module IgnoreFile =
| None -> false
| Some ignoreFile ->
let fs = ignoreFile.Location.FileSystem
let fullPath = fs.Path.GetFullPath(file)
let fullPath = AbsoluteFilePath.Create fs file

try
let path =
if fullPath.StartsWith ignoreFile.Location.Directory.FullName then
fullPath.[ignoreFile.Location.Directory.FullName.Length + 1 ..]
else
// This scenario is a bit unexpected - it suggests that we are
// trying to ask an ignorefile whether a file that is outside
// its dependency tree is ignored.
fullPath

ignoreFile.IsIgnored path
ignoreFile.IsIgnored fullPath
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smaug123 I believe the problem of using a relative path here is the following:
In loadIgnoreList we process the path with

let relativePath =
fs
.Path
.GetRelativePath(fs.Directory.GetParent(ignoreFilePath).FullName, path)
.Replace("\\", "/")

Looking at the docs, they mention GetFullPath is invoked before calculating the difference.

In the case of the cli tool, the relative will be resolved with the pwd because that is where we assume the ignore file lives. For the daemon, on the other hand, the pwd could be something really random and thus the difference is calculated wrongly.

Does this make sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh, yeah, that does make sense. But might it be better to only pass fully-qualified paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But might it be better to only pass fully-qualified paths?

Something like this?

with ex ->
printfn "%A" ex
false
13 changes: 10 additions & 3 deletions src/Fantomas/IgnoreFile.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,16 @@ namespace Fantomas

open System.IO.Abstractions

type AbsoluteFilePath =
private
| AbsoluteFilePath of string

member Path: string
static member Create: fs: IFileSystem -> filePath: string -> AbsoluteFilePath

/// The string argument is taken relative to the location
/// of the ignore-file.
type IsPathIgnored = string -> bool
type IsPathIgnored = AbsoluteFilePath -> bool

type IgnoreFile =
{ Location: IFileInfo
Expand All @@ -19,14 +26,14 @@ module IgnoreFile =
/// Note that this is intended for use only in the daemon; the command-line tool
/// does not support `.fantomasignore` files anywhere other than the current
/// working directory.
val find: fs: IFileSystem -> loadIgnoreList: (string -> string -> bool) -> filePath: string -> IgnoreFile option
val find: fs: IFileSystem -> loadIgnoreList: (string -> IsPathIgnored) -> filePath: string -> IgnoreFile option

val loadIgnoreList: fs: IFileSystem -> ignoreFilePath: string -> IsPathIgnored

val internal current':
fs: IFileSystem ->
currentDirectory: string ->
loadIgnoreList: (string -> string -> bool) ->
loadIgnoreList: (string -> IsPathIgnored) ->
Lazy<IgnoreFile option>

/// When executed from the command line, Fantomas will not dynamically locate
Expand Down