Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map workspace to iprojectsite #3425

Closed
wants to merge 3 commits into from
Closed

Map workspace to iprojectsite #3425

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
beb4e32
Add IProvideProjectSite mapping
KevinRansom Aug 7, 2017
6cfd52e
update language service for sdk files
KevinRansom Aug 8, 2017
c37d65b
Update language service to use: direct command line notifications.
KevinRansom Aug 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 133 additions & 50 deletions vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.VisualStudio.FSharp.Editor

#nowarn "40"

open System
open System.Collections.Concurrent
open System.Collections.Generic
open System.Collections.Immutable
open System.ComponentModel.Composition
open System.Diagnostics
open System.IO
open System.Linq
open System.Runtime.CompilerServices
open System.Runtime.InteropServices
open System.IO
open System.Diagnostics

open Microsoft.FSharp.Compiler.CompileOps
open Microsoft.FSharp.Compiler.SourceCodeServices
Expand All @@ -23,6 +24,7 @@ open Microsoft.CodeAnalysis.Options
open Microsoft.VisualStudio
open Microsoft.VisualStudio.Editor
open Microsoft.VisualStudio.TextManager.Interop
open Microsoft.VisualStudio.LanguageServices
open Microsoft.VisualStudio.LanguageServices.Implementation.LanguageService
open Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
open Microsoft.VisualStudio.LanguageServices.Implementation.TaskList
Expand Down Expand Up @@ -265,14 +267,12 @@ type

override this.CreateWorkspace() = this.ComponentModel.GetService<VisualStudioWorkspaceImpl>()

override this.CreateLanguageService() =
FSharpLanguageService(this)
override this.CreateLanguageService() = FSharpLanguageService(this)

override this.CreateEditorFactories() = Seq.empty<IVsEditorFactory>

override this.RegisterMiscellaneousFilesWorkspaceInformation(_) = ()

and
and
[<Guid(FSharpConstants.languageServiceGuidString)>]
[<ProvideLanguageExtension(typeof<FSharpLanguageService>, ".fs")>]
[<ProvideLanguageExtension(typeof<FSharpLanguageService>, ".fsi")>]
Expand Down Expand Up @@ -313,15 +313,28 @@ and

let optionsAssociation = ConditionalWeakTable<IWorkspaceProjectContext, string[]>()

member private this.OnProjectChanged(projectId:ProjectId, workspace:VisualStudioWorkspaceImpl, _newSolution:Solution) =
let project = workspace.CurrentSolution.GetProject(projectId)
let siteProvider = this.ProvideProjectSiteProvider(this.Workspace, project)
this.SetupProjectFile(siteProvider, this.Workspace, "setupProjectsAfterSolutionOpen")
member private this.OnProjectAdded(projectId:ProjectId, workspace:VisualStudioWorkspaceImpl, newSolution:Solution) = this.OnProjectChanged(projectId, workspace, newSolution)
member private this.OnProjectRemoved(_projectId:ProjectId, _workspace:VisualStudioWorkspaceImpl, _newSolution:Solution) = ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove this if it does nothing? Or is it a placeholder?


override this.Initialize() =
base.Initialize()

let workspaceChanged (args:WorkspaceChangeEventArgs) =
match args.Kind with
| WorkspaceChangeKind.ProjectAdded -> this.OnProjectAdded(args.ProjectId, this.Workspace, args.NewSolution)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Roslyn will fire ProjectAdded events for all the projects in a solution when SolutionAdded is fired, so we probably need to handle the Solution events too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pilchie weirdly we do get project added events. I want to dig into these quite a bit more though.

| WorkspaceChangeKind.ProjectChanged -> this.OnProjectChanged(args.ProjectId, this.Workspace, args.NewSolution)
| WorkspaceChangeKind.ProjectRemoved -> this.OnProjectRemoved(args.ProjectId, this.Workspace, args.NewSolution)
| _ -> ()

this.Workspace.Options <- this.Workspace.Options.WithChangedOption(Completion.CompletionOptions.BlockForCompletionItems, FSharpConstants.FSharpLanguageName, false)
this.Workspace.Options <- this.Workspace.Options.WithChangedOption(Shared.Options.ServiceFeatureOnOffOptions.ClosedFileDiagnostic, FSharpConstants.FSharpLanguageName, Nullable false)
this.Workspace.DocumentClosed.Add <| fun args -> tryRemoveSingleFileProject args.Document.Project.Id
this.Workspace.WorkspaceChanged.Add(workspaceChanged)

this.Workspace.DocumentClosed.Add <| fun args ->
tryRemoveSingleFileProject args.Document.Project.Id

Events.SolutionEvents.OnAfterCloseSolution.Add <| fun _ ->
//checkerProvider.Checker.StopBackgroundCompile()

Expand All @@ -334,7 +347,7 @@ and
singleFileProjects.Keys |> Seq.iter tryRemoveSingleFileProject

let ctx = System.Threading.SynchronizationContext.Current

let rec setupProjectsAfterSolutionOpen() =
async {
use openedProjects = MailboxProcessor.Start <| fun inbox ->
Expand All @@ -358,27 +371,27 @@ and

let theme = package.ComponentModel.DefaultExportProvider.GetExport<ISetThemeColors>().Value
theme.SetColors()

/// Sync the information for the project
member this.SyncProject(project: AbstractProject, projectContext: IWorkspaceProjectContext, site: IProjectSite, workspace, forceUpdate, userOpName) =
member __.SyncProject(project: AbstractProject, projectContext: IWorkspaceProjectContext, site: IProjectSite, workspace, forceUpdate, userOpName) =
let wellFormedFilePathSetIgnoreCase (paths: seq<string>) =
HashSet(paths |> Seq.filter isPathWellFormed |> Seq.map (fun s -> try System.IO.Path.GetFullPath(s) with _ -> s), StringComparer.OrdinalIgnoreCase)

let updatedFiles = site.SourceFilesOnDisk() |> wellFormedFilePathSetIgnoreCase
let originalFiles = project.GetCurrentDocuments() |> Seq.map (fun file -> file.FilePath) |> wellFormedFilePathSetIgnoreCase

let mutable updated = forceUpdate

for file in updatedFiles do
if not(originalFiles.Contains(file)) then
projectContext.AddSourceFile(file)
updated <- true

for file in originalFiles do
if not(updatedFiles.Contains(file)) then
projectContext.RemoveSourceFile(file)
updated <- true

let updatedRefs = site.AssemblyReferences() |> wellFormedFilePathSetIgnoreCase
let originalRefs = project.GetCurrentMetadataReferences() |> Seq.map (fun ref -> ref.FilePath) |> wellFormedFilePathSetIgnoreCase

Expand Down Expand Up @@ -417,49 +430,122 @@ and

member this.SetupProjectFile(siteProvider: IProvideProjectSite, workspace: VisualStudioWorkspaceImpl, userOpName) =
let userOpName = userOpName + ".SetupProjectFile"
let rec setup (site: IProjectSite) =
let rec setup (site: IProjectSite) =
let projectGuid = Guid(site.ProjectGuid)
let projectFileName = site.ProjectFileName()
let projectDisplayName = projectDisplayNameOf projectFileName

let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)

if isNull (workspace.ProjectTracker.GetProject projectId) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we used this line to guard against doing work multiple times for the same project as we traverse the project graph

You've moved the check down a bit, but this means that UpdateProjectInfo may now be called many times per project, along with actions such as creating ProjectExternalErrorReporter, and, most critically, the recursive calls and AddProjectReference calls here:

            for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites (site, this.SystemServiceProvider) do
                let referencedProjectId = setup referencedSite
                project.AddProjectReference(ProjectReference referencedProjectId)

I think you will want to somehow want to guard against doing these things many times as we traverse the project graph.

projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)
let projectContextFactory = package.ComponentModel.GetService<IWorkspaceProjectContextFactory>();
let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)

let hierarchy =
site.ProjectProvider
|> Option.map (fun p -> p :?> IVsHierarchy)
|> Option.toObj

// Roslyn is expecting site to be an IVsHierarchy.
// It just so happens that the object that implements IProvideProjectSite is also
// an IVsHierarchy. This assertion is to ensure that the assumption holds true.
Debug.Assert(hierarchy <> null, "About to CreateProjectContext with a non-hierarchy site")
projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace, userOpName)
let projectContextFactory = package.ComponentModel.GetService<IWorkspaceProjectContextFactory>();
let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)

let projectContext =
// Roslyn is expecting site to be an IVsHierarchy.
// It just so happens that the object that implements IProvideProjectSite is also
// an IVsHierarchy. This assertion is to ensure that the assumption holds true.
let hierarchy =
site.ProjectProvider
|> Option.map (fun p -> p :?> IVsHierarchy)
|> Option.toObj
Debug.Assert(hierarchy <> null, "About to CreateProjectContext with a non-hierarchy site")

let project =
match workspace.ProjectTracker.GetProject projectId with
| null ->
/// We make the project object when the project is not already tracked
projectContextFactory.CreateProjectContext(
FSharpConstants.FSharpLanguageName, projectDisplayName, projectFileName, projectGuid, hierarchy, null, errorReporter)

let project = projectContext :?> AbstractProject

FSharpConstants.FSharpLanguageName,
projectDisplayName,
projectFileName,
projectGuid,
hierarchy,
null,
errorReporter) :?> AbstractProject
| _ as p -> p
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can just be | p -> p


match project :> obj with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally I would use match box project with but it's the same thing either way, doesn't matter much

| :? IWorkspaceProjectContext as projectContext ->
this.SyncProject(project, projectContext, site, workspace, forceUpdate=false, userOpName=userOpName)
site.AdviseProjectSiteChanges(FSharpConstants.FSharpLanguageServiceCallbackName,
AdviseProjectSiteChanges(fun () -> this.SyncProject(project, projectContext, site, workspace, forceUpdate=true, userOpName="AdviseProjectSiteChanges."+userOpName)))
site.AdviseProjectSiteClosed(FSharpConstants.FSharpLanguageServiceCallbackName,
AdviseProjectSiteChanges(fun () ->
site.AdviseProjectSiteChanges(FSharpConstants.FSharpLanguageServiceCallbackName,
AdviseProjectSiteChanges(fun () -> this.SyncProject(project, projectContext, site, workspace, forceUpdate=true, userOpName="AdviseProjectSiteChanges."+userOpName)))
site.AdviseProjectSiteClosed(FSharpConstants.FSharpLanguageServiceCallbackName,
AdviseProjectSiteChanges(fun () ->
projectInfoManager.ClearInfoForProject(project.Id)
optionsAssociation.Remove(projectContext) |> ignore
project.Disconnect()))
for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites (site, this.SystemServiceProvider) do
let referencedProjectId = setup referencedSite
project.AddProjectReference(ProjectReference referencedProjectId)
| _ -> ()
for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites (site, this.SystemServiceProvider) do
let referencedProjectId = setup referencedSite
project.AddProjectReference(ProjectReference referencedProjectId)

projectId
setup (siteProvider.GetProjectSite()) |> ignore

member private __.ProvideProjectSiteProvider(workspace:Workspace, project:Project) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a helper function either outside this class (to prove that it is logically independent of any of the constructs accessible in the class), or as a let function in the class let section, to help show what data it depends in the initialization sequence. Perhaps it can even go in ProjectSitesAndFiles.fs where two other ways of creating IProjectSite are implemented (for scripts and standalone-files).

let visualStudioWorkspace = workspace :?> VisualStudioWorkspace
let hier = visualStudioWorkspace.GetHierarchy(project.Id)
{new IProvideProjectSite with
member this.GetProjectSite() =
let compileItems () = [| for document in project.Documents do yield document.FilePath |]
let compilerFlags () = [| "" |]
let caption () = project.Name
let projFileName () = project.FilePath
let taskProvider = None
let taskReporter = None
let targetFrameworkMoniker = ""
let projectGuid () = project.Id.Id.ToString()
let creationTime = System.DateTime.Now
let assemblyReferences () =
[|
for reference in project.ProjectReferences do
let p = workspace.CurrentSolution.GetProject(reference.ProjectId)
if p <> null then
let outputFilePath = p.OutputFilePath
if String.IsNullOrEmpty(outputFilePath) = false then yield outputFilePath
for r in project.MetadataReferences do
match r with
| :? PortableExecutableReference as per -> yield per.FilePath
| :? UnresolvedMetadataReference as umr -> yield umr.Reference
| _ -> () |]
{ new Microsoft.VisualStudio.FSharp.LanguageService.IProjectSite with
member __.SourceFilesOnDisk() = compileItems ()
member __.DescriptionOfProject() = caption ()
member __.CompilerFlags() = compilerFlags ()
member __.ProjectFileName() = projFileName ()
member __.ErrorListTaskProvider() = taskProvider
member __.ErrorListTaskReporter() = taskReporter
member __.AdviseProjectSiteChanges(_,_) = ()
member __.AdviseProjectSiteCleaned(_,_) = ()
member __.AdviseProjectSiteClosed(_,_) = ()
member __.IsIncompleteTypeCheckEnvironment = false
member __.TargetFrameworkMoniker = targetFrameworkMoniker
member __.ProjectGuid = projectGuid ()
member __.LoadTime = creationTime
member __.ProjectProvider = Some this
member __.AssemblyReferences() = assemblyReferences ()
}
interface IVsHierarchy with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is an IVsHierarchy implementation is needed here? I do vaguely remember - is it that crappy code in ProjectSitesAndFiles.fs that searches for IVsHierarchy for project references? However if that's the case, then let's document it, and let's also eventually get rid of this in favour of adding an appropriate method into IProjectSite, or even just using the existing AssemblyReferences method

member __.SetSite(psp) = hier.SetSite(psp)
member __.GetSite(psp) = hier.GetSite(ref psp)
member __.QueryClose(pfCanClose) = hier.QueryClose(ref pfCanClose)
member __.Close() = hier.Close()
member __.GetGuidProperty(itemid, propid, pguid) = hier.GetGuidProperty(itemid, propid, ref pguid)
member __.SetGuidProperty(itemid, propid, rguid) = hier.SetGuidProperty(itemid, propid, ref rguid)
member __.GetProperty(itemid, propid, pvar) = hier.GetProperty(itemid, propid, ref pvar)
member __.SetProperty(itemid, propid, var) = hier.SetProperty(itemid, propid, var)
member __.GetNestedHierarchy(itemid, iidHierarchyNested, ppHierarchyNested, pitemidNested) = hier.GetNestedHierarchy(itemid, ref iidHierarchyNested, ref ppHierarchyNested, ref pitemidNested)
member __.GetCanonicalName(itemid, pbstrName) = hier.GetCanonicalName(itemid, ref pbstrName)
member __.ParseCanonicalName(pszName, pitemid) = hier.ParseCanonicalName(pszName, ref pitemid)
member __.Unused0() = hier.Unused0()
member __.AdviseHierarchyEvents(pEventSink, pdwCookie) = hier.AdviseHierarchyEvents(pEventSink, ref pdwCookie)
member __.UnadviseHierarchyEvents(dwCookie) = hier.UnadviseHierarchyEvents(dwCookie)
member __.Unused1() = hier.Unused1()
member __.Unused2() = hier.Unused2()
member __.Unused3() = hier.Unused3()
member __.Unused4() = hier.Unused4()
}

member this.SetupStandAloneFile(fileName: string, fileContents: string, workspace: VisualStudioWorkspaceImpl, hier: IVsHierarchy) =

let loadTime = DateTime.Now
Expand All @@ -476,7 +562,7 @@ and

let projectContext = projectContextFactory.CreateProjectContext(FSharpConstants.FSharpLanguageName, projectDisplayName, projectFileName, projectId.Id, hier, null, errorReporter)
projectContext.AddSourceFile(fileName)

let project = projectContext :?> AbstractProject
singleFileProjects.[projectId] <- project

Expand All @@ -493,19 +579,16 @@ and
base.SetupNewTextView(textView)

let textViewAdapter = package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>()

match textView.GetBuffer() with
| (VSConstants.S_OK, textLines) ->
let filename = VsTextLines.GetFilename textLines
match VsRunningDocumentTable.FindDocumentWithoutLocking(package.RunningDocumentTable,filename) with
match VsRunningDocumentTable.FindDocumentWithoutLocking(package.RunningDocumentTable, filename) with
| Some (hier, _) ->
match hier with
| :? IProvideProjectSite as siteProvider when not (IsScript(filename)) ->
| :? IProvideProjectSite as siteProvider when not (IsScript(filename)) ->
this.SetupProjectFile(siteProvider, this.Workspace, "SetupNewTextView")
| _ ->
| _ ->
let fileContents = VsTextLines.GetFileContents(textLines, textViewAdapter)
this.SetupStandAloneFile(filename, fileContents, this.Workspace, hier)
| _ -> ()
| _ -> ()