Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Features;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -220,7 +222,30 @@ private async Task<SourceText> FormatCSharpAsync(SourceText generatedCSharpText,

var tree = CSharpSyntaxTree.ParseText(generatedCSharpText, cancellationToken: cancellationToken);
var csharpRoot = await tree.GetRootAsync(cancellationToken).ConfigureAwait(false);
var csharpChanges = RazorCSharpFormattingInteractionService.GetFormattedTextChanges(helper.HostWorkspaceServices, csharpRoot, csharpRoot.FullSpan, options.ToIndentationOptions(), options.CSharpSyntaxFormattingOptions, cancellationToken);
var csharpSyntaxFormattingOptions = options.CSharpSyntaxFormattingOptions.AssumeNotNull();

// Roslyn can be configured to insert a space after a method call, or a dot, but that can break Razor. eg:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Roslyn can be configured to insert a space after a method call, or a dot, but that can break Razor. eg:

Devil's advocate, would it be possible to only augment the C# formatting in @ scenarios instead of @{ scenarios?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible? Sure, it's just code :)

Off the top of my head I can think of 3 approaches:

  1. Add something cool like #pragma formatter disable SpaceBeforeMethodCall to Roslyn
  2. Use the C# parser to go through the formatted expressions after the fact and remove significant whitespace
  3. Do a two-pass format, one with the user options, one for just implicit expressions with these modified options

All 3 sound like they're beyond this PR, and personally I think I'd wait until someone complains about the formatting not honouring their settings before we action it (assuming when they complain, we remember that this quirk exists)

//
// <div>@PrintHello()</div>
// @DateTime.Now.ToString()
//
// Would become:
//
// <div>@PrintHello ()</div>
// @DateTime. Now. ToString()
//
// In Razor, that's not a method call, its a method group (ie C# compile error) followed by Html, and
// the dot after DateTime is also just Html, as is the rest of the line.
// We're not smart enough (yet?) to ignore this change when its inline in Razor, but allow it when
// in a code block, so we just force these options to off.
csharpSyntaxFormattingOptions = csharpSyntaxFormattingOptions with
{
Spacing = csharpSyntaxFormattingOptions.Spacing
& ~RazorSpacePlacement.AfterMethodCallName
& ~RazorSpacePlacement.AfterDot
};

var csharpChanges = RazorCSharpFormattingInteractionService.GetFormattedTextChanges(helper.HostWorkspaceServices, csharpRoot, csharpRoot.FullSpan, options.ToIndentationOptions(), csharpSyntaxFormattingOptions, cancellationToken);

return generatedCSharpText.WithChanges(csharpChanges);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,118 @@ await RunFormattingTestAsync(
expected: "");
}

[FormattingTestFact(SkipOldFormattingEngine = true)]
public async Task RoslynFormatSpaceAfterDot()
{
await RunFormattingTestAsync(
input: """
<div>

@DateTime.Now.ToString()

</div>
""",
expected: """
<div>

@DateTime.Now.ToString()

</div>
""",
csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with
{
Spacing = RazorSpacePlacement.AfterDot
});
}

[FormattingTestFact(SkipOldFormattingEngine = true)]
public async Task RoslynFormatSpaceAfterMethodCall()
{
await RunFormattingTestAsync(
input: """
<h1>count is @counter</h1>

@GetCount()

@code {
private int counter;

class Goo
{
public int GetCount()
{
return counter++;
}
}
}
""",
expected: """
<h1>count is @counter</h1>

@GetCount()

@code {
private int counter;

class Goo
{
public int GetCount()
{
return counter++;
}
}
}
""",
csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with
{
Spacing = RazorSpacePlacement.AfterMethodCallName
});
}

[FormattingTestFact(SkipOldFormattingEngine = true)]
public async Task RoslynFormatSpaceAfterMethodCallAndDecl()
{
await RunFormattingTestAsync(
input: """
<h1>count is @counter</h1>

@GetCount()

@code {
private int counter;

class Goo
{
public int GetCount()
{
return counter++;
}
}
}
""",
expected: """
<h1>count is @counter</h1>

@GetCount()

@code {
private int counter;

class Goo
{
public int GetCount ()
{
return counter++;
}
}
}
""",
csharpSyntaxFormattingOptions: RazorCSharpSyntaxFormattingOptions.Default with
{
Spacing = RazorSpacePlacement.AfterMethodCallName | RazorSpacePlacement.AfterMethodDeclarationName
});
}

[FormattingTestFact]
public async Task RoslynFormatBracesAsKandR()
{
Expand Down
Loading