refactor

src/Compiler/Driver/CompilerConfig.fs

@@ -1540,13 +1540,13 @@ type TcConfigProvider =
 
 let GetFSharpCoreLibraryName () = getFSharpCoreLibraryName
 
-/// Read and store the source file content for the `CallerArgumentExpression` feature
+/// Read and store the source file content for the `CallerArgumentExpression` feature.
+/// This should only be called in `fsc` or `fsi` processing.
 let readAndStoreFileContents (tcConfig: TcConfig) (sourceFiles: #seq<string>) =
-    for fileName in sourceFiles do
-        if FSharpImplFileSuffixes |> List.exists (FileSystemUtils.checkSuffix fileName) then
-            try
-                use fileStream = FileSystem.OpenFileForReadShim fileName
-                use reader = fileStream.GetReader(tcConfig.inputCodePage)
-                FileContent.update fileName (reader.ReadToEnd())
-            with _ ->
-                ()
+    if
+        tcConfig.langVersion.SupportsFeature LanguageFeature.SupportCallerArgumentExpression
+        && (tcConfig.compilationMode.IsOneOff || tcConfig.isInteractive)
+    then
+        for fileName in sourceFiles do
+            if FSharpImplFileSuffixes |> List.exists (FileSystemUtils.checkSuffix fileName) then
+                FileContent.update fileName (FileContent.FileCacheType.NotYetRead tcConfig.inputCodePage)

src/Compiler/Interactive/fsi.fs

@@ -3520,7 +3520,8 @@ type FsiStdinLexerProvider
                     | NonNull t ->
                         fsiStdinSyphon.Add(t + "\n")
                         // Update the stdin file content for the `CallerArgumentExpression` feature
-                        FileContent.update stdinMockFileName fsiStdinSyphon.SyphonText)
+                        FileContent.clear ()
+                        FileContent.update stdinMockFileName (FileContent.FileCacheType.FromString fsiStdinSyphon.SyphonText))
 
                 match inputOption with
                 | Some Null
@@ -4299,7 +4300,8 @@ type FsiInteractionProcessor
             let expr = ParseInteraction tokenizer
 
             // Update the file content for the `CallerArgumentExpression` feature
-            FileContent.update scriptFileName sourceText
+            FileContent.clear ()
+            FileContent.update scriptFileName (FileContent.FileCacheType.FromString sourceText)
 
             ExecuteParsedInteractionOnMainThread(ctok, diagnosticsLogger, expr, istate, cancellationToken))
         |> commitResult
@@ -4336,7 +4338,8 @@ type FsiInteractionProcessor
                 )
 
             // Update the file content for the `CallerArgumentExpression` feature
-            FileContent.update scriptFileName sourceText
+            FileContent.clear ()
+            FileContent.update scriptFileName (FileContent.FileCacheType.FromString sourceText)
 
             ExecuteParsedExpressionOnMainThread(ctok, diagnosticsLogger, exprWithSeq, istate))
         |> commitResult

src/Compiler/Utilities/range.fs

@@ -583,95 +583,81 @@ module Range =
             mkRange file (mkPos 1 0) (mkPos 1 80)
 
 module internal FileContent =
-    let private fileContentDict = ConcurrentDictionary<string, string>()
+    [<RequireQualifiedAccess>]
+    type FileCacheType =
+        | AlreadyRead of lines: string array
+        | NotYetRead of codePage: int option
 
-    let update (fileName: string) (fileContent: string) =
-        fileContentDict.AddOrUpdate(fileName, (fun _ -> fileContent), (fun _ _ -> fileContent))
-        |> ignore
+        static member FromString(s: string) = AlreadyRead(s.Split '\n')
 
-    let private seperators = [| '\r'; '\n' |]
+    let private fileContentDict = ConcurrentDictionary<string, FileCacheType>()
 
-    /// Find the index of the nearest line separator (\r or \r\n or \n) in the given string and offset.
-    let findLineEnd (input: string) lineStart =
-        if lineStart >= input.Length then
-            input.Length - 1
-        else
-            let idx = input.IndexOfAny(seperators, lineStart)
+    let update (fileName: string) (fileContent: FileCacheType) =
+        fileContentDict.AddOrUpdate(fileName, (fun _ -> fileContent), (fun _ _ -> fileContent))
+        |> ignore
 
-            if idx = -1 then
-                input.Length - 1
-            elif input.[idx] = '\r' && idx + 1 < input.Length && input.[idx + 1] = '\n' then
-                idx + 1
-            else
-                idx
+    let clear () = fileContentDict.Clear()
 
     /// Get the substring of the given range.
-    /// This can retain the line seperators in the source string.
-    let substring (input: string) (range: range) =
+    let private substring (input: string array) (range: range) =
         let startLine, startCol = range.StartLine, range.StartColumn
         let endLine, endCol = range.EndLine, range.EndColumn
 
         if
-            startLine < 1
-            || (startLine = endLine && startCol > endCol)
+            (startCol < 0 || endCol < 0)
+            || startLine < 1
             || startLine > endLine
-            || startCol < 0
-            || endCol < 0
+            || startLine > input.Length
+            || (startLine = endLine
+                && (startCol > endCol || startCol >= input.[startLine - 1].Length))
         then
             System.String.Empty
+        elif startLine = endLine then
+            let line = input.[startLine - 1]
+            let endCol = min (endCol - 1) (line.Length - 1)
+            let result = line.Substring(startCol, endCol - startCol + 1)
+
+            if endCol = line.Length - 1 && line[endCol] = '\r' then
+                result + "\n"
+            else
+                result
         else
-            // Here the loop was splited into two functions to avoid the non necessary allocation of the StringBuilder
-
-            /// Take text until reach the end line of the range.
-            let rec loopEnd (result: System.Text.StringBuilder) lineCount (startIndex, endIndex) =
-                if lineCount < endLine then
-                    result.Append(input.Substring(startIndex, endIndex - startIndex + 1)) |> ignore
-
-                    let nextLineEnd = findLineEnd input (endIndex + 1)
-
-                    if nextLineEnd = endIndex then
-                        result.ToString()
-                    else
-                        loopEnd result (lineCount + 1) (endIndex + 1, nextLineEnd)
-                elif lineCount = endLine then
-                    let len = min endCol (endIndex - startIndex + 1)
-                    result.Append(input.Substring(startIndex, len)).ToString()
-                else
-                    result.ToString()
-
-            /// Go to the start line of the range.
-            let rec loopStart lineCount (startIndex, endIndex) =
-                if lineCount < startLine then
-                    let nextLineEnd = findLineEnd input (endIndex + 1)
-
-                    if nextLineEnd = endIndex then
-                        System.String.Empty
-                    else
-                        loopStart (lineCount + 1) (endIndex + 1, nextLineEnd)
-
-                // reach the start line
-                elif lineCount = startLine && startLine = endLine then
-                    if startIndex + startCol <= endIndex then
-                        let endCol = min (endCol - 1) (endIndex - startIndex)
-                        input.Substring(startIndex + startCol, endCol - startCol + 1)
-                    else
-                        System.String.Empty
-                else
-                    let result = System.Text.StringBuilder()
+            let appendNewLineMark sb =
+                (sb: System.Text.StringBuilder).Append '\n' |> ignore
+
+            let startCol = min startCol (input.[startLine - 1].Length - 1)
+            let result = System.Text.StringBuilder()
+            result.Append(input.[startLine - 1].Substring(startCol)) |> ignore
+            appendNewLineMark result
 
-                    if lineCount = startLine then
-                        if startCol < endIndex - startIndex + 1 then
-                            result.Append(input.Substring(startIndex + startCol, endIndex - startIndex + 1 - startCol))
-                            |> ignore
+            let upperBound = min (endLine - 2) (input.Length - 1)
 
-                        loopEnd result (lineCount + 1) (endIndex + 1, findLineEnd input (endIndex + 1))
-                    else
-                        // Should not go into here
-                        loopEnd result lineCount (startIndex, endIndex)
+            for i in startLine..upperBound do
+                result.Append(input.[i]) |> ignore
+                appendNewLineMark result
 
-            loopStart 1 (0, findLineEnd input 0)
+            if endLine < input.Length then
+                let line = input.[endLine - 1]
+                let endCol = min endCol line.Length
+                result.Append(line.Substring(0, endCol)) |> ignore
+
+                if endCol = line.Length - 1 && line[endCol] = '\r' then
+                    appendNewLineMark result
+
+            result.ToString()
 
     let getCodeText (m: range) =
-        match fileContentDict.TryGetValue m.FileName with
-        | true, text -> substring text m
+        let fileName = m.FileName
+
+        match fileContentDict.TryGetValue fileName with
+        | true, FileCacheType.AlreadyRead lines -> substring lines m
+        | true, FileCacheType.NotYetRead codePage ->
+            try
+                use fileStream = FileSystem.OpenFileForReadShim fileName
+                use reader = fileStream.GetReader(codePage)
+                let lines = reader.ReadToEnd().Split('\n')
+                update fileName (FileCacheType.AlreadyRead lines)
+                substring lines m
+            with _ ->
+                String.Empty
         | _ -> String.Empty

src/Compiler/Utilities/range.fsi

@@ -271,13 +271,19 @@ module Line =
 
 /// Store code file content. Used to implement `CallerArgumentExpression`
 module internal FileContent =
+    [<RequireQualifiedAccess>]
+    type FileCacheType =
+        | AlreadyRead of lines: string array
+        | NotYetRead of codePage: int option
+
+        static member FromString: s: string -> FileCacheType
 
     /// Update the file content. Should be called before the code file/expression is typechecked.
-    val update: fileName: string -> fileContent: string -> unit
+    val update: fileName: string -> fileContent: FileCacheType -> unit
+
+    /// Clean up all the file contents. 
+    /// May be called in `fsi` processing, to avoid holding the source files which were compiled.
+    val clear: unit -> unit
 
-    /// Get the substring of the given range.
-    /// This can retain the line seperators in the source string.
-    val substring: input: string -> range: range -> string    
-
     /// Get the code text of the specific `range`
     val getCodeText: range -> string

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/CallerArgumentExpression.fs → tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/CallerArgumentExpression/CallerArgumentExpression.fs

@@ -7,32 +7,16 @@ open FSharp.Test.Compiler
 open FSharp.Test
 
 module CustomAttributes_CallerArgumentExpression =
-
+    let scriptPath = __SOURCE_DIRECTORY__ ++ "test script.fsx"
+
     [<FactForNETCOREAPP>]
     let ``Can consume CallerArgumentExpression in BCL methods`` () =
-        FSharp """let assertEqual a b = if a <> b then failwithf "not equal: %A and %A" a b
-try System.ArgumentException.ThrowIfNullOrWhiteSpace(Seq.init 50 (fun _ -> " ")
-  (* comment *) 
-  |> String.concat " ")
-with :? System.ArgumentException as ex -> 
-  assertEqual true (ex.Message.Contains("(Parameter 'Seq.init 50 (fun _ -> \" \")
-  (* comment *) 
-  |> String.concat \" \""))
-
-
-try System.ArgumentException.ThrowIfNullOrWhiteSpace(argument = (Seq.init 11 (fun _ -> " ")
-  (* comment *) 
-  |> String.concat " "))
-with :? System.ArgumentException as ex -> 
-  assertEqual true (ex.Message.Contains("(Parameter '(Seq.init 11 (fun _ -> \" \")
-  (* comment *) 
-  |> String.concat \" \")"))
-"""
-        |> withLangVersionPreview
-        |> asExe
-        |> compileAndRun
-        |> shouldSucceed
-        |> ignore
+      FsFromPath scriptPath
+      |> withLangVersionPreview
+      |> asExe
+      |> compileAndRun
+      |> shouldSucceed
+      |> ignore
 
     [<FactForNETCOREAPP>]
     let ``Can define methods using CallerArgumentExpression with C#-style optional arguments`` () =
@@ -377,28 +361,22 @@ AInCs.B (123 - 7) |> assertEqual "123 - 7"
       |> compileAndRun
       |> shouldSucceed
       |> ignore
-
+
+    (* ------------ FSI tests ------------- *)
+
     [<FactForNETCOREAPP>]
     let ``Check in fsi`` () =
-        Fsx """let assertEqual a b = if a <> b then failwithf "not equal: %A and %A" a b
-try System.ArgumentException.ThrowIfNullOrWhiteSpace(Seq.init 50 (fun _ -> " ")
-  (* comment *) 
-  |> String.concat " ")
-with :? System.ArgumentException as ex -> 
-  assertEqual true (ex.Message.Contains("(Parameter 'Seq.init 50 (fun _ -> \" \")
-  (* comment *) 
-  |> String.concat \" \""))
-
+      FsxFromPath scriptPath
+      |> withLangVersionPreview
+      |> runFsi
+      |> shouldSucceed
+      |> ignore
 
-try System.ArgumentException.ThrowIfNullOrWhiteSpace(argument = (Seq.init 11 (fun _ -> " ")
-  (* comment *) 
-  |> String.concat " "))
-with :? System.ArgumentException as ex -> 
-  assertEqual true (ex.Message.Contains("(Parameter '(Seq.init 11 (fun _ -> \" \")
-  (* comment *) 
-  |> String.concat \" \")"))
-"""
-        |> withLangVersionPreview
-        |> runFsi
-        |> shouldSucceed
-        |> ignore
+
+    [<FactForNETCOREAPP>]
+    let ``Check fsi #load`` () =
+      Fsx $"""#load @"{scriptPath}" """
+      |> withLangVersionPreview
+      |> runFsi
+      |> shouldSucceed
+      |> ignore

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/CallerArgumentExpression/test script.fsx

@@ -0,0 +1,17 @@
+let assertEqual a b = if a <> b then failwithf "not equal: %A and %A" a b
+try System.ArgumentException.ThrowIfNullOrWhiteSpace(Seq.init 50 (fun _ -> " ")
+  (* comment *) 
+  |> String.concat " ")
+with :? System.ArgumentException as ex -> 
+  assertEqual true (ex.Message.Contains("(Parameter 'Seq.init 50 (fun _ -> \" \")
+  (* comment *) 
+  |> String.concat \" \""))
+
+
+try System.ArgumentException.ThrowIfNullOrWhiteSpace(argument = (Seq.init 11 (fun _ -> " ")
+  (* comment *) 
+  |> String.concat " "))
+with :? System.ArgumentException as ex -> 
+  assertEqual true (ex.Message.Contains("(Parameter '(Seq.init 11 (fun _ -> \" \")
+  (* comment *) 
+  |> String.concat \" \")"))

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

@@ -40,7 +40,7 @@
     <Compile Include="Conformance\BasicGrammarElements\AccessibilityAnnotations\PermittedLocations\PermittedLocations.fs" />
     <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\AttributeInheritance\AttributeInheritance.fs" />
     <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\AttributeUsage\AttributeUsage.fs" />
-    <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\CallerArgumentExpression.fs" />
+    <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\CallerArgumentExpression\CallerArgumentExpression.fs" />
     <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\Basic\Basic.fs" />
     <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\ImportedAttributes\ImportedAttributes.fs" />
     <Compile Include="Conformance\BasicGrammarElements\CustomAttributes\ArgumentsOfAllTypes\ArgumentsOfAllTypes.fs" />

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj

@@ -48,7 +48,6 @@
     <Compile Include="XmlDocTests.fs" />
     <Compile Include="XmlDocTests - Units of Measure.fs" />
     <Compile Include="RangeTests.fs" />
-    <Compile Include="FileContentTests.fs" />
     <Compile Include="TooltipTests.fs" />
     <Compile Include="TokenizerTests.fs" />
     <Compile Include="CompilerTestHelpers.fs" />