From 8b7cf6ec6279a96e626f5984f037fdfa177586e1 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Mon, 4 Sep 2023 16:37:43 -0700 Subject: [PATCH 1/6] Remove. Some test failures. --- .../Formatting/CSharpFormattingPassBase.cs | 13 ++----------- .../Formatting/HtmlFormattingPass.cs | 3 +-- .../LinkedEditingRangeEndpoint.cs | 3 +-- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs index 310afa0b94f..9890a46f1c2 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs @@ -314,18 +314,9 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping var sourceText = context.SourceText; var absoluteIndex = mappingSpan.Start; - if (mappingSpan.Length > 0) - { - // Slightly ugly hack to get around the behavior of LocateOwner. - // In some cases, using the start of a mapping doesn't work well - // because LocateOwner returns the previous node due to it owning the edge. - // So, if we can try to find the owner using a position that fully belongs to the current mapping. - absoluteIndex = mappingSpan.Start + 1; - } - - var change = new SourceChange(absoluteIndex, 0, string.Empty); var syntaxTree = context.CodeDocument.GetSyntaxTree(); - var owner = syntaxTree.Root.LocateOwner(change); + var span = new Language.Syntax.TextSpan(mappingSpan.Start, mappingSpan.Length); + var owner = syntaxTree.Root.FindNode(span, getInnermostNodeForTie: true); if (owner is null) { // Can't determine owner of this position. Optimistically allow formatting. diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs index 3daccd3ef38..b44d620acac 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs @@ -224,8 +224,7 @@ private static List AdjustRazorIndentation(FormattingContext context private static bool IsPartOfHtmlTag(FormattingContext context, int position) { var syntaxTree = context.CodeDocument.GetSyntaxTree(); - var change = new SourceChange(position, 0, string.Empty); - var owner = syntaxTree.Root.LocateOwner(change); + var owner = syntaxTree.Root.FindInnermostNode(position, includeWhitespace: true); if (owner is null) { // Can't determine owner of this position. diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs index b4681d2bb85..5518bfe7de6 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs @@ -111,8 +111,7 @@ static bool TryGetNearestMarkupNameTokens( [NotNullWhen(true)] out SyntaxToken? startTagNameToken, [NotNullWhen(true)] out SyntaxToken? endTagNameToken) { - var change = new SourceChange(location.AbsoluteIndex, length: 0, newText: ""); - var owner = syntaxTree.Root.LocateOwner(change); + var owner = syntaxTree.Root.FindInnermostNode(location.AbsoluteIndex); var element = owner?.FirstAncestorOrSelf( a => a.Kind is SyntaxKind.MarkupTagHelperElement || a.Kind is SyntaxKind.MarkupElement); From 64de4e8d06ca61d12ad2d61c6bdfb793fcd5c0ac Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 5 Sep 2023 13:35:21 +1000 Subject: [PATCH 2/6] Remove unused usings --- .../Formatting/HtmlFormattingPass.cs | 1 - .../LinkedEditingRange/LinkedEditingRangeEndpoint.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs index b44d620acac..6369bc32fe4 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/HtmlFormattingPass.cs @@ -7,7 +7,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor.Language; -using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.AspNetCore.Razor.Language.Syntax; using Microsoft.AspNetCore.Razor.LanguageServer.Extensions; using Microsoft.AspNetCore.Razor.LanguageServer.Protocol; diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs index 5518bfe7de6..4eaae10c7b2 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LinkedEditingRange/LinkedEditingRangeEndpoint.cs @@ -6,7 +6,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor.Language; -using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.AspNetCore.Razor.Language.Syntax; using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts; using Microsoft.AspNetCore.Razor.LanguageServer.Extensions; From 3fadcd331d53cd8b13dfa1e71c7012fa24c61652 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 5 Sep 2023 13:52:38 +1000 Subject: [PATCH 3/6] Make debugging easier --- .../Formatting/CSharpFormattingPassBase.cs | 44 +++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs index 9890a46f1c2..99946d1f3f1 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs @@ -338,14 +338,42 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return true; } - if (IsRazorComment() || - IsInBoundComponentAttributeName() || - IsInHtmlAttributeValue() || - IsInDirectiveWithNoKind() || - IsInSingleLineDirective() || - IsImplicitExpression() || - IsInSectionDirectiveCloseBrace() || - (!allowImplicitStatements && IsImplicitStatementStart())) + if (IsRazorComment()) + { + return false; + } + + if (IsInBoundComponentAttributeName()) + { + return false; + } + + if (IsInHtmlAttributeValue()) + { + return false; + } + + if (IsInDirectiveWithNoKind()) + { + return false; + } + + if (IsInSingleLineDirective()) + { + return false; + } + + if (IsImplicitExpression()) + { + return false; + } + + if (IsInSectionDirectiveCloseBrace()) + { + return false; + } + + if (!allowImplicitStatements && IsImplicitStatementStart()) { return false; } From e9167db2c4aa2f5712d70a2c0659685deb8f9ce3 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 5 Sep 2023 14:09:40 +1000 Subject: [PATCH 4/6] Fix formatting after moving to FindToken based API --- .../Formatting/CSharpFormattingPassBase.cs | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs index 99946d1f3f1..4768747bb95 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs @@ -311,12 +311,8 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return true; } - var sourceText = context.SourceText; - var absoluteIndex = mappingSpan.Start; - var syntaxTree = context.CodeDocument.GetSyntaxTree(); - var span = new Language.Syntax.TextSpan(mappingSpan.Start, mappingSpan.Length); - var owner = syntaxTree.Root.FindNode(span, getInnermostNodeForTie: true); + var owner = syntaxTree.Root.FindInnermostNode(mappingSpan.Start, includeWhitespace: true); if (owner is null) { // Can't determine owner of this position. Optimistically allow formatting. @@ -324,7 +320,6 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return true; } - owner = FixOwnerToWorkaroundCompilerQuirks(owner); foundOwner = owner; // Special case: If we're formatting implicit statements, we want to treat the `@attribute` directive and @@ -338,7 +333,7 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return true; } - if (IsRazorComment()) + if (IsInsideRazorComment()) { return false; } @@ -363,7 +358,7 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return false; } - if (IsImplicitExpression()) + if (!allowImplicitStatements && IsImplicitExpression()) { return false; } @@ -378,11 +373,18 @@ private static bool ShouldFormatCore(FormattingContext context, TextSpan mapping return false; } + if (IsInTemplateBlock()) + { + return false; + } + return true; - bool IsRazorComment() + bool IsInsideRazorComment() { - if (owner.IsCommentSpanKind()) + // We don't want to format _in_ comments, but we do want to move the start `@*` to the right position + if (owner is RazorCommentBlockSyntax && + mappingSpan.Start != owner.SpanStart) { return true; } @@ -505,6 +507,15 @@ bool IsImplicitExpression() return owner.AncestorsAndSelf().Any(n => n is CSharpImplicitExpressionSyntax); } + bool IsInTemplateBlock() + { + // E.g, (| is position) + // + // `RenderFragment(|@);` - true + // + return owner.AncestorsAndSelf().Any(n => n is CSharpTemplateBlockSyntax); + } + bool IsInSectionDirectiveCloseBrace() { // @section Scripts { From 547290636e4520d37e154511edba63e2895613fa Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 5 Sep 2023 14:09:54 +1000 Subject: [PATCH 5/6] Remove unused function Fixes #7505 --- .../Formatting/CSharpFormattingPassBase.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs index 4768747bb95..4593df07dbc 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormattingPassBase.cs @@ -540,26 +540,6 @@ owner.Parent is MarkupBlockSyntax block && } } - private static SyntaxNode FixOwnerToWorkaroundCompilerQuirks(SyntaxNode owner) - { - // Workaround for https://github.com/dotnet/aspnetcore/issues/36689 - // A tags owner comes back as itself if it is preceeded by a HTML comment, - // because the whitespace between the comment and the tag is reported as not editable - - // Get to the outermost node first. eg in " Date: Tue, 5 Sep 2023 20:55:00 +1000 Subject: [PATCH 6/6] Update test output --- .../Formatting/TestFiles/Expected/FormatSelection.cshtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/Formatting/TestFiles/Expected/FormatSelection.cshtml b/src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/Formatting/TestFiles/Expected/FormatSelection.cshtml index c85d047a94b..2e40c8ba43f 100644 --- a/src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/Formatting/TestFiles/Expected/FormatSelection.cshtml +++ b/src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/Formatting/TestFiles/Expected/FormatSelection.cshtml @@ -12,7 +12,7 @@
  • @Html.ActionLink("Home", "Index", "Home")
    • -
  • @Html.ActionLink( "About", "About", "Home")
    • +
  • @Html.ActionLink("About", "About", "Home")
  • @Html.ActionLink("Contact", "Contact", "Home")