Include the name of the conflicting assembly as a part of warning MSB…

…3277 (dotnet#2379)

* Modify ResolveAssemblyReference to include assembly name in the message

* Modify tests that trigger MSB3277 to verify the message

* Promote check outside of inner loop

* Add new test to explicitly test the message in MSB3277

* Add unit test to ensure multiple conflicts are correctly reported

src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs

@@ -3538,9 +3538,11 @@ public void ConflictBetweenNonCopyLocalDependencies()
 
             Execute(t);
 
-            Assert.Equal(2, t.ResolvedDependencyFiles.Length);
-            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_DDllPath)); // "Expected to find assembly, but didn't."
-            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath)); // "Expected to find assembly, but didn't."
+            Assert.Equal(3, t.ResolvedDependencyFiles.Length);
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_DDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_GDllPath));
+
             Assert.Equal(1, t.SuggestedRedirects.Length);
             Assert.True(ContainsItem(t.SuggestedRedirects, @"D, Culture=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa")); // "Expected to find suggested redirect, but didn't"
             Assert.Equal(1, e.Warnings); // "Should only be one warning for suggested redirects."
@@ -3770,6 +3772,7 @@ public void ConflictWithBackVersionPrimary()
             bool result = Execute(t);
 
             Assert.Equal(1, e.Warnings); // @"Expected one warning."
+            e.AssertLogContainsMessageFromResource(AssemblyResources.GetString, "ResolveAssemblyReference.FoundConflicts", "D");
 
             Assert.Equal(0, t.SuggestedRedirects.Length);
             Assert.Equal(3, t.ResolvedFiles.Length);
@@ -3808,12 +3811,98 @@ public void ConflictWithBackVersionPrimaryWithAutoUnify()
             bool result = Execute(t);
 
             Assert.Equal(1, e.Warnings); // @"Expected one warning."
+            e.AssertLogContainsMessageFromResource(AssemblyResources.GetString, "ResolveAssemblyReference.FoundConflicts", "D");
 
             Assert.Equal(0, t.SuggestedRedirects.Length);
             Assert.Equal(3, t.ResolvedFiles.Length);
             Assert.True(ContainsItem(t.ResolvedFiles, s_myLibraries_V1_DDllPath)); // "Expected to find assembly, but didn't."
         }
 
+        /// <summary>
+        /// Consider this dependency chain:
+        ///
+        /// App
+        ///   References - B
+        ///        Depends on D version 2
+        ///   References - D, version 1
+        ///
+        /// Both D1 and D2 are CopyLocal. This is a warning because D1 is a lower version
+        /// than both D2 so that can't unify. These means that eventually when 
+        /// they're copied to the output directory they'll conflict.
+        /// </summary>
+        [Fact]
+        public void ConflictGeneratesMessageReferencingAssemblyName()
+        {
+            ResolveAssemblyReference t = new ResolveAssemblyReference();
+
+            MockEngine e = new MockEngine();
+            t.BuildEngine = e;
+
+            t.Assemblies = new ITaskItem[]
+            {
+                new TaskItem("B"),
+                new TaskItem("D, Version=1.0.0.0, Culture=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa")
+            };
+
+            t.SearchPaths = new string[]
+            {
+                s_myLibrariesRootPath, s_myLibraries_V2Path, s_myLibraries_V1Path
+            };
+
+            t.TargetFrameworkDirectories = new string[] { s_myVersion20Path };
+
+            bool result = Execute(t);
+
+            Assert.Equal(1, e.Warnings); // @"Expected one warning."
+
+            // Check that we have a message identifying conflicts with "D"
+            e.AssertLogContainsMessageFromResource(AssemblyResources.GetString, "ResolveAssemblyReference.FoundConflicts", "D");
+        }
+
+
+        /// <summary>
+        /// Consider this dependency chain:
+        ///
+        /// App
+        ///   References - B
+        ///        Depends on D version 2
+        ///        Depends on G, version 2
+        ///   References - D, version 1
+        ///   References - G, version 1
+        ///
+        /// All of Dv1, Dv2, Gv1 and Gv2 are CopyLocal. We should get two conflict warnings, one for D and one for G.
+        /// </summary>
+        [Fact]
+        public void ConflictGeneratesMessageReferencingEachConflictingAssemblyName()
+        {
+            ResolveAssemblyReference t = new ResolveAssemblyReference();
+
+            MockEngine e = new MockEngine();
+            t.BuildEngine = e;
+
+            t.Assemblies = new ITaskItem[]
+            {
+                new TaskItem("B"),
+                new TaskItem("D, Version=1.0.0.0, Culture=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa"),
+                new TaskItem("G, Version=1.0.0.0, Culture=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa")
+            };
+
+            t.SearchPaths = new string[]
+            {
+                s_myLibrariesRootPath, s_myLibraries_V2Path, s_myLibraries_V1Path
+            };
+
+            t.TargetFrameworkDirectories = new string[] { s_myVersion20Path };
+
+            bool result = Execute(t);
+
+            Assert.Equal(2, e.Warnings); // @"Expected two warnings."
+
+            // Check that we have both the expected messages
+            e.AssertLogContainsMessageFromResource(AssemblyResources.GetString, "ResolveAssemblyReference.FoundConflicts", "D");
+            e.AssertLogContainsMessageFromResource(AssemblyResources.GetString, "ResolveAssemblyReference.FoundConflicts", "G");
+        }
+
         /// <summary>
         /// Consider this dependency chain:
         ///
@@ -3852,8 +3941,11 @@ public void ConflictWithForeVersionPrimary()
 
             Assert.True(result); // @"Expected a success because this conflict is solvable."
             Assert.Equal(3, t.ResolvedFiles.Length);
-            Assert.True(ContainsItem(t.ResolvedFiles, s_myLibraries_V2_DDllPath)); // "Expected to find assembly, but didn't."
-            Assert.Equal(1, t.ResolvedDependencyFiles.Length);
+            Assert.True(ContainsItem(t.ResolvedFiles, s_myLibraries_V2_DDllPath));
+
+            Assert.Equal(2, t.ResolvedDependencyFiles.Length);
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_GDllPath));
         }
 
 
@@ -4482,9 +4574,12 @@ public void Regress265054()
             Execute(t);
 
             Assert.Equal(2, t.ResolvedFiles.Length);
-            Assert.Equal(3, t.ResolvedDependencyFiles.Length);
-            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath)); // "Expected to find assembly, but didn't."
-            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath)); // "Expected to find assembly, but didn't."
+
+            Assert.Equal(4, t.ResolvedDependencyFiles.Length);
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_DDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_DDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V2_GDllPath));
+            Assert.True(ContainsItem(t.ResolvedDependencyFiles, s_myLibraries_V1_E_EDllPath));
 
             foreach (ITaskItem item in t.ResolvedDependencyFiles)
             {

src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceTestFixture.cs

@@ -216,6 +216,8 @@ public void Dispose()
         protected static readonly string s_myLibraries_V1_DDllPath = Path.Combine(s_myLibraries_V1Path, "D.dll");
         protected static readonly string s_myLibraries_V1_E_EDllPath = Path.Combine(s_myLibraries_V1_EPath, "E.dll");
         protected static readonly string s_myLibraries_V2_DDllPath = Path.Combine(s_myLibraries_V2Path, "D.dll");
+        protected static readonly string s_myLibraries_V1_GDllPath = Path.Combine(s_myLibraries_V1Path, "G.dll");
+        protected static readonly string s_myLibraries_V2_GDllPath = Path.Combine(s_myLibraries_V2Path, "G.dll");
 
         protected static readonly string s_regress454863_ADllPath = Path.Combine(s_rootPathPrefix, "Regress454863", "A.dll");
         protected static readonly string s_regress454863_BDllPath = Path.Combine(s_rootPathPrefix, "Regress454863", "B.dll");
@@ -399,6 +401,8 @@ internal void StopIOMonitoringAndAssert_Zero_IOUse()
             s_myLibraries_V1_E_EDllPath,
             @"c:\RogueLibraries\v1\D.dll",
             s_myLibraries_V2_DDllPath,
+            s_myLibraries_V1_GDllPath,
+            s_myLibraries_V2_GDllPath,
             @"c:\MyStronglyNamed\A.dll",
             @"c:\MyWeaklyNamed\A.dll",
             @"c:\MyInaccessible\A.dll",
@@ -1302,6 +1306,16 @@ internal static AssemblyNameExtension GetAssemblyName(string path)
                 return new AssemblyNameExtension("D, VErsion=2.0.0.0, CulturE=neutral, PublicKEyToken=aaaaaaaaaaaaaaaa");
             }
 
+            if (path.EndsWith(s_myLibraries_V1_GDllPath))
+            {
+                return new AssemblyNameExtension("G, Version=1.0.0.0, Culture=neutral, PublicKEyToken=aaaaaaaaaaaaaaaa");
+            }
+
+            if (path.EndsWith(s_myLibraries_V2_GDllPath))
+            {
+                return new AssemblyNameExtension("G, Version=2.0.0.0, Culture=neutral, PublicKEyToken=aaaaaaaaaaaaaaaa");
+            }
+
             if (String.Compare(path, @"C:\Regress317975\a.dll", StringComparison.OrdinalIgnoreCase) == 0)
             {
                 return new AssemblyNameExtension("A, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null");
@@ -2124,7 +2138,8 @@ internal static AssemblyNameExtension[] GetDependencies(string path)
             {
                 return new AssemblyNameExtension[]
                 {
-                    new AssemblyNameExtension("D, Version=2.0.0.0, Culture=neutral, PuBlIcKeYToken=aaaaaaaaaaaaaaaa")
+                    new AssemblyNameExtension("D, Version=2.0.0.0, Culture=neutral, PuBlIcKeYToken=aaaaaaaaaaaaaaaa"),
+                    new AssemblyNameExtension("G, Version=2.0.0.0, Culture=neutral, PuBlIcKeYToken=aaaaaaaaaaaaaaaa")
                 };
             }
 

src/Tasks/AssemblyDependency/ReferenceTable.cs

@@ -1873,7 +1873,7 @@ out AssemblyNameReference[] conflictingReferences
                     if (entry != null)
                     {
                         // We have found an entry in the redist list that this assembly is a framework assembly of some version
-                        // also one if its parent refernces has specific version set to true, therefore we need to make sure
+                        // also one if its parent references has specific version set to true, therefore we need to make sure
                         // that we do not consider it for conflict resolution.
                         continue;
                     }

src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs

@@ -978,7 +978,6 @@ quiet at the engine level.
                     if (idealAssemblyRemappings != null)
                     {
                         bool foundAtLeastOneValidBindingRedirect = false;
-                        bool foundAtLeastOneUnresolvableConflict = false;
 
                         var buffer = new StringBuilder();
                         var ns = XNamespace.Get("urn:schemas-microsoft-com:asm.v1");
@@ -990,9 +989,10 @@ quiet at the engine level.
                             AssemblyName idealRemappingPartialAssemblyName = idealRemapping.PartialAssemblyName;
                             Reference reference = idealAssemblyRemappingsIdentities[i].reference;
 
+                            AssemblyNameExtension[] conflictVictims = reference.GetConflictVictims();
+
                             for (int j = 0; j < idealRemapping.BindingRedirects.Length; j++)
                             {
-                                AssemblyNameExtension[] conflictVictims = reference.GetConflictVictims();
                                 foreach (AssemblyNameExtension conflictVictim in conflictVictims)
                                 {
                                     // Make note we only output a conflict suggestion if the reference has at 
@@ -1052,11 +1052,13 @@ quiet at the engine level.
                                         buffer.Append(node.ToString(SaveOptions.DisableFormatting));
                                     }
                                 }
+                            }
 
-                                if (conflictVictims.Length == 0)
-                                {
-                                    foundAtLeastOneUnresolvableConflict = true;
-                                }
+                            if (conflictVictims.Length == 0)
+                            {
+                                // This warning is logged regardless of AutoUnify since it means a conflict existed where the reference
+                                // chosen was not the conflict victor in a version comparison, in other words it was older.
+                                Log.LogWarningWithCodeFromResources("ResolveAssemblyReference.FoundConflicts", idealRemappingPartialAssemblyName.Name);
                             }
                         }
 
@@ -1078,13 +1080,6 @@ quiet at the engine level.
                             // else AutoUnify is on and bindingRedirect generation is not supported
                             // we don't warn in this case since the binder will automatically unify these remappings
                         }
-
-                        if (foundAtLeastOneUnresolvableConflict)
-                        {
-                            // This warning is logged regardless of AutoUnify since it means a conflict existed where the reference
-                            // chosen was not the conflict victor in a version comparison, in other words it was older.
-                            Log.LogWarningWithCodeFromResources("ResolveAssemblyReference.FoundConflicts");
-                        }
                     }
 
                     // Fifth, log general resolution problems.

src/Tasks/Resources/Strings.resx

@@ -1532,7 +1532,7 @@
     <comment>{StrBegin="MSB3276: "}</comment>
   </data>
   <data name="ResolveAssemblyReference.FoundConflicts">
-    <value>MSB3277: Found conflicts between different versions of the same dependent assembly that could not be resolved.  These reference conflicts are listed in the build log when log verbosity is set to detailed.</value>
+    <value>MSB3277: Found conflicts between different versions of "{0}" that could not be resolved.  These reference conflicts are listed in the build log when log verbosity is set to detailed.</value>
     <comment>{StrBegin="MSB3277: "}</comment>
   </data>
   <data name="ResolveAssemblyReference.LogAttributeFormat">