Make brace indenter smarter about making edits to non-valid content kinds.

- Prior to this change our brace smart indenter would indent JavaScript blocks incorrectly because it didn't take into account where in a Razor file the brace that it was indenting existed.
- Made it so the brace smart indenter only functions in code/metacode locations within the SyntaxTree.
- Updated and added tests to account for new behavior.

#2297
This commit is contained in:
N. Taylor Mullen 2018-06-07 15:41:05 -07:00
parent ad07036020
commit b2ec939006
5 changed files with 203 additions and 17 deletions

View File

@ -4,6 +4,7 @@
using System;
using System.Diagnostics;
using System.Text;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.VisualStudio.Text;
@ -30,6 +31,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
private readonly ForegroundDispatcher _dispatcher;
private readonly ITextBuffer _textBuffer;
private readonly VisualStudioDocumentTracker _documentTracker;
private readonly TextBufferCodeDocumentProvider _codeDocumentProvider;
private readonly IEditorOperationsFactoryService _editorOperationsFactory;
private readonly StringBuilder _indentBuilder = new StringBuilder();
private BraceIndentationContext _context;
@ -42,6 +44,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
public BraceSmartIndenter(
ForegroundDispatcher dispatcher,
VisualStudioDocumentTracker documentTracker,
TextBufferCodeDocumentProvider codeDocumentProvider,
IEditorOperationsFactoryService editorOperationsFactory)
{
if (dispatcher == null)
@ -54,6 +57,11 @@ namespace Microsoft.VisualStudio.Editor.Razor
throw new ArgumentNullException(nameof(documentTracker));
}
if (codeDocumentProvider == null)
{
throw new ArgumentNullException(nameof(codeDocumentProvider));
}
if (editorOperationsFactory == null)
{
throw new ArgumentNullException(nameof(editorOperationsFactory));
@ -61,6 +69,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
_dispatcher = dispatcher;
_documentTracker = documentTracker;
_codeDocumentProvider = codeDocumentProvider;
_editorOperationsFactory = editorOperationsFactory;
_textBuffer = _documentTracker.TextBuffer;
_textBuffer.Changed += TextBuffer_OnChanged;
@ -95,7 +104,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
var newText = changeInformation.newText;
if (TryCreateIndentationContext(changeInformation.firstChange.NewPosition, newText.Length, newText, _documentTracker, out var context))
if (!_codeDocumentProvider.TryGetFromBuffer(_documentTracker.TextBuffer, out var codeDocument))
{
// Parse not available.
return;
}
var syntaxTree = codeDocument.GetSyntaxTree();
if (TryCreateIndentationContext(changeInformation.firstChange.NewPosition, newText.Length, newText, syntaxTree, _documentTracker, out var context))
{
_context = context;
}
@ -183,11 +199,23 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
// Internal for testing
internal static bool TryCreateIndentationContext(int changePosition, int changeLength, string finalText, VisualStudioDocumentTracker documentTracker, out BraceIndentationContext context)
internal static bool TryCreateIndentationContext(
int changePosition,
int changeLength,
string finalText,
RazorSyntaxTree syntaxTree,
VisualStudioDocumentTracker documentTracker,
out BraceIndentationContext context)
{
var focusedTextView = documentTracker.GetFocusedTextView();
if (focusedTextView != null && ParserHelpers.IsNewLine(finalText))
{
if (!AtValidContentKind(changePosition, syntaxTree))
{
context = null;
return false;
}
var currentSnapshot = documentTracker.TextBuffer.CurrentSnapshot;
var preChangeLineSnapshot = currentSnapshot.GetLineFromPosition(changePosition);
@ -213,6 +241,36 @@ namespace Microsoft.VisualStudio.Editor.Razor
return false;
}
// Internal for testing
internal static bool AtValidContentKind(int changePosition, RazorSyntaxTree syntaxTree)
{
var change = new SourceChange(changePosition, 0, string.Empty);
var owner = syntaxTree.Root.LocateOwner(change);
if (owner == null)
{
return false;
}
if (owner.Kind == SpanKindInternal.MetaCode)
{
// @functions{|}
return true;
}
if (owner.Kind == SpanKindInternal.Code)
{
// It's important that we still indent in C# cases because in the example below we're asked for
// a content validation kind check at a 0 length C# code Span (marker).
// In the case that we do a smart indent in a situation when Roslyn would: Roslyn respects our
// indentation attempt and applies any additional modifications to the applicable span.
// @{|}
return true;
}
return false;
}
internal static bool BeforeClosingBrace(int linePosition, ITextSnapshotLine lineSnapshot)
{
var lineText = lineSnapshot.GetText();

View File

@ -11,9 +11,11 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
private readonly IEditorOperationsFactoryService _editorOperationsFactory;
private readonly ForegroundDispatcher _dispatcher;
private readonly TextBufferCodeDocumentProvider _codeDocumentProvider;
public DefaultBraceSmartIndenterFactory(
ForegroundDispatcher dispatcher,
TextBufferCodeDocumentProvider codeDocumentProvider,
IEditorOperationsFactoryService editorOperationsFactory)
{
if (dispatcher == null)
@ -21,12 +23,18 @@ namespace Microsoft.VisualStudio.Editor.Razor
throw new ArgumentNullException(nameof(dispatcher));
}
if (codeDocumentProvider == null)
{
throw new ArgumentNullException(nameof(codeDocumentProvider));
}
if (editorOperationsFactory == null)
{
throw new ArgumentNullException(nameof(editorOperationsFactory));
}
_dispatcher = dispatcher;
_codeDocumentProvider = codeDocumentProvider;
_editorOperationsFactory = editorOperationsFactory;
}
@ -39,7 +47,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
_dispatcher.AssertForegroundThread();
var braceSmartIndenter = new BraceSmartIndenter(_dispatcher, documentTracker, _editorOperationsFactory);
var braceSmartIndenter = new BraceSmartIndenter(_dispatcher, documentTracker, _codeDocumentProvider, _editorOperationsFactory);
return braceSmartIndenter;
}

View File

@ -15,22 +15,32 @@ namespace Microsoft.VisualStudio.Editor.Razor
internal class DefaultBraceSmartIndenterFactoryFactory : ILanguageServiceFactory
{
private readonly ForegroundDispatcher _foregroundDispatcher;
private readonly TextBufferCodeDocumentProvider _codeDocumentProvider;
private readonly IEditorOperationsFactoryService _editorOperationsFactory;
[ImportingConstructor]
public DefaultBraceSmartIndenterFactoryFactory(ForegroundDispatcher foregroundDispatcher, IEditorOperationsFactoryService editorOperationsFactory)
public DefaultBraceSmartIndenterFactoryFactory(
ForegroundDispatcher foregroundDispatcher,
TextBufferCodeDocumentProvider codeDocumentProvider,
IEditorOperationsFactoryService editorOperationsFactory)
{
if (foregroundDispatcher == null)
{
throw new ArgumentNullException(nameof(foregroundDispatcher));
}
if (codeDocumentProvider == null)
{
throw new ArgumentNullException(nameof(codeDocumentProvider));
}
if (editorOperationsFactory == null)
{
throw new ArgumentNullException(nameof(editorOperationsFactory));
}
_foregroundDispatcher = foregroundDispatcher;
_codeDocumentProvider = codeDocumentProvider;
_editorOperationsFactory = editorOperationsFactory;
}
@ -41,7 +51,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
throw new ArgumentNullException(nameof(languageServices));
}
return new DefaultBraceSmartIndenterFactory(_foregroundDispatcher, _editorOperationsFactory);
return new DefaultBraceSmartIndenterFactory(_foregroundDispatcher, _codeDocumentProvider, _editorOperationsFactory);
}
}
}

View File

@ -2,8 +2,11 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Extensions;
using Microsoft.VisualStudio.Test;
using Microsoft.VisualStudio.Text;
using Moq;
using Xunit;
namespace Microsoft.VisualStudio.Editor.Razor
@ -25,8 +28,9 @@ namespace Microsoft.VisualStudio.Editor.Razor
var focusedTextView = CreateFocusedTextView(() => textBuffer, caret);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
var codeDocumentProvider = CreateCodeDocumentProvider(initialSnapshot.Content);
var editorOperationsFactory = CreateOperationsFactoryService();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory);
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory);
// Act
textBuffer.ApplyEdit(edit);
@ -50,8 +54,9 @@ namespace Microsoft.VisualStudio.Editor.Razor
var focusedTextView = CreateFocusedTextView(() => textBuffer, caret);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
var codeDocumentProvider = CreateCodeDocumentProvider(initialSnapshot.Content);
var editorOperationsFactory = CreateOperationsFactoryService();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory);
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory);
// Act
textBuffer.ApplyEdit(edit);
@ -75,8 +80,9 @@ namespace Microsoft.VisualStudio.Editor.Razor
var focusedTextView = CreateFocusedTextView(() => textBuffer, caret);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
var codeDocumentProvider = CreateCodeDocumentProvider(initialSnapshot.Content);
var editorOperationsFactory = CreateOperationsFactoryService();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory);
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory);
// Act
textBuffer.ApplyEdit(edit);
@ -84,5 +90,41 @@ namespace Microsoft.VisualStudio.Editor.Razor
// Assert
Assert.Equal(expectedIndentResult, ((StringTextSnapshot)textBuffer.CurrentSnapshot).Content);
}
[ForegroundFact]
public void TextBuffer_OnPostChanged_DoesNotIndentJavaScript()
{
// Arrange
var change = Environment.NewLine;
var initialSnapshot = new StringTextSnapshot(" <script>function foo() {}</script>");
var afterChangeSnapshot = new StringTextSnapshot(" <script>function foo() {" + change + "}</script>");
var edit = new TestEdit(28, 0, initialSnapshot, change.Length, afterChangeSnapshot, change);
var caret = CreateCaretFrom(28 + change.Length, afterChangeSnapshot);
TestTextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer, caret);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
var codeDocumentProvider = CreateCodeDocumentProvider(initialSnapshot.Content);
var editorOperationsFactory = CreateOperationsFactoryService();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory);
// Act
textBuffer.ApplyEdit(edit);
// Assert
Assert.Equal(afterChangeSnapshot.Content, ((StringTextSnapshot)textBuffer.CurrentSnapshot).Content);
}
private TextBufferCodeDocumentProvider CreateCodeDocumentProvider(string content)
{
var sourceDocument = TestRazorSourceDocument.Create(content);
var syntaxTree = RazorSyntaxTree.Parse(sourceDocument, RazorParserOptions.Create(opt => opt.Directives.Add(FunctionsDirective.Directive)));
var codeDocument = TestRazorCodeDocument.Create(content);
codeDocument.SetSyntaxTree(syntaxTree);
var codeDocumentProvider = Mock.Of<TextBufferCodeDocumentProvider>(provider => provider.TryGetFromBuffer(It.IsAny<ITextBuffer>(), out codeDocument));
return codeDocumentProvider;
}
}
}

View File

@ -3,6 +3,8 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Extensions;
using Microsoft.VisualStudio.Test;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
@ -14,6 +16,63 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
public class BraceSmartIndenterTest : BraceSmartIndenterTestBase
{
[Fact]
public void AtValidContentKind_ReturnsFalseAtMarkup()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("<p></p>"));
var changePosition = 2;
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
// Assert
Assert.False(result);
}
[Fact]
public void AtValidContentKind_ReturnsTrueAtCode()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("@{}"));
var changePosition = 2;
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtValidContentKind_ReturnsTrueAtMetacode()
{
// Arrange
var parseOptions = RazorParserOptions.Create(options => options.Directives.Add(FunctionsDirective.Directive));
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("@functions {}"), parseOptions);
var changePosition = 12;
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtValidContentKind_ReturnsFalseWhenNoOwner()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("@DateTime.Now"));
var changePosition = 14; // 1 after the end of the content
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
// Assert
Assert.False(result);
}
[Fact]
public void InsertIndent_InsertsProvidedIndentIntoBuffer()
{
@ -68,7 +127,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
var documentTracker = CreateDocumentTracker(() => Mock.Of<ITextBuffer>(), textView);
editorOperationsFactory.Setup(factory => factory.GetEditorOperations(textView))
.Returns(editorOperations.Object);
var smartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory.Object);
var codeDocumentProvider = Mock.Of<TextBufferCodeDocumentProvider>();
var smartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory.Object);
// Act
smartIndenter.TriggerSmartIndent(textView);
@ -141,7 +201,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
var changeCollection = new TestTextChangeCollection();
var textContentChangeArgs = new TestTextContentChangedEventArgs(changeCollection);
var documentTracker = CreateDocumentTracker(() => Mock.Of<ITextBuffer>(), Mock.Of<ITextView>());
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory.Object);
var codeDocumentProvider = Mock.Of<TextBufferCodeDocumentProvider>();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory.Object);
// Act & Assert
braceSmartIndenter.TextBuffer_OnChanged(null, textContentChangeArgs);
@ -156,7 +217,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
var edit = new TestEdit(0, 0, initialSnapshot, 0, initialSnapshot, string.Empty);
var editorOperationsFactory = new Mock<IEditorOperationsFactoryService>();
var documentTracker = CreateDocumentTracker(() => textBuffer, Mock.Of<ITextView>());
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, editorOperationsFactory.Object);
var codeDocumentProvider = Mock.Of<TextBufferCodeDocumentProvider>();
var braceSmartIndenter = new BraceSmartIndenter(Dispatcher, documentTracker, codeDocumentProvider, editorOperationsFactory.Object);
// Act & Assert
textBuffer.ApplyEdits(edit, edit);
@ -167,12 +229,13 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var snapshot = new StringTextSnapshot(Environment.NewLine + "Hello World");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(snapshot.Content));
ITextBuffer textBuffer = null;
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView: null);
textBuffer = CreateTextBuffer(snapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(0, Environment.NewLine.Length, Environment.NewLine, documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(0, Environment.NewLine.Length, Environment.NewLine, syntaxTree, documentTracker, out var context);
// Assert
Assert.Null(context);
@ -184,13 +247,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var snapshot = new StringTextSnapshot("This Hello World");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(snapshot.Content));
ITextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(snapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(0, 5, "This ", documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(0, 5, "This ", syntaxTree, documentTracker, out var context);
// Assert
Assert.Null(context);
@ -202,13 +266,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var initialSnapshot = new StringTextSnapshot(Environment.NewLine + "Hello World");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(initialSnapshot.Content));
ITextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(0, Environment.NewLine.Length, Environment.NewLine, documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(0, Environment.NewLine.Length, Environment.NewLine, syntaxTree, documentTracker, out var context);
// Assert
Assert.Null(context);
@ -220,13 +285,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var initialSnapshot = new StringTextSnapshot("Hello\u0085World");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(initialSnapshot.Content));
ITextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(5, 1, "\u0085", documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(5, 1, "\u0085", syntaxTree, documentTracker, out var context);
// Assert
Assert.Null(context);
@ -238,13 +304,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var initialSnapshot = new StringTextSnapshot("@{ " + Environment.NewLine + "World");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(initialSnapshot.Content));
ITextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(3, Environment.NewLine.Length, Environment.NewLine, documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(3, Environment.NewLine.Length, Environment.NewLine, syntaxTree, documentTracker, out var context);
// Assert
Assert.Null(context);
@ -256,13 +323,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
// Arrange
var initialSnapshot = new StringTextSnapshot("@{ \n}");
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create(initialSnapshot.Content));
ITextBuffer textBuffer = null;
var focusedTextView = CreateFocusedTextView(() => textBuffer);
var documentTracker = CreateDocumentTracker(() => textBuffer, focusedTextView);
textBuffer = CreateTextBuffer(initialSnapshot, documentTracker);
// Act
var result = BraceSmartIndenter.TryCreateIndentationContext(3, 1, "\n", documentTracker, out var context);
var result = BraceSmartIndenter.TryCreateIndentationContext(3, 1, "\n", syntaxTree, documentTracker, out var context);
// Assert
Assert.NotNull(context);