diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/Delegation/DelegatedCompletionHelper.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/Delegation/DelegatedCompletionHelper.cs index 4fd830ad193..33932af8231 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/Delegation/DelegatedCompletionHelper.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/Delegation/DelegatedCompletionHelper.cs @@ -255,7 +255,7 @@ static bool IsInScriptOrStyleOrHtmlComment(AspNetCore.Razor.Language.Syntax.Synt { if (node is MarkupElementSyntax elementNode) { - if (RazorSyntaxFacts.IsStyleBlock(elementNode) || RazorSyntaxFacts.IsScriptBlock(elementNode)) + if (RazorSyntaxFacts.IsScriptOrStyleBlock(elementNode)) { return true; } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs index 6c825d0c7b8..76c4cb5158b 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs @@ -172,6 +172,11 @@ private static ImmutableArray GetFormattingSpans(RazorSyntaxTree /// A whitespace string representing the indentation level based on the configuration. public string GetIndentationLevelString(int indentationLevel) { + if (indentationLevel == 0) + { + return ""; + } + var indentation = GetIndentationOffsetForLevel(indentationLevel); var indentationString = FormattingUtilities.GetIndentationString(indentation, Options.InsertSpaces, Options.TabSize); return indentationString; diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingUtilities.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingUtilities.cs index e4fbc2fca03..079e1a1b376 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingUtilities.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingUtilities.cs @@ -152,9 +152,7 @@ public static int GetIndentationLevel(TextLine line, int firstNonWhitespaceChara var currentIndentationWidth = firstNonWhitespaceCharacterPosition - line.Start; if (insertSpaces) { - var indentationLevel = currentIndentationWidth / tabSize; - additionalIndentation = new string(' ', currentIndentationWidth % tabSize); - return indentationLevel; + return GetIndentationLevel(currentIndentationWidth, tabSize, out additionalIndentation); } // For tabs, we just count the tabs, and additional is any spaces at the end. @@ -178,6 +176,16 @@ public static int GetIndentationLevel(TextLine line, int firstNonWhitespaceChara return tabCount; } + public static int GetIndentationLevel(int length, int tabSize, out string additionalIndentation) + { + var indentationLevel = length / tabSize; + var additionalIndentationLength = length % tabSize; + additionalIndentation = additionalIndentationLength == 0 + ? "" + : new string(' ', additionalIndentationLength); + return indentationLevel; + } + /// /// Given a amount of characters, generate a string representing the configured indentation. /// diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs index b0adf2f1dd2..f75f0612720 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.CSharpDocumentGenerator.cs @@ -49,12 +49,12 @@ internal partial class CSharpFormattingPass /// /// The generator will go through that syntax tree and produce the following C# document: /// - /// // <div> + /// { // <div> /// @if (true) /// { /// // Some code /// } - /// // </div> + /// } // </div> /// /// class F /// { @@ -64,9 +64,11 @@ internal partial class CSharpFormattingPass /// /// /// The class definition is clearly not present in the source Razor document, but it represents the intended - /// indentation that the user would expect to see for the property declaration. Additionally the indentation - /// of the @if block is recorded, so that after formatting the C#, which Roslyn will set back to column 0, we - /// reapply it so we end up with the C# indentation and Html indentation combined. + /// indentation that the user would expect to see for the property declaration. The Html element start and + /// end tags are emited as braces so they cause the right amount of indentation, and the indentation of the + /// @if block is recorded, so that any quirks of Roslyn formatting are the same as what the user would expect + /// when looking at equivalent code in C#. ie, there might only be one level of indentation in C# for "Some code" + /// but conceptually the user is expecting two, due to the Html element. /// /// /// For more complete examples, the full test log for every formatting test includes the generated C# document. @@ -167,13 +169,13 @@ private sealed class Generator( private RazorSyntaxToken _previousCurrentToken; /// - /// The line number of the last line of the current element, if we're inside one. + /// The line number of the last line an element, if we're inside one, where we care about the Html formatters indentation /// /// /// This is used to track if the syntax node at the start of each line is parented by an element node, without - /// having to do lots of tree traversal. + /// having to do lots of tree traversal, for formatting the contents of script and style tags. /// - private int? _elementEndLine; + private int? _honourHtmlFormattingUntilLine; /// /// The line number of the last line of a block where formatting should be completely ignored /// @@ -239,10 +241,10 @@ public void Generate() } // If we're inside an element that ends on this line, clear the field that tracks it. - if (_elementEndLine is { } endLine && + if (_honourHtmlFormattingUntilLine is { } endLine && endLine == line.LineNumber) { - _elementEndLine = null; + _honourHtmlFormattingUntilLine = null; } if (_ignoreUntilLine is { } endLine2 && @@ -267,7 +269,7 @@ private void AddAdditionalLineFormattingContent(StringBuilder additionalLinesBui // what the user expects. if (node is { Parent.Parent: MarkupTagHelperAttributeSyntax attribute } && attribute is { Parent.Parent: MarkupTagHelperElementSyntax element } && - element.TagHelperInfo.BindingResult.TagHelpers is [{ } descriptor] && + element.TagHelperInfo.BindingResult.TagHelpers is [{ } descriptor, ..] && descriptor.IsGenericTypedComponent() && descriptor.BoundAttributes.FirstOrDefault(d => d.Name == attribute.TagHelperAttributeInfo.Name) is { } boundAttribute && boundAttribute.IsTypeParameterProperty()) @@ -427,7 +429,7 @@ public override LineInfo VisitCSharpStatementLiteral(CSharpStatementLiteralSynta previousSibling is CSharpTemplateBlockSyntax && GetLineNumber(previousSibling.GetFirstToken()) != GetLineNumber(previousSibling.GetLastToken())) { - _builder.AppendLine("};"); + _builder.AppendLine(";"); return CreateLineInfo(); } @@ -526,15 +528,22 @@ public override LineInfo VisitMarkupStartTag(MarkupStartTagSyntax node) { // The contents of textareas is significant, so we never want any formatting to happen inside them _ignoreUntilLine = GetLineNumber(element.EndTag?.CloseAngle ?? element.StartTag.CloseAngle); + + return EmitCurrentLineAsComment(); } - else if (_elementEndLine is null) + + var result = VisitStartTag(node); + + if (RazorSyntaxFacts.IsScriptOrStyleBlock(element) && + _honourHtmlFormattingUntilLine is null) { // If this is an element at the root level, we want to record where it ends. We can't rely on the Visit method - // for it, because it might not be at the start of a line. - _elementEndLine = GetLineNumber(element.EndTag?.CloseAngle ?? element.StartTag.CloseAngle); + // for it, because it might not be at the start of a line. We only care about contents though, so thats why + // we are doing this after emitting this line, and minusing one from the end element line number. + _honourHtmlFormattingUntilLine = GetLineNumber(element.EndTag?.CloseAngle ?? element.StartTag.CloseAngle) - 1; } - return EmitCurrentLineAsComment(); + return result; } public override LineInfo VisitMarkupEndTag(MarkupEndTagSyntax node) @@ -544,6 +553,10 @@ public override LineInfo VisitMarkupEndTag(MarkupEndTagSyntax node) private LineInfo VisitEndTag(BaseMarkupEndTagSyntax node) { + // End tags are emited as close braces, to remove the indent their start tag caused. We don't need to worry + // about single-line elements here, because these Visit methods only ever see the first node on a line. + _builder.Append('}'); + // If this is the last line of a multi-line CSharp template (ie, RenderFragment), and the semicolon that ends // if is on the same line, then we need to close out the lambda expression that we started when we opened it. // If the semicolon is on the next line, then we'll take care of that when we get to it. @@ -554,28 +567,86 @@ private LineInfo VisitEndTag(BaseMarkupEndTagSyntax node) semiColonToken.Content == ";" && GetLineNumber(semiColonToken) == GetLineNumber(_currentToken)) { - _builder.AppendLine("};"); - return CreateLineInfo(); + _builder.Append(';'); } - return EmitCurrentLineAsComment(); +#if DEBUG + _builder.Append($" /* {_currentLine} */"); +#endif + + _builder.AppendLine(); + return CreateLineInfo(); } public override LineInfo VisitMarkupTagHelperStartTag(MarkupTagHelperStartTagSyntax node) { - // If this is an element at the root level, we want to record where it ends. We can't rely on the Visit method - // for it, because it might not be at the start of a line. - if (_elementEndLine is null) + return VisitStartTag(node); + } + + private LineInfo VisitStartTag(BaseMarkupStartTagSyntax node) + { + if (ElementCausesIndentation(node)) { - var element = (MarkupTagHelperElementSyntax)node.Parent; - _elementEndLine = GetLineNumber(element.EndTag?.CloseAngle ?? element.StartTag.CloseAngle); + // When an element causes indentation, we emit an open brace to tell the C# formatter to indent. + // Any open brace we emit that represents something "real" must have something after it to avoid + // us skipping it due to SkipNextLineIfOpenBrace on the previous line. +#if DEBUG + _builder.AppendLine($$"""{ /* {{_currentLine}} */"""); +#else + _builder.AppendLine("{ /* */"); +#endif + return CreateLineInfo(); } + // This is a single line element, so it doesn't cause indentation return EmitCurrentLineAsComment(); } + private bool ElementCausesIndentation(BaseMarkupStartTagSyntax node) + { + if (node.GetEndTag() is not { } endTag) + { + // No end tag, so it's self-closing and doesn't cause indentation + return false; + } + + var endTagLineNumber = GetLineNumber(endTag); + if (endTagLineNumber == GetLineNumber(node)) + { + // End tag and start tag are on the same line + return false; + } + + if (endTagLineNumber == GetLineNumber(node.CloseAngle)) + { + // End tag is on the same line as the start tag's close angle bracket + return false; + } + + if (node.Name.Content == "html") + { + // doesn't cause indentation in Visual Studio or VS Code, so we honour that. + return false; + } + + if (node is MarkupStartTagSyntax startTag && + (startTag.IsVoidElement() || startTag.IsSelfClosing())) + { + // Void elements and self-closing elements don't cause indentation + return false; + } + + return true; + } + public override LineInfo VisitRazorMetaCode(RazorMetaCodeSyntax node) { + // This could be a directive attribute, like @bind-Value="asdf" + if (TryVisitAttribute(node) is { } result) + { + return result; + } + // Meta code is a few things, and mostly they're valid C#, but one case we have to specifically handle is // bound attributes that start on their own line, eg the second line of: // @@ -611,26 +682,57 @@ public override LineInfo VisitRazorMetaCode(RazorMetaCodeSyntax node) public override LineInfo VisitMarkupEphemeralTextLiteral(MarkupEphemeralTextLiteralSyntax node) { // A MarkupEphemeralTextLiteral is an escaped @ sign, eg in CSS "@@font-face". We just treat it like markup text - return VisitMarkupLiteral(); + return EmitCurrentLineAsComment(); } public override LineInfo VisitMarkupTextLiteral(MarkupTextLiteralSyntax node) { - return VisitMarkupLiteral(); + if (node.Parent is MarkupCommentBlockSyntax comment) + { + return VisitMarkupCommentBlock(comment); + } + + if (TryVisitAttribute(node) is { } result) + { + return result; + } + + return EmitCurrentLineAsComment(); } - private LineInfo VisitMarkupLiteral() + public override LineInfo VisitMarkupCommentBlock(MarkupCommentBlockSyntax node) { - // For markup text literal, we always want to honour the Html formatter, so we supply the Html indent. - // Normally that would only happen if we were inside a markup element -#if DEBUG - _builder.AppendLine($"// {_currentLine}"); -#else - _builder.AppendLine($"//"); -#endif - return CreateLineInfo( - htmlIndentLevel: FormattingUtilities.GetIndentationLevel(_currentLine, _currentFirstNonWhitespacePosition, _insertSpaces, _tabSize, out var additionalIndentation), - additionalIndentation: additionalIndentation); + // The contents of html comments might be significant (eg tables or ASCII flow charts), + // so we never want any formatting to happen inside them. + _ignoreUntilLine = _sourceText.Lines.GetLineFromPosition(node.EndPosition).LineNumber; + + return EmitCurrentLineAsComment(); + } + + private LineInfo? TryVisitAttribute(RazorSyntaxNode node) + { + if (RazorSyntaxFacts.IsAttributeName(node, out var startTag)) + { + // Attributes are indented to align with the first attribute in their tag. In future, now that we're handling + // this ourselves, we can make this an option: https://github.com/dotnet/razor/issues/6551 + var firstAttribute = startTag.Attributes[0]; + var nameSpan = RazorSyntaxFacts.GetFullAttributeNameSpan(firstAttribute); + + // We need to line up with the first attribute, but the start tag might not be the first thing on the line, + // so it's really relative to the first non-whitespace character on the line + var lineStart = _sourceText.Lines[GetLineNumber(startTag)].GetFirstNonWhitespacePosition().GetValueOrDefault(); + var htmlIndentLevel = FormattingUtilities.GetIndentationLevel(nameSpan.Start - lineStart, _tabSize, out var additionalIndentation); + // If the element has caused indentation, then we'll want to take one level off our attribute indentation to + // compensate. Need to be careful here because things like ` 0) + { + htmlIndentLevel--; + } + + return EmitCurrentLineAsComment(htmlIndentLevel: htmlIndentLevel, additionalIndentation: additionalIndentation); + } + + return null; } public override LineInfo VisitMarkupTagHelperEndTag(MarkupTagHelperEndTagSyntax node) @@ -732,8 +834,33 @@ public override LineInfo VisitCSharpExplicitExpression(CSharpExplicitExpressionS if (body.CSharpCode.Children is [_, { } secondChild, ..] && GetLineNumber(secondChild) == GetLineNumber(node)) { + // Emit the whitespace, so user spacing is honoured if possible + _builder.Append(_sourceText.ToString(TextSpan.FromBounds(_currentLine.Start, _currentFirstNonWhitespacePosition))); var span = TextSpan.FromBounds(_currentFirstNonWhitespacePosition + 1, secondChild.Position); _builder.Append(_sourceText.ToString(span)); + + if (secondChild is CSharpTemplateBlockSyntax template && + _sourceText.GetLinePositionSpan(template.Span).SpansMultipleLines()) + { + _builder.AppendLine("() => {"); + // We only want to format up to the text we added, but if Roslyn inserted a newline before the brace + // then that position will be different. If we're not given the options then we assume the default behaviour of + // Roslyn which is to insert the newline. + var formattedOffsetFromEndOfLine = _csharpSyntaxFormattingOptions?.NewLines.IsFlagSet(RazorNewLinePlacement.BeforeOpenBraceInLambdaExpressionBody) ?? true + ? 5 + : 7; + + return CreateLineInfo( + skipNextLineIfBrace: true, + originOffset: 1, + formattedLength: span.Length, + formattedOffsetFromEndOfLine: formattedOffsetFromEndOfLine, + processFormatting: true, + // We turn off check for new lines because that only works if the content doesn't change from the original, + // but we're deliberately leaving out a bunch of the original file because it would confuse the Roslyn formatter. + checkForNewLines: false); + } + // Append a comment at the end so whitespace isn't removed, as Roslyn thinks its the end of the line, but we know it isn't. _builder.AppendLine(" //"); @@ -747,6 +874,9 @@ public override LineInfo VisitCSharpExplicitExpression(CSharpExplicitExpressionS checkForNewLines: false); } + // Multi-line expressions are often not formatted by Roslyn much at all, but it will often move subsequent lines + // relative to the first, so make sure we include the users indentation so everything moves together, and is stable. + _builder.Append(_sourceText.GetSubTextString(TextSpan.FromBounds(_currentLine.Start, _currentFirstNonWhitespacePosition))); _builder.AppendLine(_sourceText.GetSubTextString(TextSpan.FromBounds(_currentToken.Position + 1, _currentLine.End))); return CreateLineInfo( processFormatting: true, @@ -784,7 +914,9 @@ public override LineInfo VisitCSharpStatement(CSharpStatementSyntax node) // We don't need to worry about formatting, or offsetting, because the RazorFormattingPass will // have ensured this node is followed by a newline, and if there was a space between the "@" and "{" // then it wouldn't be a CSharpStatementSyntax so we wouldn't be here! - _builder.AppendLine("{"); + // Any open brace we emit that represents something "real" must have something after it to avoid + // us skipping it due to SkipNextLineIfOpenBrace on the previous line. + _builder.AppendLine("{ /* */"); return CreateLineInfo(); } @@ -822,7 +954,9 @@ body.CSharpCode is CSharpCodeBlockSyntax code && // If the open brace is on the same line as the directive, then we need to ensure the contents are indented. GetLineNumber(brace) == GetLineNumber(_currentToken)) { - _builder.AppendLine("{"); + // Any open brace we emit that represents something "real" must have something after it to avoid + // us skipping it due to SkipNextLineIfOpenBrace on the previous line. + _builder.AppendLine("{ /* */"); return CreateLineInfo(); } @@ -897,14 +1031,14 @@ private LineInfo EmitCurrentLineAsCSharp() return CreateLineInfo(processFormatting: true, checkForNewLines: true); } - private LineInfo EmitCurrentLineAsComment() + private LineInfo EmitCurrentLineAsComment(int htmlIndentLevel = 0, string? additionalIndentation = null) { #if DEBUG _builder.AppendLine($"// {_currentLine}"); #else _builder.AppendLine($"//"); #endif - return CreateLineInfo(); + return CreateLineInfo(htmlIndentLevel: htmlIndentLevel, additionalIndentation: additionalIndentation); } private LineInfo EmitCurrentLineWithNoFormatting() @@ -927,17 +1061,14 @@ private LineInfo CreateLineInfo( int formattedOffsetFromEndOfLine = 0, string? additionalIndentation = null) { - // We want to honour the indentation that the Html formatter supplied, but annoyingly it only actually indents - // the contents of elements, not anything which is not contained in an element. This makes sense from the point - // of view of Html, as it would expect the element to always be present, but that is not true in Razor. - // So we have to check if we're inside an element before we record the indentation, otherwise we could be - // recording incorrect information. + // We sometimes want to honour the indentation that the Html formatter supplied, when inside the right type of tag + // but we will also have added our own C# indentation on top of that, so we need to subtract one level to compensate. if (additionalIndentation is null && htmlIndentLevel == 0 && - _elementEndLine is { } endLine && + _honourHtmlFormattingUntilLine is { } endLine && endLine >= _currentLine.LineNumber) { - htmlIndentLevel = FormattingUtilities.GetIndentationLevel(_currentLine, _currentFirstNonWhitespacePosition, _insertSpaces, _tabSize, out additionalIndentation); + htmlIndentLevel = FormattingUtilities.GetIndentationLevel(_currentLine, _currentFirstNonWhitespacePosition, _insertSpaces, _tabSize, out additionalIndentation) - 1; } return new( diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs index eac8d041e9a..883d3d6d216 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs @@ -73,6 +73,8 @@ public async Task> ExecuteAsync(FormattingContext con break; } + string? indentationString = null; + var formattedLine = formattedCSharpText.Lines[iFormatted]; if (lineInfo.ProcessIndentation && formattedLine.GetFirstNonWhitespaceOffset() is { } formattedIndentation) @@ -85,7 +87,7 @@ public async Task> ExecuteAsync(FormattingContext con // First up, we take the indentation from the formatted file, and add on the Html indentation level from the line info, and // replace whatever was in the original file with it. var htmlIndentString = context.GetIndentationLevelString(lineInfo.HtmlIndentLevel); - var indentationString = formattedCSharpText.ToString(new TextSpan(formattedLine.Start, formattedIndentation)) + indentationString = formattedCSharpText.ToString(new TextSpan(formattedLine.Start, formattedIndentation)) + htmlIndentString + lineInfo.AdditionalIndentation; formattingChanges.Add(new TextChange(new TextSpan(originalLine.Start, originalLineOffset), indentationString)); @@ -173,6 +175,21 @@ public async Task> ExecuteAsync(FormattingContext con if (NextLineIsOnlyAnOpenBrace(changedText, iOriginal)) { iOriginal++; + + // We're skipping a line in the original document, because Roslyn brought it up to the previous + // line, but the fact is the opening brace was in the original document, and might need its indentation + // adjusted. Since we can't reason about this line in any way, because Roslyn has changed it, we just + // apply the indentation from the previous line. Previously the indentation on this line would have been + // adjusted by the Html formatter and we wouldn't have needed to worry. + // + // If we didn't adjust the indentation of the previous line, then we really have no information to go + // on at all, so hopefully the user is happy with where their open brace is. + if (indentationString is not null) + { + var originalLine = changedText.Lines[iOriginal]; + var originalLineOffset = originalLine.GetFirstNonWhitespaceOffset().GetValueOrDefault(); + formattingChanges.Add(new TextChange(new TextSpan(originalLine.Start, originalLineOffset), indentationString)); + } } } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/HtmlFormattingPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/HtmlFormattingPass.cs index 2b496293d3d..4b658db151c 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/HtmlFormattingPass.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/HtmlFormattingPass.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -87,13 +88,26 @@ private async Task> FilterIncomingChangesAsync(Format var syntaxRoot = codeDocument.GetRequiredSyntaxRoot(); var sourceText = codeDocument.Source.Text; SyntaxNode? csharpSyntaxRoot = null; + ImmutableArray scriptAndStyleElementBounds = default; using var changesToKeep = new PooledArrayBuilder(capacity: changes.Length); foreach (var change in changes) { - // We don't keep changes that start inside of a razor comment block. var node = syntaxRoot.FindInnermostNode(change.Span.Start); + + // We don't keep indentation changes that aren't in a script or style block + // As a quick check, we only care about dropping edits that affect indentation - ie, are before the first + // whitespace char on a line + var line = sourceText.Lines.GetLineFromPosition(change.Span.Start); + if (change.Span.Start < line.GetFirstNonWhitespacePosition() && + !ChangeIsInsideScriptOrStyleElement(change)) + { + context.Logger?.LogMessage($"Dropping change {change} because it's not in a script or style block"); + continue; + } + + // We don't keep changes that start inside of a razor comment block. var comment = node?.FirstAncestorOrSelf(); if (comment is not null && change.Span.Start > comment.SpanStart) { @@ -190,6 +204,60 @@ async Task ChangeIsInStringLiteralAsync(FormattingContext context, RazorCS return false; } + + bool ChangeIsInsideScriptOrStyleElement(TextChange change) + { + // Rather than ascend up the tree for every change, and look for script/style elements, we build up an index + // first and just reuse it for every subsequent edit. + InitializeElementBoundsArray(); + + // If there aren't any elements we're interested in, we don't need to do anything + if (scriptAndStyleElementBounds.Length == 0) + { + return false; + } + + var index = scriptAndStyleElementBounds.BinarySearch(change.Span.Start); + + // If we got a hit, then we're on the tag itself, which is not inside the tag + if (index >= 0) + { + return false; + } + + // If we don't get a hit, the complement of the result is the index to the next largest item in the array. Since we add + // things to the array in pairs, we know that if that index is even, we're outside an element, so we want to ignore identation. + // This only works if the array is ordered, but we can assume it is because we built it in order, and it makes no sense to nest + // script tags within style tags, or vice versa. + index = ~index; + if (index % 2 == 0) + { + return false; + } + + return true; + } + + void InitializeElementBoundsArray() + { + if (!scriptAndStyleElementBounds.IsDefault) + { + return; + } + + using var boundsBuilder = new PooledArrayBuilder(); + + foreach (var element in syntaxRoot.DescendantNodes(static node => node is RazorDocumentSyntax or MarkupBlockSyntax or MarkupElementSyntax or CSharpCodeBlockSyntax).OfType()) + { + if (RazorSyntaxFacts.IsScriptOrStyleBlock(element)) + { + boundsBuilder.Add(element.SpanStart); + boundsBuilder.Add(element.Span.End); + } + } + + scriptAndStyleElementBounds = boundsBuilder.ToImmutableAndClear(); + } } internal TestAccessor GetTestAccessor() => new(this); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs index ccc5c6fcf3c..92d5a1ebbb7 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/RazorSyntaxFacts.cs @@ -84,10 +84,12 @@ public static bool TryGetFullAttributeNameSpan(RazorCodeDocument codeDocument, i return attributeNameSpan != default; } - private static TextSpan GetFullAttributeNameSpan(RazorSyntaxNode? node) + public static TextSpan GetFullAttributeNameSpan(RazorSyntaxNode? node) { return node switch { + MarkupAttributeBlockSyntax att => att.Name.Span, + MarkupMinimizedAttributeBlockSyntax att => att.Name.Span, MarkupTagHelperAttributeSyntax att => att.Name.Span, MarkupMinimizedTagHelperAttributeSyntax att => att.Name.Span, MarkupTagHelperDirectiveAttributeSyntax att => CalculateFullSpan(att.Name, att.ParameterName, att.Transition), @@ -208,18 +210,29 @@ internal static bool IsInUsingDirective(RazorSyntaxNode node) return false; } - internal static bool IsElementWithName(MarkupElementSyntax? element, string name) + internal static bool IsScriptOrStyleBlock(MarkupElementSyntax? element) { - return string.Equals(element?.StartTag.Name.Content, name, StringComparison.OrdinalIgnoreCase); - } + // StartTag is annotated as not nullable, but on invalid documents it can be. The 'Format_DocumentWithDiagnostics' test + // illustrates this. + if (element?.StartTag?.Name.Content is not { } tagName) + { + return false; + } - internal static bool IsStyleBlock(MarkupElementSyntax? node) - { - return IsElementWithName(node, "style"); + return string.Equals(tagName, "script", StringComparison.OrdinalIgnoreCase) || + string.Equals(tagName, "style", StringComparison.OrdinalIgnoreCase); } - internal static bool IsScriptBlock(MarkupElementSyntax? node) + internal static bool IsAttributeName(RazorSyntaxNode node, [NotNullWhen(true)] out BaseMarkupStartTagSyntax? startTag) { - return IsElementWithName(node, "script"); + startTag = null; + + if (node.Parent.IsAnyAttributeSyntax() && + GetFullAttributeNameSpan(node.Parent).Start == node.SpanStart) + { + startTag = node.Parent.Parent as BaseMarkupStartTagSyntax; + } + + return startTag is not null; } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs index cdaff390f00..e3eb6163e8d 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs @@ -472,14 +472,14 @@ protected void ToggleIconMenu(bool iconMenuActive) """, expected: """
- @code + @code + { + private bool IconMenuActive { get; set; } = false; + protected void ToggleIconMenu(bool iconMenuActive) { - private bool IconMenuActive { get; set; } = false; - protected void ToggleIconMenu(bool iconMenuActive) - { - IconMenuActive = iconMenuActive; - } + IconMenuActive = iconMenuActive; } + }
""", csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with @@ -653,15 +653,15 @@ private void IncrementCount() """, expected: """ - @code - { - private int currentCount = 0; + @code + { + private int currentCount = 0; - private void IncrementCount() - { - currentCount++; - } + private void IncrementCount() + { + currentCount++; } + } """, inGlobalNamespace: inGlobalNamespace); @@ -686,15 +686,15 @@ private void IncrementCount() """, expected: """ - @code - { - private int currentCount = 0; + @code + { + private int currentCount = 0; - private void IncrementCount() - { - currentCount++; - } + private void IncrementCount() + { + currentCount++; } + } """); } @@ -718,15 +718,15 @@ private void IncrementCount() """, expected: """ - @code - { - private int currentCount = 0; + @code + { + private int currentCount = 0; - private void IncrementCount() - { - currentCount++; - } + private void IncrementCount() + { + currentCount++; } + } """); } @@ -1896,7 +1896,7 @@ void Method() { }

Hello, world!

- Welcome to your new app. + Welcome to your new app. @@ -3392,7 +3392,7 @@ class C
@(new C() - .M("Hello") + .M("Hello") .M("World") .M(source => { @@ -3405,15 +3405,15 @@ class C @(DateTime.Now) @(DateTime - .Now - .ToString()) + .Now + .ToString()) @(Html.DisplayNameFor(@

) - .ToString()) + .ToString()) @{ var x = @

Hi there!

; @@ -3477,29 +3477,81 @@ await RunFormattingTestAsync(

) - .ToString()) + .ToString()) @(Html.DisplayNameFor(@
, - 1, 3, 4)) + 1, 3, 4)) @(Html.DisplayNameFor(@
, - 1, 3, @
, - 2, 4)) + 1, 3, @
, + 2, 4)) @(Html.DisplayNameFor( - 1, 3, @
, - 2, 4)) + 1, 3, @
, + 2, 4)) @(Html.DisplayNameFor( - 1, 3, - 2, 4)) + 1, 3, + 2, 4)) @(Html.DisplayNameFor( - 2, 4, - 1, 3, @
, - 2, 4, - 1, 3, @
, - 4)) + 2, 4, + 1, 3, @
, + 2, 4, + 1, 3, @
, + 4)) +
+ """, + fileKind: RazorFileKind.Legacy); + } + + [FormattingTestFact] + [WorkItem("https://github.com/dotnet/razor/issues/6110")] + public async Task FormatExplicitCSharpInsideHtml3() + { + await RunFormattingTestAsync( + input: """ + @using System.Text; + +
+ @(new C() + .M("Hello") + .M("World") + .M(source => + { + if (source.Length > 0) + { + source.ToString(); + } + })) + + @(DateTime.Now) + + @(DateTime + .Now + .ToString()) +
+ """, + expected: """ + @using System.Text; + +
+ @(new C() + .M("Hello") + .M("World") + .M(source => + { + if (source.Length > 0) + { + source.ToString(); + } + })) + + @(DateTime.Now) + + @(DateTime + .Now + .ToString())
""", fileKind: RazorFileKind.Legacy); @@ -3772,6 +3824,8 @@ await RunFormattingTestAsync( [FormattingTestFact] public async Task Format_DocumentWithDiagnostics() { + // The malformed closing div in the foreach block causes confusing results in the formatter, + // but the test validates that we don't crash at least. await RunFormattingTestAsync( input: """ @page @@ -3807,14 +3861,14 @@ @model BlazorApp58.Pages.Index2Model
-
+
@foreach (var item in Model.Images) { -
-
- } -
-
+
+
+ } +
+ """, fileKind: RazorFileKind.Legacy, allowDiagnostics: true); @@ -6350,18 +6404,18 @@ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} ; } @@ -6412,18 +6466,18 @@ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} ; } @@ -6472,18 +6526,18 @@ protected RenderFragment RootFragment() => @ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} ; } @@ -6532,18 +6586,18 @@ protected RenderFragment RootFragment()=>@ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} ; } @@ -6592,18 +6646,18 @@ protected RenderFragment RootFragment() => @ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} ; } @@ -6655,21 +6709,21 @@ protected RenderFragment RootFragment() => @ @if (true) { -
- Hello - @if (true) - { - World - } - else - { - Not World - } -
+
+ Hello + @if (true) + { + World + } + else + { + Not World + } +
} - - ; + + ; } """, csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with @@ -6709,15 +6763,15 @@ protected RenderFragment RootFragment() => @code { protected RenderFragment RootFragment() => @
-
- Hello -
- World -
+
+ Hello +
+ World
- ; +
+ ; } """, csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with @@ -6955,11 +7009,11 @@ await RunFormattingTestAsync(
+ { + Title = Model.CurrentPage.TestimonialsTitle, + ContentArea = Model.CurrentPage.TestimonialsContentArea, + ChildCssClass = string.Empty + }" />
""", fileKind: RazorFileKind.Legacy); @@ -7018,11 +7072,11 @@ await RunFormattingTestAsync(
+ { + Title = Model.CurrentPage.TestimonialsTitle, + ContentArea = Model.CurrentPage.TestimonialsContentArea, + ChildCssClass = string.Empty + })" />
"""); } @@ -7052,11 +7106,11 @@ public async Task MultilineExplicitExpression_IsStable()
+ { + Title = Model.CurrentPage.TestimonialsTitle, + ContentArea = Model.CurrentPage.TestimonialsContentArea, + ChildCssClass = string.Empty + })" />
"""; await RunFormattingTestAsync( @@ -7179,15 +7233,15 @@ public Task NestedExplicitExpression1() { @((((true) ? 123d : 0d) + - (true ? 123d : 0d) - ).ToString("F2", CultureInfo.InvariantCulture)) € + (true ? 123d : 0d) + ).ToString("F2", CultureInfo.InvariantCulture)) €
@((123d + - ((true) ? 123d : 0d) + - (true ? 123d : 0d) - ).ToString("F2", CultureInfo.InvariantCulture)) € + ((true) ? 123d : 0d) + + (true ? 123d : 0d) + ).ToString("F2", CultureInfo.InvariantCulture)) € }
@@ -7296,7 +7350,7 @@ public Task NestedExplicitExpression3() @((((true) ? 123d : 0d) + (true ? 123d : 0d) ).ToString("F2", CultureInfo.InvariantCulture) - ) € + ) €
@@ -7304,7 +7358,7 @@ public Task NestedExplicitExpression3() ((true) ? 123d : 0d) + (true ? 123d : 0d) ).ToString("F2", CultureInfo.InvariantCulture) - ) € + ) € } """); @@ -7339,7 +7393,7 @@ public Task NestedExplicitExpression4() @((((true) ? 123d : 0d) + (true ? 123d : 0d) ).ToString("F2", CultureInfo.InvariantCulture) - ) € + ) €
@@ -7347,7 +7401,7 @@ public Task NestedExplicitExpression4() ((true) ? 123d : 0d) + (true ? 123d : 0d) ).ToString("F2", CultureInfo.InvariantCulture) - ) € + ) € } """); @@ -7368,4 +7422,160 @@ public Task TypeParameterAttribute()
"""); + + [FormattingTestFact] + public Task HtmlAttributes() + => RunFormattingTestAsync( + input: """ +
+ +

+
+
+ +
+ """, + expected: """ +
+ +

+
+
+ +
+ """); + + [FormattingTestFact] + [WorkItem("https://github.com/dotnet/razor/issues/12223")] + public Task ExplicitExpression_InIf() + => RunFormattingTestAsync( + input: """ + @if (true) + { + @(Html.Grid() + .Render()) + } + """, + expected: """ + @if (true) + { + @(Html.Grid() + .Render()) + } + """); + + [FormattingTestFact] + [WorkItem("https://github.com/dotnet/razor/issues/12554")] + public Task ObjectInitializers1() + => RunFormattingTestAsync( + input: """ + @{ + Func RenderTest = @
Test X: @item.X, Y: @item.Y
; + } + + @RenderTest(new Test() + { + X = 10, + Y = 20, + }) + +
+ @RenderTest(new Test() + { + X = 1, + Y = 2, + }) +
+ """, + expected: """ + @{ + Func RenderTest = @
Test X: @item.X, Y: @item.Y
; + } + + @RenderTest(new Test() + { + X = 10, + Y = 20, + }) + +
+ @RenderTest(new Test() + { + X = 1, + Y = 2, + }) +
+ """); + + [FormattingTestFact] + [WorkItem("https://github.com/dotnet/razor/issues/12554")] + public Task ObjectInitializers2() + => RunFormattingTestAsync( + input: """ +
+ @if (true) + { + @Html.TextBox(new Test() + { + test = 5 + }) + } +
+ """, + expected: """ +
+ @if (true) + { + @Html.TextBox(new Test() + { + test = 5 + }) + } +
+ """); + + [FormattingTestFact] + [WorkItem("https://github.com/dotnet/razor/issues/12554")] + public Task ObjectInitializers3() + => RunFormattingTestAsync( + input: """ +
+ @{ + var a = new int[] { 1, 2, 3 } + .Where(i => i % 2 == 0) + .ToArray(); + } +
+ """, + expected: """ +
+ @{ + var a = new int[] { 1, 2, 3 } + .Where(i => i % 2 == 0) + .ToArray(); + } +
+ """); } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/HtmlFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/HtmlFormattingTest.cs index 23f85daf973..f1c3b6359ae 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/HtmlFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/HtmlFormattingTest.cs @@ -263,7 +263,7 @@ await RunFormattingTestAsync( tagHelpers: [.. GetComponents()]); } - [FormattingTestFact(Skip = "Requires fix")] + [FormattingTestFact] [WorkItem("https://github.com/dotnet/razor/issues/8228")] public async Task FormatNestedComponents4() { @@ -279,9 +279,9 @@ await RunFormattingTestAsync( expected: """ @{ RenderFragment fragment = - @ - ; + @ + ; } """, tagHelpers: [.. GetComponents()]); @@ -307,7 +307,7 @@ await RunFormattingTestAsync( @{ RenderFragment fragment = @ + Caption="Title"> ; } @@ -464,11 +464,11 @@ await RunFormattingTestAsync(
@{ - #if DEBUG - } -
- @{ - #endif + #if DEBUG + } +
+ @{ + #endif }