Skip to content

Commit

Permalink
[FUSE] Fix OnAutoInsert and override completion and possible others (#…
Browse files Browse the repository at this point in the history
…11122)

Fixes #11112
Hopefully fixes
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2290299/ but I'm
not sure, as whilst OnAutoInsert was definitely broken for me, it didn't
cause that repro. It just sent telemetry for a fault and did nothing in
the IDE.
Fixes
https://developercommunity.visualstudio.com/t/Razor-editor-is-broken-in-1712-Preview-/10777527
~but awaiting confirmation that FUSE is on for the user who reported the
issue (@richardhauer I don't suppose that's you?)~
Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2294069

Sadly, in order to get a working test for this, I had to enable FUSE in
cohosting. I say sadly because whilst it proves the cohosting tests are
awesome, it does potentially break us into jail a little bit. But only
in cohosting, so it's a pretty small jail :)

The changes aren't very exciting, as this is just
#10967 but for more than code
actions (and these changes probably should have been part of that PR
originally 🤦‍♂️), but each commit has an explanation for why it's there
so commit-at-a-time might be enlightening. I could not resist a little
clean up, but what's one little deleted service between friends?
  • Loading branch information
davidwengier authored Oct 30, 2024
2 parents 76cd92a + 5838cd5 commit c5babf1
Show file tree
Hide file tree
Showing 21 changed files with 167 additions and 175 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -18,7 +17,6 @@
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.CodeAnalysis.Razor.Protocol.Completion;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;

Expand All @@ -28,7 +26,6 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.InlineCompletion;
internal sealed class InlineCompletionEndpoint(
IDocumentMappingService documentMappingService,
IClientConnection clientConnection,
IFormattingCodeDocumentProvider formattingCodeDocumentProvider,
RazorLSPOptionsMonitor optionsMonitor,
ILoggerFactory loggerFactory)
: IRazorRequestHandler<VSInternalInlineCompletionRequest, VSInternalInlineCompletionList?>, ICapabilitiesProvider
Expand All @@ -40,7 +37,6 @@ internal sealed class InlineCompletionEndpoint(

private readonly IDocumentMappingService _documentMappingService = documentMappingService;
private readonly IClientConnection _clientConnection = clientConnection;
private readonly IFormattingCodeDocumentProvider _formattingCodeDocumentProvider = formattingCodeDocumentProvider;
private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<InlineCompletionEndpoint>();

Expand Down Expand Up @@ -125,8 +121,7 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalInlineCompleti
var formattingContext = FormattingContext.Create(
documentContext.Snapshot,
codeDocument,
options,
_formattingCodeDocumentProvider);
options);
if (!TryGetSnippetWithAdjustedIndentation(formattingContext, item.Text, hostDocumentIndex, out var newSnippetText))
{
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ protected override ILspServices ConstructLspServices()
// Add the logger as a service in case anything in CLaSP pulls it out to do logging
services.AddSingleton<ILspLogger>(Logger);

services.AddSingleton<IFormattingCodeDocumentProvider, LspFormattingCodeDocumentProvider>();

var featureOptions = _featureOptions ?? new DefaultLanguageServerFeatureOptions();
services.AddSingleton(featureOptions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,17 @@ namespace Microsoft.CodeAnalysis.Razor.Formatting;

internal sealed class FormattingContext
{
private readonly IFormattingCodeDocumentProvider _codeDocumentProvider;

private IReadOnlyList<FormattingSpan>? _formattingSpans;
private IReadOnlyDictionary<int, IndentationContext>? _indentations;

private FormattingContext(
IFormattingCodeDocumentProvider codeDocumentProvider,
IDocumentSnapshot originalSnapshot,
RazorCodeDocument codeDocument,
RazorFormattingOptions options,
bool automaticallyAddUsings,
int hostDocumentIndex,
char triggerCharacter)
{
_codeDocumentProvider = codeDocumentProvider;
OriginalSnapshot = originalSnapshot;
CodeDocument = codeDocument;
Options = options;
Expand Down Expand Up @@ -229,14 +225,12 @@ public async Task<FormattingContext> WithTextAsync(SourceText changedText, Cance
{
var changedSnapshot = OriginalSnapshot.WithText(changedText);

var codeDocument = await _codeDocumentProvider
.GetCodeDocumentAsync(changedSnapshot, cancellationToken)
.ConfigureAwait(false);
// Formatting always uses design time document
var codeDocument = await changedSnapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false);

DEBUG_ValidateComponents(CodeDocument, codeDocument);

var newContext = new FormattingContext(
_codeDocumentProvider,
OriginalSnapshot,
codeDocument,
Options,
Expand Down Expand Up @@ -269,13 +263,11 @@ public static FormattingContext CreateForOnTypeFormatting(
IDocumentSnapshot originalSnapshot,
RazorCodeDocument codeDocument,
RazorFormattingOptions options,
IFormattingCodeDocumentProvider codeDocumentProvider,
bool automaticallyAddUsings,
int hostDocumentIndex,
char triggerCharacter)
{
return new FormattingContext(
codeDocumentProvider,
originalSnapshot,
codeDocument,
options,
Expand All @@ -287,17 +279,14 @@ public static FormattingContext CreateForOnTypeFormatting(
public static FormattingContext Create(
IDocumentSnapshot originalSnapshot,
RazorCodeDocument codeDocument,
RazorFormattingOptions options,
IFormattingCodeDocumentProvider codeDocumentProvider)
RazorFormattingOptions options)
{
return new FormattingContext(
codeDocumentProvider,
originalSnapshot,
codeDocument,
options,
automaticallyAddUsings: false,
hostDocumentIndex: 0,
triggerCharacter: '\0'
);
triggerCharacter: '\0');
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
if (!DocumentMappingService.TryMapToGeneratedDocumentPosition(codeDocument.GetCSharpDocument(), context.HostDocumentIndex, out _, out var projectedIndex))
{
_logger.LogWarning($"Failed to map to projected position for document {context.OriginalSnapshot.FilePath}.");
return changes;
return [];
}

// Ask C# for formatting changes.
Expand All @@ -64,7 +64,7 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
if (formattingChanges.IsEmpty)
{
_logger.LogInformation($"Received no results.");
return changes;
return [];
}

changes = formattingChanges;
Expand All @@ -80,10 +80,10 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
var startPos = edit.Span.Start;
var endPos = edit.Span.End;
var count = csharpText.Length;
if (startPos >= count || endPos >= count)
if (startPos > count || endPos > count)
{
_logger.LogWarning($"Got a bad edit that couldn't be applied. Edit is {startPos}-{endPos} but there are only {count} characters in C#.");
return changes;
return [];
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,16 @@ internal class RazorFormattingService : IRazorFormattingService
private static readonly FrozenSet<string> s_csharpTriggerCharacterSet = FrozenSet.ToFrozenSet(["}", ";"], StringComparer.Ordinal);
private static readonly FrozenSet<string> s_htmlTriggerCharacterSet = FrozenSet.ToFrozenSet(["\n", "{", "}", ";"], StringComparer.Ordinal);

private readonly IFormattingCodeDocumentProvider _codeDocumentProvider;

private readonly ImmutableArray<IFormattingPass> _documentFormattingPasses;
private readonly ImmutableArray<IFormattingPass> _validationPasses;
private readonly CSharpOnTypeFormattingPass _csharpOnTypeFormattingPass;
private readonly HtmlOnTypeFormattingPass _htmlOnTypeFormattingPass;

public RazorFormattingService(
IFormattingCodeDocumentProvider codeDocumentProvider,
IDocumentMappingService documentMappingService,
IHostServicesProvider hostServicesProvider,
ILoggerFactory loggerFactory)
{
_codeDocumentProvider = codeDocumentProvider;

_htmlOnTypeFormattingPass = new HtmlOnTypeFormattingPass(loggerFactory);
_csharpOnTypeFormattingPass = new CSharpOnTypeFormattingPass(documentMappingService, hostServicesProvider, loggerFactory);
_validationPasses =
Expand All @@ -67,9 +62,7 @@ public async Task<ImmutableArray<TextChange>> GetDocumentFormattingChangesAsync(
RazorFormattingOptions options,
CancellationToken cancellationToken)
{
var codeDocument = await _codeDocumentProvider
.GetCodeDocumentAsync(documentContext.Snapshot, cancellationToken)
.ConfigureAwait(false);
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false);

// Range formatting happens on every paste, and if there are Razor diagnostics in the file
// that can make some very bad results. eg, given:
Expand Down Expand Up @@ -100,8 +93,7 @@ public async Task<ImmutableArray<TextChange>> GetDocumentFormattingChangesAsync(
var context = FormattingContext.Create(
documentSnapshot,
codeDocument,
options,
_codeDocumentProvider);
options);
var originalText = context.SourceText;

var result = htmlChanges;
Expand All @@ -119,56 +111,78 @@ public async Task<ImmutableArray<TextChange>> GetDocumentFormattingChangesAsync(
return originalText.MinimizeTextChanges(normalizedChanges);
}

public Task<ImmutableArray<TextChange>> GetCSharpOnTypeFormattingChangesAsync(DocumentContext documentContext, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken)
=> ApplyFormattedChangesAsync(
documentContext,
generatedDocumentChanges: [],
options,
hostDocumentIndex,
triggerCharacter,
[_csharpOnTypeFormattingPass, .. _validationPasses],
collapseChanges: false,
isCodeActionFormattingRequest: false,
cancellationToken: cancellationToken);
public async Task<ImmutableArray<TextChange>> GetCSharpOnTypeFormattingChangesAsync(DocumentContext documentContext, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken)
{
var documentSnapshot = documentContext.Snapshot;
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false);

return await ApplyFormattedChangesAsync(
documentSnapshot,
codeDocument,
generatedDocumentChanges: [],
options,
hostDocumentIndex,
triggerCharacter,
[_csharpOnTypeFormattingPass, .. _validationPasses],
collapseChanges: false,
automaticallyAddUsings: false,
cancellationToken: cancellationToken).ConfigureAwait(false);
}

public Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsync(DocumentContext documentContext, ImmutableArray<TextChange> htmlChanges, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken)
=> ApplyFormattedChangesAsync(
documentContext,
htmlChanges,
options,
hostDocumentIndex,
triggerCharacter,
[_htmlOnTypeFormattingPass, .. _validationPasses],
collapseChanges: false,
isCodeActionFormattingRequest: false,
cancellationToken: cancellationToken);
public async Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsync(DocumentContext documentContext, ImmutableArray<TextChange> htmlChanges, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken)
{
var documentSnapshot = documentContext.Snapshot;
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false);

return await ApplyFormattedChangesAsync(
documentSnapshot,
codeDocument,
htmlChanges,
options,
hostDocumentIndex,
triggerCharacter,
[_htmlOnTypeFormattingPass, .. _validationPasses],
collapseChanges: false,
automaticallyAddUsings: false,
cancellationToken: cancellationToken).ConfigureAwait(false);
}

public async Task<TextChange?> TryGetSingleCSharpEditAsync(DocumentContext documentContext, TextChange csharpEdit, RazorFormattingOptions options, CancellationToken cancellationToken)
{
var documentSnapshot = documentContext.Snapshot;
// Since we've been provided with an edit from the C# generated doc, forcing design time would make things not line up
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false);

var razorChanges = await ApplyFormattedChangesAsync(
documentContext,
documentSnapshot,
codeDocument,
[csharpEdit],
options,
hostDocumentIndex: 0,
triggerCharacter: '\0',
[_csharpOnTypeFormattingPass, .. _validationPasses],
collapseChanges: false,
isCodeActionFormattingRequest: false,
automaticallyAddUsings: false,
cancellationToken: cancellationToken).ConfigureAwait(false);
return razorChanges.SingleOrDefault();
}

public async Task<TextChange?> TryGetCSharpCodeActionEditAsync(DocumentContext documentContext, ImmutableArray<TextChange> csharpChanges, RazorFormattingOptions options, CancellationToken cancellationToken)
{
var documentSnapshot = documentContext.Snapshot;
// Since we've been provided with edits from the C# generated doc, forcing design time would make things not line up
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false);

var razorChanges = await ApplyFormattedChangesAsync(
documentContext,
documentSnapshot,
codeDocument,
csharpChanges,
options,
hostDocumentIndex: 0,
triggerCharacter: '\0',
[_csharpOnTypeFormattingPass],
collapseChanges: true,
isCodeActionFormattingRequest: true,
automaticallyAddUsings: true,
cancellationToken: cancellationToken).ConfigureAwait(false);
return razorChanges.SingleOrDefault();
}
Expand All @@ -177,15 +191,20 @@ public Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsync(Docu
{
csharpChanges = WrapCSharpSnippets(csharpChanges);

var documentSnapshot = documentContext.Snapshot;
// Since we've been provided with edits from the C# generated doc, forcing design time would make things not line up
var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false);

var razorChanges = await ApplyFormattedChangesAsync(
documentContext,
documentSnapshot,
codeDocument,
csharpChanges,
options,
hostDocumentIndex: 0,
triggerCharacter: '\0',
[_csharpOnTypeFormattingPass],
collapseChanges: true,
isCodeActionFormattingRequest: false,
automaticallyAddUsings: false,
cancellationToken: cancellationToken).ConfigureAwait(false);

razorChanges = UnwrapCSharpSnippets(razorChanges);
Expand All @@ -206,34 +225,26 @@ public bool TryGetOnTypeFormattingTriggerKind(RazorCodeDocument codeDocument, in
}

private async Task<ImmutableArray<TextChange>> ApplyFormattedChangesAsync(
DocumentContext documentContext,
IDocumentSnapshot documentSnapshot,
RazorCodeDocument codeDocument,
ImmutableArray<TextChange> generatedDocumentChanges,
RazorFormattingOptions options,
int hostDocumentIndex,
char triggerCharacter,
ImmutableArray<IFormattingPass> formattingPasses,
bool collapseChanges,
bool isCodeActionFormattingRequest,
bool automaticallyAddUsings,
CancellationToken cancellationToken)
{
// If we only received a single edit, let's always return a single edit back.
// Otherwise, merge only if explicitly asked.
collapseChanges |= generatedDocumentChanges.Length == 1;

var documentSnapshot = documentContext.Snapshot;

// Code actions were computed on the regular document, which with FUSE could be a runtime document. We have to make
// sure for code actions specifically we are formatting that same document, or TextChange spans may not line up
var codeDocument = isCodeActionFormattingRequest
? await documentSnapshot.GetGeneratedOutputAsync(cancellationToken).ConfigureAwait(false)
: await _codeDocumentProvider.GetCodeDocumentAsync(documentSnapshot, cancellationToken).ConfigureAwait(false);

var context = FormattingContext.CreateForOnTypeFormatting(
documentSnapshot,
codeDocument,
options,
_codeDocumentProvider,
automaticallyAddUsings: isCodeActionFormattingRequest,
automaticallyAddUsings: automaticallyAddUsings,
hostDocumentIndex,
triggerCharacter);
var result = generatedDocumentChanges;
Expand Down
Loading

0 comments on commit c5babf1

Please sign in to comment.