Skip to content

Conversation

@vzarytovskii
Copy link
Member

Workaround, until we have a proper fix from the project system or move to Roslyn as a source of truth:
Do not update cache if we receive completely empty list from project system.
A workaround for #12982.
More details:

When we open the solution, HandleCommandLineChanges is getting called by project system (CPS):

member _.HandleCommandLineChanges(path:string, sources:ImmutableArray<CommandLineSourceFile>, _references:ImmutableArray<CommandLineReference>, options:ImmutableArray<string>) =
use _logBlock = Logger.LogBlock(LogEditorFunctionId.LanguageService_HandleCommandLineArgs)
let projectId =
match Microsoft.CodeAnalysis.ExternalAccess.FSharp.LanguageServices.FSharpVisualStudioWorkspaceExtensions.TryGetProjectIdByBinPath(workspace, path) with
| true, projectId -> projectId
| false, _ -> Microsoft.CodeAnalysis.ExternalAccess.FSharp.LanguageServices.FSharpVisualStudioWorkspaceExtensions.GetOrCreateProjectIdForPath(workspace, path, projectDisplayNameOf path)
let path = Microsoft.CodeAnalysis.ExternalAccess.FSharp.LanguageServices.FSharpVisualStudioWorkspaceExtensions.GetProjectFilePath(workspace, projectId)
let getFullPath p =
let p' =
if Path.IsPathRooted(p) || path = null then p
else Path.Combine(Path.GetDirectoryName(path), p)
Path.GetFullPathSafe(p')
let sourcePaths = sources |> Seq.map(fun s -> getFullPath s.Path) |> Seq.toArray
let workspaceService = workspace.Services.GetRequiredService<IFSharpWorkspaceService>()
workspaceService.FSharpProjectOptionsManager.SetCommandLineOptions(projectId, sourcePaths, options)

When project system calls us, it sends us project id, source files and options, and we store them in cache:

member this.SetCommandLineOptions(projectId, sourcePaths, options: ImmutableArray<string>) =
reactor.SetCommandLineOptions(projectId, sourcePaths, options.ToArray())

Project system calls us here:
https://github.com/dotnet/project-system/blob/2d8184bf78a11568ae502597ed5ba6d24bdd736c/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/FSharp/FSharpCommandLineParserService.cs#L83-L94

Notification, apparently, originates from here:

https://github.com/dotnet/project-system/blob/9815fdcba096f21ae64a0dd1803d61b21b0c8b62/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/LanguageServices/Handlers/CommandLineNotificationHandler.cs#L26-L33

We use that cache afterwards, to get documents from it and then apply colouring, tooltips, etc.

However, what's happening now is:

  1. We open a solution
  2. We receive project system"events" once for every project, we store it in cache, and we successfully store everything in cache.
  3. We close solution, we empty the cache.
  4. We REopen the solution, we receive project system "events" for every projects, we store it in cache, BUT
  5. We again receive project system events, for each project (usually, for all of them, except if there are any documents open already).
    HOWEVER those events contain empty source file list as well as options list, and we are rewriting the cache with empty lists.
  6. Later on, when we use this cache, we don't find any documents in it, and cannot apply colour, tooltips, etc.

@vzarytovskii vzarytovskii marked this pull request as draft June 16, 2022 17:16
@vzarytovskii
Copy link
Member Author

Converting to draft, since I will need to do some extensive manual testing before merging.

@KevinRansom KevinRansom changed the title Workaroud for emptying FSharp.Editor cache due to duplicate PS events Workaround for emptying FSharp.Editor cache due to duplicate PS events Jun 16, 2022
@KevinRansom KevinRansom changed the title Workaround for emptying FSharp.Editor cache due to duplicate PS events Workaround for emptying FSharp.Editor cache due to duplicate ProjectSystem events Jun 16, 2022
@KevinRansom
Copy link
Contributor

Undrafting.

@KevinRansom KevinRansom marked this pull request as ready for review June 16, 2022 21:06
vzarytovskii and others added 2 commits June 17, 2022 12:41
…e.fs

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
…e.fs

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
@vzarytovskii
Copy link
Member Author

OK, I have verified that it works (there's still some lag between opening the document and colouring kick in, but it's most likely due to my slow laptop).

@vzarytovskii vzarytovskii merged commit 2420593 into dotnet:release/dev17.3 Jun 17, 2022
@vzarytovskii vzarytovskii deleted the sources-cache-workaround branch June 17, 2022 13:56
@vzarytovskii vzarytovskii linked an issue Jun 17, 2022 that may be closed by this pull request
KevinRansom pushed a commit to KevinRansom/fsharp that referenced this pull request Jun 27, 2022
…ystem events (dotnet#13313)

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
dsyme pushed a commit that referenced this pull request Jun 28, 2022
…ystem events (#13313) (#13396)

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>

Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
vzarytovskii added a commit to vzarytovskii/fsharp that referenced this pull request Jun 28, 2022
…ystem events (dotnet#13313)

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
vzarytovskii added a commit that referenced this pull request Jun 29, 2022
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
vzarytovskii added a commit that referenced this pull request Jun 29, 2022
* Cherry-pick #13313 to 17.2 (#13399)

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>

* Update azure-pipelines.yml

Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
vzarytovskii added a commit that referenced this pull request Jun 29, 2022
* Cherry-pick #13313 to 17.2 (#13399)

Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>

* Update azure-pipelines.yml

Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
vzarytovskii added a commit that referenced this pull request Jun 29, 2022
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>

Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

F# intellisense sometimes stops working

3 participants