Skip to content

RewriteCsprojStep has two too greedy patterns #612

Description

@ITaluone

Usage Information

current main

Description

Running fallout-migrate on a project, resulted in a csproj with unnecessary changes.

Run the test below and see the output:

The test fails with: (see the missing whitespace before <IsPackable>

Expected buildCsproj to be a match with the expectation, but it differs at column 1 of line 10 (index 382):
                ↓ (actual)
  "…rectory>\r\n<IsPackable>false</IsPackable>\r\n    </PropertyGroup>\r\n\r\n    <ItemGroup>\r\n  …"
  "…rectory>\r\n        <IsPackable>false</IsPackable>\r\n    </PropertyGroup>\r\n\r\n   …"
                ↑ (expected)

Full expectation:

    "<Project Sdk="Microsoft.NET.Sdk">
        <PropertyGroup>
            <FalloutScriptDirectory>..</FalloutScriptDirectory>
            <IsPackable>false</IsPackable>
        </PropertyGroup>
        <ItemGroup>
            <PackageReference Include="Fallout.Common" Version="10.3.49" />
        </ItemGroup
    </Project>",

Full subject:

    "<Project Sdk="Microsoft.NET.Sdk">
        <PropertyGroup>
            <FalloutScriptDirectory>..</FalloutScriptDirectory>
            <FalloutTelemetryVersion>1</FalloutTelemetryVersion>
     <IsPackable>false</IsPackable>
        </PropertyGroup>
        <ItemGroup>
            <PackageReference Include="Fallout.Common" Version="10.3.49" />
        </ItemGroup>
    </Project>"

Reproduction Steps

    [Fact]
    public async Task Telemetry_pattern_does_not_act_greedy()
    {
        const string input = """
                             <Project Sdk="Microsoft.NET.Sdk">
                                 <PropertyGroup>
                                     <FalloutScriptDirectory>..</FalloutScriptDirectory>
                                     <FalloutTelemetryVersion>1</FalloutTelemetryVersion>
                                     <IsPackable>false</IsPackable>
                                 </PropertyGroup>
                                 <ItemGroup>
                                     <PackageReference Include="Fallout.Common" Version="10.3.49" />
                                 </ItemGroup>
                             </Project>
                             """;

        (tempDirectory / "build" / "_build.csproj").WriteAllText(input, eofLineBreak: false);

        await new RewriteCsprojsStep().ExecuteAsync(context, summary);

        var buildCsproj = (tempDirectory / "build" / "_build.csproj").ReadAllText();
        buildCsproj.Should().Be("""
                                <Project Sdk="Microsoft.NET.Sdk">
                                    <PropertyGroup>
                                        <FalloutScriptDirectory>..</FalloutScriptDirectory>
                                        <IsPackable>false</IsPackable>
                                    </PropertyGroup>
                                    <ItemGroup>
                                        <PackageReference Include="Fallout.Common" Version="10.3.49" />
                                    </ItemGroup>
                                </Project>
                                """);
    }

Expected Behavior

The intendation is preserved

Actual Behavior

The regex pattern actis too greedy @"^[ \t]*<(?<tag>(?:Nuke|Fallout)TelemetryVersion)>.*?</\k<tag>>\s*\r?\n?" <-- the \s* consumes everything until the next < (from <IsPackable> in my case).

Regression?

Maybe, a bug in every case

Known Workarounds

Adding the intendation manually again

Could you help with a pull-request?

Yes

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions