From d4994eb4a451203f5cf6c5f7dc1543886f940b96 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 22 Dec 2017 12:05:54 -0800 Subject: [PATCH] Added DefaultRazorIndentationFactsService unit tests. - Already had a good variety of integration tests so refactored the service to properly unit test each piece. - Found several pieces of unneeded code (wasn't being used) so removed it. - Removed the `LocateOwner` logic that was embedded in the service. We already have an equivalent locate owner on our `SyntaxTreeNode` items. #1698 --- .../DefaultRazorIndentationFactsService.cs | 187 +++++------------- ...DefaultRazorIndentationFactsServiceTest.cs | 155 +++++++++++++++ 2 files changed, 205 insertions(+), 137 deletions(-) diff --git a/src/Microsoft.VisualStudio.Editor.Razor/DefaultRazorIndentationFactsService.cs b/src/Microsoft.VisualStudio.Editor.Razor/DefaultRazorIndentationFactsService.cs index 76f704eb99..c0ed95ad11 100644 --- a/src/Microsoft.VisualStudio.Editor.Razor/DefaultRazorIndentationFactsService.cs +++ b/src/Microsoft.VisualStudio.Editor.Razor/DefaultRazorIndentationFactsService.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.ComponentModel.Composition; using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language.Legacy; @@ -15,6 +14,21 @@ namespace Microsoft.VisualStudio.Editor.Razor [Export(typeof(RazorIndentationFactsService))] internal class DefaultRazorIndentationFactsService : RazorIndentationFactsService { + // This method dives down a syntax tree looking for open curly braces, every time + // it finds one it increments its indent until it finds the provided "line". + // + // Examples: + // @{ + // Hello World + // } + // Asking for desired indentation of the @{ or } lines should result in a desired indentation of 4. + // + //
+ // @{ + // Hello World + // } + //
+ // Asking for desired indentation of the @{ or } lines should result in a desired indentation of 8. public override int? GetDesiredIndentation( RazorSyntaxTree syntaxTree, ITextSnapshot syntaxTreeSnapshot, @@ -47,12 +61,9 @@ namespace Microsoft.VisualStudio.Editor.Razor throw new ArgumentOutOfRangeException(nameof(tabSize)); } - var previousLine = line.Snapshot.GetLineFromLineNumber(line.LineNumber - 1); - var trackingPoint = previousLine.Snapshot.CreateTrackingPoint(previousLine.End, PointTrackingMode.Negative); - var previousLineEndIndex = trackingPoint.GetPosition(syntaxTreeSnapshot); - + var previousLineEndIndex = GetPreviousLineEndIndex(syntaxTreeSnapshot, line); var simulatedChange = new SourceChange(previousLineEndIndex, 0, string.Empty); - var owningSpan = LocateOwner(syntaxTree.Root, simulatedChange); + var owningSpan = syntaxTree.Root.LocateOwner(simulatedChange); if (owningSpan.Kind == SpanKindInternal.Code) { // Example, @@ -63,40 +74,17 @@ namespace Microsoft.VisualStudio.Editor.Razor } int? desiredIndentation = null; - SyntaxTreeNode owningChild = owningSpan; - while ((owningChild.Parent != null) && !desiredIndentation.HasValue) + while (owningChild.Parent != null) { var owningParent = owningChild.Parent; - var children = new List(owningParent.Children); - for (var i = 0; i < children.Count; i++) + for (var i = 0; i < owningParent.Children.Count; i++) { - var currentChild = children[i]; - if (!currentChild.IsBlock) + var currentChild = owningParent.Children[i]; + if (IsCSharpOpenCurlyBrace(currentChild)) { - var currentSpan = currentChild as Span; - if (currentSpan.Symbols.Count == 1 && - currentSpan.Symbols[0] is CSharpSymbol symbol && - symbol.Type == CSharpSymbolType.LeftBrace) - { - var extraIndent = 0; - - // Dev11 337312: Only indent one level deeper if the item after the open curly brace is a markup block - if (i < children.Count - 1) - { - var nextChild = children[i + 1]; - if (nextChild.IsBlock && ((nextChild as Block).Type == BlockKindInternal.Markup)) - { - extraIndent = indentSize; - } - } - - // We can't rely on the syntax trees representation of the source document because partial parses may have mutated - // the underlying SyntaxTree text buffer. Because of this, if we want to provide accurate indentations we need to - // operate on the current line representation as indicated by the provider. - var lineText = line.Snapshot.GetLineFromLineNumber(currentSpan.Start.LineIndex).GetText(); - desiredIndentation = GetIndentLevelOfLine(lineText, tabSize) + indentSize; - } + var lineText = line.Snapshot.GetLineFromLineNumber(currentChild.Start.LineIndex).GetText(); + desiredIndentation = GetIndentLevelOfLine(lineText, tabSize) + indentSize; } if (currentChild == owningChild) @@ -105,113 +93,20 @@ namespace Microsoft.VisualStudio.Editor.Razor } } + if (desiredIndentation.HasValue) + { + return desiredIndentation; + } + owningChild = owningParent; } - return desiredIndentation; + // Couldn't determine indentation + return null; } - private Span LocateOwner(Block root, SourceChange change) - { - // Ask each child recursively - Span owner = null; - foreach (var element in root.Children) - { - if (element.Start.AbsoluteIndex > change.Span.AbsoluteIndex) - { - // too far - break; - } - - var elementLen = element.Length; - if (element.Start.AbsoluteIndex + elementLen < change.Span.AbsoluteIndex) - { - // not far enough - continue; - } - - if (element.IsBlock) - { - var block = element as Block; - - if (element.Start.AbsoluteIndex + elementLen == change.Span.AbsoluteIndex) - { - var lastDescendant = block.FindLastDescendentSpan(); - if ((lastDescendant == null) && (block is TagHelperBlock)) - { - var tagHelperBlock = (TagHelperBlock)block; - if (tagHelperBlock.SourceEndTag != null) - { - lastDescendant = tagHelperBlock.SourceEndTag.FindLastDescendentSpan(); - } - else if (tagHelperBlock.SourceStartTag != null) - { - lastDescendant = tagHelperBlock.SourceStartTag.FindLastDescendentSpan(); - } - } - - // Conceptually, lastDescendant should always be non-null, but runtime errs on some - // cases and makes empty blocks. Runtime will fix these issues as we find them, but make - // no guarantee that they catch them all. - if (lastDescendant == null) - { - owner = LocateOwner(block, change); - if (owner != null) - { - break; - } - } - else if (lastDescendant.EditHandler.OwnsChange(lastDescendant, change)) - { - owner = lastDescendant; - break; - } - } - else - { - owner = LocateOwner(block, change); - if (owner != null) - { - break; - } - } - } - else - { - var span = element as Span; - if (span.EditHandler.OwnsChange(span, change)) - { - owner = span; - break; - } - } - } - - if (owner == null) - { - if (root is TagHelperBlock tagHelperNode) - { - var sourceStartTag = tagHelperNode.SourceStartTag; - var sourceEndTag = tagHelperNode.SourceEndTag; - if ((sourceStartTag.Start.AbsoluteIndex <= change.Span.AbsoluteIndex) && - (sourceStartTag.Start.AbsoluteIndex + sourceStartTag.Length >= change.Span.AbsoluteIndex)) - { - // intersects the start tag - return LocateOwner(sourceStartTag, change); - } - else if ((sourceEndTag.Start.AbsoluteIndex <= change.Span.AbsoluteIndex) && - (sourceEndTag.Start.AbsoluteIndex + sourceEndTag.Length >= change.Span.AbsoluteIndex)) - { - // intersects the end tag - return LocateOwner(sourceEndTag, change); - } - } - } - - return owner; - } - - private int GetIndentLevelOfLine(string line, int tabSize) + // Internal for testing + internal int GetIndentLevelOfLine(string line, int tabSize) { var indentLevel = 0; @@ -233,5 +128,23 @@ namespace Microsoft.VisualStudio.Editor.Razor return indentLevel; } + + // Internal for testing + internal static int GetPreviousLineEndIndex(ITextSnapshot syntaxTreeSnapshot, ITextSnapshotLine line) + { + var previousLine = line.Snapshot.GetLineFromLineNumber(line.LineNumber - 1); + var trackingPoint = previousLine.Snapshot.CreateTrackingPoint(previousLine.End, PointTrackingMode.Negative); + var previousLineEndIndex = trackingPoint.GetPosition(syntaxTreeSnapshot); + return previousLineEndIndex; + } + + // Internal for testing + internal static bool IsCSharpOpenCurlyBrace(SyntaxTreeNode currentChild) + { + return currentChild is Span currentSpan && + currentSpan.Symbols.Count == 1 && + currentSpan.Symbols[0] is CSharpSymbol symbol && + symbol.Type == CSharpSymbolType.LeftBrace; + } } } diff --git a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultRazorIndentationFactsServiceTest.cs b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultRazorIndentationFactsServiceTest.cs index 3c4f2912c6..ad5c50b9a6 100644 --- a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultRazorIndentationFactsServiceTest.cs +++ b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultRazorIndentationFactsServiceTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Razor.Language; +using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.VisualStudio.Text; using Xunit; @@ -11,6 +12,160 @@ namespace Microsoft.VisualStudio.Editor.Razor { public class DefaultRazorIndentationFactsServiceTest { + [Fact] + public void GetPreviousLineEndIndex_ReturnsPreviousLine() + { + // Arrange + var textSnapshot = new StringTextSnapshot(@"@{ +

Hello World

+}"); + var line = textSnapshot.GetLineFromLineNumber(2); + + // Act + var previousLineEndIndex = DefaultRazorIndentationFactsService.GetPreviousLineEndIndex(textSnapshot, line); + + // Assert + Assert.Equal(26, previousLineEndIndex); + } + + [Fact] + public void IsCSharpOpenCurlyBrace_SpanWithLeftBrace_ReturnTrue() + { + // Arrange + var childBuilder = new SpanBuilder(SourceLocation.Zero); + childBuilder.Accept(new CSharpSymbol("{", CSharpSymbolType.LeftBrace)); + var child = childBuilder.Build(); + + // Act + var result = DefaultRazorIndentationFactsService.IsCSharpOpenCurlyBrace(child); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("if", CSharpSymbolType.Keyword)] + [InlineData("}", CSharpSymbolType.RightBrace)] + [InlineData("++", CSharpSymbolType.Increment)] + [InlineData("text", CSharpSymbolType.Identifier)] + public void IsCSharpOpenCurlyBrace_SpanWithUnsupportedSymbolType_ReturnFalse(string content, object symbolTypeObject) + { + // Arrange + var symbolType = (CSharpSymbolType)symbolTypeObject; + var childBuilder = new SpanBuilder(SourceLocation.Zero); + childBuilder.Accept(new CSharpSymbol(content, symbolType)); + var child = childBuilder.Build(); + + // Act + var result = DefaultRazorIndentationFactsService.IsCSharpOpenCurlyBrace(child); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsCSharpOpenCurlyBrace_MultipleSymbols_ReturnFalse() + { + // Arrange + var childBuilder = new SpanBuilder(SourceLocation.Zero); + childBuilder.Accept(new CSharpSymbol("hello", CSharpSymbolType.Identifier)); + childBuilder.Accept(new CSharpSymbol(",", CSharpSymbolType.Comma)); + var child = childBuilder.Build(); + + // Act + var result = DefaultRazorIndentationFactsService.IsCSharpOpenCurlyBrace(child); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsCSharpOpenCurlyBrace_SpanWithHtmlSymbol_ReturnFalse() + { + // Arrange + var childBuilder = new SpanBuilder(SourceLocation.Zero); + childBuilder.Accept(new HtmlSymbol("hello", HtmlSymbolType.Text)); + var child = childBuilder.Build(); + + // Act + var result = DefaultRazorIndentationFactsService.IsCSharpOpenCurlyBrace(child); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsCSharpOpenCurlyBrace_Blocks_ReturnFalse() + { + // Arrange + var child = new BlockBuilder() + { + Type = BlockKindInternal.Markup, + }.Build(); + + // Act + var result = DefaultRazorIndentationFactsService.IsCSharpOpenCurlyBrace(child); + + // Assert + Assert.False(result); + } + + [Fact] + public void GetIndentLevelOfLine_AddsTabsOnlyAtBeginningOfLine() + { + // Arrange + var text = "\t\tHello\tWorld.\t"; + var service = new DefaultRazorIndentationFactsService(); + + // Act + var indentLevel = service.GetIndentLevelOfLine(text, 4); + + // Assert + Assert.Equal(8, indentLevel); + } + + [Fact] + public void GetIndentLevelOfLine_AddsSpacesOnlyAtBeginningOfLine() + { + // Arrange + var text = " Hello World. "; + var service = new DefaultRazorIndentationFactsService(); + + // Act + var indentLevel = service.GetIndentLevelOfLine(text, 4); + + // Assert + Assert.Equal(3, indentLevel); + } + + [Fact] + public void GetIndentLevelOfLine_AddsTabsAndSpacesOnlyAtBeginningOfLine() + { + // Arrange + var text = " \t \tHello\t World.\t "; + var service = new DefaultRazorIndentationFactsService(); + + // Act + var indentLevel = service.GetIndentLevelOfLine(text, 4); + + // Assert + Assert.Equal(11, indentLevel); + } + + [Fact] + public void GetIndentLevelOfLine_NoIndent() + { + // Arrange + var text = "Hello World."; + var service = new DefaultRazorIndentationFactsService(); + + // Act + var indentLevel = service.GetIndentLevelOfLine(text, 4); + + // Assert + Assert.Equal(0, indentLevel); + } + [Fact] public void GetDesiredIndentation_ReturnsNull_IfOwningSpanIsCode() {