Repository navigation
Conversation
GetProjectSnapshotFromScript fell back to the file system when no document source was passed, also when the checker was created with DocumentSource.Custom. The TransparentCompiler's GetProjectOptionsFromScript always passed DocumentSource.FileSystem. Both now use the document source of the checker. A file from a custom document source got DateTime.Now as its version, so every snapshot gave unchanged files a new version and the TransparentCompiler caches missed. CreateFromDocumentSource now gets the text when the snapshot is created and uses its checksum as the version. It returns Async<FSharpFileSnapshot> for that reason. When the custom source returned None, getting the source failed with "Couldn't get source for file". The file is now read from disk, as the BackgroundCompiler does. See dotnet#20750.
Contributor
✅ Release notes checked
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This fixes points 1 to 3 of #20750. FsAutoComplete wants to stop installing a process-wide
FileSystemshim and give each checker its own view of the files (ionide/FsAutoComplete#1555). For scripts,DocumentSource.Customdid not work well enough for that yet.FSharpChecker.GetProjectSnapshotFromScriptfell back toDocumentSource.FileSystemwhen nodocumentSourcewas passed, also when the checker was created withDocumentSource.Custom. The TransparentCompiler'sGetProjectOptionsFromScriptalways passedDocumentSource.FileSystem. Both now use the document source of the checker.FSharpFileSnapshot.CreateFromDocumentSourceusedDateTime.Now.Ticksas the version, so every call gave unchanged files a new version and the TransparentCompiler caches missed. This hit the#loaded files of a script and everyFSharpProjectOptionsbased call on a TransparentCompiler created withDocumentSource.Custom. It now gets the text when the snapshot is created and uses its checksum as the version. Files read from disk keep the last write time.Nonereads from disk. When the custom source returnedNone, getting the source failed with "Couldn't get source for file". The file is now read from disk, as the BackgroundCompiler does (FSharpSource.fs).API change:
FSharpFileSnapshot.CreateFromDocumentSourcenow returnsAsync<FSharpFileSnapshot>, because the custom source is async and the version needs the text. The type is marked experimental and nothing outside FCS calls it. Surface area baseline is updated.A custom source is now called for every file each time a snapshot is created, not only when a file needs parsing. Before, those calls missed the caches and reparsed everything anyway.
Not in this PR (see this comment):
#loaded files through the globalFileSystem(ScriptClosure.fs,ClosureSourceOfFilename). This was already known in Ensure script load closure is always executed for GetProjectSnapshotFromScript #16880 (comment), where @0101 suggested a two-step approach: first gather the loaded files, then take their snapshots as input.ScriptClosurecache is keyed on the root script only, so a cached closure is not updated when only a#loaded file changes (review comment).documentSourceonFSharpChecker.Createis still marked "likely to be removed in the future". If that is the plan, the question from #20750 still stands: how should an editor give the TransparentCompiler the text of open files that a script#loads?Part of #20750
Checklist
Test cases added
Performance benchmarks added in case of performance changes
Release notes entry updated:
Tests: three new theories in
tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs, each run with the BackgroundCompiler and the TransparentCompiler. All six fail onmainand pass with this change.