Skip to content

Fix ProjectCracker, optimize Filename.checkPathForIllegalChars #659

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 25, 2016
Merged

Fix ProjectCracker, optimize Filename.checkPathForIllegalChars #659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
427a0b6
memoize Filename.checkPathForIllegalChars
vasily-kirichenko Oct 22, 2016
0c1f673
fixed: ProjectCracker returns empty ProjectFileName list
vasily-kirichenko Oct 23, 2016
f070db4
do not use StringComparison.InvariantCultureIgnoreCase because it mak…
vasily-kirichenko Oct 24, 2016
f4d78bb
try to fix test: ProjectCracker do not pass project file names as `Ot…
vasily-kirichenko Oct 24, 2016
e28cf5b
fix tests
Oct 25, 2016
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
18 changes: 15 additions & 3 deletions src/fsharp/FSharp.Compiler.Service.ProjectCracker/ProjectCracker.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ open System.IO
open System

type ProjectCracker =

static member GetProjectOptionsFromProjectFileLogged(projectFileName : string, ?properties : (string * string) list, ?loadedTimeStamp, ?enableLogging) =
let loadedTimeStamp = defaultArg loadedTimeStamp DateTime.MaxValue // Not 'now', we don't want to force reloading
let properties = defaultArg properties []
Expand All @@ -19,10 +18,23 @@ type ProjectCracker =

let rec convert (opts: Microsoft.FSharp.Compiler.SourceCodeServices.ProjectCrackerTool.ProjectOptions) : FSharpProjectOptions =
let referencedProjects = Array.map (fun (a, b) -> a, convert b) opts.ReferencedProjectOptions

let sourceFiles, otherOptions =
opts.Options |> Array.partition (fun x -> Path.GetExtension(x).ToLower() = ".fs")

let sepChar = Path.DirectorySeparatorChar

let sourceFiles = sourceFiles |> Array.map (fun x ->
match sepChar with
| '\\' -> x.Replace('/', '\\')
| '/' -> x.Replace('\\', '/')
| _ -> x
)

logMap := Map.add opts.ProjectFile opts.LogOutput !logMap
{ ProjectFileName = opts.ProjectFile
ProjectFileNames = [| |]
OtherOptions = opts.Options
ProjectFileNames = sourceFiles
OtherOptions = otherOptions
ReferencedProjects = referencedProjects
IsIncompleteTypeCheckEnvironment = false
UseScriptResolutionRules = false
Expand Down
30 changes: 21 additions & 9 deletions src/utils/filename.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,28 @@ let illegalPathChars =
let chars = Path.GetInvalidPathChars ()
chars

let checkPathForIllegalChars (path:string) =
let len = path.Length
for i = 0 to len - 1 do
let c = path.[i]
type private PathState =
| Legal
| Illegal of path: string * illegalChar: char

let private checkPathForIllegalChars =
let cache = System.Collections.Concurrent.ConcurrentDictionary<string, PathState>()
fun (path: string) ->
match cache.TryGetValue path with
| true, Legal -> ()
| true, Illegal (path, c) -> raise(IllegalFileNameChar(path, c))
| _ ->
let len = path.Length
for i = 0 to len - 1 do
let c = path.[i]

// Determine if this character is disallowed within a path by
// attempting to find it in the array of illegal path characters.
for badChar in illegalPathChars do
if c = badChar then
raise(IllegalFileNameChar(path, c))
// Determine if this character is disallowed within a path by
// attempting to find it in the array of illegal path characters.
for badChar in illegalPathChars do
if c = badChar then
cache.[path] <- Illegal(path, c)
raise(IllegalFileNameChar(path, c))
cache.[path] <- Legal

// Case sensitive (original behaviour preserved).
let checkSuffix (x:string) (y:string) = x.EndsWith(y,System.StringComparison.Ordinal)
Expand Down
2 changes: 1 addition & 1 deletion tests/service/ExprTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ let ``Check use of type provider that provides calls to F# code`` () =
|> checker.ParseAndCheckProject
|> Async.RunSynchronously

res.Errors.Length |> shouldEqual 0
Assert.AreEqual ([||], res.Errors, sprintf "Should not be errors, but: %A" res.Errors)

let results =
[ for f in res.AssemblyContents.ImplementationFiles do
Expand Down
69 changes: 33 additions & 36 deletions tests/service/ProjectOptionsTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,22 @@ let checkOptionNotPresent (opts:string[]) s =
let getReferencedFilenames = Array.choose (fun (o:string) -> if o.StartsWith("-r:") then o.[3..] |> (Path.GetFileName >> Some) else None)
let getReferencedFilenamesAndContainingFolders = Array.choose (fun (o:string) -> if o.StartsWith("-r:") then o.[3..] |> (fun r -> ((r |> Path.GetFileName), (r |> Path.GetDirectoryName |> Path.GetFileName)) |> Some) else None)
let getOutputFile = Array.pick (fun (o:string) -> if o.StartsWith("--out:") then o.[6..] |> Some else None)
let getCompiledFilenames = Array.choose (fun (o:string) -> if o.EndsWith(".fs") then o |> (Path.GetFileName >> Some) else None)

let getCompiledFilenames =
Array.choose (fun (opt: string) ->
if opt.EndsWith ".fs" then
opt |> Path.GetFileName |> Some
else None)
>> Array.distinct

[<Test>]
let ``Project file parsing example 1 Default Configuration`` () =
let projectFile = __SOURCE_DIRECTORY__ + @"/FSharp.Compiler.Service.Tests.fsproj"
let options = ProjectCracker.GetProjectOptionsFromProjectFile(projectFile)

checkOption options.ProjectFileNames "FileSystemTests.fs"

checkOption options.OtherOptions "FSharp.Compiler.Service.dll"
checkOption options.OtherOptions "FileSystemTests.fs"
checkOption options.OtherOptions "--define:TRACE"
checkOption options.OtherOptions "--define:DEBUG"
checkOption options.OtherOptions "--flaterrors"
Expand All @@ -58,8 +65,9 @@ let ``Project file parsing example 1 Release Configuration`` () =
// Check with Configuration = Release
let options = ProjectCracker.GetProjectOptionsFromProjectFile(projectFile, [("Configuration", "Release")])

checkOption options.ProjectFileNames "FileSystemTests.fs"

checkOption options.OtherOptions "FSharp.Compiler.Service.dll"
checkOption options.OtherOptions "FileSystemTests.fs"
checkOption options.OtherOptions "--define:TRACE"
checkOptionNotPresent options.OtherOptions "--define:DEBUG"
checkOption options.OtherOptions "--debug:pdbonly"
Expand All @@ -68,11 +76,11 @@ let ``Project file parsing example 1 Release Configuration`` () =
let ``Project file parsing example 1 Default configuration relative path`` () =
let projectFile = "FSharp.Compiler.Service.Tests.fsproj"
Directory.SetCurrentDirectory(__SOURCE_DIRECTORY__)

let options = ProjectCracker.GetProjectOptionsFromProjectFile(projectFile)

checkOption options.ProjectFileNames "FileSystemTests.fs"

checkOption options.OtherOptions "FSharp.Compiler.Service.dll"
checkOption options.OtherOptions "FileSystemTests.fs"
checkOption options.OtherOptions "--define:TRACE"
checkOption options.OtherOptions "--define:DEBUG"
checkOption options.OtherOptions "--flaterrors"
Expand Down Expand Up @@ -110,21 +118,16 @@ let ``Project file parsing Sample_VS2013_FSharp_Portable_Library_net451_adjusted

[<Test>]
let ``Project file parsing -- compile files 1``() =
let p = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Test1.fsproj")

p.OtherOptions
|> getCompiledFilenames
|> set
|> should equal (set [ "Test1File1.fs"; "Test1File2.fs" ])
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Test1.fsproj")
CollectionAssert.AreEqual (["Test1File2.fs"; "Test1File1.fs"], opts.ProjectFileNames |> Array.map Path.GetFileName)
CollectionAssert.IsEmpty (getCompiledFilenames opts.OtherOptions)

[<Test>]
let ``Project file parsing -- compile files 2``() =
let p = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")

p.OtherOptions
|> getCompiledFilenames
|> set
|> should equal (set [ "Test2File1.fs"; "Test2File2.fs" ])
CollectionAssert.AreEqual (["Test2File2.fs"; "Test2File1.fs"], opts.ProjectFileNames |> Array.map Path.GetFileName)
CollectionAssert.IsEmpty (getCompiledFilenames opts.OtherOptions)

[<Test>]
let ``Project file parsing -- bad project file``() =
Expand Down Expand Up @@ -192,18 +195,16 @@ let ``Project file parsing -- reference project output file``() =
|> Array.choose (fun (o:string) -> if o.StartsWith("-r:") then o.[3..] |> Some else None)
|> should contain (normalizePath (__SOURCE_DIRECTORY__ + @"/data/DifferingOutputDir/Dir1/OutputDir1/Test1.dll"))


[<Test>]
let ``Project file parsing -- Tools Version 12``() =
let p = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/ToolsVersion12.fsproj")

checkOption (getReferencedFilenames p.OtherOptions) "System.Core.dll"
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/ToolsVersion12.fsproj")
checkOption (getReferencedFilenames opts.OtherOptions) "System.Core.dll"

[<Test>]
let ``Project file parsing -- Logging``() =
let f = normalizePath (__SOURCE_DIRECTORY__ + @"/data/ToolsVersion12.fsproj")
let p, logMap = ProjectCracker.GetProjectOptionsFromProjectFileLogged(f, enableLogging=true)
let log = logMap.[f]
let projectFileName = normalizePath (__SOURCE_DIRECTORY__ + @"/data/ToolsVersion12.fsproj")
let _, logMap = ProjectCracker.GetProjectOptionsFromProjectFileLogged(projectFileName, enableLogging=true)
let log = logMap.[projectFileName]

if runningOnMono then
Assert.That(log, Is.StringContaining("Reference System.Core resolved"))
Expand Down Expand Up @@ -386,30 +387,26 @@ let ``Project file parsing -- Exe with a PCL reference``() =

[<Test>]
let ``Project file parsing -- project file contains project reference to out-of-solution project and is used in release mode``() =

let f = normalizePath(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")
let p = ProjectCracker.GetProjectOptionsFromProjectFile(f,[("Configuration","Release")])
let references = getReferencedFilenamesAndContainingFolders p.OtherOptions |> set
let projectFileName = normalizePath(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(projectFileName,[("Configuration","Release")])
let references = getReferencedFilenamesAndContainingFolders opts.OtherOptions |> set
// Check the reference is to a release DLL
references |> should contain ("Test1.dll", "Release")

[<Test>]
let ``Project file parsing -- project file contains project reference to out-of-solution project and is used in debug mode``() =

let f = normalizePath(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")
let p = ProjectCracker.GetProjectOptionsFromProjectFile(f,[("Configuration","Debug")])
let references = getReferencedFilenamesAndContainingFolders p.OtherOptions |> set
let projectFileName = normalizePath(__SOURCE_DIRECTORY__ + @"/data/Test2.fsproj")
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(projectFileName,[("Configuration","Debug")])
let references = getReferencedFilenamesAndContainingFolders opts.OtherOptions |> set
// Check the reference is to a debug DLL
references |> should contain ("Test1.dll", "Debug")

[<Test>]
let ``Project file parsing -- space in file name``() =
let p = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Space in name.fsproj")

p.OtherOptions
|> getCompiledFilenames
|> set
|> should equal (set [ "Test2File1.fs"; "Test2File2.fs" ])
let opts = ProjectCracker.GetProjectOptionsFromProjectFile(__SOURCE_DIRECTORY__ + @"/data/Space in name.fsproj")
CollectionAssert.AreEqual (["Test2File2.fs"; "Test2File1.fs"], opts.ProjectFileNames |> Array.map Path.GetFileName)
CollectionAssert.IsEmpty (getCompiledFilenames opts.OtherOptions)

[<Test>]
let ``Project file parsing -- report files``() =
Expand Down