Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SourceText.GetTextChanges produces invalid edits after successive changes #41413

Closed
ajaybhargavb opened this issue Feb 5, 2020 · 2 comments · Fixed by #48420
Closed

SourceText.GetTextChanges produces invalid edits after successive changes #41413

ajaybhargavb opened this issue Feb 5, 2020 · 2 comments · Fixed by #48420
Assignees
Milestone

Comments

@ajaybhargavb
Copy link

ajaybhargavb commented Feb 5, 2020

When working on formatting for .razor files, I noticed something odd. Under certain conditions, GetTextChanges would return invalid edits. I'm not sure exactly under what conditions. I was able to come up with a (somewhat) minimal repro.

Version Used:
Microsoft.CodeAnalysis 3.4.0
dotnet 3.1.101

Steps to Reproduce:

  1. dotnet new console -o SourceTextBug
  2. cd SourceTextBug
  3. Replace Program.cs content with the following,
Program.cs

using System;
using Microsoft.CodeAnalysis.Text;

namespace SourceTextBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var content = @"@functions{
    public class Foo
    {
void Method()
{
    
}
    }
}";
            Console.WriteLine("Initial:");
            Console.WriteLine(content);
            Console.WriteLine();

            var text = SourceText.From(content);
            var edits1 = new TextChange[]
            {
                new TextChange(new TextSpan(39, 0), "    "),
                new TextChange(new TextSpan(42, 0), "            "),
                new TextChange(new TextSpan(57, 0), "            "),
                new TextChange(new TextSpan(58, 0), "\r\n"),
                new TextChange(new TextSpan(64, 2), "        "),
                new TextChange(new TextSpan(69, 0), "    "),
            };
            var changedText = text.WithChanges(edits1);

            Console.WriteLine("After first set of edits:");
            Console.WriteLine(changedText.ToString());
            Console.WriteLine();

            var edits2 = new TextChange[]
            {
                new TextChange(new TextSpan(35, 4), string.Empty),
                new TextChange(new TextSpan(46, 4), string.Empty),
                new TextChange(new TextSpan(73, 4), string.Empty),
                new TextChange(new TextSpan(88, 0), "    "),
                new TextChange(new TextSpan(90, 4), string.Empty),
                new TextChange(new TextSpan(105, 4), string.Empty),
            };
            var changedText2 = changedText.WithChanges(edits2);

            Console.WriteLine("After second set of edits:");
            Console.WriteLine(changedText2.ToString());
            Console.WriteLine();

            Console.WriteLine("Diff of final text vs original:");
            var changes = changedText2.GetTextChanges(text);
            foreach (var change in changes)
            {
                Console.WriteLine(change);
            }
        }
    }
}

4. Add the following in the csproj,
<PackageReference Include="Microsoft.CodeAnalysis" Version="3.4.0" />`
  1. dotnet run

Expected Behavior:
The set of TextChanges produced should be valid and not overlap.

Actual Behavior:
Overlapping changes are produced.

Output

Initial:
@functions{
    public class Foo
    {
void Method()
{

}
    }
}

After first set of edits:
@functions{
    public class Foo
        {
            void Method()
            {

            }
        }
}

After second set of edits:
@functions{
    public class Foo
    {
        void Method()
        {

        }
    }
}

Diff of final text vs original:
TextChange: { [35..39), "    " }
TextChange: { [42..42), "        " }
TextChange: { [57..57), "        " }
TextChange: { [58..58), "
" }
TextChange: { [64..66), "        }
 " }
TextChange: { [62..66), "" }
TextChange: { [69..69), "    " }
TextChange: { [73..77), "" }

Notice the two overlapping edits,
[64..66) and [62..66)

@jasonmalinowski
Copy link
Member

FYI to @jinujoseph and @vatsalyaagrawal that this is potentially impacting ASP.NET's team to take on some ownership of the ASP.NET formatting code paths. I'm not sure if we really have an owner of this code or not. It was most recently touched in #39258 but it seems there's still some issues in it.

@ajaybhargavb
Copy link
Author

ajaybhargavb commented Feb 11, 2020

Looks like this also happens with GetChangeRanges(). Calling this method after successive changes, sometimes provides bad ranges.

@jinujoseph jinujoseph modified the milestones: 16.6, 16.7 Apr 19, 2020
@JoeRobich JoeRobich modified the milestones: 16.7, 16.8 Jul 30, 2020
@RikkiGibson RikkiGibson self-assigned this Oct 10, 2020
@sharwell sharwell modified the milestones: 16.8, 16.9.P1 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants