huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't smart indent code inside of Razor block constructs. 

- Prior to this change we weren't strict enough with where our smart indenter ran. We made the assumption that every code block could be smart indented and then Roslyn would "do the right thing". However, in nested code block scenarios we found that Roslyn and us would both indent resulting in extra newlines. These changes make the criteria for applying smart indentation a little stricter.
- Updated directive code block parsing to add a C# marker symbol in cases of an empty code block directive.
- Added unit tests to verify new smart indenter behavior.
- Updated existing tests to expect new syntax tree marker symbol for empty directive bits.
- Regenerated baselines.

#2410
This commit is contained in:
N. Taylor Mullen 2018-06-18 17:00:23 -07:00
parent 1aa15374b5
commit 41fad8a33a
9 changed files with 287 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs
View File

@ -1820,6 +1820,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
Span.ChunkGenerator = new StatementChunkGenerator();
var existingEditHandler = Span.EditHandler;
Span.EditHandler = new CodeBlockEditHandler(Language.TokenizeString);
AddMarkerSymbolIfNecessary();
Output(SpanKindInternal.Code);
Span.EditHandler = existingEditHandler;

63
src/Microsoft.VisualStudio.Editor.Razor/BraceSmartIndenter.cs
View File

@ -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.Diagnostics;
using System.Text;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Legacy;
@ -11,6 +10,7 @@ using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Operations;
using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer;
using Span = Microsoft.AspNetCore.Razor.Language.Legacy.Span;
namespace Microsoft.VisualStudio.Editor.Razor
{
@ -210,7 +210,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
var focusedTextView = documentTracker.GetFocusedTextView();
if (focusedTextView != null && ParserHelpers.IsNewLine(finalText))
{
if (!AtValidContentKind(changePosition, syntaxTree))
if (!AtApplicableRazorBlock(changePosition, syntaxTree))
{
context = null;
return false;
@ -242,35 +242,68 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
// Internal for testing
internal static bool AtValidContentKind(int changePosition, RazorSyntaxTree syntaxTree)
internal static bool AtApplicableRazorBlock(int changePosition, RazorSyntaxTree syntaxTree)
{
// Our goal here is to return true when we're acting on code blocks that have all
// whitespace content and are surrounded by metacode.
// Some examples:
// @functions { |}
// @section foo { |}
// @{ |}
var change = new SourceChange(changePosition, 0, string.Empty);
var owner = syntaxTree.Root.LocateOwner(change);
if (owner == null)
if (IsUnlinkedSpan(owner))
{
return false;
}
if (owner.Kind == SpanKindInternal.MetaCode)
if (SurroundedByInvalidContent(owner))
{
// @functions{|}
return true;
return false;
}
if (owner.Kind == SpanKindInternal.Code)
if (ContainsInvalidContent(owner))
{
// 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;
}
// Indentable content inside of a code block.
return true;
}
// Internal for testing
internal static bool ContainsInvalidContent(Span owner)
{
// We only support whitespace based content. Any non-whitespace content is an unkonwn to us
// in regards to indentation.
for (var i = 0; i < owner.Symbols.Count; i++)
{
if (!string.IsNullOrWhiteSpace(owner.Symbols[i].Content))
{
return true;
}
}
return false;
}
// Internal for testing
internal static bool IsUnlinkedSpan(Span owner)
{
return owner == null ||
owner.Next == null ||
owner.Previous == null;
}
// Internal for testing
internal static bool SurroundedByInvalidContent(Span owner)
{
return owner.Next.Kind != SpanKindInternal.MetaCode ||
owner.Previous.Kind != SpanKindInternal.MetaCode;
}
internal static bool BeforeClosingBrace(int linePosition, ITextSnapshotLine lineSnapshot)
{
var lineText = lineSnapshot.GetText();

3
test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpAutoCompleteTest.cs
View File

@ -26,7 +26,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
new DirectiveBlock(chunkGenerator,
Factory.CodeTransition(),
Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None),
Factory.MetaCode("{").AutoCompleteWith("}", atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None)));
Factory.MetaCode("{").AutoCompleteWith("}", atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None),
Factory.EmptyCSharp().AsCodeBlock()));
}
[Fact]

3
test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs
View File

@ -949,7 +949,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
Factory.Span(SpanKindInternal.Markup, " ", markup: false).Accepts(AcceptedCharactersInternal.AllWhiteSpace),
Factory.MetaCode("{")
.AutoCompleteWith("}", atEndOfSpan: true)
.Accepts(AcceptedCharactersInternal.None)));
.Accepts(AcceptedCharactersInternal.None),
Factory.EmptyCSharp().AsCodeBlock()));
}
[Fact]

5
test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives_DesignTime.codegen.cs
View File

@ -63,6 +63,11 @@ global::System.Object __typeHelper = ";
#pragma warning disable 1998
public async System.Threading.Tasks.Task ExecuteAsync()
{
#line 25 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml"
#line default
#line hidden
}
#pragma warning restore 1998
}

2
test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives_DesignTime.ir.txt
View File

@ -82,3 +82,5 @@ Document -
HtmlContent - (339:23,9 [3] IncompleteDirectives.cshtml)
IntermediateToken - (339:23,9 [3] IncompleteDirectives.cshtml) - Html - {\n
MalformedDirective - (342:24,0 [12] IncompleteDirectives.cshtml) - functions
CSharpCode - (354:24,12 [0] IncompleteDirectives.cshtml)
IntermediateToken - (354:24,12 [0] IncompleteDirectives.cshtml) - CSharp -

5
test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives_DesignTime.mappings.txt
View File

@ -63,3 +63,8 @@ Source Location: (326:21,9 [0] TestFiles/IntegrationTests/CodeGenerationIntegrat
Generated Location: (1463:55,0 [0] )
||
Source Location: (354:24,12 [0] TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml)
||
Generated Location: (1879:66,12 [0] )
||

2
test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives_Runtime.ir.txt
View File

@ -52,3 +52,5 @@ Document -
HtmlContent - (339:23,9 [3] IncompleteDirectives.cshtml)
IntermediateToken - (339:23,9 [3] IncompleteDirectives.cshtml) - Html - {\n
MalformedDirective - (342:24,0 [12] IncompleteDirectives.cshtml) - functions
CSharpCode - (354:24,12 [0] IncompleteDirectives.cshtml)
IntermediateToken - (354:24,12 [0] IncompleteDirectives.cshtml) - CSharp -

230
test/Microsoft.VisualStudio.Editor.Razor.Test/BraceSmartIndenterTest.cs
View File

@ -5,47 +5,232 @@ using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Extensions;
using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.VisualStudio.Test;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Operations;
using Moq;
using Xunit;
using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer;
using Span = Microsoft.AspNetCore.Razor.Language.Legacy.Span;
namespace Microsoft.VisualStudio.Editor.Razor
{
public class BraceSmartIndenterTest : BraceSmartIndenterTestBase
{
[Fact]
public void AtValidContentKind_ReturnsFalseAtMarkup()
public void AtApplicableRazorBlock_NestedIfBlock_ReturnsFalse()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("<p></p>"));
var changePosition = 2;
var syntaxTree = GetSyntaxTree("@{ if (true) { } }");
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
var result = BraceSmartIndenter.AtApplicableRazorBlock(14, syntaxTree);
// Assert
Assert.False(result);
}
[Fact]
public void AtValidContentKind_ReturnsTrueAtCode()
public void AtApplicableRazorBlock_SectionBlock_ReturnsTrue()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("@{}"));
var changePosition = 2;
var syntaxTree = GetSyntaxTree("@section Foo { }");
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
var result = BraceSmartIndenter.AtApplicableRazorBlock(15, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtValidContentKind_ReturnsTrueAtMetacode()
public void AtApplicableRazorBlock_FunctionsBlock_ReturnsTrue()
{
// Arrange
var syntaxTree = GetSyntaxTree("@functions { }");
// Act
var result = BraceSmartIndenter.AtApplicableRazorBlock(13, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtApplicableRazorBlock_ExplicitCodeBlock_ReturnsTrue()
{
// Arrange
var syntaxTree = GetSyntaxTree("@{ }");
// Act
var result = BraceSmartIndenter.AtApplicableRazorBlock(3, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void ContainsInvalidContent_NewLineSpan_ReturnsFalse()
{
// Arrange
var span = ExtractSpan(2, "@{" + Environment.NewLine + "}");
// Act
var result = BraceSmartIndenter.ContainsInvalidContent(span);
// Assert
Assert.False(result);
}
[Fact]
public void ContainsInvalidContent_WhitespaceSpan_ReturnsFalse()
{
// Arrange
var span = ExtractSpan(2, "@{ }");
// Act
var result = BraceSmartIndenter.ContainsInvalidContent(span);
// Assert
Assert.False(result);
}
[Fact]
public void ContainsInvalidContent_MarkerSpan_ReturnsFalse()
{
// Arrange
var span = ExtractSpan(3, "@{}");
// Act
var result = BraceSmartIndenter.ContainsInvalidContent(span);
// Assert
Assert.False(result);
}
[Fact]
public void ContainsInvalidContent_NonWhitespaceMarker_ReturnsTrue()
{
// Arrange
var span = ExtractSpan(2, "@{ if}");
// Act
var result = BraceSmartIndenter.ContainsInvalidContent(span);
// Assert
Assert.True(result);
}
[Fact]
public void IsUnlinkedSpan_NullPrevious_ReturnsTrue()
{
// Arrange
var span = ExtractSpan(0, "@{}");
// Act
var result = BraceSmartIndenter.IsUnlinkedSpan(span);
// Assert
Assert.True(result);
}
[Fact]
public void IsUnlinkedSpan_NullNext_ReturnsTrue()
{
// Arrange
var span = ExtractSpan(3, "@{}");
// Act
var result = BraceSmartIndenter.IsUnlinkedSpan(span);
// Assert
Assert.True(result);
}
[Fact]
public void IsUnlinkedSpan_NullOwner_ReturnsTrue()
{
// Arrange
Span owner = null;
// Act
var result = BraceSmartIndenter.IsUnlinkedSpan(owner);
// Assert
Assert.True(result);
}
[Fact]
public void SurroundedByInvalidContent_MetacodeSurroundings_ReturnsFalse()
{
// Arrange
var span = ExtractSpan(2, "@{}");
// Act
var result = BraceSmartIndenter.SurroundedByInvalidContent(span);
// Assert
Assert.False(result);
}
[Fact]
public void SurroundedByInvalidContent_OnlyNextMetacode_ReturnsTrue()
{
// Arrange
var span = ExtractSpan(9, "@{<p></p>}");
// Act
var result = BraceSmartIndenter.SurroundedByInvalidContent(span);
// Assert
Assert.True(result);
}
[Fact]
public void SurroundedByInvalidContent_OnlyPreviousMetacode_ReturnsTrue()
{
// Arrange
var span = ExtractSpan(2, "@{<p>");
// Act
var result = BraceSmartIndenter.SurroundedByInvalidContent(span);
// Assert
Assert.True(result);
}
[Fact]
public void AtApplicableRazorBlock_AtMarkup_ReturnsFalse()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("<p></p>"));
var changePosition = 2;
// Act
var result = BraceSmartIndenter.AtApplicableRazorBlock(changePosition, syntaxTree);
// Assert
Assert.False(result);
}
[Fact]
public void AtApplicableRazorBlock_AtExplicitCodeBlocksCode_ReturnsTrue()
{
// Arrange
var syntaxTree = RazorSyntaxTree.Parse(TestRazorSourceDocument.Create("@{}"));
var changePosition = 2;
// Act
var result = BraceSmartIndenter.AtApplicableRazorBlock(changePosition, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtApplicableRazorBlock_AtMetacode_ReturnsTrue()
{
// Arrange
var parseOptions = RazorParserOptions.Create(options => options.Directives.Add(FunctionsDirective.Directive));
@ -53,21 +238,21 @@ namespace Microsoft.VisualStudio.Editor.Razor
var changePosition = 12;
// Act
var result = BraceSmartIndenter.AtValidContentKind(changePosition, syntaxTree);
var result = BraceSmartIndenter.AtApplicableRazorBlock(changePosition, syntaxTree);
// Assert
Assert.True(result);
}
[Fact]
public void AtValidContentKind_ReturnsFalseWhenNoOwner()
public void AtApplicableRazorBlock_WhenNoOwner_ReturnsFalse()
{
// 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);
var result = BraceSmartIndenter.AtApplicableRazorBlock(changePosition, syntaxTree);
// Assert
Assert.False(result);
@ -339,6 +524,27 @@ namespace Microsoft.VisualStudio.Editor.Razor
Assert.True(result);
}
private static RazorSyntaxTree GetSyntaxTree(string content)
{
var syntaxTree = RazorSyntaxTree.Parse(
TestRazorSourceDocument.Create(content),
RazorParserOptions.Create(options =>
{
options.Directives.Add(FunctionsDirective.Directive);
options.Directives.Add(SectionDirective.Directive);
}));
syntaxTree.Root.LinkNodes();
return syntaxTree;
}
private static Span ExtractSpan(int spanLocation, string content)
{
var syntaxTree = GetSyntaxTree(content);
var span = syntaxTree.Root.LocateOwner(new SourceChange(new SourceSpan(spanLocation, 0), string.Empty));
return span;
}
protected class TestTextContentChangedEventArgs : TextContentChangedEventArgs
{
public TestTextContentChangedEventArgs(INormalizedTextChangeCollection changeCollection)