Skip to content

Commit 15d9391

Browse files
jasonmalinowskiTIHan
authored andcommitted
Decouple the F# language service from AbstractProject (#5553)
The F# language service presumed that IWorkspaceProjectContext is implemented by an AbstractProject, which is an assumption that isn't safe going forward. This removes that presumption, and tweaks a few other things that should allow F# to continue to function once we further refactor the project system work in Roslyn.
1 parent e4012ad commit 15d9391

File tree

1 file changed

+32
-25
lines changed

1 file changed

+32
-25
lines changed

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -408,13 +408,13 @@ type internal FSharpLanguageService(package : FSharpPackage) =
408408
if String.IsNullOrWhiteSpace projectFileName then projectFileName
409409
else Path.GetFileNameWithoutExtension projectFileName
410410

411-
let singleFileProjects = ConcurrentDictionary<_, AbstractProject>()
411+
let singleFileProjects = ConcurrentDictionary<_, IWorkspaceProjectContext>()
412412

413413
let tryRemoveSingleFileProject projectId =
414414
match singleFileProjects.TryRemove(projectId) with
415415
| true, project ->
416416
projectInfoManager.ClearInfoForSingleFileProject(projectId)
417-
project.Disconnect()
417+
project.Dispose()
418418
| _ -> ()
419419

420420
let invalidPathChars = set (Path.GetInvalidPathChars())
@@ -501,16 +501,18 @@ type internal FSharpLanguageService(package : FSharpPackage) =
501501

502502
/// Sync the Roslyn information for the project held in 'projectContext' to match the information given by 'site'.
503503
/// Also sync the info in ProjectInfoManager if necessary.
504-
member this.SyncProject(project: AbstractProject, projectContext: IWorkspaceProjectContext, site: IProjectSite, workspace, forceUpdate, userOpName) =
504+
member this.SyncProject(projectContext: IWorkspaceProjectContext, site: IProjectSite, workspace: VisualStudioWorkspaceImpl, forceUpdate, userOpName) =
505505
let wellFormedFilePathSetIgnoreCase (paths: seq<string>) =
506506
HashSet(paths |> Seq.filter isPathWellFormed |> Seq.map (fun s -> try Path.GetFullPath(s) with _ -> s), StringComparer.OrdinalIgnoreCase)
507507

508508
let mutable updated = forceUpdate
509509

510+
let project = workspace.CurrentSolution.Projects |> Seq.filter (fun p -> p.Name = projectContext.DisplayName) |> Seq.exactlyOne
511+
510512
// Sync the source files in projectContext. Note that these source files are __not__ maintained in order in projectContext
511513
// as edits are made. It seems this is ok because the source file list is only used to drive roslyn per-file checking.
512514
let updatedFiles = site.CompilationSourceFiles |> wellFormedFilePathSetIgnoreCase
513-
let originalFiles = project.GetCurrentDocuments() |> Seq.map (fun file -> file.FilePath) |> wellFormedFilePathSetIgnoreCase
515+
let originalFiles = project.Documents |> Seq.map (fun file -> file.FilePath) |> wellFormedFilePathSetIgnoreCase
514516

515517
for file in updatedFiles do
516518
if not(originalFiles.Contains(file)) then
@@ -523,7 +525,7 @@ type internal FSharpLanguageService(package : FSharpPackage) =
523525
updated <- true
524526

525527
let updatedRefs = site.CompilationReferences |> wellFormedFilePathSetIgnoreCase
526-
let originalRefs = project.GetCurrentMetadataReferences() |> Seq.map (fun ref -> ref.FilePath) |> wellFormedFilePathSetIgnoreCase
528+
let originalRefs = project.MetadataReferences |> Enumerable.OfType<PortableExecutableReference> |> Seq.map (fun ref -> ref.FilePath) |> wellFormedFilePathSetIgnoreCase
527529

528530
for ref in updatedRefs do
529531
if not(originalRefs.Contains(ref)) then
@@ -566,11 +568,14 @@ type internal FSharpLanguageService(package : FSharpPackage) =
566568
let projectFileName = site.ProjectFileName
567569
let projectDisplayName = projectDisplayNameOf projectFileName
568570

569-
let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
571+
// This projectId is not guaranteed to be the same ProjectId that will actually be created once we call CreateProjectContext
572+
// in Roslyn versions once https://github.com/dotnet/roslyn/pull/26931 is merged. Roslyn will still guarantee that once
573+
// there is a project in the workspace with the same path, it'll return the ID of that. So this is sufficient to use
574+
// in that case as long as we only use it to call GetProject.
575+
let fakeProjectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
570576

571-
if isNull (workspace.ProjectTracker.GetProject projectId) then
577+
if isNull (workspace.ProjectTracker.GetProject fakeProjectId) then
572578
let projectContextFactory = package.ComponentModel.GetService<IWorkspaceProjectContextFactory>();
573-
let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)
574579

575580
let hierarchy =
576581
site.ProjectProvider
@@ -589,27 +594,27 @@ type internal FSharpLanguageService(package : FSharpPackage) =
589594
projectFileName,
590595
projectGuid,
591596
hierarchy,
592-
Option.toObj site.CompilationBinOutputPath,
593-
errorReporter)
594-
595-
let project = projectContext :?> AbstractProject
597+
Option.toObj site.CompilationBinOutputPath)
598+
599+
// The real project ID that was actually added. See comments for fakeProjectId why this one is actually good.
600+
let realProjectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
596601

597602
// Sync IProjectSite --> projectContext, and IProjectSite --> ProjectInfoManage
598-
this.SyncProject(project, projectContext, site, workspace, forceUpdate=true, userOpName=userOpName)
603+
this.SyncProject(projectContext, site, workspace, forceUpdate=true, userOpName=userOpName)
599604

600-
site.BuildErrorReporter <- Some (errorReporter :> Microsoft.VisualStudio.Shell.Interop.IVsLanguageServiceBuildErrorReporter2)
605+
site.BuildErrorReporter <- Some (projectContext :?> Microsoft.VisualStudio.Shell.Interop.IVsLanguageServiceBuildErrorReporter2)
601606

602607
// TODO: consider forceUpdate = false here. forceUpdate=true may be causing repeated computation?
603608
site.AdviseProjectSiteChanges(FSharpConstants.FSharpLanguageServiceCallbackName,
604-
AdviseProjectSiteChanges(fun () -> this.SyncProject(project, projectContext, site, workspace, forceUpdate=true, userOpName="AdviseProjectSiteChanges."+userOpName)))
609+
AdviseProjectSiteChanges(fun () -> this.SyncProject(projectContext, site, workspace, forceUpdate=true, userOpName="AdviseProjectSiteChanges."+userOpName)))
605610

606611
site.AdviseProjectSiteClosed(FSharpConstants.FSharpLanguageServiceCallbackName,
607612
AdviseProjectSiteChanges(fun () ->
608-
projectInfoManager.ClearInfoForProject(project.Id)
613+
projectInfoManager.ClearInfoForProject(realProjectId)
609614
optionsAssociation.Remove(projectContext) |> ignore
610-
project.Disconnect()))
615+
projectContext.Dispose()))
611616

612-
for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites(Some projectId, site, this.SystemServiceProvider, Some (this.Workspace :>obj), Some projectInfoManager.FSharpOptions ) do
617+
for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites(Some realProjectId, site, this.SystemServiceProvider, Some (this.Workspace :>obj), Some projectInfoManager.FSharpOptions ) do
613618
setup referencedSite
614619

615620
setup (siteProvider.GetProjectSite())
@@ -619,19 +624,21 @@ type internal FSharpLanguageService(package : FSharpPackage) =
619624
let projectFileName = fileName
620625
let projectDisplayName = projectDisplayNameOf projectFileName
621626

622-
let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
623-
let _referencedProjectFileNames, parsingOptions, projectOptions = projectInfoManager.ComputeSingleFileOptions (tryGetOrCreateProjectId workspace, fileName, loadTime, fileContents) |> Async.RunSynchronously
624-
projectInfoManager.AddOrUpdateSingleFileProject(projectId, (loadTime, parsingOptions, projectOptions))
627+
let mutable projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
625628

626629
if isNull (workspace.ProjectTracker.GetProject projectId) then
627630
let projectContextFactory = package.ComponentModel.GetService<IWorkspaceProjectContextFactory>();
628-
let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)
629631

630-
let projectContext = projectContextFactory.CreateProjectContext(FSharpConstants.FSharpLanguageName, projectDisplayName, projectFileName, projectId.Id, hier, null, errorReporter)
632+
let projectContext = projectContextFactory.CreateProjectContext(FSharpConstants.FSharpLanguageName, projectDisplayName, projectFileName, projectId.Id, hier, null)
633+
634+
projectId <- workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)
635+
631636
projectContext.AddSourceFile(fileName)
632637

633-
let project = projectContext :?> AbstractProject
634-
singleFileProjects.[projectId] <- project
638+
singleFileProjects.[projectId] <- projectContext
639+
640+
let _referencedProjectFileNames, parsingOptions, projectOptions = projectInfoManager.ComputeSingleFileOptions (tryGetOrCreateProjectId workspace, fileName, loadTime, fileContents) |> Async.RunSynchronously
641+
projectInfoManager.AddOrUpdateSingleFileProject(projectId, (loadTime, parsingOptions, projectOptions))
635642

636643
override this.ContentTypeName = FSharpConstants.FSharpContentTypeName
637644
override this.LanguageName = FSharpConstants.FSharpLanguageName

0 commit comments

Comments
 (0)