diff --git a/src/Build.UnitTests/Construction/ElementLocation_Tests.cs b/src/Build.UnitTests/Construction/ElementLocation_Tests.cs
index 7a33e0d5343..ce99b967fa5 100644
--- a/src/Build.UnitTests/Construction/ElementLocation_Tests.cs
+++ b/src/Build.UnitTests/Construction/ElementLocation_Tests.cs
@@ -2,9 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.
using System;
+using System.Collections.Generic;
using System.IO;
+using System.Linq;
using System.Reflection;
+using System.Text;
using System.Xml;
+using System.Xml.Linq;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Construction;
using Microsoft.Build.Exceptions;
@@ -13,442 +17,333 @@
using Microsoft.Build.UnitTests.BackEnd;
using Xunit;
-#nullable disable
+#pragma warning disable CA3075 // Insecure DTD processing in XML
+#pragma warning disable IDE0022 // Use expression body for method
+#pragma warning disable IDE0058 // Expression value is never used
+#pragma warning disable SA1010 // Opening square brackets should be spaced correctly
namespace Microsoft.Build.UnitTests.Construction
{
///
- /// Tests for the ElementLocation class
+ /// Unit tests for .
///
+ [Collection("ElementLocation")]
public class ElementLocation_Tests
{
///
- /// Path to the common targets
+ /// Reset the file path cache index to zero. We have tests which validate that
+ /// returns a specific storage type, and
+ /// that requires the index to be within certain ranges.
///
- private string _pathToCommonTargets =
-#if FEATURE_INSTALLED_MSBUILD
- Path.Combine(FrameworkLocationHelper.PathToDotNetFrameworkV45, "Microsoft.Common.targets");
-#else
- Path.Combine(AppContext.BaseDirectory, "Microsoft.Common.targets");
-#endif
+ public ElementLocation_Tests() => ElementLocation.DangerousInternalResetFileIndex();
- ///
- /// Tests constructor specifying only file.
- ///
- [Fact]
- public void ConstructorTest1()
+ [Theory]
+ [MemberData(nameof(GetCreateTestCases))]
+ public void Create(string? file, int line, int column, string typeName)
{
- IElementLocation location = ElementLocation.Create("file", 65536, 0);
- Assert.Equal("file", location.File);
- Assert.Equal(65536, location.Line);
- Assert.Equal(0, location.Column);
- Assert.Contains("RegularElementLocation", location.GetType().FullName);
- }
+ ElementLocation location = ElementLocation.Create(file, line, column);
- ///
- /// Tests constructor specifying only file.
- ///
- [Fact]
- public void ConstructorTest2()
- {
- IElementLocation location = ElementLocation.Create("file", 0, 65536);
- Assert.Equal("file", location.File);
- Assert.Equal(0, location.Line);
- Assert.Equal(65536, location.Column);
- Assert.Contains("RegularElementLocation", location.GetType().FullName);
- }
+ Assert.Equal(file ?? "", location.File);
+ Assert.Equal(line, location.Line);
+ Assert.Equal(column, location.Column);
- ///
- /// Tests constructor specifying only file.
- ///
- [Fact]
- public void ConstructorTest3()
- {
- IElementLocation location = ElementLocation.Create("file", 65536, 65537);
- Assert.Equal("file", location.File);
- Assert.Equal(65536, location.Line);
- Assert.Equal(65537, location.Column);
- Assert.Contains("RegularElementLocation", location.GetType().FullName);
+ Assert.Contains(typeName, location.GetType().FullName);
}
- ///
- /// Test equality
- ///
- [Fact]
- public void Equality()
+ [Theory]
+ [InlineData("file", -1, 0)]
+ [InlineData("file", 0, -1)]
+ [InlineData("file", int.MaxValue, -1)]
+ [InlineData("file", -1, int.MaxValue)]
+ [InlineData("file", -1, -1)]
+ [InlineData("file", int.MinValue, 0)]
+ [InlineData("file", 0, int.MinValue)]
+ [InlineData("file", int.MinValue, int.MinValue)]
+ public void Create_NegativeValuesThrow(string file, int line, int column)
{
- IElementLocation location1 = ElementLocation.Create("file", 65536, 65537);
- IElementLocation location2 = ElementLocation.Create("file", 0, 1);
- IElementLocation location3 = ElementLocation.Create("file", 0, 65537);
- IElementLocation location4 = ElementLocation.Create("file", 65536, 1);
- IElementLocation location5 = ElementLocation.Create("file", 0, 1);
- IElementLocation location6 = ElementLocation.Create("file", 65536, 65537);
-
- Assert.True(location1.Equals(location6));
- Assert.True(location2.Equals(location5));
- Assert.False(location3.Equals(location1));
- Assert.False(location4.Equals(location2));
- Assert.False(location4.Equals(location6));
+ _ = Assert.Throws(
+ () => ElementLocation.Create(file, line, column));
}
- ///
- /// Check it will use large element location when it should.
- /// Using file as BIZARRELY XmlTextReader+StringReader crops or trims.
- ///
[Fact]
- public void TestLargeElementLocationUsedLargeColumn()
+ public void Create_FileIndexPacking()
{
- string file = null;
-
- try
- {
- file = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+ int i = 0;
- File.WriteAllText(file, ObjectModelHelpers.CleanupFileContents("\r\n") + new string(' ', 70000) + @"");
-
- ProjectRootElement.Open(file);
- }
- catch (InvalidProjectFileException ex)
+ for (int j = 0; j < ushort.MaxValue; j++)
{
- Assert.Equal(70012, ex.ColumnNumber);
- Assert.Equal(2, ex.LineNumber);
+ Assert.Contains("SmallFileElementLocation", Next());
}
- finally
- {
- File.Delete(file);
- }
- }
-
- ///
- /// Check it will use large element location when it should.
- /// Using file as BIZARRELY XmlTextReader+StringReader crops or trims.
- ///
- [Fact]
- public void TestLargeElementLocationUsedLargeLine()
- {
- string file = null;
- try
- {
- string longstring = String.Empty;
-
- for (int i = 0; i < 7000; i++)
- {
- longstring += "\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n";
- }
-
- file = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
-
- File.WriteAllText(file, ObjectModelHelpers.CleanupFileContents("\r\n") + longstring + @" ");
+ // If the file index exceed 65,535 items, we use a larger storage type.
+ Assert.Contains("LargeFileElementLocation", Next());
- ProjectRootElement.Open(file);
- }
- catch (InvalidProjectFileException ex)
- {
- Assert.Equal(70002, ex.LineNumber);
- Assert.Equal(2, ex.ColumnNumber);
- }
- finally
- {
- File.Delete(file);
- }
+ string? Next() => ElementLocation.Create("file" + i++, 0, 0).GetType().FullName;
}
- ///
- /// Tests serialization.
- ///
[Fact]
- public void SerializationTest()
+ public void Create_NullFile()
{
- IElementLocation location = ElementLocation.Create("file", 65536, 65537);
-
- TranslationHelpers.GetWriteTranslator().Translate(ref location, ElementLocation.FactoryForDeserialization);
- IElementLocation deserializedLocation = null;
- TranslationHelpers.GetReadTranslator().Translate(ref deserializedLocation, ElementLocation.FactoryForDeserialization);
+ ElementLocation location = ElementLocation.Create(null);
- Assert.Equal(location.File, deserializedLocation.File);
- Assert.Equal(location.Line, deserializedLocation.Line);
- Assert.Equal(location.Column, deserializedLocation.Column);
- Assert.Contains("RegularElementLocation", location.GetType().FullName);
+ Assert.Equal("", location.File);
+ Assert.Equal(0, location.Line);
+ Assert.Equal(0, location.Column);
}
- ///
- /// Tests serialization of empty location.
- ///
- [Fact]
- public void SerializationTestForEmptyLocation()
+ [Theory]
+ [MemberData(nameof(GetCreateTestCases))]
+ public void RoundTripSerialisation(string? file, int line, int column, string typeName)
{
- IElementLocation location = ElementLocation.EmptyLocation;
+ ElementLocation location = ElementLocation.Create(file, line, column);
TranslationHelpers.GetWriteTranslator().Translate(ref location, ElementLocation.FactoryForDeserialization);
- IElementLocation deserializedLocation = null;
+ ElementLocation? deserializedLocation = null;
TranslationHelpers.GetReadTranslator().Translate(ref deserializedLocation, ElementLocation.FactoryForDeserialization);
+ Assert.NotNull(deserializedLocation);
- Assert.Equal(location.File, deserializedLocation.File);
- Assert.Equal(location.Line, deserializedLocation.Line);
- Assert.Equal(location.Column, deserializedLocation.Column);
- Assert.Contains("SmallElementLocation", deserializedLocation.GetType().FullName);
- }
+ Assert.Equal(file ?? "", deserializedLocation.File);
+ Assert.Equal(line, deserializedLocation.Line);
+ Assert.Equal(column, deserializedLocation.Column);
- ///
- /// Tests constructor specifying file, line and column.
- ///
- [Fact]
- public void ConstructorWithIndicesTest_SmallElementLocation()
- {
- IElementLocation location = ElementLocation.Create("file", 65535, 65534);
- Assert.Equal("file", location.File);
- Assert.Equal(65535, location.Line);
- Assert.Equal(65534, location.Column);
- Assert.Contains("SmallElementLocation", location.GetType().FullName);
+ Assert.Contains(typeName, deserializedLocation.GetType().FullName);
}
- ///
- /// Tests constructor specifying file, negative line, column
- ///
- [Fact]
- public void ConstructorWithNegativeIndicesTest1()
- {
- Assert.Throws(() =>
- {
- ElementLocation.Create("file", -1, 2);
- });
- }
- ///
- /// Tests constructor specifying file, line, negative column
- ///
- [Fact]
- public void ConstructorWithNegativeIndicesTest2n()
- {
- Assert.Throws(() =>
- {
- ElementLocation.Create("file", 1, -2);
- });
- }
- ///
- /// Tests constructor with invalid null file.
- ///
- [Fact]
- public void ConstructorTestNullFile()
+ public static IEnumerable GetCreateTestCases()
{
- IElementLocation location = ElementLocation.Create(null);
- Assert.Equal(location.File, String.Empty);
+ yield return [null, 0, 0, "EmptyElementLocation"];
+ yield return ["", 0, 0, "EmptyElementLocation"];
+ yield return ["file", byte.MaxValue, 0, "SmallFileElementLocation"];
+ yield return ["file", byte.MaxValue + 1, 0, "SmallLineElementLocation"];
+ yield return ["file", 0, byte.MaxValue, "SmallFileElementLocation"];
+ yield return ["file", 0, byte.MaxValue + 1, "SmallColumnElementLocation"];
+ yield return ["file", ushort.MaxValue, 0, "SmallLineElementLocation"];
+ yield return ["file", ushort.MaxValue + 1, 0, "LargeLineElementLocation"];
+ yield return ["file", 0, ushort.MaxValue, "SmallColumnElementLocation"];
+ yield return ["file", 0, ushort.MaxValue + 1, "LargeColumnElementLocation"];
+ yield return ["file", ushort.MaxValue, ushort.MaxValue, "LargeFileElementLocation"];
+ yield return ["file", ushort.MaxValue + 1, ushort.MaxValue, "FullElementLocation"];
+ yield return ["file", ushort.MaxValue, ushort.MaxValue + 1, "FullElementLocation"];
+ yield return ["file", ushort.MaxValue + 1, ushort.MaxValue + 1, "FullElementLocation"];
}
- ///
- /// Tests constructor specifying only file.
- ///
[Fact]
- public void ConstructorTest1_SmallElementLocation()
+ public void Equality()
{
- IElementLocation location = ElementLocation.Create("file", 65535, 0);
- Assert.Equal("file", location.File);
- Assert.Equal(65535, location.Line);
- Assert.Equal(0, location.Column);
- Assert.Contains("SmallElementLocation", location.GetType().FullName);
- }
+ ElementLocation location1 = ElementLocation.Create("file", line: 65536, column: 65537);
+ ElementLocation location2 = ElementLocation.Create("file", line: 0, column: 1);
+ ElementLocation location3 = ElementLocation.Create("file", line: 0, column: 65537);
+ ElementLocation location4 = ElementLocation.Create("file", line: 65536, column: 1);
+ ElementLocation location5 = ElementLocation.Create("file", line: 0, column: 1);
+ ElementLocation location6 = ElementLocation.Create("file", line: 65536, column: 65537);
- ///
- /// Tests constructor specifying only file.
- ///
- [Fact]
- public void ConstructorTest2_SmallElementLocation()
- {
- IElementLocation location = ElementLocation.Create("file", 0, 65535);
- Assert.Equal("file", location.File);
- Assert.Equal(0, location.Line);
- Assert.Equal(65535, location.Column);
- Assert.Contains("SmallElementLocation", location.GetType().FullName);
+ Assert.True(location1.Equals(location6));
+ Assert.True(location2.Equals(location5));
+ Assert.False(location3.Equals(location1));
+ Assert.False(location4.Equals(location2));
+ Assert.False(location4.Equals(location6));
}
///
- /// Tests constructor specifying only file.
+ /// Check it will use large element location when it should.
///
[Fact]
- public void ConstructorTest3_SmallElementLocation()
+ public void TestLargeElementLocationUsedLargeColumn()
{
- IElementLocation location = ElementLocation.Create("file", 65535, 65534);
- Assert.Equal("file", location.File);
- Assert.Equal(65535, location.Line);
- Assert.Equal(65534, location.Column);
- Assert.Contains("SmallElementLocation", location.GetType().FullName);
+ StringBuilder xml = new(capacity: 71_000);
+
+ xml.AppendLine(ObjectModelHelpers.CleanupFileContents(
+ """
+
+
+ """));
+ xml.Append(' ', 70_000);
+ xml.Append("""
+
+
+
+ """);
+
+ Assert.Equal(71_000, xml.Capacity);
+
+ XmlDocumentWithLocation doc = new();
+ doc.LoadXml(xml.ToString());
+
+ InvalidProjectFileException ex = Assert.Throws(
+ () => ProjectRootElement.Open(doc));
+
+ Assert.Equal(70_001, ex.ColumnNumber);
+ Assert.Equal(3, ex.LineNumber);
}
///
- /// Tests serialization.
+ /// Check it will use large element location when it should.
///
[Fact]
- public void SerializationTest_SmallElementLocation()
+ public void TestLargeElementLocationUsedLargeLine()
{
- IElementLocation location = ElementLocation.Create("file", 65535, 2);
+ StringBuilder xml = new(capacity: 71_000);
- TranslationHelpers.GetWriteTranslator().Translate(ref location, ElementLocation.FactoryForDeserialization);
- IElementLocation deserializedLocation = null;
- TranslationHelpers.GetReadTranslator().Translate(ref deserializedLocation, ElementLocation.FactoryForDeserialization);
+ xml.Append(ObjectModelHelpers.CleanupFileContents(
+ """
+
+
+ """));
+
+ xml.Append('\n', 70_000);
- Assert.Equal(location.File, deserializedLocation.File);
- Assert.Equal(location.Line, deserializedLocation.Line);
- Assert.Equal(location.Column, deserializedLocation.Column);
- Assert.Contains("SmallElementLocation", location.GetType().FullName);
+ xml.Append(
+ """
+
+
+
+ """);
+
+ Assert.Equal(71_000, xml.Capacity);
+
+ XmlDocumentWithLocation doc = new();
+ doc.LoadXml(xml.ToString());
+
+ InvalidProjectFileException ex = Assert.Throws(
+ () => ProjectRootElement.Open(doc));
+
+ Assert.Equal(70_002, ex.LineNumber);
+ Assert.Equal(5, ex.ColumnNumber);
}
- ///
- /// Test many of the getters
- ///
[Fact]
[Trait("Category", "netcore-osx-failing")]
[Trait("Category", "netcore-linux-failing")]
- public void LocationStringsMedleyReadOnlyLoad()
+ public void LocationsMatchWhenReadOnlyOrWriteable()
{
- string content = ObjectModelHelpers.CleanupFileContents(@"
-
-
-
-
- foo bar
-
+ string content = ObjectModelHelpers.CleanupFileContents(
+ """
+
+
+
+
+ foo bar
+
-
-
+
+
-
+
-
-
+
+
-
-
+
+
-
+
-
-
+
+
-
+
-
+
- ");
+ """);
string readWriteLoadLocations = GetLocations(content, readOnly: false);
string readOnlyLoadLocations = GetLocations(content, readOnly: true);
- Console.WriteLine(readWriteLoadLocations);
-
Helpers.VerifyAssertLineByLine(readWriteLoadLocations, readOnlyLoadLocations);
+
+ static string GetLocations(string content, bool readOnly)
+ {
+ string file = FileUtilities.GetTemporaryFileName();
+
+ XmlDocumentWithLocation doc = new(loadAsReadOnly: readOnly, fullPath: file);
+ doc.LoadXml(content);
+
+ Assert.Equal(readOnly, doc.IsReadOnly);
+
+ XmlNodeList? allNodes = doc.SelectNodes("//*|//@*");
+
+ Assert.NotNull(allNodes);
+
+ StringBuilder locations = new();
+
+ foreach (object node in allNodes)
+ {
+ PropertyInfo? property = node.GetType().GetProperty("Location", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
+
+ var value = (ElementLocation?)property?.GetValue(node, null);
+
+ if (value is not null) // null means attribute is not present
+ {
+ locations.Append(((XmlNode)node).Name).Append("==").Append(((XmlNode)node).Value ?? "").Append(": ").Append(value.LocationString.Replace(file, """c:\foo\bar.csproj""")).Append("\r\n");
+ }
+ }
+
+ return locations.ToString();
+ }
}
- ///
- /// Save read only fails
- ///
[Fact]
- public void SaveReadOnly1()
+ public void SaveReadOnly_FilePath_Throws()
{
- Assert.Throws(() =>
+ XmlDocumentWithLocation doc = new(loadAsReadOnly: true);
+
+ Assert.True(doc.IsReadOnly);
+
+ _ = Assert.Throws(() =>
{
- var doc = new XmlDocumentWithLocation(loadAsReadOnly: true);
- doc.Load(_pathToCommonTargets);
- Assert.True(doc.IsReadOnly);
doc.Save(FileUtilities.GetTemporaryFile());
});
}
- ///
- /// Save read only fails
- ///
[Fact]
[Trait("Category", "netcore-osx-failing")]
[Trait("Category", "netcore-linux-failing")]
- public void SaveReadOnly2()
+ public void SaveReadOnly_Stream_Throws()
{
- var doc = new XmlDocumentWithLocation(loadAsReadOnly: true);
- doc.Load(_pathToCommonTargets);
+ XmlDocumentWithLocation doc = new(loadAsReadOnly: true);
+
Assert.True(doc.IsReadOnly);
- Assert.Throws(() =>
+
+ _ = Assert.Throws(() =>
{
doc.Save(new MemoryStream());
});
}
- ///
- /// Save read only fails
- ///
[Fact]
[Trait("Category", "netcore-osx-failing")]
[Trait("Category", "netcore-linux-failing")]
- public void SaveReadOnly3()
+ public void SaveReadOnly_TextWriter_Throws()
{
- var doc = new XmlDocumentWithLocation(loadAsReadOnly: true);
- doc.Load(_pathToCommonTargets);
+ XmlDocumentWithLocation doc = new(loadAsReadOnly: true);
+
Assert.True(doc.IsReadOnly);
- Assert.Throws(() =>
+
+ _ = Assert.Throws(() =>
{
doc.Save(new StringWriter());
});
}
- ///
- /// Save read only fails
- ///
[Fact]
[Trait("Category", "netcore-osx-failing")]
[Trait("Category", "netcore-linux-failing")]
- public void SaveReadOnly4()
- {
- var doc = new XmlDocumentWithLocation(loadAsReadOnly: true);
- doc.Load(_pathToCommonTargets);
- Assert.True(doc.IsReadOnly);
- using (XmlWriter wr = XmlWriter.Create(new FileStream(FileUtilities.GetTemporaryFileName(), FileMode.Create)))
- {
- Assert.Throws(() =>
- {
- doc.Save(wr);
- });
- }
- }
-
- ///
- /// Get location strings for the content, loading as readonly if specified
- ///
- private string GetLocations(string content, bool readOnly)
+ public void SaveReadOnly_XmlWriter_Throws()
{
- string file = null;
+ XmlDocumentWithLocation doc = new(loadAsReadOnly: true);
- try
- {
- file = FileUtilities.GetTemporaryFileName();
- File.WriteAllText(file, content);
- var doc = new XmlDocumentWithLocation(loadAsReadOnly: readOnly);
- doc.Load(file);
- Assert.Equal(readOnly, doc.IsReadOnly);
- var allNodes = doc.SelectNodes("//*|//@*");
+ Assert.True(doc.IsReadOnly);
- string locations = String.Empty;
- foreach (var node in allNodes)
- {
- foreach (var property in node.GetType().GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance))
- {
- if (property.Name.Equals("Location"))
- {
- var value = ((ElementLocation)property.GetValue(node, null));
-
- if (value != null) // null means attribute is not present
- {
- locations += ((XmlNode)node).Name + "==" + ((XmlNode)node).Value ?? String.Empty + ": " + value.LocationString + "\r\n";
- }
- }
- }
- }
+ using XmlWriter wr = XmlWriter.Create(new MemoryStream());
- return locations.Replace(file, "c:\\foo\\bar.csproj");
- }
- finally
+ _ = Assert.Throws(() =>
{
- File.Delete(file);
- }
+ doc.Save(wr);
+ });
}
}
}
diff --git a/src/Build/ElementLocation/ElementLocation.cs b/src/Build/ElementLocation/ElementLocation.cs
index c9bf42b48b2..7cc973f6f45 100644
--- a/src/Build/ElementLocation/ElementLocation.cs
+++ b/src/Build/ElementLocation/ElementLocation.cs
@@ -2,13 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.
using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
using System.Diagnostics;
+using System.Threading;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Collections;
using Microsoft.Build.Shared;
-#nullable disable
-
namespace Microsoft.Build.Construction
{
///
@@ -18,51 +19,47 @@ namespace Microsoft.Build.Construction
///
/// This object is IMMUTABLE, so that it can be passed around arbitrarily.
/// DO NOT make these objects any larger. There are huge numbers of them and they are transmitted between nodes.
+ ///
+ /// Although this class is called "element" location, it is also used for other XML node types, such as in .
///
[Serializable]
public abstract class ElementLocation : IElementLocation, ITranslatable, IImmutable
{
///
- /// The singleton empty element location.
+ /// Gets the empty element location.
+ /// This is not to be used when something is "missing": that should have a null location.
+ /// It is to be used for the project location when the project has not been given a name.
+ /// In that case, it exists, but can't have a specific location.
///
- private static ElementLocation s_emptyElementLocation = new SmallElementLocation(null, 0, 0);
+ public static ElementLocation EmptyLocation { get; } = new EmptyElementLocation();
///
- /// The file from which this particular element originated. It may
+ /// Gets the file from which this particular element originated. It may
/// differ from the ProjectFile if, for instance, it was part of
/// an import or originated in a targets file.
/// If not known, returns empty string.
///
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public abstract string File
- {
- get;
- }
+ public abstract string File { get; }
///
- /// The line number where this element exists in its file.
+ /// Gets the line number where this element exists in its file.
/// The first line is numbered 1.
/// Zero indicates "unknown location".
///
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public abstract int Line
- {
- get;
- }
+ public abstract int Line { get; }
///
- /// The column number where this element exists in its file.
+ /// Gets the column number where this element exists in its file.
/// The first column is numbered 1.
/// Zero indicates "unknown location".
///
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public abstract int Column
- {
- get;
- }
+ public abstract int Column { get; }
///
- /// The location in a form suitable for replacement
+ /// Gets the location in a form suitable for replacement
/// into a message.
/// Example: "c:\foo\bar.csproj (12,34)"
/// Calling this creates and formats a new string.
@@ -71,43 +68,30 @@ public abstract class ElementLocation : IElementLocation, ITranslatable, IImmuta
///
public string LocationString
{
- get { return GetLocationString(File, Line, Column); }
- }
-
- ///
- /// Gets the empty element location.
- /// This is not to be used when something is "missing": that should have a null location.
- /// It is to be used for the project location when the project has not been given a name.
- /// In that case, it exists, but can't have a specific location.
- ///
- public static ElementLocation EmptyLocation
- {
- get { return s_emptyElementLocation; }
+ get
+ {
+ int line = Line;
+ int column = Column;
+ return (line, column) switch
+ {
+ (not 0, not 0) => ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("FileLocation", File, line, column),
+ (not 0, 0) => $"{File} ({line})",
+ _ => File,
+ };
+ }
}
- ///
- /// Get reasonable hash code.
- ///
+ ///
public override int GetHashCode()
{
- // Line and column are good enough
- return Line.GetHashCode() ^ Column.GetHashCode();
+ // We don't include the file path in the hash
+ return (Line * 397) ^ Column;
}
- ///
- /// Override Equals so that identical
- /// fields imply equal objects.
- ///
- public override bool Equals(object obj)
+ ///
+ public override bool Equals(object? obj)
{
- if (obj == null)
- {
- return false;
- }
-
- IElementLocation that = obj as IElementLocation;
-
- if (that == null)
+ if (obj is not IElementLocation that)
{
return false;
}
@@ -125,23 +109,19 @@ public override bool Equals(object obj)
return true;
}
- ///
- /// Location of element.
- ///
+ ///
public override string ToString()
{
return LocationString;
}
- ///
- /// Writes the packet to the serializer.
- /// Always send as ints, even if ushorts are being used: otherwise it'd
- /// need a byte to discriminate and the savings would be microscopic.
- ///
+ ///
void ITranslatable.Translate(ITranslator translator)
{
ErrorUtilities.VerifyThrow(translator.Mode == TranslationDirection.WriteToStream, "write only");
+ // Translate int, even if ushort is being used.
+ // Internally, the translator uses a variable length (prefix) encoding.
string file = File;
int line = Line;
int column = Column;
@@ -156,7 +136,7 @@ void ITranslatable.Translate(ITranslator translator)
///
internal static ElementLocation FactoryForDeserialization(ITranslator translator)
{
- string file = null;
+ string? file = null;
int line = 0;
int column = 0;
translator.Translate(ref file);
@@ -171,11 +151,22 @@ internal static ElementLocation FactoryForDeserialization(ITranslator translator
/// This is the case when we are creating a new item, for example, and it has
/// not been evaluated from some XML.
///
- internal static ElementLocation Create(string file)
+ internal static ElementLocation Create(string? file)
{
return Create(file, 0, 0);
}
+ private static string[] s_fileByIndex = new string[32];
+ private static int s_nextFileIndex;
+ private static ImmutableDictionary s_indexByFile = ImmutableDictionary.Empty.WithComparers(StringComparer.OrdinalIgnoreCase);
+
+ internal static void DangerousInternalResetFileIndex()
+ {
+ s_nextFileIndex = 0;
+ s_fileByIndex = new string[32];
+ s_indexByFile = ImmutableDictionary.Empty.WithComparers(StringComparer.OrdinalIgnoreCase);
+ }
+
///
/// Constructor for the case where we have most or all information.
/// Numerical values must be 1-based, non-negative; 0 indicates unknown
@@ -185,192 +176,288 @@ internal static ElementLocation Create(string file)
/// In AG there are 600 locations that have a file but zero line and column.
/// In theory yet another derived class could be made for these to save 4 bytes each.
///
- internal static ElementLocation Create(string file, int line, int column)
+ internal static ElementLocation Create(string? filePath, int line, int column)
{
- if (string.IsNullOrEmpty(file) && line == 0 && column == 0)
+ // Combine line and column values with bitwise OR so we can perform various
+ // checks on both values in a single comparison, reducing the amount of branching
+ // in the code.
+ int combinedValue = line | column;
+
+ if (string.IsNullOrEmpty(filePath) && combinedValue == 0)
{
- return EmptyLocation;
+ // When combinedValue is zero, it implies that both line and column are zero.
+ return EmptyElementLocation.Instance;
}
- if (line <= 65535 && column <= 65535)
+ // When combinedValue is negative, it implies that either line or column were negative.
+ ErrorUtilities.VerifyThrow(combinedValue > -1, "Use zero for unknown");
+
+ // TODO store the last run's value and check if this is for the same file. If so, skip the dictionary lookup (tree walk).
+ int fileIndex = GetOrAddFileIndex(filePath);
+
+#if DEBUG
+ string lookedUpFilePath = LookupFileByIndex(fileIndex);
+ if (!StringComparer.OrdinalIgnoreCase.Equals(filePath ?? "", lookedUpFilePath))
{
- return new ElementLocation.SmallElementLocation(file, line, column);
+ Debug.Fail($"File index {fileIndex} returned for path '{filePath}', but lookup for that index returns '{lookedUpFilePath}'.");
+ }
+#endif
+
+ // We use multiple packing schemes for this data. TypeSize below excludes the CLR's per-object overhead.
+ //
+ // Name TypeSize FileIndex Line Column
+ //
+ // EmptyElementLocation 0 0 (max 0) 0 (max 0) 0 (max 0)
+ //
+ // SmallFileElementLocation 8 4 (max 65,535) 2 (max 256) 2 (max 256)
+ // SmallLineElementLocation 8 2 (max 256) 4 (max 65,535) 2 (max 256)
+ // SmallColumnElementLocation 8 2 (max 256) 2 (max 256) 4 (max 65,535)
+ //
+ // LargeFileElementLocation 16 8 (max 2b) 4 (max 65,535) 4 (max 65,535)
+ // LargeLineElementLocation 16 4 (max 65,535) 8 (max 2b) 4 (max 65,535)
+ // LargeColumnElementLocation 16 4 (max 65,535) 4 (max 65,535) 8 (max 2b)
+ //
+ // FullElementLocation 24 8 (max 2b) 8 (max 2b) 8 (max 2b)
+
+ // Check for empty first
+ if (fileIndex is 0 && line is 0 && column is 0)
+ {
+ return EmptyElementLocation.Instance;
}
- return new ElementLocation.RegularElementLocation(file, line, column);
- }
+ combinedValue |= fileIndex;
- ///
- /// The location in a form suitable for replacement
- /// into a message.
- /// Example: "c:\foo\bar.csproj (12,34)"
- /// Calling this creates and formats a new string.
- /// PREFER TO PUT THE LOCATION INFORMATION AT THE START OF THE MESSAGE INSTEAD.
- /// Only in rare cases should the location go within the message itself.
- ///
- private static string GetLocationString(string file, int line, int column)
- {
- string locationString;
- if (line != 0 && column != 0)
+ // We want class sizes as multiples of 8 on 64-bit architectures, for alignment reasons.
+ // Any space used between these multiples leads to unused bytes in padding. We want to
+ // take advantage of these sizes effectively, so we check for various requirements and
+ // choose.
+ //
+ // Search in popularity order to reduce the number of branches taken through this code.
+ //
+ // When combinedValue is less than a threshold, it implies that both line and column are less
+ // than that threshold.
+
+ // Handle cases that fit in 0xFF and 0XFFFF.
+ if (combinedValue <= byte.MaxValue)
{
- locationString = ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("FileLocation", file, line, column);
+ // All values fit within a byte. Pick one of the 8-byte types.
+ return new SmallFileElementLocation((ushort)fileIndex, (byte)line, (byte)column);
}
- else if (line != 0)
+ else if (combinedValue <= ushort.MaxValue)
{
- locationString = file + " (" + line + ")";
+ // At least one value needs ushort. Try to use an 8-byte type all the same.
+ if (line <= byte.MaxValue && column <= byte.MaxValue)
+ {
+ // Only fileIndex needs 4 bytes
+ return new SmallFileElementLocation((ushort)fileIndex, (byte)line, (byte)column);
+ }
+ else if (fileIndex <= byte.MaxValue && column <= byte.MaxValue)
+ {
+ // Only line needs 4 bytes
+ return new SmallLineElementLocation((byte)fileIndex, (ushort)line, (byte)column);
+ }
+ else if (fileIndex <= byte.MaxValue && line <= byte.MaxValue)
+ {
+ // Only column needs 4 bytes
+ return new SmallColumnElementLocation((byte)fileIndex, (byte)line, (ushort)column);
+ }
+ else
+ {
+ // All three values need ushort. Choose an implementation that gives the file
+ // index an easily-read value (i.e. within 4 bytes) to simplify reads. The
+ // assumption is that if you need line, you probably also need column.
+ return new LargeFileElementLocation(fileIndex, (ushort)line, (ushort)column);
+ }
}
else
{
- locationString = file;
+ // At least one value needs int.
+ if (fileIndex <= short.MaxValue && column <= short.MaxValue)
+ {
+ // Only line needs 8 bytes
+ return new LargeLineElementLocation((ushort)fileIndex, line, (ushort)column);
+ }
+ else if (line <= short.MaxValue && column <= short.MaxValue)
+ {
+ // Only fileIndex needs 8 bytes
+ return new LargeFileElementLocation(fileIndex, (ushort)line, (ushort)column);
+ }
+ else if (fileIndex <= short.MaxValue && line <= short.MaxValue)
+ {
+ // Only column needs 8 bytes
+ return new LargeColumnElementLocation((ushort)fileIndex, (ushort)line, column);
+ }
+ else
+ {
+ return new FullElementLocation(fileIndex, line, column);
+ }
}
- return locationString;
- }
+ static int GetOrAddFileIndex(string? file)
+ {
+ if (file is null)
+ {
+ return 0;
+ }
- ///
- /// Rarer variation for when the line and column won't each fit in a ushort.
- ///
- private class RegularElementLocation : ElementLocation
- {
- ///
- /// The source file.
- ///
- private string file;
+ if (s_indexByFile.TryGetValue(file, out int index))
+ {
+ return index + 1;
+ }
- ///
- /// The source line.
- ///
- private int line;
+ return AddFile();
- ///
- /// The source column.
- ///
- private int column;
+ int AddFile()
+ {
+ int index = Interlocked.Increment(ref s_nextFileIndex) - 1;
- ///
- /// Constructor for the case where we have most or all information.
- /// Numerical values must be 1-based, non-negative; 0 indicates unknown
- /// File may be null, indicating the file was not loaded from disk.
- ///
- internal RegularElementLocation(string file, int line, int column)
- {
- ErrorUtilities.VerifyThrowArgumentLengthIfNotNull(file, nameof(file));
- ErrorUtilities.VerifyThrow(line > -1 && column > -1, "Use zero for unknown");
+ SetValue(index);
- this.file = file ?? String.Empty;
- this.line = line;
- this.column = column;
- }
+ _ = ImmutableInterlocked.TryAdd(ref s_indexByFile, file, index);
- ///
- /// The file from which this particular element originated. It may
- /// differ from the ProjectFile if, for instance, it was part of
- /// an import or originated in a targets file.
- /// If not known, returns empty string.
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override string File
- {
- get { return file; }
+ return index + 1;
+ }
+
+ void SetValue(int index)
+ {
+ while (true)
+ {
+ string[] array = Volatile.Read(ref s_fileByIndex);
+
+ if (index < array.Length)
+ {
+ array[index] = file;
+ return;
+ }
+
+ // Need to grow the array
+
+ // Wait for the last value to be non-null, so that we have all values to copy
+ while (array[array.Length - 1] is null)
+ {
+ Thread.SpinWait(100);
+ }
+
+ int newArrayLength = array.Length * 2;
+
+ while (index >= newArrayLength)
+ {
+ newArrayLength *= 2;
+ }
+
+ string[] newArray = new string[newArrayLength];
+ array.AsSpan().CopyTo(newArray);
+ newArray[index] = file;
+
+ string[] exchanged = Interlocked.CompareExchange(ref s_fileByIndex, newArray, array);
+
+ if (ReferenceEquals(exchanged, array))
+ {
+ // We replaced it
+ return;
+ }
+
+ // Otherwise, loop around again. We can't just return exchanged here,
+ // as theoretically the array might have been grown more than once.
+ }
+ }
}
+ }
- ///
- /// The line number where this element exists in its file.
- /// The first line is numbered 1.
- /// Zero indicates "unknown location".
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override int Line
+ internal static string LookupFileByIndex(int index)
+ {
+ if (index is 0)
{
- get { return line; }
+ return "";
}
- ///
- /// The column number where this element exists in its file.
- /// The first column is numbered 1.
- /// Zero indicates "unknown location".
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override int Column
+ index -= 1;
+
+ Thread.MemoryBarrier();
+
+ string[] array = Volatile.Read(ref s_fileByIndex);
+
+ while (index >= array.Length)
{
- get { return column; }
+ // Data race! Spin.
+ array = Volatile.Read(ref s_fileByIndex);
}
+
+ return array[index];
}
- ///
- /// For when the line and column each fit in a short - under 65536
- /// (almost always will: microsoft.common.targets is less than 5000 lines long)
- /// When loading Australian Government, for example, there are over 31,000 ElementLocation
- /// objects so this saves 4 bytes each = 123KB
- ///
- /// A "very small" variation that used two bytes (or halves of a short) would fit about half of them
- /// and save 4 more bytes each, but the CLR packs each field to 4 bytes, so it isn't actually any smaller.
- ///
- private class SmallElementLocation : ElementLocation
- {
- ///
- /// The source file.
- ///
- private string file;
+ #region Element implementations
- ///
- /// The source line.
- ///
- private ushort line;
+#pragma warning disable SA1516 // Elements should be separated by blank line
+ private sealed class EmptyElementLocation() : ElementLocation
+ {
///
- /// The source column.
+ /// Gets the singleton, immutable empty element location.
///
- private ushort column;
+ ///
+ /// Not to be be used when something is "missing". Use a location for that.
+ /// Use only for the project location when the project has not been given a name.
+ /// In that case, it exists, but can't have a specific location.
+ ///
+ public static EmptyElementLocation Instance { get; } = new();
+
+ public override string File => "";
+ public override int Line => 0;
+ public override int Column => 0;
+ }
- ///
- /// Constructor for the case where we have most or all information.
- /// Numerical values must be 1-based, non-negative; 0 indicates unknown
- /// File may be null or empty, indicating the file was not loaded from disk.
- ///
- internal SmallElementLocation(string file, int line, int column)
- {
- ErrorUtilities.VerifyThrow(line > -1 && column > -1, "Use zero for unknown");
- ErrorUtilities.VerifyThrow(line <= 65535 && column <= 65535, "Use ElementLocation instead");
+ private sealed class SmallFileElementLocation(ushort file, byte line, byte column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
+
+ private sealed class SmallLineElementLocation(byte file, ushort line, byte column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
+
+ private sealed class SmallColumnElementLocation(byte file, byte line, ushort column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
- this.file = file ?? String.Empty;
- this.line = Convert.ToUInt16(line);
- this.column = Convert.ToUInt16(column);
- }
+ private sealed class LargeFileElementLocation(int file, ushort line, ushort column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
- ///
- /// The file from which this particular element originated. It may
- /// differ from the ProjectFile if, for instance, it was part of
- /// an import or originated in a targets file.
- /// If not known, returns empty string.
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override string File
- {
- get { return file; }
- }
+ private sealed class LargeLineElementLocation(ushort file, int line, ushort column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
- ///
- /// The line number where this element exists in its file.
- /// The first line is numbered 1.
- /// Zero indicates "unknown location".
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override int Line
- {
- get { return (int)line; }
- }
+ private sealed class LargeColumnElementLocation(ushort file, ushort line, int column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
+ }
- ///
- /// The column number where this element exists in its file.
- /// The first column is numbered 1.
- /// Zero indicates "unknown location".
- ///
- [DebuggerBrowsable(DebuggerBrowsableState.Never)]
- public override int Column
- {
- get { return (int)column; }
- }
+ private sealed class FullElementLocation(int file, int line, int column) : ElementLocation
+ {
+ public override string File => LookupFileByIndex(file);
+ public override int Line => line;
+ public override int Column => column;
}
+
+#pragma warning restore SA1516 // Elements should be separated by blank line
+
+ #endregion
}
}
diff --git a/src/Build/ElementLocation/XmlDocumentWithLocation.cs b/src/Build/ElementLocation/XmlDocumentWithLocation.cs
index 71f51e9f272..b70dcd17d88 100644
--- a/src/Build/ElementLocation/XmlDocumentWithLocation.cs
+++ b/src/Build/ElementLocation/XmlDocumentWithLocation.cs
@@ -84,6 +84,16 @@ internal XmlDocumentWithLocation(bool? loadAsReadOnly)
_loadAsReadOnly = loadAsReadOnly;
}
+ ///
+ /// Constructor that allows specifying the full path. Intended for unit tests only,
+ /// to avoid having to files to disk during tests just to see location paths populated.
+ ///
+ internal XmlDocumentWithLocation(bool? loadAsReadOnly, string fullPath)
+ : this(loadAsReadOnly)
+ {
+ _fullPath = fullPath;
+ }
+
///
/// Whether to load files read only
///
diff --git a/src/Shared/ErrorUtilities.cs b/src/Shared/ErrorUtilities.cs
index 9bbd30e09c8..446e1918b25 100644
--- a/src/Shared/ErrorUtilities.cs
+++ b/src/Shared/ErrorUtilities.cs
@@ -537,17 +537,19 @@ internal static void VerifyThrowArgumentInvalidPath(string parameter, string par
}
}
+#nullable enable
///
/// Throws an ArgumentException if the string has zero length, unless it is
/// null, in which case no exception is thrown.
///
- internal static void VerifyThrowArgumentLengthIfNotNull(string parameter, string parameterName)
+ internal static void VerifyThrowArgumentLengthIfNotNull(string? parameter, string parameterName)
{
if (parameter?.Length == 0)
{
ThrowArgumentLength(parameterName);
}
}
+#nullable disable
///
/// Throws an ArgumentNullException if the given parameter is null.