From 0bf10d9873577f8c943cae1f50dedf68565847ee Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Sat, 16 Dec 2023 09:03:44 -0800 Subject: 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 ------ .../Link/FindEntrySectionAndLoadSymbolsCommand.cs | 6 +- .../Link/ReportConflictingSymbolsCommand.cs | 61 +++++++++++++------ .../Link/ResolveReferencesCommand.cs | 57 +++++++++--------- src/wix/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 ++++++ .../AccessModifier/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 889d1762..e149d54a 100644 --- a/src/api/wix/WixToolset.Data/ErrorMessages.cs +++ b/src/api/wix/WixToolset.Data/ErrorMessages.cs @@ -323,21 +323,6 @@ namespace WixToolset.Data 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 @@ namespace WixToolset.Data 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 e614d4eb..f9d6ab59 100644 --- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs @@ -36,7 +36,7 @@ namespace WixToolset.Core.Link /// /// 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 @@ namespace WixToolset.Core.Link { 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 3a07d366..2851fa60 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 @@ namespace WixToolset.Core.Link private IMessaging Messaging { get; } - private 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 0edbf39c..0e7a7a52 100644 --- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs +++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs @@ -27,9 +27,9 @@ namespace WixToolset.Core.Link 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 @@ namespace WixToolset.Core.Link // 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 @@ namespace WixToolset.Core.Link } 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 @@ namespace WixToolset.Core.Link 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 979aa44f..5bdf8360 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 @@ namespace WixToolset.Core.Link /// 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 @@ namespace WixToolset.Core.Link 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 78cd76f0..c234087e 100644 --- a/src/wix/WixToolset.Core/LinkerErrors.cs +++ b/src/wix/WixToolset.Core/LinkerErrors.cs @@ -11,6 +11,41 @@ namespace WixToolset.Core 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 @@ namespace WixToolset.Core 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 @@ namespace WixToolset.Core public enum Ids { + DuplicateSymbol = 91, + DuplicateSymbol2 = 92, + OrphanedPayload = 7000, PackageInMultipleContainers = 7001, PayloadSharedWithBA = 7002, @@ -67,6 +130,7 @@ namespace WixToolset.Core 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 d0e31760..5e40114f 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs @@ -78,8 +78,8 @@ namespace WixToolsetTest.CoreIntegration 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 @@ namespace WixToolsetTest.CoreIntegration 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 @@ namespace WixToolsetTest.CoreIntegration 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 7bf10e3f..9931a45a 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 @@ namespace WixToolsetTest.CoreIntegration "-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 8d8ac801..54375f67 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs @@ -66,7 +66,7 @@ namespace WixToolsetTest.CoreIntegration 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 00000000..7ecf4445 --- /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 00000000..e97d0b57 --- /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 00000000..3db854f5 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolWithoutOverride.wxs @@ -0,0 +1,12 @@ + + + + + + + + + + + + -- cgit v1.2.3-55-g6feb