diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/src/AssemblyItem.cs b/src/Razor/Microsoft.NET.Sdk.Razor/src/AssemblyItem.cs index 1f57029bf0..74b3f43d77 100644 --- a/src/Razor/Microsoft.NET.Sdk.Razor/src/AssemblyItem.cs +++ b/src/Razor/Microsoft.NET.Sdk.Razor/src/AssemblyItem.cs @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Razor.Tasks { public string Path { get; set; } - public bool IsSystemReference { get; set; } + public bool IsFrameworkReference { get; set; } public string AssemblyName { get; set; } } diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/src/FindAssembliesWithReferencesTo.cs b/src/Razor/Microsoft.NET.Sdk.Razor/src/FindAssembliesWithReferencesTo.cs index c936eb4c9b..00ab6845dc 100644 --- a/src/Razor/Microsoft.NET.Sdk.Razor/src/FindAssembliesWithReferencesTo.cs +++ b/src/Razor/Microsoft.NET.Sdk.Razor/src/FindAssembliesWithReferencesTo.cs @@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.Razor.Tasks referenceItems.Add(new AssemblyItem { AssemblyName = assemblyName, - IsSystemReference = item.GetMetadata("IsSystemReference") == "true", + IsFrameworkReference = item.GetMetadata("IsFrameworkReference") == "true", Path = item.ItemSpec, }); } diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/src/ReferenceResolver.cs b/src/Razor/Microsoft.NET.Sdk.Razor/src/ReferenceResolver.cs index 657e9432cf..8a44c9b7d1 100644 --- a/src/Razor/Microsoft.NET.Sdk.Razor/src/ReferenceResolver.cs +++ b/src/Razor/Microsoft.NET.Sdk.Razor/src/ReferenceResolver.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; +using System.Linq; using System.Reflection.Metadata; using System.Reflection.PortableExecutable; @@ -15,155 +17,141 @@ namespace Microsoft.AspNetCore.Razor.Tasks public class ReferenceResolver { private readonly HashSet _mvcAssemblies; - private readonly Dictionary _lookup = new Dictionary(StringComparer.Ordinal); + private readonly IReadOnlyList _assemblyItems; + private readonly Dictionary _classifications; public ReferenceResolver(IReadOnlyList targetAssemblies, IReadOnlyList assemblyItems) { _mvcAssemblies = new HashSet(targetAssemblies, StringComparer.Ordinal); + _assemblyItems = assemblyItems; + _classifications = new Dictionary(); + Lookup = new Dictionary(StringComparer.Ordinal); foreach (var item in assemblyItems) { - var classifiedItem = new ClassifiedAssemblyItem(item); - _lookup[item.AssemblyName] = classifiedItem; + Lookup[item.AssemblyName] = item; } } + protected Dictionary Lookup { get; } + public IReadOnlyList ResolveAssemblies() { var applicationParts = new List(); - foreach (var item in _lookup) - { - var classification = Resolve(item.Value); - if (classification == DependencyClassification.ReferencesMvc) - { - applicationParts.Add(item.Key); - } - // It's not interesting for us to know if a dependency has a classification of MvcReference. - // All applications targeting the Microsoft.AspNetCore.App will have have a reference to Mvc. + foreach (var item in _assemblyItems) + { + var classification = Resolve(item); + if (classification == Classification.ReferencesMvc) + { + applicationParts.Add(item.AssemblyName); + } } return applicationParts; } - private DependencyClassification Resolve(ClassifiedAssemblyItem classifiedItem) + private Classification Resolve(AssemblyItem assemblyItem) { - if (classifiedItem.DependencyClassification != DependencyClassification.Unknown) + if (_classifications.TryGetValue(assemblyItem, out var classification)) { - return classifiedItem.DependencyClassification; + return classification; } - if (classifiedItem.AssemblyItem == null) + // Initialize the dictionary with a value to short-circuit recursive references. + classification = Classification.Unknown; + _classifications[assemblyItem] = classification; + + if (assemblyItem.Path == null) { - // We encountered a dependency that isn't part of this assembly's dependency set. We'll see if it happens to be an MVC assembly. - // This might be useful in scenarios where the app does not have a framework reference at the entry point, - // but the transitive dependency does. - classifiedItem.DependencyClassification = _mvcAssemblies.Contains(classifiedItem.Name) ? - DependencyClassification.MvcReference : - DependencyClassification.DoesNotReferenceMvc; - - return classifiedItem.DependencyClassification; + // We encountered a dependency that isn't part of this assembly's dependency set. We'll see if it happens to be an MVC assembly + // since that's the only useful determination we can make given the assembly name. + classification = _mvcAssemblies.Contains(assemblyItem.AssemblyName) ? + Classification.IsMvc : + Classification.DoesNotReferenceMvc; } - - if (classifiedItem.AssemblyItem.IsSystemReference) + else if (assemblyItem.IsFrameworkReference) { // We do not allow transitive references to MVC via a framework reference to count. // e.g. depending on Microsoft.AspNetCore.SomeThingNewThatDependsOnMvc would not result in an assembly being treated as // referencing MVC. - classifiedItem.DependencyClassification = _mvcAssemblies.Contains(classifiedItem.Name) ? - DependencyClassification.MvcReference : - DependencyClassification.DoesNotReferenceMvc; - - return classifiedItem.DependencyClassification; + classification = _mvcAssemblies.Contains(assemblyItem.AssemblyName) ? + Classification.IsMvc : + Classification.DoesNotReferenceMvc; } - - if (_mvcAssemblies.Contains(classifiedItem.Name)) + else if (_mvcAssemblies.Contains(assemblyItem.AssemblyName)) { - classifiedItem.DependencyClassification = DependencyClassification.MvcReference; - return classifiedItem.DependencyClassification; + classification = Classification.IsMvc; } - - var dependencyClassification = DependencyClassification.DoesNotReferenceMvc; - foreach (var assemblyItem in GetReferences(classifiedItem.AssemblyItem.Path)) + else { - var classification = Resolve(assemblyItem); - if (classification == DependencyClassification.MvcReference || classification == DependencyClassification.ReferencesMvc) + classification = Classification.DoesNotReferenceMvc; + foreach (var reference in GetReferences(assemblyItem.Path)) { - dependencyClassification = DependencyClassification.ReferencesMvc; - break; + var referenceClassification = Resolve(reference); + + if (referenceClassification == Classification.IsMvc || referenceClassification == Classification.ReferencesMvc) + { + classification = Classification.ReferencesMvc; + break; + } } } - classifiedItem.DependencyClassification = dependencyClassification; - return dependencyClassification; + Debug.Assert(classification != Classification.Unknown); + _classifications[assemblyItem] = classification; + return classification; } - protected virtual IReadOnlyList GetReferences(string file) + protected virtual IReadOnlyList GetReferences(string file) { try { using var peReader = new PEReader(File.OpenRead(file)); if (!peReader.HasMetadata) { - return Array.Empty(); // not a managed assembly + return Array.Empty(); // not a managed assembly } var metadataReader = peReader.GetMetadataReader(); - var assemblyItems = new List(); + var references = new List(); foreach (var handle in metadataReader.AssemblyReferences) { var reference = metadataReader.GetAssemblyReference(handle); var referenceName = metadataReader.GetString(reference.Name); - if (_lookup.TryGetValue(referenceName, out var classifiedItem)) - { - assemblyItems.Add(classifiedItem); - } - else + if (!Lookup.TryGetValue(referenceName, out var assemblyItem)) { // A dependency references an item that isn't referenced by this project. // We'll construct an item for so that we can calculate the classification based on it's name. - assemblyItems.Add(new ClassifiedAssemblyItem(referenceName)); + assemblyItem = new AssemblyItem + { + AssemblyName = referenceName, + }; + + Lookup[referenceName] = assemblyItem; } + + references.Add(assemblyItem); } - return assemblyItems; + return references; } catch (BadImageFormatException) { // not a PE file, or invalid metadata } - return Array.Empty(); // not a managed assembly + return Array.Empty(); // not a managed assembly } - protected enum DependencyClassification + protected enum Classification { Unknown, DoesNotReferenceMvc, ReferencesMvc, - MvcReference, - } - - protected class ClassifiedAssemblyItem - { - public ClassifiedAssemblyItem(AssemblyItem classifiedItem) - : this(classifiedItem.AssemblyName) - { - AssemblyItem = classifiedItem; - } - - public ClassifiedAssemblyItem(string name) - { - Name = name; - } - - public string Name { get; } - - public AssemblyItem AssemblyItem { get; } - - public DependencyClassification DependencyClassification { get; set; } + IsMvc, } } } diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/test/ReferenceResolverTest.cs b/src/Razor/Microsoft.NET.Sdk.Razor/test/ReferenceResolverTest.cs index 7a24101947..05dd86b430 100644 --- a/src/Razor/Microsoft.NET.Sdk.Razor/test/ReferenceResolverTest.cs +++ b/src/Razor/Microsoft.NET.Sdk.Razor/test/ReferenceResolverTest.cs @@ -59,13 +59,13 @@ namespace Microsoft.AspNetCore.Razor.Tasks var resolver = new TestReferencesToMvcResolver(new[] { CreateAssemblyItem("MyApp.Models"), - CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isSystemReference: true), - CreateAssemblyItem("Microsoft.Extensions.Primitives", isSystemReference: true), - CreateAssemblyItem("System.Net.Http", isSystemReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.Extensions.Primitives", isFrameworkReference: true), + CreateAssemblyItem("System.Net.Http", isFrameworkReference: true), CreateAssemblyItem("Microsoft.EntityFrameworkCore"), }); @@ -89,16 +89,16 @@ namespace Microsoft.AspNetCore.Razor.Tasks // Arrange var resolver = new TestReferencesToMvcResolver(new[] { - CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.Mvc.TagHelpers", isSystemReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc.TagHelpers", isFrameworkReference: true), CreateAssemblyItem("MyTagHelpers"), CreateAssemblyItem("MyControllers"), CreateAssemblyItem("MyApp.Models"), - CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isSystemReference: true), - CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isSystemReference: true), - CreateAssemblyItem("Microsoft.Extensions.Primitives", isSystemReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isFrameworkReference: true), + CreateAssemblyItem("Microsoft.Extensions.Primitives", isFrameworkReference: true), CreateAssemblyItem("Microsoft.EntityFrameworkCore"), }); @@ -126,13 +126,12 @@ namespace Microsoft.AspNetCore.Razor.Tasks { CreateAssemblyItem("MyCMS"), CreateAssemblyItem("MyCMS.Core"), - CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isSystemReference: true), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true), }); resolver.Add("MyCMS", "MyCMS.Core"); resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures"); - // Act var assemblies = resolver.ResolveAssemblies(); @@ -140,41 +139,93 @@ namespace Microsoft.AspNetCore.Razor.Tasks Assert.Equal(new[] { "MyCMS", "MyCMS.Core" }, assemblies.OrderBy(a => a)); } - public AssemblyItem CreateAssemblyItem(string name, bool isSystemReference = false) + [Fact] + public void Resolve_Works_WhenAssemblyReferencesAreRecursive() + { + // Test for https://github.com/aspnet/AspNetCore/issues/12693 + // Arrange + var resolver = new TestReferencesToMvcResolver(new[] + { + CreateAssemblyItem("PresentationFramework"), + CreateAssemblyItem("ReachFramework"), + CreateAssemblyItem("MyCMS"), + CreateAssemblyItem("MyCMS.Core"), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true), + }); + + resolver.Add("PresentationFramework", "ReachFramework"); + resolver.Add("ReachFramework", "PresentationFramework"); + + resolver.Add("MyCMS", "MyCMS.Core"); + resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures"); + + // Act + var assemblies = resolver.ResolveAssemblies(); + + // Assert + Assert.Equal(new[] { "MyCMS", "MyCMS.Core" }, assemblies.OrderBy(a => a)); + } + + [Fact] + public void Resolve_Works_WhenAssemblyReferencesAreRecursive_ButAlsoReferencesMvc() + { + // Arrange + var resolver = new TestReferencesToMvcResolver(new[] + { + CreateAssemblyItem("MyCoolLibrary"), + CreateAssemblyItem("PresentationFramework"), + CreateAssemblyItem("ReachFramework"), + CreateAssemblyItem("MyCMS"), + CreateAssemblyItem("MyCMS.Core"), + CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true), + }); + + resolver.Add("MyCoolLibrary", "PresentationFramework"); + resolver.Add("PresentationFramework", "ReachFramework"); + resolver.Add("ReachFramework", "PresentationFramework", "MyCMS"); + + resolver.Add("MyCMS", "MyCMS.Core"); + resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures"); + + // Act + var assemblies = resolver.ResolveAssemblies(); + + // Assert + Assert.Equal(new[] { "MyCMS", "MyCMS.Core", "MyCoolLibrary", "PresentationFramework", "ReachFramework" }, assemblies.OrderBy(a => a)); + } + + public AssemblyItem CreateAssemblyItem(string name, bool isFrameworkReference = false) { return new AssemblyItem { AssemblyName = name, - IsSystemReference = isSystemReference, + IsFrameworkReference = isFrameworkReference, Path = name, }; } private class TestReferencesToMvcResolver : ReferenceResolver { - private readonly Dictionary> _references = new Dictionary>(); - private readonly Dictionary _lookup; + private readonly Dictionary _references = new Dictionary(); public TestReferencesToMvcResolver(AssemblyItem[] referenceItems) : base(MvcAssemblies, referenceItems) { - _lookup = referenceItems.ToDictionary(r => r.AssemblyName, r => new ClassifiedAssemblyItem(r)); } public void Add(string assembly, params string[] references) { - var assemblyItems = references.Select(r => _lookup[r]).ToList(); - _references[assembly] = assemblyItems; + _references.Add(assembly, references); } - protected override IReadOnlyList GetReferences(string file) + protected override IReadOnlyList GetReferences(string file) { if (_references.TryGetValue(file, out var result)) { - return result; + return result.Select(r => Lookup[r]).ToArray(); } - return Array.Empty(); + return Array.Empty(); } } }