Aspnetcore: Light bulb refactorings can break C# semantic colorization

Created on 6 Nov 2020  路  4Comments  路  Source: dotnet/aspnetcore

In a Blazor app in Codespaces open up Counter.razor and add:

@code {
    class Test : IDisposable
    {
    }
}

Invoke the light bulb to implement the dispose pattern for IDisposable and see colorization break (ignore the double light bulbs, that's another issue but this happens even without htat):

ZH9VLkgiuO

I wasn't able to reproduce this in a pure C# file so I'm imagining it has something to do with Razor. Also it doesn't seem to reproduce locally. @gundermanc I imagine this is a LiveShare sync issue? I would have imagined that it would have been "eventually" correct but it never corrects

area-razor.tooling bug feature-razor.vs

Most helpful comment

FYI: I have filed a proposed user story to harden the LSP client's portion of these scenarios: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1243527.

The intent is that if possible, we support server-side edits and have a 'gating' mechanism where we don't actually inform the LSP server about the edit until it has been acknowledged by the client.

I don't have any more details yet, this will probably require some prototyping and possibly a change to the in-proc Roslyn bits.

All 4 comments

~Hmm, something odd may have been happening because I can't reproduce anymore but also looking at the gif I attached there were also multiple light bulbs for the same thing~

Just takes a few invocations of the dispose pattern light bulb + the generate constructor with field assignment

I would have imagined that it would have been "eventually" correct but it never corrects

Unfortunately no, once we're out of sync, the only way we can detect the error condition at the moment is by getting semantic tokens edits from the server that place tokens at positions that don't exist. Until that happens, we exist in a corrupt state because there's ATM no standardized checksum for the post-edit buffer state in LSP protocol standard. :(

Hints as to what's going on:

  • _Some_ .NET lightbulbs can trigger server-side changes, which are violation of the LSP protocol's assumption that all edits to open documents happen on the client via workspace/applyEdit. Issue on Roslyn here: https://github.com/dotnet/roslyn/issues/48991.
  • I saw at least one case where the colors would get mis-aligned with lightbulbs that didn't seem to be doing any server edits, so it's possible there's a race condition bug in application of edits in workspace/applyEdit in the IDE's LSP client too.

We should fix both of these by 16.9 GA. Both of these seem to be covered by my work on IDE's LSP message + text synchronization and remoting story.

@NTaylorMullen do you have a set of lightbulb actions that are known to consistently cause this issue? Doing so will help me prioritize which end needs fixed first.

@NTaylorMullen do you have a set of lightbulb actions that are known to consistently cause this issue? Doing so will help me prioritize which end needs fixed first.

I get this with Add 'Debugger Display' Attribute as well.

image

@page "/counter"


<h1>Counter</h1>

<p>Current count: @currentCount</p>

<button class="btn btn-primary" @onclick="IncrementCount">Click me</button>

<NonExistantComponent></NonExistantComponent>

@code {
    private int currentCount = 0;

    private void IncrementCount()
    {
        currentCount++;
    }

    public override bool Equals(object obj)
    {
        return obj is Counter counter &&
            currentCount == counter.currentCount;
    }

    public override int GetHashCode()
    {
        return HashCode.Combine(currentCount);
    }

    class Skj : IDisposable
    {
        public Skj()
        {
        }

        public Skj(string lksdajf)
        {
            Lksdajf = lksdajf ?? throw new ArgumentNullException(nameof(lksdajf));
        }

        public string Lksdajf { get; }

        public void Dispose()
        {
            throw new NotImplementedException();
        }
    }
}

FYI: I have filed a proposed user story to harden the LSP client's portion of these scenarios: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1243527.

The intent is that if possible, we support server-side edits and have a 'gating' mechanism where we don't actually inform the LSP server about the edit until it has been acknowledged by the client.

I don't have any more details yet, this will probably require some prototyping and possibly a change to the in-proc Roslyn bits.

Was this page helpful?
0 / 5 - 0 ratings