Skip to content

Commit 74e3362

Browse files
authored
Ensure script load closure is always executed for GetProjectSnapshotFromScript (#16880)
* Ensure script load closure is always executed for GetProjectSnapshotFromScript. * assumeDotNetFramework is false for non Windows. * Update Surface Area * Check for FF in test. * Address bad merge.
1 parent dc78363 commit 74e3362

File tree

8 files changed

+249
-63
lines changed

8 files changed

+249
-63
lines changed

src/Compiler/Service/BackgroundCompiler.fs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ type internal IBackgroundCompiler =
126126
abstract GetProjectSnapshotFromScript:
127127
fileName: string *
128128
sourceText: ISourceTextNew *
129+
documentSource: DocumentSource *
129130
previewEnabled: bool option *
130131
loadedTimeStamp: System.DateTime option *
131132
otherFlags: string array option *
@@ -1627,6 +1628,7 @@ type internal BackgroundCompiler
16271628
(
16281629
fileName: string,
16291630
sourceText: ISourceTextNew,
1631+
documentSource: DocumentSource,
16301632
previewEnabled: bool option,
16311633
loadedTimeStamp: DateTime option,
16321634
otherFlags: string array option,
@@ -1653,7 +1655,7 @@ type internal BackgroundCompiler
16531655
userOpName
16541656
)
16551657

1656-
let! snapshot = FSharpProjectSnapshot.FromOptions(options, DocumentSource.FileSystem)
1658+
let! snapshot = FSharpProjectSnapshot.FromOptions(options, documentSource)
16571659
return snapshot, diagnostics
16581660
}
16591661

src/Compiler/Service/BackgroundCompiler.fsi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ type internal IBackgroundCompiler =
105105
abstract GetProjectSnapshotFromScript:
106106
fileName: string *
107107
sourceText: ISourceTextNew *
108+
documentSource: DocumentSource *
108109
previewEnabled: bool option *
109110
loadedTimeStamp: System.DateTime option *
110111
otherFlags: string array option *

src/Compiler/Service/TransparentCompiler.fs

Lines changed: 133 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,76 @@ type internal TransparentCompiler
376376
)
377377
:> IBackgroundCompiler
378378

379+
let ComputeScriptClosureInner
380+
(fileName: string)
381+
(source: ISourceTextNew)
382+
(defaultFSharpBinariesDir: string)
383+
(useSimpleResolution: bool)
384+
(useFsiAuxLib: bool)
385+
(useSdkRefs: bool)
386+
(sdkDirOverride: string option)
387+
(assumeDotNetFramework: bool)
388+
(otherOptions: string list)
389+
=
390+
let reduceMemoryUsage = ReduceMemoryFlag.Yes
391+
392+
let applyCompilerOptions tcConfig =
393+
let fsiCompilerOptions = GetCoreFsiCompilerOptions tcConfig
394+
ParseCompilerOptions(ignore, fsiCompilerOptions, otherOptions)
395+
396+
let closure =
397+
LoadClosure.ComputeClosureOfScriptText(
398+
legacyReferenceResolver,
399+
defaultFSharpBinariesDir,
400+
fileName,
401+
source,
402+
CodeContext.Editing,
403+
useSimpleResolution,
404+
useFsiAuxLib,
405+
useSdkRefs,
406+
sdkDirOverride,
407+
Lexhelp.LexResourceManager(),
408+
applyCompilerOptions,
409+
assumeDotNetFramework,
410+
tryGetMetadataSnapshot,
411+
reduceMemoryUsage,
412+
dependencyProviderForScripts
413+
)
414+
415+
closure
416+
417+
let mkScriptClosureCacheKey
418+
(fileName: string)
419+
(source: ISourceTextNew)
420+
(useSimpleResolution: bool)
421+
(useFsiAuxLib: bool)
422+
(useSdkRefs: bool)
423+
(assumeDotNetFramework: bool)
424+
(projectIdentifier: ProjectIdentifier)
425+
(otherOptions: string list)
426+
(stamp: int64 option)
427+
=
428+
{ new ICacheKey<string * ProjectIdentifier, string> with
429+
member _.GetKey() = fileName, projectIdentifier
430+
member _.GetLabel() = $"ScriptClosure for %s{fileName}"
431+
432+
member _.GetVersion() =
433+
Md5Hasher.empty
434+
|> Md5Hasher.addStrings
435+
[|
436+
yield! otherOptions
437+
match stamp with
438+
| None -> ()
439+
| Some stamp -> string stamp
440+
|]
441+
|> Md5Hasher.addBytes (source.GetChecksum().ToArray())
442+
|> Md5Hasher.addBool useSimpleResolution
443+
|> Md5Hasher.addBool useFsiAuxLib
444+
|> Md5Hasher.addBool useSdkRefs
445+
|> Md5Hasher.addBool assumeDotNetFramework
446+
|> Md5Hasher.toString
447+
}
448+
379449
let ComputeScriptClosure
380450
(fileName: string)
381451
(source: ISourceTextNew)
@@ -393,57 +463,32 @@ type internal TransparentCompiler
393463
let useSdkRefs = defaultArg useSdkRefs true
394464
let assumeDotNetFramework = defaultArg assumeDotNetFramework false
395465

396-
let key =
397-
{ new ICacheKey<string * ProjectIdentifier, string> with
398-
member _.GetKey() = fileName, projectIdentifier
399-
member _.GetLabel() = $"ScriptClosure for %s{fileName}"
400-
401-
member _.GetVersion() =
402-
Md5Hasher.empty
403-
|> Md5Hasher.addStrings
404-
[|
405-
yield! otherOptions
406-
match stamp with
407-
| None -> ()
408-
| Some stamp -> string stamp
409-
|]
410-
|> Md5Hasher.addBytes (source.GetChecksum().ToArray())
411-
|> Md5Hasher.addBool useFsiAuxLib
412-
|> Md5Hasher.addBool useFsiAuxLib
413-
|> Md5Hasher.addBool useSdkRefs
414-
|> Md5Hasher.addBool assumeDotNetFramework
415-
|> Md5Hasher.toString
416-
}
466+
let key: ICacheKey<string * ProjectIdentifier, string> =
467+
mkScriptClosureCacheKey
468+
fileName
469+
source
470+
useSimpleResolution
471+
useFsiAuxLib
472+
useSdkRefs
473+
assumeDotNetFramework
474+
projectIdentifier
475+
otherOptions
476+
stamp
417477

418478
caches.ScriptClosure.Get(
419479
key,
420480
node {
421-
let reduceMemoryUsage = ReduceMemoryFlag.Yes
422-
423-
let applyCompilerOptions tcConfig =
424-
let fsiCompilerOptions = GetCoreFsiCompilerOptions tcConfig
425-
ParseCompilerOptions(ignore, fsiCompilerOptions, otherOptions)
426-
427-
let closure =
428-
LoadClosure.ComputeClosureOfScriptText(
429-
legacyReferenceResolver,
430-
defaultFSharpBinariesDir,
431-
fileName,
432-
source,
433-
CodeContext.Editing,
434-
useSimpleResolution,
435-
useFsiAuxLib,
436-
useSdkRefs,
437-
sdkDirOverride,
438-
Lexhelp.LexResourceManager(),
439-
applyCompilerOptions,
440-
assumeDotNetFramework,
441-
tryGetMetadataSnapshot,
442-
reduceMemoryUsage,
443-
dependencyProviderForScripts
444-
)
445-
446-
return closure
481+
return
482+
ComputeScriptClosureInner
483+
fileName
484+
source
485+
defaultFSharpBinariesDir
486+
useSimpleResolution
487+
useFsiAuxLib
488+
useSdkRefs
489+
sdkDirOverride
490+
assumeDotNetFramework
491+
otherOptions
447492
}
448493
)
449494

@@ -676,8 +721,13 @@ type internal TransparentCompiler
676721
(getSwitchValue useSimpleResolutionSwitch) |> Option.isSome
677722

678723
let! (loadClosureOpt: LoadClosure option) =
679-
match projectSnapshot.SourceFiles, projectSnapshot.UseScriptResolutionRules with
680-
| [ fsxFile ], true -> // assuming UseScriptResolutionRules and a single source file means we are doing this for a script
724+
let lastScriptFile =
725+
match List.tryLast projectSnapshot.SourceFiles with
726+
| None -> None
727+
| Some file -> if IsScript file.FileName then Some file else None
728+
729+
match lastScriptFile, projectSnapshot.UseScriptResolutionRules with
730+
| Some fsxFile, true -> // assuming UseScriptResolutionRules and a single source file means we are doing this for a script
681731
node {
682732
let! source = fsxFile.GetSource() |> NodeCode.AwaitTask
683733

@@ -2158,6 +2208,7 @@ type internal TransparentCompiler
21582208
bc.GetProjectSnapshotFromScript(
21592209
fileName,
21602210
SourceTextNew.ofISourceText sourceText,
2211+
DocumentSource.FileSystem,
21612212
previewEnabled,
21622213
loadedTimeStamp,
21632214
otherFlags,
@@ -2177,6 +2228,7 @@ type internal TransparentCompiler
21772228
(
21782229
fileName: string,
21792230
sourceText: ISourceTextNew,
2231+
documentSource: DocumentSource,
21802232
previewEnabled: bool option,
21812233
loadedTimeStamp: DateTime option,
21822234
otherFlags: string array option,
@@ -2199,7 +2251,8 @@ type internal TransparentCompiler
21992251
let previewEnabled = defaultArg previewEnabled false
22002252

22012253
// Do we assume .NET Framework references for scripts?
2202-
let assumeDotNetFramework = defaultArg assumeDotNetFramework true
2254+
// No, because the bootstrap info call also doesn't
2255+
let assumeDotNetFramework = defaultArg assumeDotNetFramework false
22032256

22042257
let extraFlags =
22052258
if previewEnabled then
@@ -2219,20 +2272,22 @@ type internal TransparentCompiler
22192272
let currentSourceFile =
22202273
FSharpFileSnapshot.Create(fileName, sourceText.GetHashCode().ToString(), (fun () -> Task.FromResult sourceText))
22212274

2222-
let! loadClosure =
2223-
ComputeScriptClosure
2275+
let otherFlags = List.ofArray otherFlags
2276+
2277+
// Always perform the load closure as we cannot be sure that the incoming file does not load any new additional files.
2278+
// Consider the scenario where a.fsx loads b.fsx. Based purely on a.fsx, we cannot know if b.fsx loads another file.
2279+
// Therefore we cannot rely on any caching for the script closure in this API.
2280+
let loadClosure =
2281+
ComputeScriptClosureInner
22242282
fileName
22252283
sourceText
22262284
FSharpCheckerResultsSettings.defaultFSharpBinariesDir
22272285
useSimpleResolution
2228-
(Some useFsiAuxLib)
2229-
(Some useSdkRefs)
2286+
useFsiAuxLib
2287+
useSdkRefs
22302288
sdkDirOverride
2231-
(Some assumeDotNetFramework)
2232-
(projectFileName, fileName)
2233-
(List.ofArray otherFlags)
2234-
optionsStamp
2235-
|> Async.AwaitNodeCode
2289+
assumeDotNetFramework
2290+
otherFlags
22362291

22372292
let otherFlags =
22382293
[
@@ -2243,13 +2298,31 @@ type internal TransparentCompiler
22432298
yield "--nowarn:" + code
22442299
]
22452300

2301+
// Once we do have the script closure, we can populate the cache to re-use can later.
2302+
let loadClosureKey =
2303+
mkScriptClosureCacheKey
2304+
fileName
2305+
sourceText
2306+
useSimpleResolution
2307+
useFsiAuxLib
2308+
useSdkRefs
2309+
assumeDotNetFramework
2310+
(projectFileName, "")
2311+
otherFlags
2312+
optionsStamp
2313+
2314+
// Populate the cache.
2315+
let! _ =
2316+
caches.ScriptClosure.Get(loadClosureKey, node { return loadClosure })
2317+
|> Async.AwaitNodeCode
2318+
22462319
let sourceFiles =
22472320
loadClosure.SourceFiles
22482321
|> List.map (fun (sf, _) ->
22492322
if sf = fileName then
22502323
currentSourceFile
22512324
else
2252-
FSharpFileSnapshot.CreateFromFileSystem sf)
2325+
FSharpFileSnapshot.CreateFromDocumentSource(sf, documentSource))
22532326

22542327
let references =
22552328
loadClosure.References

src/Compiler/Service/service.fs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ type FSharpChecker
566566
(
567567
fileName,
568568
source,
569+
?documentSource,
569570
?previewEnabled,
570571
?loadedTimeStamp,
571572
?otherFlags,
@@ -577,10 +578,12 @@ type FSharpChecker
577578
?userOpName: string
578579
) =
579580
let userOpName = defaultArg userOpName "Unknown"
581+
let documentSource = defaultArg documentSource DocumentSource.FileSystem
580582

581583
backgroundCompiler.GetProjectSnapshotFromScript(
582584
fileName,
583585
source,
586+
documentSource,
584587
previewEnabled,
585588
loadedTimeStamp,
586589
otherFlags,

src/Compiler/Service/service.fsi

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ type public FSharpChecker =
253253

254254
/// <param name="fileName">Used to differentiate between scripts, to consider each script a separate project. Also used in formatted error messages.</param>
255255
/// <param name="source">The source for the file.</param>
256+
/// <param name="documentSource">DocumentSource to load any additional files.</param>
256257
/// <param name="previewEnabled">Is the preview compiler enabled.</param>
257258
/// <param name="loadedTimeStamp">Indicates when the script was loaded into the editing environment,
258259
/// so that an 'unload' and 'reload' action will cause the script to be considered as a new project,
@@ -268,6 +269,7 @@ type public FSharpChecker =
268269
member GetProjectSnapshotFromScript:
269270
fileName: string *
270271
source: ISourceTextNew *
272+
?documentSource: DocumentSource *
271273
?previewEnabled: bool *
272274
?loadedTimeStamp: DateTime *
273275
?otherFlags: string[] *

0 commit comments

Comments
 (0)