[DotNetCore] Support transitive project references #2753
Conversation
The existing GetReferencedAssemblyProjects returns the DotNetProjects that are directly referenced without the alias information. The type system service also requires the associated aliases for the referenced project and so previously was determining them separately based on the directly referenced projects. This prevents the DotNetProject from returning transitively referenced projects from the GetReferencedAssemblyProjects method. So a new method has been added GetReferencedAssemblyProjectAliases which returns the referenced projects and any associated aliases. This will allow the DotNetProject to return transitively referenced projects and the type system can just use this information without having to lookup the aliases separately and missing projects that are not directly referenced.
The type system now gets the referenced projects and associated aliases from the DotNetProject instead of determine the aliases itself from the DotNetProject's Reference collection. This allows the DotNetProject to return transitively referenced projects if supported by the project extension.
Fixed bug #57990 - Transitively referenced SDK projects do not show up in IntelliSense https://bugzilla.xamarin.com/show_bug.cgi?id=57990 With three .NET Core projects: LibC references LibB references LibA The types defined by LibA were not available to LibC since it was not directly referencing LibA. .NET Core projects support transitive project references so the types defined in LibA should be available to LibC.
| void GetProjectAliases (HashSet<string> traversedProjects, List<DotNetProjectAliases> projectAliases, ConfigurationSelector configuration) | ||
| { | ||
| foreach (var projectAlias in base.OnGetReferencedAssemblyProjectAliases (configuration)) { | ||
| if (traversedProjects.Contains (projectAlias.Project.ItemId)) |
There was a problem hiding this comment.
if (!traversedProjects.Add())?
|
In PR #2445 @mhutch is introducing new API for getting all information about references. AssemblyReference will have an Aliases property. In fact, the list of references is obtained from an msbuild target, so maybe for .net core projects it will take into account transitive references, which would make this PR unnecessary. It should be verified. |
|
FWIW, I did some more testing on my PR and found a blocking bug in the dotnet sdk: dotnet/sdk#1393 |
DotNetProject now has a GetReferences method that returns project and assembly references. The assemblies are returned from the existing GetReferencedAssemblies method and the project references are taken from the project. In the future both project and assembly references will be taken from MSBuild. The GetReferences and associated project extension methods allows the .NET Core project extension to provide transitive project references. In the future this will be information will be available from MSBuild.
GetReferences is a new method that returns all reference information including project references.
|
OK I have updated the code to use the better API from the unify project/assembly references pull request PR #2445. The changes included from the other PR were the new DotNetProject.GetReferences method and the extra properties on the AssemblyReference. Also added some tests. |
|
Some of that code looks suspiciously similar to mine :) |
|
|
||
| public Task<List<AssemblyReference>> GetReferences (ConfigurationSelector configuration, CancellationToken token) | ||
| { | ||
| return ProjectExtension.OnGetReferences (configuration, token); |
There was a problem hiding this comment.
For correctness you should use BindTask here too. The token provided to OnGetReferences would be a combination of the token parameter and the token provided by BindTask, created with CancellationTokenSource.CreateLinkedTokenSource ()
| var traversedProjects = new HashSet<string> (); | ||
| traversedProjects.Add (Project.ItemId); | ||
|
|
||
| await GetProjectAssemblyReferences (traversedProjects, references, configuration, token); |
There was a problem hiding this comment.
Isn't OnGetReferences() supposed to return all references, not only project references?
| public AssemblyReference (FilePath path, string aliases = null) | ||
| { | ||
| FilePath = path; | ||
| Aliases = aliases ?? ""; | ||
| metadata = new Dictionary<string, string> { { "Aliases", aliases } }; |
There was a problem hiding this comment.
For consistency with other API, I'd like metadata to be made available through a Metadata property of type IReadOnlyPropertySet, like here https://github.com/mono/monodevelop/blob/master/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/TargetEvaluationResult.cs#L58.
This property set can be created like this: https://github.com/mono/monodevelop/blob/master/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/Project.cs#L1287.
Metadata is now available from a IReadOnlyPropertySet Metadata property to make it more consistent with other project model classes.
The GetReferences method that took a cancellation token was not using a BindTask. The GetPackageDependencies method was not combining the BindTask cancellation token with the cancellation token passed to its method.
The DotNetProject's GetReferences should return project references as well as other assembly references. The DotNetCoreProjectExtension was only returning project references.
|
|
||
| bool MetadataIsTrue (string name) | ||
| { | ||
| return string.Equals (GetMetadata (name), "true", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
MSBuildPropertyGroupEvaluated already implements boolean conversion logic, so this would be better: return metadata.GetValue(name, false);
| } | ||
|
|
||
| public FilePath FilePath { get; private set; } | ||
| public string Aliases { get; private set; } | ||
| public string Aliases => GetMetadata ("Aliases") ?? ""; |
There was a problem hiding this comment.
better use of api: metadata.GetValue ("Aliases","")
Use the IReadOnlyPropertySet's GetValue methods in more places.
Fixed bug #57990 - Transitively referenced SDK projects do not show up
in IntelliSense
https://bugzilla.xamarin.com/show_bug.cgi?id=57990
With three .NET Core projects: LibC references LibB references LibA
The types defined by LibA were not available to LibC since it was not
directly referencing LibA. .NET Core projects support transitive
project references so the types defined in LibA should be available to
LibC.
Remove code duplication in DotNetProject. New method has code similar to the existing GetReferencedAssemblyProject.Tests to be added for type system changes when finding project referencesCannot see a simple way to test this.