Ensure virtual symbols are included when overridden but not referenced

13 files changed, 322 insertions, 156 deletions

diff --git a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs
index 610fd5ed..bcd7654f 100644
--- a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs
+++ b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs
@@ -125,7 +125,7 @@ namespace WixToolsetTest.UI
             }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray());
         }
 
-        [Fact(Skip = "Linker problem")]
+        [Fact]
         public void CanBuildUsingWixUIFeatureTree()
         {
             var folder = TestData.Get(@"TestData", "WixUI_FeatureTree");
@@ -161,7 +161,7 @@ namespace WixToolsetTest.UI
             }, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
         }
 
-        [Fact(Skip = "Linker problem")]
+        [Fact]
         public void CanBuildWithWixUIInstallDirWithCustomizedEula()
         {
             var folder = TestData.Get(@"TestData", "WixUI_InstallDir");
@@ -274,7 +274,7 @@ namespace WixToolsetTest.UI
             }
         }
 
-        [Fact(Skip = "Linker problem")]
+        [Fact]
         public void CanBuildUsingWixUIMondo()
         {
             var folder = TestData.Get(@"TestData", "WixUI_Mondo");
@@ -307,7 +307,7 @@ namespace WixToolsetTest.UI
             }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray());
             WixAssert.CompareLineByLine(new[]
             {
-                "InstallUISequence:WelcomeDlg\tInstalled AND PATCH\t1295",
+                "InstallUISequence:WelcomeDlg\tNOT Installed OR PATCH\t1297",
             }, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
         }
 
@@ -325,7 +325,7 @@ namespace WixToolsetTest.UI
             }, results.Where(s => s.StartsWith("Control:ErrorDlg\tY")).Select(s => s.Split('\t')[9]).ToArray());
         }
 
-        [Fact(Skip = "Linker problem")]
+        [Fact]
         public void CanBuildWithInstallDirAndRemovedDialog()
         {
             var folder = TestData.Get(@"TestData", "InstallDir_NoLicense");
@@ -366,7 +366,7 @@ namespace WixToolsetTest.UI
             }, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
         }
 
-        [Fact(Skip = "Linker problem")]
+        [Fact]
         public void CanBuildWithInstallDirAndAddedDialog()
         {
             var folder = TestData.Get(@"TestData", "InstallDir_SpecialDlg");
diff --git a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
index f9d6ab59..8eaac27d 100644
--- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
+++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
@@ -45,10 +45,9 @@ namespace WixToolset.Core.Link
         public ISet<IntermediateSymbol> IdenticalDirectorySymbols { get; private set; }
 
         /// <summary>
-        /// Gets the collection of overridden symbols that should not be included
-        /// in the final output.
+        /// Gets the collection of symbols that are marked as overrides.
         /// </summary>
-        public ISet<IntermediateSymbol> OverriddenSymbols { get; private set; }
+        public IReadOnlyCollection<SymbolWithSection> OverrideSymbols { get; private set; }
 
         public void Execute()
         {
@@ -56,7 +55,6 @@ namespace WixToolset.Core.Link
             var possibleConflicts = new HashSet<SymbolWithSection>();
             var identicalDirectorySymbols = new HashSet<IntermediateSymbol>();
             var overrideSymbols = new List<SymbolWithSection>();
-            var overriddenSymbols = new HashSet<IntermediateSymbol>();
 
             if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType))
             {
@@ -92,34 +90,15 @@ namespace WixToolset.Core.Link
 
                     if (!symbolsByName.TryGetValue(fullName, out var existingSymbol))
                     {
-                        if (symbolWithSection.Access == AccessModifier.Override)
-                        {
-                            overrideSymbols.Add(symbolWithSection);
-                        }
-
                         symbolsByName.Add(fullName, symbolWithSection);
                     }
-                    else // uh-oh, duplicate symbols.
+                    else // duplicate symbol ids MAY be a problem, but not always (e.g. identical directories and virtual symbols that get overridden) so we do NOT report errors here.
                     {
-                        if (AccessModifier.Virtual == existingSymbol.Access && AccessModifier.Override == symbolWithSection.Access)
-                        {
-                            symbolWithSection.OverrideVirtualSymbol(existingSymbol);
-                            symbolsByName[fullName] = symbolWithSection; // replace the virtual symbol with the override symbol.
-
-                            overrideSymbols.Add(symbolWithSection);
-                            overriddenSymbols.Add(existingSymbol.Symbol);
-                        }
-                        else if (AccessModifier.Override == existingSymbol.Access && AccessModifier.Virtual == symbolWithSection.Access)
-                        {
-                            existingSymbol.OverrideVirtualSymbol(symbolWithSection);
-
-                            overriddenSymbols.Add(symbolWithSection.Symbol);
-                        }
                         // If the duplicate symbols are both private directories, there is a chance that they
                         // point to identical symbols. Identical directory symbols are redundant and will not cause
                         // conflicts.
-                        else if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access &&
-                                 SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol))
+                        if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access &&
+                            SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol))
                         {
                             // Ensure identical symbols are tracked to ensure that only one symbol will end up in linked intermediate.
                             identicalDirectorySymbols.Add(existingSymbol.Symbol);
@@ -129,25 +108,21 @@ namespace WixToolset.Core.Link
                         {
                             symbolWithSection.AddPossibleConflict(existingSymbol);
                             existingSymbol.AddPossibleConflict(symbolWithSection);
-                            possibleConflicts.Add(symbolWithSection);
+                            possibleConflicts.Add(existingSymbol);
                         }
                     }
-                }
-            }
 
-            // Ensure override symbols actually overrode a virtual symbol.
-            foreach (var symbolWithSection in overrideSymbols)
-            {
-                if (symbolWithSection.Overrides is null)
-                {
-                    this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(symbolWithSection.Symbol));
+                    if (symbolWithSection.Access == AccessModifier.Override)
+                    {
+                        overrideSymbols.Add(symbolWithSection);
+                    }
                 }
             }
 
             this.SymbolsByName = symbolsByName;
             this.PossibleConflicts = possibleConflicts;
             this.IdenticalDirectorySymbols = identicalDirectorySymbols;
-            this.OverriddenSymbols = overriddenSymbols;
+            this.OverrideSymbols = overrideSymbols;
         }
     }
 }
diff --git a/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs
new file mode 100644
index 00000000..556a24d7
--- /dev/null
+++ b/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs
@@ -0,0 +1,159 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+
+namespace WixToolset.Core.Link
+{
+    using System.Collections.Generic;
+    using System.Linq;
+    using WixToolset.Data;
+    using WixToolset.Data.Symbols;
+    using WixToolset.Extensibility.Services;
+
+    internal class ProcessConflictingSymbolsCommand
+    {
+        public ProcessConflictingSymbolsCommand(IMessaging messaging, IReadOnlyCollection<SymbolWithSection> possibleConflicts, IReadOnlyCollection<SymbolWithSection> overrideSymbols, ISet<IntermediateSection> resolvedSections)
+        {
+            this.Messaging = messaging;
+            this.PossibleConflicts = possibleConflicts;
+            this.OverrideSymbols = overrideSymbols;
+            this.ResolvedSections = resolvedSections;
+        }
+
+        private IMessaging Messaging { get; }
+
+        private IReadOnlyCollection<SymbolWithSection> PossibleConflicts { get; }
+
+        private ISet<IntermediateSection> ResolvedSections { get; }
+
+        private IReadOnlyCollection<SymbolWithSection> OverrideSymbols { get; }
+
+        /// <summary>
+        /// Gets the collection of overridden symbols that should not be included
+        /// in the final output.
+        /// </summary>
+        public ISet<IntermediateSymbol> OverriddenSymbols { get; private set; }
+
+        public void Execute()
+        {
+            var overriddenSymbols = new HashSet<IntermediateSymbol>();
+
+            foreach (var symbolWithConflicts in this.PossibleConflicts)
+            {
+                var conflicts = YieldReferencedConflicts(symbolWithConflicts, this.ResolvedSections).ToList();
+
+                if (conflicts.Count > 1)
+                {
+                    IEnumerable<SymbolWithSection> reportDuplicates;
+
+                    var virtualConflicts = conflicts.Where(s => s.Access == AccessModifier.Virtual).ToList();
+
+                    // No virtual symbols, just plain old duplicate errors. This is the easy case.
+                    if (virtualConflicts.Count == 0)
+                    {
+                        var first = conflicts[0];
+                        reportDuplicates = conflicts.Skip(1);
+
+                        var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
+
+                        this.Messaging.Write(LinkerErrors.DuplicateSymbol(first.Symbol, referencingSourceLineNumber));
+                    }
+                    else // there are virtual symbols, which complicates conflict resolution and may not be an error at all.
+                    {
+                        var firstVirtualSymbol = virtualConflicts[0];
+                        var overrideConflicts = conflicts.Where(s => s.Access == AccessModifier.Override).ToList();
+
+                        // If there is a single virtual symbol, there may be a single override symbol to make this a success case.
+                        // All other scenarios are errors.
+                        if (virtualConflicts.Count == 1)
+                        {
+                            var otherConflicts = conflicts.Where(s => s.Access != AccessModifier.Virtual && s.Access != AccessModifier.Override).ToList();
+
+                            if (otherConflicts.Count > 0)
+                            {
+                                var first = otherConflicts[0];
+                                var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
+
+                                reportDuplicates = virtualConflicts;
+
+                                switch (first.Symbol)
+                                {
+                                    case WixActionSymbol action:
+                                        this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(action));
+                                        break;
+                                    default:
+                                        this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(first.Symbol, referencingSourceLineNumber));
+                                        break;
+                                }
+                            }
+                            else if (overrideConflicts.Count > 1) // multiple overrides report as normal duplicates.
+                            {
+                                var first = overrideConflicts[0];
+                                var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
+
+                                reportDuplicates = overrideConflicts.Skip(1);
+
+                                this.Messaging.Write(LinkerErrors.DuplicateSymbol(first.Symbol, referencingSourceLineNumber));
+                            }
+                            else // the single virtual symbol is overridden by a single override symbol. This is a success case.
+                            {
+                                var overrideSymbol = overrideConflicts[0];
+
+                                overriddenSymbols.Add(firstVirtualSymbol.Symbol);
+
+                                reportDuplicates = Enumerable.Empty<SymbolWithSection>();
+                            }
+                        }
+                        else // multiple symbols are virtual, use the duplicate virtual symbol message.
+                        {
+                            var first = virtualConflicts[0];
+                            var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
+
+                            reportDuplicates = virtualConflicts.Skip(1);
+
+                            this.Messaging.Write(LinkerErrors.DuplicateVirtualSymbol(first.Symbol, referencingSourceLineNumber));
+                        }
+
+                        // Always point the override symbols at the first virtual symbol to prevent error being reported about missing overrides.
+                        // There may have been errors reported above, but there was at least one virtual symbol to satisfy the overrides so we
+                        // don't want extra errors in this case.
+                        foreach (var overrideSymbol in overrideConflicts)
+                        {
+                            overrideSymbol.OverrideVirtualSymbol(firstVirtualSymbol);
+                        }
+                    }
+
+                    foreach (var duplicate in reportDuplicates)
+                    {
+                        this.Messaging.Write(LinkerErrors.DuplicateSymbol2(duplicate.Symbol));
+                    }
+                }
+            }
+
+            // Ensure referenced override symbols actually overrode a virtual symbol.
+            foreach (var referencedOverrideSymbol in this.OverrideSymbols.Where(s => this.ResolvedSections.Contains(s.Section)))
+            {
+                if (referencedOverrideSymbol.Overrides is null)
+                {
+                    this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(referencedOverrideSymbol.Symbol));
+                }
+            }
+
+            this.OverriddenSymbols = overriddenSymbols;
+        }
+
+        private static IEnumerable<SymbolWithSection> YieldReferencedConflicts(SymbolWithSection symbolWithConflicts, ISet<IntermediateSection> resolvedSections)
+        {
+            if (resolvedSections.Contains(symbolWithConflicts.Section))
+            {
+                yield return symbolWithConflicts;
+            }
+
+            foreach (var possibleConflict in symbolWithConflicts.PossiblyConflicts)
+            {
+                if (resolvedSections.Contains(possibleConflict.Section))
+                {
+                    yield return possibleConflict;
+                }
+            }
+        }
+    }
+}
diff --git a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs
deleted file mode 100644
index b1db46a2..00000000
--- a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs
+++ /dev/null
@@ -1,92 +0,0 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
-
-namespace WixToolset.Core.Link
-{
-    using System.Collections.Generic;
-    using System.Linq;
-    using WixToolset.Data;
-    using WixToolset.Data.Symbols;
-    using WixToolset.Extensibility.Services;
-
-    internal class ReportConflictingSymbolsCommand
-    {
-        public ReportConflictingSymbolsCommand(IMessaging messaging, IReadOnlyCollection<SymbolWithSection> possibleConflicts, ISet<IntermediateSection> resolvedSections)
-        {
-            this.Messaging = messaging;
-            this.PossibleConflicts = possibleConflicts;
-            this.ResolvedSections = resolvedSections;
-        }
-
-        private IMessaging Messaging { get; }
-
-        private IReadOnlyCollection<SymbolWithSection> PossibleConflicts { get; }
-
-        private ISet<IntermediateSection> ResolvedSections { get; }
-
-        public void Execute()
-        {
-            // Do a quick check if there are any possibly conflicting symbols. Hopefully the symbols with possible conflicts
-            // list is a very short list (empty should be the most common).
-            //
-            // If we have conflicts then we'll do a more costly check to see if the possible conflicting
-            // symbols are in sections we actually referenced. From the resulting set, show an error for each duplicate
-            // (aka: conflicting) symbol.
-            if (0 < this.PossibleConflicts.Count)
-            {
-                foreach (var referencedDuplicate in this.PossibleConflicts.Where(s => this.ResolvedSections.Contains(s.Section)))
-                {
-                    var actuallyReferencedDuplicates = referencedDuplicate.PossiblyConflicts.Where(s => this.ResolvedSections.Contains(s.Section)).ToList();
-
-                    if (actuallyReferencedDuplicates.Count > 0)
-                    {
-                        var conflicts = actuallyReferencedDuplicates.Append(referencedDuplicate).ToList();
-                        var virtualConflicts = conflicts.Where(s => s.Access == AccessModifier.Virtual).ToList();
-                        var overrideConflicts = conflicts.Where(s => s.Access == AccessModifier.Override).ToList();
-                        var otherConflicts = conflicts.Where(s => s.Access != AccessModifier.Virtual && s.Access != AccessModifier.Override).ToList();
-
-                        IEnumerable<SymbolWithSection> reportDuplicates = actuallyReferencedDuplicates;
-
-                        // If multiple symbols are virtual, use the duplicate virtual symbol message.
-                        if (virtualConflicts.Count > 1)
-                        {
-                            var first = virtualConflicts[0];
-                            var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
-
-                            reportDuplicates = virtualConflicts.Skip(1);
-
-                            this.Messaging.Write(LinkerErrors.DuplicateVirtualSymbol(first.Symbol, referencingSourceLineNumber));
-                        }
-                        else if (virtualConflicts.Count == 1 && otherConflicts.Count > 0)
-                        {
-                            var first = otherConflicts[0];
-                            var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
-
-                            reportDuplicates = virtualConflicts;
-
-                            switch (first.Symbol)
-                            {
-                                case WixActionSymbol action:
-                                    this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(action));
-                                    break;
-                                default:
-                                    this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(first.Symbol, referencingSourceLineNumber));
-                                    break;
-                            }
-                        }
-                        else
-                        {
-                            var referencingSourceLineNumber = referencedDuplicate.DirectReferences.FirstOrDefault()?.SourceLineNumbers;
-
-                            this.Messaging.Write(LinkerErrors.DuplicateSymbol(referencedDuplicate.Symbol, referencingSourceLineNumber));
-                        }
-
-                        foreach (var duplicate in reportDuplicates)
-                        {
-                            this.Messaging.Write(LinkerErrors.DuplicateSymbol2(duplicate.Symbol));
-                        }
-                    }
-                }
-            }
-        }
-    }
-}
diff --git a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs
index 0e7a7a52..7a3c502a 100644
--- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs
+++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs
@@ -76,6 +76,7 @@ namespace WixToolset.Core.Link
                 if (this.symbolsWithSections.TryGetValue(reference.SymbolicName, out var symbolWithSection))
                 {
                     var accessible = this.DetermineAccessibleSymbols(section, symbolWithSection);
+
                     if (accessible.Count == 1)
                     {
                         var accessibleSymbol = accessible[0];
@@ -91,7 +92,7 @@ namespace WixToolset.Core.Link
                     {
                         this.Messaging.Write(ErrorMessages.UnresolvedReference(reference.SourceLineNumbers, reference.SymbolicName, symbolWithSection.Access));
                     }
-                    else // multiple symbols referenced creates conflicting symbols.
+                    else // multiple accessible symbols referenced creates conflicting symbols.
                     {
                         // Remember the direct reference to the symbol for the error reporting later,
                         // but do NOT continue resolving references found in these conflicting symbols.
@@ -122,7 +123,7 @@ namespace WixToolset.Core.Link
         }
 
         /// <summary>
-        /// Determine if the symbol and any of its duplicates are accessbile by referencing section.
+        /// Determine if the symbol and any of its duplicates are accessbile by the referencing section.
         /// </summary>
         /// <param name="referencingSection">Section referencing the symbol.</param>
         /// <param name="symbolWithSection">Symbol being referenced.</param>
@@ -130,17 +131,48 @@ namespace WixToolset.Core.Link
         private List<SymbolWithSection> DetermineAccessibleSymbols(IntermediateSection referencingSection, SymbolWithSection symbolWithSection)
         {
             var accessibleSymbols = new List<SymbolWithSection>();
+            List<SymbolWithSection> virtualSymbols = null;
 
             if (this.AccessibleSymbol(referencingSection, symbolWithSection))
             {
-                accessibleSymbols.Add(symbolWithSection);
+                AddSymbolToCorrectCollection(symbolWithSection, accessibleSymbols, ref virtualSymbols);
             }
 
             foreach (var dupe in symbolWithSection.PossiblyConflicts)
             {
                 if (this.AccessibleSymbol(referencingSection, dupe))
                 {
-                    accessibleSymbols.Add(dupe);
+                    AddSymbolToCorrectCollection(dupe, accessibleSymbols, ref virtualSymbols);
+                }
+            }
+
+            // If a virtual symbol is accessible we have some extra work to do.
+            if (virtualSymbols != null)
+            {
+                // If there are multiple virtual symbols accessible that's an error case so add them all and
+                // they'll be reported as conflicts later.
+                if (virtualSymbols.Count > 1)
+                {
+                    accessibleSymbols.AddRange(virtualSymbols);
+                }
+                else
+                {
+                    var overrode = false;
+
+                    // If there are any override symbols, skip adding the virtual symbols because they are "overridden".
+                    foreach (var overrideSymbol in accessibleSymbols.Where(symbol => symbol.Access == AccessModifier.Override))
+                    {
+                        overrideSymbol.OverrideVirtualSymbol(virtualSymbols[0]);
+
+                        overrode = true;
+                    }
+
+                    // If there are no overriding symbols, add the virtual symbol (there is only one but we'll add the collection just in case)
+                    // so it will be reported as a conflict later.
+                    if (!overrode)
+                    {
+                        accessibleSymbols.AddRange(virtualSymbols);
+                    }
                 }
             }
 
@@ -148,6 +180,29 @@ namespace WixToolset.Core.Link
         }
 
         /// <summary>
+        /// Add a symbol to the correct collection based on its access.
+        /// </summary>
+        /// <param name="symbolWithSection"></param>
+        /// <param name="accessibleSymbols">Collection of non-virtual symbol that was accessible.</param>
+        /// <param name="virtualSymbols">Collection of virtual symbol that was accessible</param>
+        private static void AddSymbolToCorrectCollection(SymbolWithSection symbolWithSection, List<SymbolWithSection> accessibleSymbols, ref List<SymbolWithSection> virtualSymbols)
+        {
+            if (symbolWithSection.Access == AccessModifier.Virtual)
+            {
+                if (virtualSymbols == null)
+                {
+                    virtualSymbols = new List<SymbolWithSection>();
+                }
+
+                virtualSymbols.Add(symbolWithSection);
+            }
+            else
+            {
+                accessibleSymbols.Add(symbolWithSection);
+            }
+        }
+
+        /// <summary>
         /// Determine if a single symbol is accessible by the referencing section.
         /// </summary>
         /// <param name="referencingSection">Section referencing the symbol.</param>
diff --git a/src/wix/WixToolset.Core/Linker.cs b/src/wix/WixToolset.Core/Linker.cs
index a536c049..43a41eac 100644
--- a/src/wix/WixToolset.Core/Linker.cs
+++ b/src/wix/WixToolset.Core/Linker.cs
@@ -190,10 +190,13 @@ namespace WixToolset.Core
                     }
                 }
 
-                // Report duplicates that would ultimately end up being primary key collisions.
+                // Process conflicts that may be overridden virtual symbols (that's okay) or end up as primary key collisions (those need to be reported as errors).
+                ISet<IntermediateSymbol> overriddenSymbols;
                 {
-                    var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections);
+                    var reportDupes = new ProcessConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, find.OverrideSymbols, resolve.ResolvedSections);
                     reportDupes.Execute();
+
+                    overriddenSymbols = reportDupes.OverriddenSymbols;
                 }
 
                 if (this.Messaging.EncounteredError)
@@ -222,7 +225,7 @@ namespace WixToolset.Core
                                 continue;
                             }
                         }
-                        else if (find.OverriddenSymbols.Contains(symbol))
+                        else if (overriddenSymbols.Contains(symbol))
                         {
                             // Skip the symbols that were overridden.
                             continue;
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs
index 5e40114f..13eb15be 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs
@@ -63,6 +63,18 @@ namespace WixToolsetTest.CoreIntegration
         }
 
         [Fact]
+        public void CanCompileVirtualSymbolThatDoesNotGetOverridden()
+        {
+            var dirSymbols = BuildToGetDirectorySymbols("TestData", "AccessModifier", "VirtualSymbolThatDoesNotGetOverridden.wxs");
+            WixAssert.CompareLineByLine(new[]
+            {
+                "virtual:ProgramFilesFolder:TARGETDIR:PFiles",
+                "virtual:TARGETDIR::SourceDir",
+                "virtual:TestFolder:ProgramFilesFolder:Used",
+            }, dirSymbols.OrderBy(d => d.Id.Id).Select(d => d.Id.Access.AsString() + ":" + d.Id.Id + ":" + d.ParentDirectoryRef + ":" + d.Name).ToArray());
+        }
+
+        [Fact]
         public void CannotCompileInvalidCrossFragmentReference()
         {
             var errors = BuildForFailure("TestData", "AccessModifier", "InvalidCrossFragmentReference.wxs");
@@ -78,8 +90,9 @@ namespace WixToolsetTest.CoreIntegration
             var errors = BuildForFailure("TestData", "AccessModifier", "DuplicateCrossFragmentReference.wxs");
             WixAssert.CompareLineByLine(new[]
             {
-                "ln 12: Duplicate Directory with identifier 'TestFolder' referenced by <sourceFolder>\\DuplicateCrossFragmentReference.wxs(4). Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.",
-                "ln 8: Location of symbol related to previous error."
+                "ln 8: Duplicate Directory with identifier 'TestFolder' referenced by <sourceFolder>\\DuplicateCrossFragmentReference.wxs(4). Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.",
+                "ln 12: Location of symbol related to previous error.",
+                "ln 16: Location of symbol related to previous error."
             }, errors);
         }
 
@@ -99,8 +112,8 @@ namespace WixToolsetTest.CoreIntegration
             var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatedOverrideVirtualSymbol.wxs");
             WixAssert.CompareLineByLine(new[]
             {
-                "ln 14: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
-                "ln 6: Location of symbol related to previous error."
+                "ln 6: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
+                "ln 14: Location of symbol related to previous error."
             }, errors);
         }
 
@@ -132,8 +145,8 @@ namespace WixToolsetTest.CoreIntegration
             var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatePublicOverrideVirtualSymbol.wxs");
             WixAssert.CompareLineByLine(new[]
             {
-                "ln 14: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
-                "ln 6: Location of symbol related to previous error."
+                "ln 14: The Directory symbol 'TestFolder' conflicts with a virtual symbol. Use the 'override' access modifier to override the virtual symbol or use a different Id to avoid the conflict.",
+                "ln 10: Location of symbol related to previous error."
             }, errors);
         }
 
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs
index f735b53a..17c8db95 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs
@@ -4,9 +4,9 @@ namespace WixToolsetTest.CoreIntegration
 {
     using System.IO;
     using System.Linq;
+    using WixInternal.Core.TestPackage;
     using WixInternal.TestSupport;
     using WixToolset.Core;
-    using WixInternal.Core.TestPackage;
     using WixToolset.Data;
     using WixToolset.Data.Symbols;
     using WixToolset.Extensibility.Data;
@@ -101,7 +101,7 @@ namespace WixToolsetTest.CoreIntegration
         [Fact]
         public void VariableRedefinitionIsAWarning()
         {
-            var folder = TestData.Get(@"TestData\Variables");
+            var folder = TestData.Get(@"TestData", "Variables");
 
             using (var fs = new DisposableFileSystem())
             {
@@ -121,8 +121,15 @@ namespace WixToolsetTest.CoreIntegration
 
                 result.AssertSuccess();
 
-                var warning = result.Messages.Where(message => message.Id == (int)WarningMessages.Ids.VariableDeclarationCollision);
-                Assert.Single(warning);
+                var warning = result.Messages.Where(m => m.Level == MessageLevel.Warning || m.Level == MessageLevel.Error)
+                                      .Select(m => $"ln {m.SourceLineNumbers.LineNumber}: {m.Level.ToString().ToLowerInvariant()}: {m}".Replace(baseFolder, "<baseFolder>"))
+                                      .ToArray();
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "ln 5: warning: The variable 'Bar' with value 'Baz' was previously declared with value 'Bar'.",
+                    "ln 24: warning: It is no longer necessary to define the standard directory 'TARGETDIR'. Use the StandardDirectory element instead.",
+                    "ln 25: warning: It is no longer necessary to define the standard directory 'ProgramFilesFolder'. Use the StandardDirectory element instead."
+                }, warning);
             }
         }
 
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs
index 9931a45a..5b357994 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs
@@ -102,10 +102,9 @@ namespace WixToolsetTest.CoreIntegration
                                      .ToArray();
                 WixAssert.CompareLineByLine(new[]
                 {
-                    "ln 8: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
-                    "ln 7: Location of symbol related to previous error.",
-                    "ln 9: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
-                    "ln 7: Location of symbol related to previous error."
+                    "ln 7: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.",
+                    "ln 8: Location of symbol related to previous error.",
+                    "ln 9: Location of symbol related to previous error."
                 }, errors);
             }
         }
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs
index f987fe13..73678cb6 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs
@@ -11,4 +11,8 @@
     <Fragment>
         <Directory Id="TestFolder" Name="Dupe 2" />
     </Fragment>
+
+    <Fragment>
+        <Directory Id="TestFolder" Name="Dupe 3" />
+    </Fragment>
 </Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs
index 13f03040..9afec003 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs
@@ -2,9 +2,22 @@
 <Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
     <Package Name="Override Virtual Symbol With Fragments" Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="047730a5-30fe-4a62-a520-da9381b8226a">
         <DirectoryRef Id="TestFolder" />
+
+        <PropertyRef Id="A" />
+        <PropertyRef Id="B" />
     </Package>
 
     <Fragment>
+        <Property Id="A" Secure="true"/>
+        <DirectoryRef Id="TestFolder" />
+    </Fragment>
+
+    <Fragment>
+        <Property Id="B" Secure="true"/>
+        <DirectoryRef Id="TestFolder" />
+    </Fragment>
+
+    <Fragment>
         <StandardDirectory Id="ProgramFilesFolder">
             <Directory Id="override TestFolder" Name="Override Test Folder Includes Another" />
             <Directory Id="AlsoIncluded" Name="Also Included" />
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs
new file mode 100644
index 00000000..123b856d
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Package Name="Override Without Virtual Symbol" Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="047730a5-30fe-4a62-a520-da9381b8226a">
+        <PropertyRef Id="DoesGetReferenced" />
+    </Package>
+
+    <Fragment>
+        <Property Id="DoesNotGetReferenced1" Value="Does Not Get Referenced" />
+
+        <StandardDirectory Id="ProgramFilesFolder">
+            <Directory Id="override TestFolder" Name="IsNotUsed1" />
+        </StandardDirectory>
+    </Fragment>
+
+    <Fragment>
+        <Property Id="DoesNotGetReferenced2" Value="Does Not Get Referenced" />
+
+        <StandardDirectory Id="ProgramFilesFolder">
+            <Directory Id="override TestFolder" Name="IsNotUsed2" />
+        </StandardDirectory>
+    </Fragment>
+
+    <Fragment>
+        <Property Id="DoesGetReferenced" Value="Is Referenced" />
+
+        <StandardDirectory Id="ProgramFilesFolder">
+            <Directory Id="virtual TestFolder" Name="Used" />
+        </StandardDirectory>
+    </Fragment>
+</Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs
index bcbdb00f..d22676d4 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs
@@ -22,7 +22,7 @@
 
   <Fragment>
     <Directory Id="override TARGETDIR" Name="SourceDir">
-      <Directory Id="override ProgramFilesFolder">
+      <Directory Id="ProgramFilesFolder">
         <Directory Id="INSTALLFOLDER" Name="MsiPackage" />
       </Directory>
     </Directory>