Skip to content

Stop calling GetHierarchy in FSharpProjectOptionsReactor #6946

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

Merged
TIHan merged 2 commits into from
Jun 6, 2019
Merged

Stop calling GetHierarchy in FSharpProjectOptionsReactor #6946

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbe91c4
Stop calling GetHierarchy in FSharpProjectOptionsReactor
TIHan Jun 5, 2019
116c1f7
Fixing build
TIHan Jun 6, 2019
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
Next Next commit
Stop calling GetHierarchy in FSharpProjectOptionsReactor
  • Loading branch information
@TIHan
TIHan committed Jun 5, 2019
commit dbe91c4de7feb5ee16addff0a612de63d2e34322
101 changes: 43 additions & 58 deletions vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,58 +24,30 @@ open Microsoft.CodeAnalysis.ExternalAccess.FSharp.LanguageServices
[<AutoOpen>]
module private FSharpProjectOptionsHelpers =

let mapCpsProjectToSite(workspace:VisualStudioWorkspace, project:Project, serviceProvider:System.IServiceProvider, cpsCommandLineOptions: IDictionary<ProjectId, string[] * string[]>) =
let hier = workspace.GetHierarchy(project.Id)
let mapCpsProjectToSite(project:Project, cpsCommandLineOptions: IDictionary<ProjectId, string[] * string[]>) =
let sourcePaths, referencePaths, options =
match cpsCommandLineOptions.TryGetValue(project.Id) with
| true, (sourcePaths, options) -> sourcePaths, [||], options
| false, _ -> [||], [||], [||]
let mutable errorReporter = Unchecked.defaultof<_>
{
new IProvideProjectSite with
member x.GetProjectSite() =
let mutable errorReporter =
let reporter = ProjectExternalErrorReporter(project.Id, "FS", serviceProvider)
Some(reporter:> IVsLanguageServiceBuildErrorReporter2)

{
new IProjectSite with
member __.Description = project.Name
member __.CompilationSourceFiles = sourcePaths
member __.CompilationOptions =
Array.concat [options; referencePaths |> Array.map(fun r -> "-r:" + r)]
member __.CompilationReferences = referencePaths
member site.CompilationBinOutputPath = site.CompilationOptions |> Array.tryPick (fun s -> if s.StartsWith("-o:") then Some s.[3..] else None)
member __.ProjectFileName = project.FilePath
member __.AdviseProjectSiteChanges(_,_) = ()
member __.AdviseProjectSiteCleaned(_,_) = ()
member __.AdviseProjectSiteClosed(_,_) = ()
member __.IsIncompleteTypeCheckEnvironment = false
member __.TargetFrameworkMoniker = ""
member __.ProjectGuid = project.Id.Id.ToString()
member __.LoadTime = System.DateTime.Now
member __.ProjectProvider = Some (x)
member __.BuildErrorReporter with get () = errorReporter and set (v) = errorReporter <- v
}
interface IVsHierarchy with
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()
new IProjectSite with
member __.Description = project.Name
member __.CompilationSourceFiles = sourcePaths
member __.CompilationOptions =
Array.concat [options; referencePaths |> Array.map(fun r -> "-r:" + r)]
member __.CompilationReferences = referencePaths
member site.CompilationBinOutputPath = site.CompilationOptions |> Array.tryPick (fun s -> if s.StartsWith("-o:") then Some s.[3..] else None)
member __.ProjectFileName = project.FilePath
member __.AdviseProjectSiteChanges(_,_) = ()
member __.AdviseProjectSiteCleaned(_,_) = ()
member __.AdviseProjectSiteClosed(_,_) = ()
member __.IsIncompleteTypeCheckEnvironment = false
member __.TargetFrameworkMoniker = ""
member __.ProjectGuid = project.Id.Id.ToString()
member __.LoadTime = System.DateTime.Now
member __.ProjectProvider = None
member __.BuildErrorReporter with get () = errorReporter and set (v) = errorReporter <- v
}

let hasProjectVersionChanged (oldProject: Project) (newProject: Project) =
Expand Down Expand Up @@ -108,11 +80,13 @@ type private FSharpProjectOptionsMessage =
| ClearSingleFileOptionsCache of DocumentId

[<Sealed>]
type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, settings: EditorOptions, serviceProvider, checkerProvider: FSharpCheckerProvider) =
type private FSharpProjectOptionsReactor (_workspace: VisualStudioWorkspace, settings: EditorOptions, _serviceProvider, checkerProvider: FSharpCheckerProvider) =
Copy link
Contributor

@cartermp cartermp Jun 6, 2019

Choose a reason for hiding this comment

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

Are these unused values needed as parameters given the _?

Copy link
Contributor Author

@TIHan TIHan Jun 6, 2019

Choose a reason for hiding this comment

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

They are unused for now, they will be used when I start doing more refactoring here.

Copy link
Contributor

@cartermp cartermp Jun 6, 2019

Choose a reason for hiding this comment

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

👍

let cancellationTokenSource = new CancellationTokenSource()

// Hack to store command line options from HandleCommandLineChanges
let cpsCommandLineOptions = new ConcurrentDictionary<ProjectId, string[] * string[]>()
let cpsCommandLineOptions = ConcurrentDictionary<ProjectId, string[] * string[]>()

let legacyProjectSites = ConcurrentDictionary<ProjectId, IProjectSite>()

let cache = Dictionary<ProjectId, Project * FSharpParsingOptions * FSharpProjectOptions>()
let singleFileCache = Dictionary<DocumentId, VersionStamp * FSharpParsingOptions * FSharpProjectOptions>()
Expand Down Expand Up @@ -158,6 +132,16 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
else
return Some(parsingOptions, projectOptions)
}

let tryGetProjectSite (project: Project) =
// Cps
if cpsCommandLineOptions.ContainsKey project.Id then
Some (mapCpsProjectToSite(project, cpsCommandLineOptions))
else
// Legacy
match legacyProjectSites.TryGetValue project.Id with
| true, site -> Some site
| _ -> None

let rec tryComputeOptions (project: Project) =
async {
Expand All @@ -183,15 +167,9 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
return None
else

let hier = workspace.GetHierarchy(projectId)
let projectSite =
match hier with
// Legacy
| (:? IProvideProjectSite as provideSite) -> provideSite.GetProjectSite()
// Cps
| _ ->
let provideSite = mapCpsProjectToSite(workspace, project, serviceProvider, cpsCommandLineOptions)
provideSite.GetProjectSite()
match tryGetProjectSite project with
| None -> return None
| Some projectSite ->

let otherOptions =
project.ProjectReferences
Expand Down Expand Up @@ -283,6 +261,7 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett

| FSharpProjectOptionsMessage.ClearOptions(projectId) ->
cache.Remove(projectId) |> ignore
legacyProjectSites.TryRemove(projectId) |> ignore
| FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) ->
singleFileCache.Remove(documentId) |> ignore
}
Expand All @@ -304,6 +283,9 @@ type private FSharpProjectOptionsReactor (workspace: VisualStudioWorkspace, sett
member __.SetCpsCommandLineOptions(projectId, sourcePaths, options) =
cpsCommandLineOptions.[projectId] <- (sourcePaths, options)

member __.SetLegacyProjectSite (projectId, projectSite) =
legacyProjectSites.[projectId] <- projectSite

member __.TryGetCachedOptionsByProjectId(projectId) =
match cache.TryGetValue(projectId) with
| true, result -> Some(result)
Expand Down Expand Up @@ -344,6 +326,9 @@ type internal FSharpProjectOptionsManager
| _ -> ()
)

member __.SetLegacyProjectSite (projectId, projectSite) =
reactor.SetLegacyProjectSite (projectId, projectSite)

/// Clear a project from the project table
member this.ClearInfoForProject(projectId:ProjectId) =
reactor.ClearOptionsByProjectId(projectId)
Expand Down
2 changes: 2 additions & 0 deletions vsintegration/src/FSharp.Editor/LanguageService/LegacyProjectWorkspaceMap.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ type internal LegacyProjectWorkspaceMap(solution: IVsSolution,

let projectId = projectContext.Id

projectInfoManager.SetLegacyProjectSite (projectId, site)

// Sync the source files in projectContext. Note that these source files are __not__ maintained in order in projectContext
// as edits are made. It seems this is ok because the source file list is only used to drive roslyn per-file checking.
let updatedFiles = site.CompilationSourceFiles |> wellFormedFilePathSetIgnoreCase
Expand Down