Design new interface between Project System and Roslyn

[tmat]

WIP Proposal

Project System:

// MEF interface: exported by language services, imported by Project System
public interface IProjectChangeListener
{   
   void EvaluationCompleted(ProjectSnapshotId project, ProjectEvaluationResults results); 

   // Triggered on any change to the build inputs, including change in a source file, metadata 
   // reference, etc.
   void BuildCompleted(ProjectSnapshotId project, ProjectBuildKind kind, ProjectBuildResults results);
}


// Uniquely identifies project and its complete state --
// all the inputs to the build (the content of files, references, etc.).
// For now, it can be a full file path to the proj file. In future we would add a snapshot checksum.
public readonly struct ProjectSnapshotId
{
   string ProjectPath { get; }
   Checksum Checksum { get; }
}

public enum ProjectBuildKind 
{
  DesignTime,
  Full
}

public readonly struct ProjectEvaluationResults
{
   // Values of properties and items that the class exporting IProjectChangeListener asked for via export metadata.
   bool TryGetPropertyValue(string name, out string value);
   bool TryGetItems(string name, out ImmutableArray<ProjectBuildItemValue> values);
}

public readonly struct ProjectBuildItemValue
{
  bool TryGetMetadata(string name, out string value);
}

public readonly struct ProjectBuildResults 
{
  // Complete command line as returned by the compiler build task (e.g. csc).
  string CompilerCommandLine { get; }
  
  // Paths listed in the command line that correspond to P2P references.
  ImmutableArray<string> ProjectReferences { get; }

  // Values of properties and items that the class exporting IProjectChangeListener asked 
  // for via export metadata.
  bool TryGetPropertyValue(string name, out string value);
  bool TryGetItems(string name, out ImmutableArray<ProjectBuildItemValue> values);
}

// Design-time build phases
public enum ProjectBuildPhase
{
  Evaluation,
  Build,
}

// MEF metadata describing what properties does IProjectChangeListener implementation read.
public ReadsProjectBuildProperty : Attribute
{
  string Name { get; set; }

  // After what phase is this property read.
  ProjectBuildPhase { get; set; }
}

// MEF metadata describing what items does IProjectChangeListener implementation read.
public ReadsProjectBuildItem : Attribute
{
  string Name { get; set; }

  // After what phase is this property read.
  ProjectBuildPhase { get; set; }
}

Roslyn:

[Export(typeof(IProjectChangeListener))]
[ReadsProjectBuildProperty("Foo", ProjectBuildPhase.Evaluation)]
[ReadsProjectBuildItem("Bar", ProjectBuildPhase.Build)]
internal sealed class ProjectChangeListener : IProjectChangeListener
{
   void EvaluationCompleted(ProjectSnapshotId project, ProjectEvaluationResults results)
   {
     // apply changes to the project
   }

   void DesignTimeBuildCompleted(ProjectSnapshotId project, ProjectBuildResults results)
   {
     // apply changes to the project
   }

   void FileContentChanged(ProjectSnapshotId project, string changedFilePath) 
   {
     // ...
   }
}

[tmat]

@panopticoncentral Looks reasonable?

[tmat]

We concluded that the interface proposed above is conceptually very close to IWorkspaceProjectContext. Instead of a new interface we just need to make minor changes to the context and improve the current implementation.

1. IWorkspaceProjectContextFactory.CreateProjectContext currently passes IVsHierarchy object. It needs to be removed.
2. There is a thread affinity that needs to removed
3. We need an indication that a design time build has been completed. LastDesignTimeBuildSucceeded doesn't provide the right info. This is being worked on.
4. void AddProjectReference(IWorkspaceProjectContext project, MetadataReferenceProperties properties)
void RemoveProjectReference(IWorkspaceProjectContext project) are not remotable, can we remove them?

We had a deep discussion on versions and checksums. The insight is that versions are necessary to facilitate optimizations within CPS. System that is fully based on checksums would be ideal and theoretically possible, but in practice might need too much throughput that we can't provide. Consider a system where the VS client starts by checking out a repo at certain commit sha. Then the cloud service calculates the view model based on that state and presents the view in the UI to the user. Every time user makes a change it results to new commit sha and the service calculates the new state of the UI for that commit sha and renders it. In such system the versions are not needed because we have a deterministic function of commit sha to UI state and an ordering of the commits managed by the client.

However if the user makes new commits faster than the service is able to produce results (even when caching and minimal diff based calculations are considered) we would not make any progress. That's what splitting the system into multiple date sources that each produce versioned outputs that can be aggregated together addresses. Some outputs are available faster than others and can produce UI updates sooner.

We concluded that CPS needs the version system, but we can encapsulate it and don't need versions to be used for everything in the entire cloud service.

Exposing evaluation results and design time build results separately as the original proposal above indicates would need to merge the results of these computations based on the versions. IWorkspaceProjectContext already aggregates the data coming from eval and DTB.

We need a guarantee that F5 blocks launching until DTB of all projects in the cone of startup project is complete and Roslyn notified.

[tmat]

@davkean @heejaechang Does the above comment summarize what we discussed or am I missing anything?

[jasonmalinowski]

@tmat: regarding API, you shouldn't be looking at IWorkspaceProjectContext as an example of the API; look at VisualStudioProject. That's the API that is intended to go public and cleans up a bunch of warts. IWorkspaceProjectContext is just a hacked up shim forwarding to the better code at this point. For example, the AddProjectReference/RemoveProjectReference switches to taking a ProjectId which is trivially remotable.

IWorkspaceProjectContextFactory.CreateProjectContext currently passes IVsHierarchy object. It needs to be removed.

The only reason that code takes an IVsHierarchy is to:

1. Provide a return value from VisualStudioWorkspace.GetHierarchy, and
2. Let us line up entries in the running document table to projects which is necessary for driving the project context dropdown in the editor.

When I rewrote all of this I was very careful that when that hierarchy comes in it goes straight to the VisualStudioWorkspace to store it, and nothing in the actual implementation of the project stuff itself touches (or even holds) the IVsHierarchy. As much as I'd love to not touch IVsHierarchy at all, other things in VS would need to change before we can remove it from the VS layer. An appropriate refactoring can of course let us move things further down.

[tmat]

look at VisualStudioProject. That's the API that is intended to go public and cleans up a bunch of warts.

I don't think this is compatible with what we are trying to do here. VisualStudioProject can't be made public. It actually needs to not exist eventually.

As much as I'd love to not touch IVsHierarchy at all, other things in VS would need to change before we can remove it from the VS layer.

Can you file issues on what needs to be changed?

[jasonmalinowski]

I don't think this is compatible with what we are trying to do here. VisualStudioProject can't be made public. It actually needs to not exist eventually.

Almost nothing of VisualStudioProject is actually tied to VS; if you want to refactor some of it that's easy for us to do. We just didn't have an ask; now we have one.

Can you file issues on what needs to be changed?

It'd be bugs on at least six or seven different teams. We should track it somewhere....I'm not sure a bug per se is going to cut it though. Is there some other all-up tracking mechanism for this?

[tmat]

I'd start by filing the bugs in Roslyn repo, so we can track them here (tag with label Concept-OOP). Then we can follow up with each team and link bugs filed in their repos to Roslyn issues.