From 92119c839e2e36c5961915b661f7a0b8fecbd293 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Sat, 16 Dec 2023 09:03:44 -0800 Subject: [PATCH] Improve error reporting of duplicate symbols Virtual symbols provide more interesting ways to have (and avoid) conflicts. Adding additional messages and cleaning up the existing messages should help users know what options they have to address conflicts. This also puts all the conflict resolution in ReportConflictingSymbolsCommand instead of spreading it across reference resolution as well. --- src/api/wix/WixToolset.Data/ErrorMessages.cs | 17 ----- .../FindEntrySectionAndLoadSymbolsCommand.cs | 6 +- .../Link/ReportConflictingSymbolsCommand.cs | 61 ++++++++++++----- .../Link/ResolveReferencesCommand.cs | 57 ++++++++-------- .../WixToolset.Core/Link/SymbolWithSection.cs | 21 ++++++ src/wix/WixToolset.Core/LinkerErrors.cs | 68 ++++++++++++++++++- .../AccessModifierFixture.cs | 41 +++++++++-- .../RegistryFixture.cs | 14 +++- .../RollbackBoundaryFixture.cs | 2 +- .../DuplicatePublicOverrideVirtualSymbol.wxs | 17 +++++ .../DuplicatedVirtualSymbol.wxs | 13 ++++ .../VirtualSymbolWithoutOverride.wxs | 12 ++++ 12 files changed, 251 insertions(+), 78 deletions(-) create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatePublicOverrideVirtualSymbol.wxs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatedVirtualSymbol.wxs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolWithoutOverride.wxs diff --git a/src/api/wix/WixToolset.Data/ErrorMessages.cs b/src/api/wix/WixToolset.Data/ErrorMessages.cs index 889d1762f..e149d54a5 100644 --- a/src/api/wix/WixToolset.Data/ErrorMessages.cs +++ b/src/api/wix/WixToolset.Data/ErrorMessages.cs @@ -323,21 +323,6 @@ public static Message DuplicateSourcesForOutput(string sourceList, string output return Message(null, Ids.DuplicateSourcesForOutput, "Multiple source files ({0}) have resulted in the same output file '{1}'. This is likely because the source files only differ in extension or path. Rename the source files to avoid this problem.", sourceList, outputFile); } - public static Message DuplicateSymbol(SourceLineNumber sourceLineNumbers, string symbolName) - { - return Message(sourceLineNumbers, Ids.DuplicateSymbol, "Duplicate symbol '{0}' found. This typically means that an Id is duplicated. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", symbolName); - } - - public static Message DuplicateSymbol(SourceLineNumber sourceLineNumbers, string symbolName, string referencingSourceLineNumber) - { - return Message(sourceLineNumbers, Ids.DuplicateSymbol, "Duplicate symbol '{0}' referenced by {1}. This typically means that an Id is duplicated. Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.", symbolName, referencingSourceLineNumber); - } - - public static Message DuplicateSymbol2(SourceLineNumber sourceLineNumbers) - { - return Message(sourceLineNumbers, Ids.DuplicateSymbol2, "Location of symbol related to previous error."); - } - public static Message DuplicateTransform(string transform) { return Message(null, Ids.DuplicateTransform, "The transform {0} was included twice on the command line. Each transform can be applied to a patch only once.", transform); @@ -2369,8 +2354,6 @@ public enum Ids InvalidDateTimeFormat = 88, MultipleEntrySections = 89, MultipleEntrySections2 = 90, - DuplicateSymbol = 91, - DuplicateSymbol2 = 92, MissingEntrySection = 93, UnresolvedReference = 94, MultiplePrimaryReferences = 95, diff --git a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs index e614d4eb1..f9d6ab598 100644 --- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs @@ -36,7 +36,7 @@ public FindEntrySectionAndLoadSymbolsCommand(IMessaging messaging, IEnumerable /// Gets the collection of possibly conflicting symbols. /// - public IEnumerable PossibleConflicts { get; private set; } + public IReadOnlyCollection PossibleConflicts { get; private set; } /// /// Gets the collection of redundant symbols that should not be included @@ -140,9 +140,7 @@ public void Execute() { if (symbolWithSection.Overrides is null) { - var fullName = symbolWithSection.GetFullName(); - - this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(symbolWithSection.Symbol.SourceLineNumbers, fullName)); + this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(symbolWithSection.Symbol)); } } diff --git a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs index 3a07d3662..2851fa60a 100644 --- a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs @@ -9,7 +9,7 @@ namespace WixToolset.Core.Link internal class ReportConflictingSymbolsCommand { - public ReportConflictingSymbolsCommand(IMessaging messaging, IEnumerable possibleConflicts, IEnumerable resolvedSections) + public ReportConflictingSymbolsCommand(IMessaging messaging, IReadOnlyCollection possibleConflicts, ISet resolvedSections) { this.Messaging = messaging; this.PossibleConflicts = possibleConflicts; @@ -18,35 +18,62 @@ public ReportConflictingSymbolsCommand(IMessaging messaging, IEnumerable PossibleConflicts { get; } + private IReadOnlyCollection PossibleConflicts { get; } - private IEnumerable ResolvedSections { get; } + private ISet ResolvedSections { get; } public void Execute() { - // Do a quick check if there are any possibly conflicting symbols that don't come from tables that allow - // overriding. Hopefully the symbols with possible conflicts list is usually very short list (empty should - // be the most common). If we find any matches, we'll do a more costly check to see if the possible conflicting + // 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. - var illegalDuplicates = this.PossibleConflicts.Where(s => s.Symbol.Definition.Type != SymbolDefinitionType.WixAction && s.Symbol.Definition.Type != SymbolDefinitionType.WixVariable).ToList(); - if (0 < illegalDuplicates.Count) + if (0 < this.PossibleConflicts.Count) { - var referencedSections = new HashSet(this.ResolvedSections); - - foreach (var referencedDuplicate in illegalDuplicates.Where(s => referencedSections.Contains(s.Section))) + foreach (var referencedDuplicate in this.PossibleConflicts.Where(s => this.ResolvedSections.Contains(s.Section))) { - var actuallyReferencedDuplicates = referencedDuplicate.PossiblyConflicts.Where(s => referencedSections.Contains(s.Section)).ToList(); + var actuallyReferencedDuplicates = referencedDuplicate.PossiblyConflicts.Where(s => this.ResolvedSections.Contains(s.Section)).ToList(); - if (actuallyReferencedDuplicates.Any()) + if (actuallyReferencedDuplicates.Count > 0) { - var fullName = referencedDuplicate.GetFullName(); + 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 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; - this.Messaging.Write(ErrorMessages.DuplicateSymbol(referencedDuplicate.Symbol.SourceLineNumbers, fullName)); + 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; + + this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(first.Symbol, referencingSourceLineNumber)); + } + else + { + var referencingSourceLineNumber = referencedDuplicate.DirectReferences.FirstOrDefault()?.SourceLineNumbers; + + this.Messaging.Write(LinkerErrors.DuplicateSymbol(referencedDuplicate.Symbol, referencingSourceLineNumber)); + } - foreach (var duplicate in actuallyReferencedDuplicates) + foreach (var duplicate in reportDuplicates) { - this.Messaging.Write(ErrorMessages.DuplicateSymbol2(duplicate.Symbol.SourceLineNumbers)); + 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 0edbf39c8..0e7a7a52f 100644 --- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs +++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs @@ -27,9 +27,9 @@ public ResolveReferencesCommand(IMessaging messaging, IntermediateSection entryS this.BuildingMergeModule = (SectionType.Module == entrySection.Type); } - public IEnumerable ReferencedSymbolWithSections => this.referencedSymbols; + public IReadOnlyCollection ReferencedSymbolWithSections => this.referencedSymbols; - public IEnumerable ResolvedSections => this.resolvedSections; + public ISet ResolvedSections => this.resolvedSections; private bool BuildingMergeModule { get; } @@ -63,22 +63,25 @@ private void RecursivelyResolveReferences(IntermediateSection section) // symbols provided. Then recursively call this method to process the // located symbol's section. All in all this is a very simple depth-first // search of the references per-section. - foreach (var wixSimpleReferenceRow in section.Symbols.OfType()) + foreach (var reference in section.Symbols.OfType()) { // If we're building a Merge Module, ignore all references to the Media table // because Merge Modules don't have Media tables. - if (this.BuildingMergeModule && wixSimpleReferenceRow.Table == "Media") + if (this.BuildingMergeModule && reference.Table == "Media") { continue; } // See if the symbol (and any of its duplicates) are appropriately accessible. - if (this.symbolsWithSections.TryGetValue(wixSimpleReferenceRow.SymbolicName, out var symbolWithSection)) + if (this.symbolsWithSections.TryGetValue(reference.SymbolicName, out var symbolWithSection)) { var accessible = this.DetermineAccessibleSymbols(section, symbolWithSection); if (accessible.Count == 1) { var accessibleSymbol = accessible[0]; + + accessibleSymbol.AddDirectReference(reference); + if (this.referencedSymbols.Add(accessibleSymbol) && null != accessibleSymbol.Section) { this.RecursivelyResolveReferences(accessibleSymbol.Section); @@ -86,32 +89,34 @@ private void RecursivelyResolveReferences(IntermediateSection section) } else if (accessible.Count == 0) { - this.Messaging.Write(ErrorMessages.UnresolvedReference(wixSimpleReferenceRow.SourceLineNumbers, wixSimpleReferenceRow.SymbolicName, symbolWithSection.Access)); + this.Messaging.Write(ErrorMessages.UnresolvedReference(reference.SourceLineNumbers, reference.SymbolicName, symbolWithSection.Access)); } - else // display errors for the duplicate symbols. + else // multiple symbols referenced creates conflicting symbols. { - var accessibleSymbol = accessible[0]; - var accessibleFullName = accessibleSymbol.GetFullName(); - var referencingSourceLineNumber = wixSimpleReferenceRow.SourceLineNumbers?.ToString(); - - if (String.IsNullOrEmpty(referencingSourceLineNumber)) - { - this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName)); - } - else + // Remember the direct reference to the symbol for the error reporting later, + // but do NOT continue resolving references found in these conflicting symbols. + foreach (var conflict in accessible) { - this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName, referencingSourceLineNumber)); - } + // This should NEVER happen. + if (!conflict.PossiblyConflicts.Any()) + { + throw new InvalidOperationException("If a reference can reference multiple symbols, those symbols MUST have already been recognized as possible conflicts."); + } - foreach (var accessibleDuplicate in accessible.Skip(1)) - { - this.Messaging.Write(ErrorMessages.DuplicateSymbol2(accessibleDuplicate.Symbol.SourceLineNumbers)); + conflict.AddDirectReference(reference); + + this.referencedSymbols.Add(conflict); + + if (conflict.Section != null) + { + this.resolvedSections.Add(conflict.Section); + } } } } else { - this.Messaging.Write(ErrorMessages.UnresolvedReference(wixSimpleReferenceRow.SourceLineNumbers, wixSimpleReferenceRow.SymbolicName)); + this.Messaging.Write(ErrorMessages.UnresolvedReference(reference.SourceLineNumbers, reference.SymbolicName)); } } } @@ -133,14 +138,6 @@ private List DetermineAccessibleSymbols(IntermediateSection r foreach (var dupe in symbolWithSection.PossiblyConflicts) { - //// don't count overridable WixActionSymbols - //var symbolAction = symbolWithSection.Symbol as WixActionSymbol; - //var dupeAction = dupe.Symbol as WixActionSymbol; - //if (symbolAction?.Overridable != dupeAction?.Overridable) - //{ - // continue; - //} - if (this.AccessibleSymbol(referencingSection, dupe)) { accessibleSymbols.Add(dupe); diff --git a/src/wix/WixToolset.Core/Link/SymbolWithSection.cs b/src/wix/WixToolset.Core/Link/SymbolWithSection.cs index 979aa44fd..5bdf83603 100644 --- a/src/wix/WixToolset.Core/Link/SymbolWithSection.cs +++ b/src/wix/WixToolset.Core/Link/SymbolWithSection.cs @@ -6,12 +6,14 @@ namespace WixToolset.Core.Link using System.Collections.Generic; using System.Linq; using WixToolset.Data; + using WixToolset.Data.Symbols; /// /// Symbol with section representing a single unique symbol. /// internal class SymbolWithSection { + private List directReferences; private HashSet possibleConflicts; /// @@ -43,6 +45,11 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol) /// Section for the symbol. public IntermediateSection Section { get; } + /// + /// Gets any duplicates of this symbol with sections that are possible conflicts. + /// + public IEnumerable DirectReferences => this.directReferences ?? Enumerable.Empty(); + /// /// Gets any duplicates of this symbol with sections that are possible conflicts. /// @@ -67,6 +74,20 @@ public void AddPossibleConflict(SymbolWithSection symbolWithSection) this.possibleConflicts.Add(symbolWithSection); } + /// + /// Adds a reference that directly points to this symbol. + /// + /// The direct reference. + public void AddDirectReference(WixSimpleReferenceSymbol reference) + { + if (null == this.directReferences) + { + this.directReferences = new List(); + } + + this.directReferences.Add(reference); + } + /// /// Override a virtual symbol. /// diff --git a/src/wix/WixToolset.Core/LinkerErrors.cs b/src/wix/WixToolset.Core/LinkerErrors.cs index 78cd76f00..c234087e3 100644 --- a/src/wix/WixToolset.Core/LinkerErrors.cs +++ b/src/wix/WixToolset.Core/LinkerErrors.cs @@ -11,6 +11,41 @@ public static Message DuplicateBindPathVariableOnCommandLine(string argument, st return Message(null, Ids.DuplicateBindPathVariableOnCommandLine, "", argument, bindName, bindValue, collisionValue); } + public static Message DuplicateSymbol(IntermediateSymbol symbol) + { + return Message(symbol.SourceLineNumbers, Ids.DuplicateSymbol, "Duplicate {0} with identifier '{1}' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", symbol.Definition.Name, symbol.Id.Id); + } + + public static Message DuplicateSymbol(IntermediateSymbol symbol, SourceLineNumber referencingSourceLineNumber) + { + if (referencingSourceLineNumber is null) + { + return DuplicateSymbol(symbol); + } + + return Message(symbol.SourceLineNumbers, Ids.DuplicateSymbol, "Duplicate {0} with identifier '{1}' referenced by {2}. Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.", symbol.Definition.Name, symbol.Id.Id, referencingSourceLineNumber); + } + + public static Message DuplicateVirtualSymbol(IntermediateSymbol symbol) + { + return Message(symbol.SourceLineNumbers, Ids.DuplicateSymbol, "The virtual {0} with identifier '{1}' is duplicated. Ensure identifiers of a given type (Directory, File, etc.) are unique or did you mean to make one an override for the virtual symbol?", symbol.Definition.Name, symbol.Id.Id); + } + + public static Message DuplicateVirtualSymbol(IntermediateSymbol symbol, SourceLineNumber referencingSourceLineNumber) + { + if (referencingSourceLineNumber is null) + { + return DuplicateVirtualSymbol(symbol); + } + + return Message(symbol.SourceLineNumbers, Ids.DuplicateSymbol, "The virtual {0} with identifier '{1}' is duplicated. Ensure identifiers of a given type (Directory, File, etc.) are unique or did you mean to make one an override for the virtual symbol? Referenced from {2}", symbol.Definition.Name, symbol.Id.Id, referencingSourceLineNumber); + } + + public static Message DuplicateSymbol2(IntermediateSymbol symbol) + { + return Message(symbol.SourceLineNumbers, Ids.DuplicateSymbol2, "Location of symbol related to previous error."); + } + public static Message OrphanedPayload(SourceLineNumber sourceLineNumbers, string payloadId) { return Message(sourceLineNumbers, Ids.OrphanedPayload, "Found orphaned Payload '{0}'. Make sure to reference it from a Package, the BootstrapperApplication, or the Bundle or move it into its own Fragment so it only gets linked in when actually used.", payloadId); @@ -46,9 +81,34 @@ public static Message UncompressedPayloadInContainer(SourceLineNumber sourceLine return Message(sourceLineNumbers, Ids.UncompressedPayloadInContainer, "The payload '{0}' is uncompressed and cannot be added to container '{1}'. Remove its Compressed attribute and provide a @SourceFile value to allow it to be added to a container.", payloadId, containerId); } - public static Message VirtualSymbolNotFoundForOverride(SourceLineNumber sourceLineNumbers, string id) + public static Message VirtualSymbolNotFoundForOverride(IntermediateSymbol symbol) + { + return Message(symbol.SourceLineNumbers, Ids.VirtualSymbolNotFoundForOverride, "Could not find a virtual symbol to override with the {0} symbol '{1}'. Remove the override access modifier or include the code with the virtual symbol.", symbol.Definition.Name, symbol.Id.Id); + } + + public static Message VirtualSymbolNotFoundForOverride(IntermediateSymbol symbol, SourceLineNumber referencingSourceLineNumber) + { + if (referencingSourceLineNumber is null) + { + return VirtualSymbolNotFoundForOverride(symbol); + } + + return Message(symbol.SourceLineNumbers, Ids.VirtualSymbolNotFoundForOverride, "Could not find a virtual symbol to override with the {0} symbol '{1}'. Remove the override access modifier or include the code with the virtual symbol. Referenced from {2}", symbol.Definition.Name, symbol.Id.Id, referencingSourceLineNumber); + } + + public static Message VirtualSymbolMustBeOverridden(IntermediateSymbol symbol) { - return Message(sourceLineNumbers, Ids.VirtualSymbolNotFoundForOverride, "Did not find virtual symbol for override symbol '{0}'",id); + return Message(symbol.SourceLineNumbers, Ids.VirtualSymbolMustBeOverridden, "The {0} symbol '{1}' conflicts with a virtual symbol. Use the 'override' access modifier to override the virtual symbol or use a different Id to avoid the conflict.", symbol.Definition.Name, symbol.Id.Id); + } + + public static Message VirtualSymbolMustBeOverridden(IntermediateSymbol symbol, SourceLineNumber referencingSourceLineNumber) + { + if (referencingSourceLineNumber is null) + { + return VirtualSymbolMustBeOverridden(symbol); + } + + return Message(symbol.SourceLineNumbers, Ids.VirtualSymbolMustBeOverridden, "The {0} symbol '{1}' conflicts with a virtual symbol. Use the 'override' access modifier to override the virtual symbol or use a different Id to avoid the conflict. Referenced from {2}", symbol.Definition.Name, symbol.Id.Id, referencingSourceLineNumber); } private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string format, params object[] args) @@ -58,6 +118,9 @@ private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string public enum Ids { + DuplicateSymbol = 91, + DuplicateSymbol2 = 92, + OrphanedPayload = 7000, PackageInMultipleContainers = 7001, PayloadSharedWithBA = 7002, @@ -67,6 +130,7 @@ public enum Ids BAContainerCannotContainRemotePayload = 7006, DuplicateBindPathVariableOnCommandLine = 7007, VirtualSymbolNotFoundForOverride = 7008, + VirtualSymbolMustBeOverridden = 7009, } // last available is 7099. 7100 is WindowsInstallerBackendWarnings. } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs index d0e317604..5e40114fa 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs @@ -78,8 +78,8 @@ public void CannotCompileDuplicateCrossFragmentReference() var errors = BuildForFailure("TestData", "AccessModifier", "DuplicateCrossFragmentReference.wxs"); WixAssert.CompareLineByLine(new[] { - @"ln 8: Duplicate symbol 'Directory:TestFolder' referenced by \DuplicateCrossFragmentReference.wxs(4). This typically means that an Id is duplicated. 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 12: Duplicate Directory with identifier 'TestFolder' referenced by \\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." }, errors); } @@ -89,7 +89,7 @@ public void CannotCompileOverrideWithoutVirtualSymbol() var errors = BuildForFailure("TestData", "AccessModifier", "OverrideWithoutVirtualSymbol.wxs"); WixAssert.CompareLineByLine(new[] { - "ln 5: Did not find virtual symbol for override symbol 'Directory:TestFolder'", + "ln 5: Could not find a virtual symbol to override with the Directory symbol 'TestFolder'. Remove the override access modifier or include the code with the virtual symbol.", }, errors); } @@ -99,7 +99,40 @@ public void CannotCompileDuplicatedOverride() var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatedOverrideVirtualSymbol.wxs"); WixAssert.CompareLineByLine(new[] { - "ln 14: Duplicate symbol 'Directory:TestFolder' found. This typically means that an Id is duplicated. 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: 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." + }, errors); + } + + [Fact] + public void CannotCompileDuplicatedVirtual() + { + var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatedVirtualSymbol.wxs"); + WixAssert.CompareLineByLine(new[] + { + "ln 6: The virtual Directory with identifier 'TestFolder' is duplicated. Ensure identifiers of a given type (Directory, File, etc.) are unique or did you mean to make one an override for the virtual symbol?", + "ln 10: Location of symbol related to previous error." + }, errors); + } + + [Fact] + public void CannotCompilePublicWithVirtualSymbol() + { + var errors = BuildForFailure("TestData", "AccessModifier", "VirtualSymbolWithoutOverride.wxs"); + WixAssert.CompareLineByLine(new[] + { + "ln 9: 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 5: Location of symbol related to previous error." + }, errors); + } + + [Fact] + public void CannotCompilePublicAndOverrideWithVirtualSymbol() + { + 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." }, errors); } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs index 7bf10e3f8..9931a45ae 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs @@ -4,8 +4,8 @@ namespace WixToolsetTest.CoreIntegration { using System.IO; using System.Linq; - using WixInternal.TestSupport; using WixInternal.Core.TestPackage; + using WixInternal.TestSupport; using WixToolset.Data; using Xunit; @@ -97,8 +97,16 @@ public void DuplicateRegistryValueIdsAreDetectedSmoothly() "-o", msiPath }, out var messages); - Assert.Equal(2, messages.Where(m => m.Id == (int)ErrorMessages.Ids.DuplicateSymbol).Count()); - Assert.Equal(2, messages.Where(m => m.Id == (int)ErrorMessages.Ids.DuplicateSymbol2).Count()); + var errors = messages.Where(m => m.Level == MessageLevel.Error) + .Select(m => $"ln {m.SourceLineNumbers.LineNumber}: {m}".Replace(baseFolder, "").Replace(folder, "")) + .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." + }, errors); } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs index 8d8ac8011..54375f672 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs @@ -66,7 +66,7 @@ public void CannotHaveRollbackBoundaryAndChainPackageWithSameId() WixAssert.CompareLineByLine(new[] { - "Duplicate symbol 'WixChainItem:collision' found. This typically means that an Id is duplicated. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", + "Duplicate WixChainItem with identifier 'collision' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", "Location of symbol related to previous error.", }, result.Messages.Select(m => m.ToString()).ToArray()); diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatePublicOverrideVirtualSymbol.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatePublicOverrideVirtualSymbol.wxs new file mode 100644 index 000000000..7ecf44451 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatePublicOverrideVirtualSymbol.wxs @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatedVirtualSymbol.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatedVirtualSymbol.wxs new file mode 100644 index 000000000..e97d0b57a --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicatedVirtualSymbol.wxs @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolWithoutOverride.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolWithoutOverride.wxs new file mode 100644 index 000000000..3db854f5a --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolWithoutOverride.wxs @@ -0,0 +1,12 @@ + + + + + + + + + + + +