From 7a3e122c730421c96157ec918606915861b591c9 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 8 Apr 2015 12:30:14 -0700 Subject: [PATCH 1/3] * Adding FilePath to SourceLocation * Using SourceLocation.FilePath when printing line pragmas, if available. --- .../Common/HashCodeCombiner.cs | 3 +- .../CodeBuilder/CSharp/CSharpCodeWriter.cs | 10 +- .../CSharp/Visitors/CSharpCodeVisitor.cs | 2 +- .../Properties/RazorResources.Designer.cs | 16 ++ .../RazorResources.resx | 3 + src/Microsoft.AspNet.Razor/SourceLocation.cs | 145 ++++++++-- .../Text/SourceLocationTracker.cs | 6 +- .../Compiler/CSharp/CSharpCodeWriterTest.cs | 48 ++++ .../RazorErrorTest.cs | 24 +- .../SourceLocationTest.cs | 255 ++++++++++++++++++ 10 files changed, 480 insertions(+), 32 deletions(-) create mode 100644 test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs diff --git a/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs b/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs index 041bd911b..5dfec7cbb 100644 --- a/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs +++ b/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs @@ -49,7 +49,8 @@ public HashCodeCombiner Add(object o) public HashCodeCombiner Add(TValue value, IEqualityComparer comparer) { - return Add(comparer.GetHashCode(value)); + var hashCode = value != null ? comparer.GetHashCode(value) : 0; + return Add(0); } public static HashCodeCombiner Start() diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs index 15b569fd8..2d39947dc 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs @@ -190,18 +190,18 @@ public CSharpCodeWriter WriteReturn(string value, bool endLine) /// The current instance of . public CSharpCodeWriter WriteLineNumberDirective(SourceLocation location, string file) { - return WriteLineNumberDirective(location.LineIndex + 1, file); - } + if (!string.IsNullOrEmpty(location.FilePath)) + { + file = location.FilePath; + } - public CSharpCodeWriter WriteLineNumberDirective(int lineNumber, string file) - { if (!string.IsNullOrEmpty(LastWrite) && !LastWrite.EndsWith(NewLine, StringComparison.Ordinal)) { WriteLine(); } - var lineNumberAsString = lineNumber.ToString(CultureInfo.InvariantCulture); + var lineNumberAsString = (location.LineIndex + 1).ToString(CultureInfo.InvariantCulture); return Write("#line ").Write(lineNumberAsString).Write(" \"").Write(file).WriteLine("\""); } diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs index 4742c9b2f..e2cc70e5e 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs @@ -371,7 +371,7 @@ public void RenderDesignTimeExpressionBlockChunk(ExpressionBlockChunk chunk) var documentLocation = firstChild.Association.Start; // This is only here to enable accurate formatting by the C# editor. - Writer.WriteLineNumberDirective(documentLocation.LineIndex + 1, Context.SourceFile); + Writer.WriteLineNumberDirective(documentLocation, Context.SourceFile); // We build the padding with an offset of the design time assignment statement. Writer.Write(_paddingBuilder.BuildExpressionPadding((Span)firstChild.Association, designTimeAssignment.Length)) diff --git a/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs b/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs index 39a2afc85..84bfaa098 100644 --- a/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs +++ b/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs @@ -1466,6 +1466,22 @@ internal static string FormatRewriterError_EmptyTagHelperBoundAttribute(object p return string.Format(CultureInfo.CurrentCulture, GetString("RewriterError_EmptyTagHelperBoundAttribute"), p0, p1, p2); } + /// + /// Cannot perform operations on '{0}' instances with different file paths. + /// + internal static string SourceLocationFilePathDoesNotMatch + { + get { return GetString("SourceLocationFilePathDoesNotMatch"); } + } + + /// + /// Cannot perform operations on '{0}' instances with different file paths. + /// + internal static string FormatSourceLocationFilePathDoesNotMatch(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("SourceLocationFilePathDoesNotMatch"), p0); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Razor/RazorResources.resx b/src/Microsoft.AspNet.Razor/RazorResources.resx index c206b3cc5..e1c4a407f 100644 --- a/src/Microsoft.AspNet.Razor/RazorResources.resx +++ b/src/Microsoft.AspNet.Razor/RazorResources.resx @@ -410,4 +410,7 @@ Instead, wrap the contents of the block in "{{}}": Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of type '{2}' cannot be empty or contain only whitespace. + + Cannot perform operations on '{0}' instances with different file paths. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Razor/SourceLocation.cs b/src/Microsoft.AspNet.Razor/SourceLocation.cs index 6ee98d7a4..7bc2bb4d0 100644 --- a/src/Microsoft.AspNet.Razor/SourceLocation.cs +++ b/src/Microsoft.AspNet.Razor/SourceLocation.cs @@ -4,9 +4,13 @@ using System; using System.Globalization; using Microsoft.AspNet.Razor.Text; +using Microsoft.Internal.Web.Utils; namespace Microsoft.AspNet.Razor { + /// + /// A location in a Razor file. + /// #if NET45 // No Serializable attribute in CoreCLR (no need for it anymore?) [Serializable] @@ -16,13 +20,38 @@ public struct SourceLocation : IEquatable, IComparable + /// Initializes a new instance of . + /// + /// The absolute index. + /// The line index. + /// The character index. public SourceLocation(int absoluteIndex, int lineIndex, int characterIndex) + : this(filePath: null, absoluteIndex: absoluteIndex, lineIndex: lineIndex, characterIndex: characterIndex) + { + } + + /// + /// Initializes a new instance of . + /// + /// The file path. + /// The absolute index. + /// The line index. + /// The character index. + public SourceLocation(string filePath, int absoluteIndex, int lineIndex, int characterIndex) { + FilePath = filePath; AbsoluteIndex = absoluteIndex; LineIndex = lineIndex; CharacterIndex = characterIndex; } + /// + /// Path of the file. + /// + /// When null, the parser assumes it is the file currently being processed. + public string FilePath { get; set; } + /// Set property is only accessible for deserialization purposes. public int AbsoluteIndex { get; set; } @@ -35,6 +64,7 @@ public SourceLocation(int absoluteIndex, int lineIndex, int characterIndex) /// Set property is only accessible for deserialization purposes. public int CharacterIndex { get; set; } + /// public override string ToString() { return string.Format( @@ -45,30 +75,52 @@ public override string ToString() CharacterIndex); } + + /// public override bool Equals(object obj) { return (obj is SourceLocation) && Equals((SourceLocation)obj); } + /// public override int GetHashCode() { // LineIndex and CharacterIndex can be calculated from AbsoluteIndex and the document content. - return AbsoluteIndex; + return HashCodeCombiner.Start() + .Add(FilePath, StringComparer.Ordinal) + .Add(AbsoluteIndex) + .CombinedHash; } + /// public bool Equals(SourceLocation other) { - return + return string.Equals(FilePath, other.FilePath, StringComparison.Ordinal) && AbsoluteIndex == other.AbsoluteIndex && LineIndex == other.LineIndex && CharacterIndex == other.CharacterIndex; } + /// + /// if the of the left and right operands + /// are different. public int CompareTo(SourceLocation other) { + var filePathOrdinal = string.Compare(FilePath, other.FilePath, StringComparison.Ordinal); + if (filePathOrdinal != 0) + { + return filePathOrdinal; + } + return AbsoluteIndex.CompareTo(other.AbsoluteIndex); } + /// + /// Advances the by the length of the . + /// + /// The to advance. + /// The to advance by. + /// The advanced . public static SourceLocation Advance(SourceLocation left, string text) { var tracker = new SourceLocationTracker(left); @@ -76,12 +128,37 @@ public static SourceLocation Advance(SourceLocation left, string text) return tracker.CurrentLocation; } - public static SourceLocation Add(SourceLocation left, SourceLocation right) + private static SourceLocation CreateUndefined() { + var sl = new SourceLocation(); + sl.AbsoluteIndex = -1; + sl.LineIndex = -1; + sl.CharacterIndex = -1; + return sl; + } + + /// + /// Adds two s. + /// + /// The left operand. + /// The right operand. + /// A that is the sum of the left and right operands. + /// if the of the left and right operands + /// are different. + public static SourceLocation operator +(SourceLocation left, SourceLocation right) + { + if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal)) + { + throw new ArgumentException( + RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation)), + nameof(right)); + } + if (right.LineIndex > 0) { // Column index doesn't matter return new SourceLocation( + left.FilePath, left.AbsoluteIndex + right.AbsoluteIndex, left.LineIndex + right.LineIndex, right.CharacterIndex); @@ -89,57 +166,79 @@ public static SourceLocation Add(SourceLocation left, SourceLocation right) else { return new SourceLocation( + left.FilePath, left.AbsoluteIndex + right.AbsoluteIndex, left.LineIndex + right.LineIndex, left.CharacterIndex + right.CharacterIndex); } } - public static SourceLocation Subtract(SourceLocation left, SourceLocation right) + /// + /// Subtracts two s. + /// + /// The left operand. + /// The right operand. + /// A that is the difference of the left and right operands. + /// if the of the left and right operands + /// are different. + public static SourceLocation operator -(SourceLocation left, SourceLocation right) { + if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal)) + { + throw new ArgumentException( + RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation)), + nameof(right)); + } + return new SourceLocation( + left.FilePath, left.AbsoluteIndex - right.AbsoluteIndex, left.LineIndex - right.LineIndex, left.LineIndex != right.LineIndex ? left.CharacterIndex : left.CharacterIndex - right.CharacterIndex); } - private static SourceLocation CreateUndefined() - { - var sl = new SourceLocation(); - sl.AbsoluteIndex = -1; - sl.LineIndex = -1; - sl.CharacterIndex = -1; - return sl; - } - + /// + /// Determines whether the first operand is lesser than the second operand. + /// + /// The left operand. + /// The right operand. + /// true if is lesser than . public static bool operator <(SourceLocation left, SourceLocation right) { return left.CompareTo(right) < 0; } + /// + /// Determines whether the first operand is greater than the second operand. + /// + /// The left operand. + /// The right operand. + /// true if is greater than . public static bool operator >(SourceLocation left, SourceLocation right) { return left.CompareTo(right) > 0; } + /// + /// Determines whether the operands are equal. + /// + /// The left operand. + /// The right operand. + /// true if and are equal. public static bool operator ==(SourceLocation left, SourceLocation right) { return left.Equals(right); } + /// + /// Determines whether the operands are not equal. + /// + /// The left operand. + /// The right operand. + /// true if and are not equal. public static bool operator !=(SourceLocation left, SourceLocation right) { return !left.Equals(right); } - - public static SourceLocation operator +(SourceLocation left, SourceLocation right) - { - return Add(left, right); - } - - public static SourceLocation operator -(SourceLocation left, SourceLocation right) - { - return Subtract(left, right); - } } } diff --git a/src/Microsoft.AspNet.Razor/Text/SourceLocationTracker.cs b/src/Microsoft.AspNet.Razor/Text/SourceLocationTracker.cs index a1479c31d..6b825c91a 100644 --- a/src/Microsoft.AspNet.Razor/Text/SourceLocationTracker.cs +++ b/src/Microsoft.AspNet.Razor/Text/SourceLocationTracker.cs @@ -85,7 +85,11 @@ private void UpdateInternalState() private void RecalculateSourceLocation() { - _currentLocation = new SourceLocation(_absoluteIndex, _lineIndex, _characterIndex); + _currentLocation = new SourceLocation( + _currentLocation.FilePath, + _absoluteIndex, + _lineIndex, + _characterIndex); } public static SourceLocation CalculateNewLocation(SourceLocation lastPosition, string newContent) diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs b/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs new file mode 100644 index 000000000..8bc50b47b --- /dev/null +++ b/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp +{ + public class CSharpCodeWriterTest + { + [Theory] + [InlineData(null)] + [InlineData("")] + public void WriteLineNumberDirective_UsesFilePath_WhenFileInSourceLocationIsNullOrEmpty( + string sourceLocationFilePath) + { + // Arrange + var filePath = "some-path"; + var writer = new CSharpCodeWriter(); + var expected = $"#line 5 \"{filePath}\"" + writer.NewLine; + var sourceLocation = new SourceLocation(sourceLocationFilePath, 10, 4, 3); + + // Act + writer.WriteLineNumberDirective(sourceLocation, filePath); + var code = writer.GenerateCode(); + + // Assert + Assert.Equal(expected, code); + } + + [Fact] + public void WriteLineNumberDirectives_UsesSourceLocationFilePath_IfAvailable() + { + // Arrange + var filePath = "some-path"; + var sourceLocationFilePath = "source-location-file-path"; + var writer = new CSharpCodeWriter(); + var expected = $"#line 5 \"{sourceLocationFilePath}\"" + writer.NewLine; + var sourceLocation = new SourceLocation(sourceLocationFilePath, 10, 4, 3); + + // Act + writer.WriteLineNumberDirective(sourceLocation, filePath); + var code = writer.GenerateCode(); + + // Assert + Assert.Equal(expected, code); + } + } +} diff --git a/test/Microsoft.AspNet.Razor.Test/RazorErrorTest.cs b/test/Microsoft.AspNet.Razor.Test/RazorErrorTest.cs index 3abe46fcc..75616cdb9 100644 --- a/test/Microsoft.AspNet.Razor.Test/RazorErrorTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/RazorErrorTest.cs @@ -19,6 +19,7 @@ public void RazorError_CanBeSerialized() length: 456); var expectedSerializedError = $"{{\"{nameof(RazorError.Message)}\":\"Testing\",\"{nameof(RazorError.Location)}\":{{\"" + + $"{nameof(SourceLocation.FilePath)}\":null,\"" + $"{nameof(SourceLocation.AbsoluteIndex)}\":1,\"{nameof(SourceLocation.LineIndex)}\":2,\"" + $"{nameof(SourceLocation.CharacterIndex)}\":3}},\"{nameof(RazorError.Length)}\":456}}"; @@ -29,13 +30,34 @@ public void RazorError_CanBeSerialized() Assert.Equal(expectedSerializedError, serializedError, StringComparer.Ordinal); } + [Fact] + public void RazorError_WithFilePath_CanBeSerialized() + { + // Arrange + var error = new RazorError( + message: "Testing", + location: new SourceLocation("some-path", absoluteIndex: 1, lineIndex: 2, characterIndex: 56), + length: 3); + var expectedSerializedError = + $"{{\"{nameof(RazorError.Message)}\":\"Testing\",\"{nameof(RazorError.Location)}\":{{\"" + + $"{nameof(SourceLocation.FilePath)}\":\"some-path\",\"" + + $"{nameof(SourceLocation.AbsoluteIndex)}\":1,\"{nameof(SourceLocation.LineIndex)}\":2,\"" + + $"{nameof(SourceLocation.CharacterIndex)}\":56}},\"{nameof(RazorError.Length)}\":3}}"; + + // Act + var serializedError = JsonConvert.SerializeObject(error); + + // Assert + Assert.Equal(expectedSerializedError, serializedError, StringComparer.Ordinal); + } + [Fact] public void RazorError_CanBeDeserialized() { // Arrange var error = new RazorError( message: "Testing", - location: new SourceLocation(absoluteIndex: 1, lineIndex: 2, characterIndex: 3), + location: new SourceLocation("somepath", absoluteIndex: 1, lineIndex: 2, characterIndex: 3), length: 456); var serializedError = JsonConvert.SerializeObject(error); diff --git a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs index a8157b148..f896c8852 100644 --- a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using Microsoft.AspNet.Testing; using Xunit; namespace Microsoft.AspNet.Razor @@ -14,9 +16,262 @@ public void ConstructorWithLineAndCharacterIndexSetsAssociatedProperties() var loc = new SourceLocation(0, 42, 24); // Assert + Assert.Null(loc.FilePath); Assert.Equal(0, loc.AbsoluteIndex); Assert.Equal(42, loc.LineIndex); Assert.Equal(24, loc.CharacterIndex); } + + [Fact] + public void Constructor_SetsFilePathAndIndexes() + { + // Arrange + var filePath = "some-file-path"; + var absoluteIndex = 133; + var lineIndex = 23; + var characterIndex = 12; + + // Act + var sourceLocation = new SourceLocation(filePath, absoluteIndex, lineIndex, characterIndex); + + // Assert + Assert.Equal(filePath, sourceLocation.FilePath); + Assert.Equal(absoluteIndex, sourceLocation.AbsoluteIndex); + Assert.Equal(lineIndex, sourceLocation.LineIndex); + Assert.Equal(characterIndex, sourceLocation.CharacterIndex); + } + + [Fact] + public void GetHashCode_ReturnsHashCode_UsingAbsoluteIndex() + { + // Arrange + var sourceLocationA = new SourceLocation(10, 3, 4); + var sourceLocationB = new SourceLocation(10, 45, 8754); + + // Act + var hashCodeA = sourceLocationA.GetHashCode(); + var hashCodeB = sourceLocationB.GetHashCode(); + + // Assert + Assert.Equal(hashCodeA, hashCodeB); + } + + [Fact] + public void GetHashCode_ReturnsHashCode_UsingFilePathAndAbsoluteIndex_WhenFilePathIsNonNull() + { + // Arrange + var sourceLocationA = new SourceLocation("some-path", 3, 53, 94); + var sourceLocationB = new SourceLocation("some-path", 3, 37, 46); + + // Act + var hashCodeA = sourceLocationA.GetHashCode(); + var hashCodeB = sourceLocationB.GetHashCode(); + + // Assert + Assert.Equal(hashCodeA, hashCodeB); + } + + [Fact] + public void Equal_ReturnsFalse_IfIndexesDiffer() + { + // Arrange + var sourceLocationA = new SourceLocation(10, 3, 4); + var sourceLocationB = new SourceLocation(10, 45, 8754); + + // Act + var result = sourceLocationA.Equals(sourceLocationB); + + // Assert + Assert.False(result); + } + + [Fact] + public void Equal_ReturnsFalse_IfFilePathIsDifferent() + { + // Arrange + var sourceLocationA = new SourceLocation(10, 3, 4); + var sourceLocationB = new SourceLocation("different-file", 10, 3, 4); + + // Act + var result = sourceLocationA.Equals(sourceLocationB); + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData(null)] + [InlineData("some-file")] + public void Equal_ReturnsTrue_IfFilePathAndIndexesAreSame(string path) + { + // Arrange + var sourceLocationA = new SourceLocation(path, 10, 3, 4); + var sourceLocationB = new SourceLocation(path, 10, 3, 4); + var sourceLocationC = new SourceLocation("different-path", 10, 3, 4); + + // Act + var result1 = sourceLocationA.Equals(sourceLocationB); + var result2 = sourceLocationA.Equals(sourceLocationC); + + // Assert + Assert.True(result1); + Assert.False(result2); + } + + [Fact] + public void CompareTo_ReturnsResultOfFilePathComparisons_IfSourceLocationsDoNotMatch() + { + // Arrange + var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); + var sourceLocationB = new SourceLocation("b-path", 1, 1, 1); + + // Act + var result = sourceLocationA.CompareTo(sourceLocationB); + + // Assert + Assert.Equal(string.Compare(sourceLocationA.FilePath, sourceLocationB.FilePath, StringComparison.Ordinal), + result); + } + + [Theory] + [InlineData(null, 1, 2)] + [InlineData(null, 32, 32)] + [InlineData("same-path", 34, 32)] + [InlineData("same-path-b", 18, 32)] + public void CompareTo_ReturnsResultOfAbsoluteIndexComparisons_IfSourceLocationsMatch( + string path, int indexA, int indexB) + { + // Arrange + var sourceLocationA = new SourceLocation(path, indexA, 1, 1); + var sourceLocationB = new SourceLocation(path, indexB, 1, 1); + + // Act + var result = sourceLocationA.CompareTo(sourceLocationB); + + // Assert + Assert.Equal(indexA.CompareTo(indexB), result); + } + + [Fact] + public void Add_Throws_IfSourceLocationsDoNotMatch() + { + // Arrange + var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); + var sourceLocationB = new SourceLocation("b-path", 1, 1, 1); + + // Act and Assert + ExceptionAssert.ThrowsArgument( + () => { var result = sourceLocationA + sourceLocationB; }, + "right", + $"Cannot perform operations on 'SourceLocation' instances with different file paths."); + } + + [Theory] + [InlineData(null)] + [InlineData("same-path")] + public void Add_IgnoresCharacterIndexIfLineIndexIsNonZero(string path) + { + // Arrange + var sourceLocationA = new SourceLocation(path, 1, 2, 3); + var sourceLocationB = new SourceLocation(path, 4, 5, 6); + + // Act + var result = sourceLocationA + sourceLocationB; + + // Assert + Assert.Equal(path, result.FilePath); + Assert.Equal(5, result.AbsoluteIndex); + Assert.Equal(7, result.LineIndex); + Assert.Equal(6, result.CharacterIndex); + } + + [Theory] + [InlineData(null)] + [InlineData("same-path")] + public void Add_UsesCharacterIndexIfRightLineIndexIsZero(string path) + { + // Arrange + var sourceLocationA = new SourceLocation(path, 2, 5, 3); + var sourceLocationB = new SourceLocation(path, 4, 0, 6); + + // Act + var result = sourceLocationA + sourceLocationB; + + // Assert + Assert.Equal(path, result.FilePath); + Assert.Equal(6, result.AbsoluteIndex); + Assert.Equal(5, result.LineIndex); + Assert.Equal(9, result.CharacterIndex); + } + + [Fact] + public void Subtract_Throws_IfSourceLocationsDoNotMatch() + { + // Arrange + var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); + var sourceLocationB = new SourceLocation("b-path", 1, 1, 1); + + // Act and Assert + ExceptionAssert.ThrowsArgument( + () => { var result = sourceLocationA - sourceLocationB; }, + "right", + "Cannot perform operations on 'SourceLocation' instances with different file paths."); + } + + [Theory] + [InlineData(null)] + [InlineData("same-path")] + public void Subtract_UsesDifferenceOfCharacterIndexesIfLineIndexesAreSame(string path) + { + // Arrange + var sourceLocationA = new SourceLocation(path, 1, 5, 3); + var sourceLocationB = new SourceLocation(path, 5, 5, 6); + + // Act + var result = sourceLocationB - sourceLocationA; + + // Assert + Assert.Equal(path, result.FilePath); + Assert.Equal(4, result.AbsoluteIndex); + Assert.Equal(0, result.LineIndex); + Assert.Equal(3, result.CharacterIndex); + } + + [Theory] + [InlineData(null)] + [InlineData("same-path")] + public void Subtract_UsesLeftCharacterIndexIfLineIndexesAreDifferent(string path) + { + // Arrange + var sourceLocationA = new SourceLocation(path, 2, 0, 3); + var sourceLocationB = new SourceLocation(path, 4, 5, 6); + + // Act + var result = sourceLocationB - sourceLocationA; + + // Assert + Assert.Equal(path, result.FilePath); + Assert.Equal(2, result.AbsoluteIndex); + Assert.Equal(5, result.LineIndex); + Assert.Equal(6, result.CharacterIndex); + } + + [Theory] + [InlineData(null)] + [InlineData("path-to-file")] + public void Advance_PreservesSourceLocationFilePath(string path) + { + // Arrange + var sourceLocation = new SourceLocation(path, 15, 2, 8); + + // Act + var result = SourceLocation.Advance(sourceLocation, "Hello world"); + + // Assert + Assert.Equal(path, result.FilePath); + Assert.Equal(26, result.AbsoluteIndex); + Assert.Equal(2, result.LineIndex); + Assert.Equal(19, result.CharacterIndex); + } } } From 318d34422b51fbb6b93cf8137777f64b0b379d11 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 16 Apr 2015 06:47:20 -0700 Subject: [PATCH 2/3] Changes per PR comments --- .../Common/HashCodeCombiner.cs | 2 +- .../CodeBuilder/CSharp/CSharpCodeWriter.cs | 2 +- .../Properties/RazorResources.Designer.cs | 8 +-- .../RazorResources.resx | 2 +- src/Microsoft.AspNet.Razor/SourceLocation.cs | 52 ++++++++++--------- .../Compiler/CSharp/CSharpCodeWriterTest.cs | 17 +++--- .../SourceLocationTest.cs | 24 +++++---- 7 files changed, 57 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs b/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs index 5dfec7cbb..9e9023a92 100644 --- a/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs +++ b/src/Microsoft.AspNet.Razor/Common/HashCodeCombiner.cs @@ -50,7 +50,7 @@ public HashCodeCombiner Add(object o) public HashCodeCombiner Add(TValue value, IEqualityComparer comparer) { var hashCode = value != null ? comparer.GetHashCode(value) : 0; - return Add(0); + return Add(hashCode); } public static HashCodeCombiner Start() diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs index 2d39947dc..83d66a943 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs @@ -190,7 +190,7 @@ public CSharpCodeWriter WriteReturn(string value, bool endLine) /// The current instance of . public CSharpCodeWriter WriteLineNumberDirective(SourceLocation location, string file) { - if (!string.IsNullOrEmpty(location.FilePath)) + if (location.FilePath != null) { file = location.FilePath; } diff --git a/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs b/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs index 84bfaa098..d798e7f7f 100644 --- a/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs +++ b/src/Microsoft.AspNet.Razor/Properties/RazorResources.Designer.cs @@ -1467,7 +1467,7 @@ internal static string FormatRewriterError_EmptyTagHelperBoundAttribute(object p } /// - /// Cannot perform operations on '{0}' instances with different file paths. + /// Cannot perform '{1}' operations on '{0}' instances with different file paths. /// internal static string SourceLocationFilePathDoesNotMatch { @@ -1475,11 +1475,11 @@ internal static string SourceLocationFilePathDoesNotMatch } /// - /// Cannot perform operations on '{0}' instances with different file paths. + /// Cannot perform '{1}' operations on '{0}' instances with different file paths. /// - internal static string FormatSourceLocationFilePathDoesNotMatch(object p0) + internal static string FormatSourceLocationFilePathDoesNotMatch(object p0, object p1) { - return string.Format(CultureInfo.CurrentCulture, GetString("SourceLocationFilePathDoesNotMatch"), p0); + return string.Format(CultureInfo.CurrentCulture, GetString("SourceLocationFilePathDoesNotMatch"), p0, p1); } private static string GetString(string name, params string[] formatterNames) diff --git a/src/Microsoft.AspNet.Razor/RazorResources.resx b/src/Microsoft.AspNet.Razor/RazorResources.resx index e1c4a407f..5b6770917 100644 --- a/src/Microsoft.AspNet.Razor/RazorResources.resx +++ b/src/Microsoft.AspNet.Razor/RazorResources.resx @@ -411,6 +411,6 @@ Instead, wrap the contents of the block in "{{}}": Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of type '{2}' cannot be empty or contain only whitespace. - Cannot perform operations on '{0}' instances with different file paths. + Cannot perform '{1}' operations on '{0}' instances with different file paths. \ No newline at end of file diff --git a/src/Microsoft.AspNet.Razor/SourceLocation.cs b/src/Microsoft.AspNet.Razor/SourceLocation.cs index 7bc2bb4d0..6c293c30f 100644 --- a/src/Microsoft.AspNet.Razor/SourceLocation.cs +++ b/src/Microsoft.AspNet.Razor/SourceLocation.cs @@ -17,8 +17,18 @@ namespace Microsoft.AspNet.Razor #endif public struct SourceLocation : IEquatable, IComparable { - public static readonly SourceLocation Undefined = CreateUndefined(); - public static readonly SourceLocation Zero = new SourceLocation(0, 0, 0); + /// + /// An undefined . + /// + public static readonly SourceLocation Undefined = + new SourceLocation(absoluteIndex: -1, lineIndex: -1, characterIndex: -1); + + /// + /// A with , , and + /// initialized to 0. + /// + public static readonly SourceLocation Zero = + new SourceLocation(absoluteIndex: 0, lineIndex: 0, characterIndex: 0); /// /// Initializes a new instance of . @@ -38,7 +48,7 @@ public SourceLocation(int absoluteIndex, int lineIndex, int characterIndex) /// The absolute index. /// The line index. /// The character index. - public SourceLocation(string filePath, int absoluteIndex, int lineIndex, int characterIndex) + public SourceLocation([NotNull] string filePath, int absoluteIndex, int lineIndex, int characterIndex) { FilePath = filePath; AbsoluteIndex = absoluteIndex; @@ -49,7 +59,8 @@ public SourceLocation(string filePath, int absoluteIndex, int lineIndex, int cha /// /// Path of the file. /// - /// When null, the parser assumes it is the file currently being processed. + /// When null, the parser assumes the location is in the file currently being processed. + /// public string FilePath { get; set; } /// Set property is only accessible for deserialization purposes. @@ -75,7 +86,6 @@ public override string ToString() CharacterIndex); } - /// public override bool Equals(object obj) { @@ -102,8 +112,6 @@ public bool Equals(SourceLocation other) } /// - /// if the of the left and right operands - /// are different. public int CompareTo(SourceLocation other) { var filePathOrdinal = string.Compare(FilePath, other.FilePath, StringComparison.Ordinal); @@ -121,22 +129,13 @@ public int CompareTo(SourceLocation other) /// The to advance. /// The to advance by. /// The advanced . - public static SourceLocation Advance(SourceLocation left, string text) + public static SourceLocation Advance(SourceLocation left, [NotNull] string text) { var tracker = new SourceLocationTracker(left); tracker.UpdateLocation(text); return tracker.CurrentLocation; } - private static SourceLocation CreateUndefined() - { - var sl = new SourceLocation(); - sl.AbsoluteIndex = -1; - sl.LineIndex = -1; - sl.CharacterIndex = -1; - return sl; - } - /// /// Adds two s. /// @@ -150,7 +149,7 @@ private static SourceLocation CreateUndefined() if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal)) { throw new ArgumentException( - RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation)), + RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation), "+"), nameof(right)); } @@ -186,23 +185,26 @@ private static SourceLocation CreateUndefined() if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal)) { throw new ArgumentException( - RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation)), + RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation), "-"), nameof(right)); } + var characterIndex = left.LineIndex != right.LineIndex ? + left.CharacterIndex : left.CharacterIndex - right.CharacterIndex; + return new SourceLocation( - left.FilePath, - left.AbsoluteIndex - right.AbsoluteIndex, - left.LineIndex - right.LineIndex, - left.LineIndex != right.LineIndex ? left.CharacterIndex : left.CharacterIndex - right.CharacterIndex); + filePath: null, + absoluteIndex: left.AbsoluteIndex - right.AbsoluteIndex, + lineIndex: left.LineIndex - right.LineIndex, + characterIndex: characterIndex); } /// - /// Determines whether the first operand is lesser than the second operand. + /// Determines whether the first operand is less than the second operand. /// /// The left operand. /// The right operand. - /// true if is lesser than . + /// true if is less than . public static bool operator <(SourceLocation left, SourceLocation right) { return left.CompareTo(right) < 0; diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs b/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs index 8bc50b47b..d03b31ea4 100644 --- a/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/Generator/Compiler/CSharp/CSharpCodeWriterTest.cs @@ -7,17 +7,14 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp { public class CSharpCodeWriterTest { - [Theory] - [InlineData(null)] - [InlineData("")] - public void WriteLineNumberDirective_UsesFilePath_WhenFileInSourceLocationIsNullOrEmpty( - string sourceLocationFilePath) + [Fact] + public void WriteLineNumberDirective_UsesFilePath_WhenFileInSourceLocationIsNull() { // Arrange var filePath = "some-path"; var writer = new CSharpCodeWriter(); var expected = $"#line 5 \"{filePath}\"" + writer.NewLine; - var sourceLocation = new SourceLocation(sourceLocationFilePath, 10, 4, 3); + var sourceLocation = new SourceLocation(10, 4, 3); // Act writer.WriteLineNumberDirective(sourceLocation, filePath); @@ -27,12 +24,14 @@ public void WriteLineNumberDirective_UsesFilePath_WhenFileInSourceLocationIsNull Assert.Equal(expected, code); } - [Fact] - public void WriteLineNumberDirectives_UsesSourceLocationFilePath_IfAvailable() + [Theory] + [InlineData("")] + [InlineData("source-location-file-path")] + public void WriteLineNumberDirective_UsesSourceLocationFilePath_IfAvailable( + string sourceLocationFilePath) { // Arrange var filePath = "some-path"; - var sourceLocationFilePath = "source-location-file-path"; var writer = new CSharpCodeWriter(); var expected = $"#line 5 \"{sourceLocationFilePath}\"" + writer.NewLine; var sourceLocation = new SourceLocation(sourceLocationFilePath, 10, 4, 3); diff --git a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs index f896c8852..85293d5bc 100644 --- a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs @@ -47,13 +47,16 @@ public void GetHashCode_ReturnsHashCode_UsingAbsoluteIndex() // Arrange var sourceLocationA = new SourceLocation(10, 3, 4); var sourceLocationB = new SourceLocation(10, 45, 8754); + var sourceLocationC = new SourceLocation(12, 45, 8754); // Act var hashCodeA = sourceLocationA.GetHashCode(); var hashCodeB = sourceLocationB.GetHashCode(); + var hashCodeC = sourceLocationC.GetHashCode(); // Assert Assert.Equal(hashCodeA, hashCodeB); + Assert.NotEqual(hashCodeA, hashCodeC); } [Fact] @@ -61,14 +64,17 @@ public void GetHashCode_ReturnsHashCode_UsingFilePathAndAbsoluteIndex_WhenFilePa { // Arrange var sourceLocationA = new SourceLocation("some-path", 3, 53, 94); - var sourceLocationB = new SourceLocation("some-path", 3, 37, 46); + var sourceLocationB = new SourceLocation("some-path", 3, 43, 87); + var sourceLocationC = new SourceLocation(3, 53, 94); // Act var hashCodeA = sourceLocationA.GetHashCode(); var hashCodeB = sourceLocationB.GetHashCode(); + var hashCodeC = sourceLocationC.GetHashCode(); // Assert Assert.Equal(hashCodeA, hashCodeB); + Assert.NotEqual(hashCodeA, hashCodeC); } [Fact] @@ -119,7 +125,7 @@ public void Equal_ReturnsTrue_IfFilePathAndIndexesAreSame(string path) } [Fact] - public void CompareTo_ReturnsResultOfFilePathComparisons_IfSourceLocationsDoNotMatch() + public void CompareTo_ReturnsResultOfFilePathComparisons_WhenFilePathsAreDifferent() { // Arrange var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); @@ -138,7 +144,7 @@ public void CompareTo_ReturnsResultOfFilePathComparisons_IfSourceLocationsDoNotM [InlineData(null, 32, 32)] [InlineData("same-path", 34, 32)] [InlineData("same-path-b", 18, 32)] - public void CompareTo_ReturnsResultOfAbsoluteIndexComparisons_IfSourceLocationsMatch( + public void CompareTo_ReturnsResultOfAbsoluteIndexComparisons_IfFilePathsMatch( string path, int indexA, int indexB) { // Arrange @@ -153,7 +159,7 @@ public void CompareTo_ReturnsResultOfAbsoluteIndexComparisons_IfSourceLocationsM } [Fact] - public void Add_Throws_IfSourceLocationsDoNotMatch() + public void Add_Throws_IfFilePathsDoNotMatch() { // Arrange var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); @@ -163,7 +169,7 @@ public void Add_Throws_IfSourceLocationsDoNotMatch() ExceptionAssert.ThrowsArgument( () => { var result = sourceLocationA + sourceLocationB; }, "right", - $"Cannot perform operations on 'SourceLocation' instances with different file paths."); + $"Cannot perform '+' operations on 'SourceLocation' instances with different file paths."); } [Theory] @@ -205,7 +211,7 @@ public void Add_UsesCharacterIndexIfRightLineIndexIsZero(string path) } [Fact] - public void Subtract_Throws_IfSourceLocationsDoNotMatch() + public void Subtract_Throws_IfFilePathsDoNotMatch() { // Arrange var sourceLocationA = new SourceLocation("a-path", 1, 1, 1); @@ -215,7 +221,7 @@ public void Subtract_Throws_IfSourceLocationsDoNotMatch() ExceptionAssert.ThrowsArgument( () => { var result = sourceLocationA - sourceLocationB; }, "right", - "Cannot perform operations on 'SourceLocation' instances with different file paths."); + "Cannot perform '-' operations on 'SourceLocation' instances with different file paths."); } [Theory] @@ -231,7 +237,7 @@ public void Subtract_UsesDifferenceOfCharacterIndexesIfLineIndexesAreSame(string var result = sourceLocationB - sourceLocationA; // Assert - Assert.Equal(path, result.FilePath); + Assert.Null(result.FilePath); Assert.Equal(4, result.AbsoluteIndex); Assert.Equal(0, result.LineIndex); Assert.Equal(3, result.CharacterIndex); @@ -250,7 +256,7 @@ public void Subtract_UsesLeftCharacterIndexIfLineIndexesAreDifferent(string path var result = sourceLocationB - sourceLocationA; // Assert - Assert.Equal(path, result.FilePath); + Assert.Null(result.FilePath); Assert.Equal(2, result.AbsoluteIndex); Assert.Equal(5, result.LineIndex); Assert.Equal(6, result.CharacterIndex); From 701fce584aa5ac533d36eef509e08508b7c8d6e0 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 16 Apr 2015 10:14:30 -0700 Subject: [PATCH 3/3] Changes per PR comments --- src/Microsoft.AspNet.Razor/SourceLocation.cs | 12 ++++--- .../SourceLocationTest.cs | 36 ++++++++++++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNet.Razor/SourceLocation.cs b/src/Microsoft.AspNet.Razor/SourceLocation.cs index 6c293c30f..25f98a502 100644 --- a/src/Microsoft.AspNet.Razor/SourceLocation.cs +++ b/src/Microsoft.AspNet.Razor/SourceLocation.cs @@ -48,7 +48,7 @@ public SourceLocation(int absoluteIndex, int lineIndex, int characterIndex) /// The absolute index. /// The line index. /// The character index. - public SourceLocation([NotNull] string filePath, int absoluteIndex, int lineIndex, int characterIndex) + public SourceLocation(string filePath, int absoluteIndex, int lineIndex, int characterIndex) { FilePath = filePath; AbsoluteIndex = absoluteIndex; @@ -146,18 +146,22 @@ public static SourceLocation Advance(SourceLocation left, [NotNull] string text) /// are different. public static SourceLocation operator +(SourceLocation left, SourceLocation right) { - if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal)) + if (!string.Equals(left.FilePath, right.FilePath, StringComparison.Ordinal) && + left.FilePath != null && + right.FilePath != null) { + // Throw if FilePath for left and right are different, and neither is null. throw new ArgumentException( RazorResources.FormatSourceLocationFilePathDoesNotMatch(nameof(SourceLocation), "+"), nameof(right)); } + var resultFilePath = left.FilePath ?? right.FilePath; if (right.LineIndex > 0) { // Column index doesn't matter return new SourceLocation( - left.FilePath, + resultFilePath, left.AbsoluteIndex + right.AbsoluteIndex, left.LineIndex + right.LineIndex, right.CharacterIndex); @@ -165,7 +169,7 @@ public static SourceLocation Advance(SourceLocation left, [NotNull] string text) else { return new SourceLocation( - left.FilePath, + resultFilePath, left.AbsoluteIndex + right.AbsoluteIndex, left.LineIndex + right.LineIndex, left.CharacterIndex + right.CharacterIndex); diff --git a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs index 85293d5bc..9296952e4 100644 --- a/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/SourceLocationTest.cs @@ -175,7 +175,7 @@ public void Add_Throws_IfFilePathsDoNotMatch() [Theory] [InlineData(null)] [InlineData("same-path")] - public void Add_IgnoresCharacterIndexIfLineIndexIsNonZero(string path) + public void Add_IgnoresCharacterIndexIfRightLineIndexIsNonZero(string path) { // Arrange var sourceLocationA = new SourceLocation(path, 1, 2, 3); @@ -210,6 +210,40 @@ public void Add_UsesCharacterIndexIfRightLineIndexIsZero(string path) Assert.Equal(9, result.CharacterIndex); } + [Fact] + public void Add_AllowsRightFilePathToBeNull_WhenLeftFilePathIsNonNull() + { + // Arrange + var left = new SourceLocation("left-path", 7, 1, 7); + var right = new SourceLocation(13, 1, 4); + + // Act + var result = left + right; + + // Assert + Assert.Equal(left.FilePath, result.FilePath); + Assert.Equal(20, result.AbsoluteIndex); + Assert.Equal(2, result.LineIndex); + Assert.Equal(4, result.CharacterIndex); + } + + [Fact] + public void Add_AllowsLeftFilePathToBeNull_WhenRightFilePathIsNonNull() + { + // Arrange + var left = new SourceLocation(4, 5, 6); + var right = new SourceLocation("right-path", 7, 8, 9); + + // Act + var result = left + right; + + // Assert + Assert.Equal(right.FilePath, result.FilePath); + Assert.Equal(11, result.AbsoluteIndex); + Assert.Equal(13, result.LineIndex); + Assert.Equal(9, result.CharacterIndex); + } + [Fact] public void Subtract_Throws_IfFilePathsDoNotMatch() {