Skip to content
Open
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
2 changes: 2 additions & 0 deletions src/Fallout.Migrate/Migration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ internal sealed class Migration(AbsolutePath rootDirectory, bool dryRun, TextWri
new VerifyBuildTargetFrameworkStep(),
new ConfirmMigrationStep(),
new RewriteCsprojsStep(),
new RemoveNugetFrameworkPinStep(),
new RemoveRelatedNugetPinsStep(),
new BumpDotNetVersionStep(),
new RewriteCsFilesStep(),
new RewriteBootstrapScriptsStep(),
Expand Down
39 changes: 39 additions & 0 deletions src/Fallout.Migrate/Steps/RemoveNugetFrameworkPinStep.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Fallout.Migrate.Common;

namespace Fallout.Migrate.Steps;

/// <summary>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧🤖 The summary lists Framework, Protocol, Packaging and Resolver, but the regex only removes NuGet.Frameworks. Fix the text.

/// Removes explicit <c>NuGet.Framework</c>, <c>NuGet.Protocol</c>, <c>NuGet.Packaging</c>, and
/// <c>NuGet.Resolver</c> package pins from project files. The pins are no longer needed.
/// </summary>
internal sealed class RemoveNugetFrameworkPinStep : IMigrationStep
{
private static readonly Regex explicitPinPattern = new(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 Add a human-friendly explanation to the Regex, e.g. something like

/// <summary>
/// Finds a whole line with a self-closing <c>&lt;PackageReference&gt;</c>
/// for <c>NuGet.Frameworks</c> that has a <c>Version</c> attribute.
/// </summary>
/// <remarks>
/// The match includes leading spaces, trailing spaces, and the line break.
/// Replace it with an empty string to remove the line.
/// The tag must be on one line and must end with <c>/&gt;</c>.
/// <c>Include</c> and <c>Version</c> can appear in any order.
/// Use this with <see cref="RegexOptions.Multiline"/>.
/// The match is case-sensitive.
/// </remarks>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓🤖 Under central package management the pin is a <PackageVersion> in Directory.Packages.props, which neither step reads. Is that acceptable, or should it build on #694 (#694), which adds *.props support to the migration?

@"^[ \t]*<PackageReference\s+(?=[^>\r\n]*\bInclude=""NuGet\.Frameworks"")(?=[^>\r\n]*\bVersion=""[^""]+"")[^>\r\n]*/>[ \t]*\r?\n?",
RegexOptions.Compiled | RegexOptions.Multiline);

/// <inheritdoc />
public Task ExecuteAsync(MigrationContext context, Summary summary)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔🤖 This step is not version-gated, so migrating to a Fallout version that predates the #677 fix would strip a pin that is still needed. I could not verify which version contains #677.

{
foreach (var path in MigrationFileOperations.EnumerateFiles(context.RootDirectory, "*.csproj"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓🤖 This scans every *.csproj, so a library that uses NuGet.Frameworks directly loses its reference. Is that intended, or should it only touch _build.csproj? Same for RemoveRelatedNugetPinsStep.

{
MigrationFileOperations.ApplyRewrite(context, path, Rewrite, summary);
}

return Task.CompletedTask;
}

private static RewriteResult Rewrite(string original)
{
var edits = 0;
var content = explicitPinPattern.Replace(original, _ =>
{
edits++;
return string.Empty;
});

return new RewriteResult(content, edits);
}
}
56 changes: 56 additions & 0 deletions src/Fallout.Migrate/Steps/RemoveRelatedNugetPinsStep.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using System.Globalization;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Fallout.Migrate.Common;

namespace Fallout.Migrate.Steps;

/// <summary>
/// Removes explicit <c>NuGet.Protocol</c>, <c>NuGet.Packaging</c>, and <c>NuGet.Resolver</c>
/// package pins from project files when migrating to Fallout 11 or later.
/// </summary>
internal sealed class RemoveRelatedNugetPinsStep : IMigrationStep
{
private const int MinimumFalloutMajor = 11;

private static readonly Regex explicitPinPattern = new(
@"^[ \t]*<PackageReference\s+(?=[^>\r\n]*\bInclude=""NuGet\.(?:Protocol|Packaging|Resolver)"")(?=[^>\r\n]*\bVersion=""[^""]+"")[^>\r\n]*/>[ \t]*\r?\n?",
RegexOptions.Compiled | RegexOptions.Multiline);

/// <inheritdoc />
public Task ExecuteAsync(MigrationContext context, Summary summary)
{
if (!RunsFor(context.FalloutVersion))
{
return Task.CompletedTask;
}

foreach (var path in MigrationFileOperations.EnumerateFiles(context.RootDirectory, "*.csproj"))
{
MigrationFileOperations.ApplyRewrite(context, path, Rewrite, summary);
}

return Task.CompletedTask;
}

private static bool RunsFor(string version)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧🤖 RunsFor has no docs. Say what it returns for null or unparseable input. Same for MinimumFalloutMajor.

{
int separatorIndex = version?.IndexOf('.') ?? -1;
string majorSegment = separatorIndex == -1 ? version : version[..separatorIndex];

return int.TryParse(majorSegment, NumberStyles.None, CultureInfo.InvariantCulture, out int major) &&
major >= MinimumFalloutMajor;
}

private static RewriteResult Rewrite(string original)
{
var edits = 0;
var content = explicitPinPattern.Replace(original, _ =>
{
edits++;
return string.Empty;
});

return new RewriteResult(content, edits);
}
}
108 changes: 108 additions & 0 deletions tests/Fallout.Migrate.Specs/RemoveNugetFrameworkPinStepSpecs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
using System;
using System.IO;
using System.Threading.Tasks;
using Fallout.Common.IO;
using Fallout.Migrate.Common;
using Fallout.Migrate.Steps;
using FluentAssertions;
using Xunit;

namespace Fallout.Migrate.Specs;

public class RemoveNugetFrameworkPinStepSpecs : IDisposable
{
private readonly AbsolutePath tempDirectory;
private readonly MigrationContext context;
private readonly Summary summary = new();

public RemoveNugetFrameworkPinStepSpecs()
{
tempDirectory = AbsolutePath.Temp("fallout-remove-nuget-framework-pin");
context = new MigrationContext(tempDirectory, dryRun: false, TextWriter.Null);
}

[Fact]
public async Task Explicit_nuget_framework_pin_is_removed()
{
// Arrange
var project = tempDirectory / "Library.csproj";
project.WriteAllText(
"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<PackageReference Version="7.9.0" PrivateAssets="all" Include="NuGet.Frameworks" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>
</Project>
""", eofLineBreak: false);

// Act
await new RemoveNugetFrameworkPinStep().ExecuteAsync(context, summary);

// Assert
project.ReadAllText().Should().Be(
"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>
</Project>
""");

summary.FilesChanged.Should().Be(1);
summary.EditCount.Should().Be(1);
}

[Fact]
public async Task Nuget_framework_reference_without_version_is_left_unchanged()
{
// Arrange
var project = tempDirectory / "Library.csproj";
const string input =
"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<PackageReference Include="NuGet.Frameworks" />
</ItemGroup>
</Project>
""";

project.WriteAllText(input, eofLineBreak: false);

// Act
await new RemoveNugetFrameworkPinStep().ExecuteAsync(context, summary);

// Assert
project.ReadAllText().Should().Be(input);
summary.EditCount.Should().Be(0);
}

[Fact]
public async Task Missing_nuget_framework_pin_is_left_unchanged()
{
// Arrange
var project = tempDirectory / "build" / "_build.csproj";
const string input =
"""
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
</Project>
""";

project.WriteAllText(input, eofLineBreak: false);

// Act
await new RemoveNugetFrameworkPinStep().ExecuteAsync(context, summary);

// Assert
project.ReadAllText().Should().Be(input);
summary.EditCount.Should().Be(0);
}

public void Dispose()
{
tempDirectory.DeleteDirectory();
}
}
94 changes: 94 additions & 0 deletions tests/Fallout.Migrate.Specs/RemoveRelatedNugetPinsStepSpecs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using System;
using System.IO;
using System.Threading.Tasks;
using Fallout.Common.IO;
using Fallout.Migrate.Common;
using Fallout.Migrate.Steps;
using FluentAssertions;
using Xunit;

namespace Fallout.Migrate.Specs;

public class RemoveRelatedNugetPinsStepSpecs : IDisposable
{
private readonly AbsolutePath tempDirectory;
private readonly Summary summary = new();

public RemoveRelatedNugetPinsStepSpecs()
{
tempDirectory = AbsolutePath.Temp("fallout-remove-related-nuget-pins");
}

[Theory]
[InlineData("NuGet.Protocol", "11.0.0")]
[InlineData("NuGet.Packaging", "11.0.0-preview.1")]
[InlineData("NuGet.Resolver", "12.0.0")]
public async Task Explicit_related_nuget_pin_is_removed_for_v11_or_later(
string packageName,
string falloutVersion)
{
// Arrange
var project = tempDirectory / "Library.csproj";
project.WriteAllText(
$"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<PackageReference Include="{packageName}" Version="7.9.0" />
</ItemGroup>
</Project>
""", eofLineBreak: false);
var context = CreateContext(falloutVersion);

// Act
await new RemoveRelatedNugetPinsStep().ExecuteAsync(context, summary);

// Assert
project.ReadAllText().Should().Be(
"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
</ItemGroup>
</Project>
""");
summary.FilesChanged.Should().Be(1);
summary.EditCount.Should().Be(1);
}

[Theory]
[InlineData("10.9.0")]
[InlineData(null)]
public async Task Explicit_related_nuget_pin_is_left_unchanged_before_v11(string falloutVersion)
{
// Arrange
var project = tempDirectory / "Library.csproj";
const string input =
"""
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<PackageReference Include="NuGet.Protocol" Version="7.9.0" />
</ItemGroup>
</Project>
""";

project.WriteAllText(input, eofLineBreak: false);
var context = CreateContext(falloutVersion);

// Act
await new RemoveRelatedNugetPinsStep().ExecuteAsync(context, summary);

// Assert
project.ReadAllText().Should().Be(input);
summary.EditCount.Should().Be(0);
}

private MigrationContext CreateContext(string falloutVersion) =>
new(tempDirectory, dryRun: false, TextWriter.Null)
{
FalloutVersion = falloutVersion
};

public void Dispose()
{
tempDirectory.DeleteDirectory();
}
}
Loading