Skip to content

Commit 0b668b7

Browse files
macsuxmacsux
authored
XML: recognize additional .NET XML file extensions (#7448)
* XML: recognize additional .NET XML file extensions Adds `targets`, `slnx`, `ruleset`, and `dotsettings` to the list of accepted XML extensions, and accepts `App.*.config` / `Web.*.config` XDT transform variants (e.g. `Web.Release.config`). * XML/C#: preserve UTF-8 BOM across csproj round-trip XmlParser was leaving the BOM byte in the input string for the lexer to choke on, and the XmlPrinter wasn't writing the BOM back out. csproj files saved by Visual Studio commonly start with a BOM, so we'd round- trip them without it. Also fixes File.ReadAllText silently stripping BOMs in RewriteRpcServer when reading project files for parsing. * C#: match short-form TFMs in ChangeDotNetTargetFramework MSBuild treats `net8` and `net8.0` as the same target framework moniker (.NET 5+). The recipe was doing strict string equality, so users with short-form TFMs in their csproj files (or short-form input on the recipe parameter) would silently no-op. Normalize both sides before comparing. * C#: handle wildcard and floating versions in UpgradeNuGetPackageVersion Three related fixes for version-spec resolution: - Accept the OpenRewrite wildcard convention `10.0.x` and translate it to NuGet's `10.0.*` so composite migration recipes that pass `x`-form specs no longer silently no-op. - When the NuGet feed query returns nothing (offline, private feed, or package not indexed), fall back to the range's MinVersion as the upgrade target instead of giving up. - Recognize a floating `currentVersion` like `14.0.*` when deciding whether the project is already at or above the target, comparing against `VersionRange.MinVersion` rather than treating it as unknown. * C#: drop redundant BOM emit from CSharpPrinter CharsetBomMarked is captured from raw file bytes (Roslyn strips the BOM from the source string before it reaches the parser). The print-equals- input idempotency check in SolutionParser compares the printer output against the BOM-stripped source, so emitting `` from the visitor caused every BOM-marked file to flip to ParseError — breaking SolutionParserTests.ParseFileWithBomPreservesCharsetBomMarked and WorkingSetRoundTripTests.CharsetBomMarked_SetFromParser. The flag is preserved on the LST for downstream patch generation, which is what was actually needed.
1 parent 00226af commit 0b668b7

10 files changed

Lines changed: 216 additions & 23 deletions

File tree

‎rewrite-csharp/csharp/OpenRewrite/CSharp/Recipes/ChangeDotNetTargetFrameworkVisitor.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
using System.Text.RegularExpressions;
1617
using OpenRewrite.Core;
1718
using OpenRewrite.Xml;
1819
using ExecutionContext = OpenRewrite.Core.ExecutionContext;
@@ -26,6 +27,8 @@ namespace OpenRewrite.CSharp.Recipes;
2627
/// </summary>
2728
public class ChangeDotNetTargetFrameworkVisitor(string oldTfm, string newTfm) : XmlVisitor<ExecutionContext>
2829
{
30+
private static readonly Regex ShortFormTfm = new(@"^net\d+$", RegexOptions.Compiled);
31+
2932
private bool _modified;
3033

3134
public override Xml.Xml VisitDocument(Document document, ExecutionContext ctx)
@@ -44,7 +47,7 @@ public override Xml.Xml VisitTag(Tag tag, ExecutionContext ctx)
4447
if (t.Name == "TargetFramework")
4548
{
4649
var value = t.GetValue() ?? "";
47-
if (oldTfm == value)
50+
if (TfmEquals(oldTfm, value))
4851
{
4952
_modified = true;
5053
DoAfterVisit(new ChangeTagValueVisitor<ExecutionContext>(t, newTfm));
@@ -59,7 +62,7 @@ public override Xml.Xml VisitTag(Tag tag, ExecutionContext ctx)
5962
foreach (var framework in frameworks)
6063
{
6164
var fw = framework.Trim();
62-
if (oldTfm == fw)
65+
if (TfmEquals(oldTfm, fw))
6366
{
6467
changed = true;
6568
fw = newTfm;
@@ -78,4 +81,11 @@ public override Xml.Xml VisitTag(Tag tag, ExecutionContext ctx)
7881

7982
return t;
8083
}
84+
85+
// net8 and net8.0 are equivalent TFMs in MSBuild for .NET 5+. Match both forms.
86+
private static bool TfmEquals(string a, string b) =>
87+
a == b || Normalize(a) == Normalize(b);
88+
89+
private static string Normalize(string tfm) =>
90+
ShortFormTfm.IsMatch(tfm) ? tfm + ".0" : tfm;
8191
}

‎rewrite-csharp/csharp/OpenRewrite/CSharp/Recipes/UpgradeNuGetPackageVersion.cs‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
using System.Net.Http;
1717
using System.Text.Json;
18+
using System.Text.RegularExpressions;
1819
using NuGet.Versioning;
1920
using OpenRewrite.Core;
2021
using OpenRewrite.Xml;
@@ -322,7 +323,8 @@ private void ScanPackageVersionElements(Document doc, Accumulator acc, MSBuildPr
322323

323324
/// <summary>
324325
/// Resolves the target version using NuGet.Versioning.
325-
/// Handles exact versions, NuGet version ranges, and "latest" keyword.
326+
/// Handles exact versions, NuGet version ranges, "latest" keyword, and OpenRewrite
327+
/// convention wildcards like "10.0.x" (equivalent to NuGet "10.0.*").
326328
/// </summary>
327329
internal static string? ResolveTargetVersion(
328330
string packageInclude,
@@ -331,14 +333,14 @@ private void ScanPackageVersionElements(Document doc, Accumulator acc, MSBuildPr
331333
string newVersionSpec,
332334
bool includePrerelease)
333335
{
336+
// OpenRewrite convention: "10.0.x" is equivalent to NuGet's "10.0.*".
337+
newVersionSpec = NormalizeVersionSpec(newVersionSpec);
338+
334339
// Exact version: just return it if it's higher than current
335340
if (NuGetVersion.TryParse(newVersionSpec, out var exactVersion))
336341
{
337-
if (currentVersion != null && NuGetVersion.TryParse(currentVersion, out var current))
338-
{
339-
if (exactVersion <= current)
340-
return null; // already at or above target
341-
}
342+
if (IsCurrentAtOrAbove(currentVersion, exactVersion))
343+
return null;
342344
return exactVersion.ToNormalizedString();
343345
}
344346

@@ -355,7 +357,7 @@ private void ScanPackageVersionElements(Document doc, Accumulator acc, MSBuildPr
355357
if (candidates.Count == 0) return null;
356358
var best = candidates.Max()!;
357359

358-
if (currentVersion != null && NuGetVersion.TryParse(currentVersion, out var current) && best <= current)
360+
if (IsCurrentAtOrAbove(currentVersion, best))
359361
return null;
360362

361363
return best.ToNormalizedString();
@@ -365,16 +367,34 @@ private void ScanPackageVersionElements(Document doc, Accumulator acc, MSBuildPr
365367
if (VersionRange.TryParse(newVersionSpec, out var range))
366368
{
367369
var available = FetchAvailableVersions(packageInclude, marker);
368-
if (available.Count == 0) return null;
369-
370-
var candidates = includePrerelease
371-
? available
372-
: available.Where(v => !v.IsPrerelease).ToList();
370+
NuGetVersion? best = null;
371+
if (available.Count > 0)
372+
{
373+
var candidates = includePrerelease
374+
? available
375+
: available.Where(v => !v.IsPrerelease).ToList();
376+
377+
best = range.FindBestMatch(candidates);
378+
379+
// The package exists on NuGet but has no version matching the range
380+
// (e.g., Microsoft.AspNetCore.Mvc has no 10.0.x because it was consolidated
381+
// into the shared framework). Don't upgrade — writing a non-existent
382+
// version would cause NU1102 at restore time.
383+
if (best == null)
384+
return null;
385+
}
386+
else
387+
{
388+
// We couldn't query NuGet (offline, private feed, or package not indexed).
389+
// Fall back to the range's minimum version. That's still a valid concrete
390+
// upgrade target when the current version is lower.
391+
if (range.MinVersion != null)
392+
best = range.MinVersion;
393+
}
373394

374-
var best = range.FindBestMatch(candidates);
375395
if (best == null) return null;
376396

377-
if (currentVersion != null && NuGetVersion.TryParse(currentVersion, out var current) && best <= current)
397+
if (IsCurrentAtOrAbove(currentVersion, best))
378398
return null;
379399

380400
return best.ToNormalizedString();
@@ -383,6 +403,34 @@ private void ScanPackageVersionElements(Document doc, Accumulator acc, MSBuildPr
383403
return null;
384404
}
385405

406+
// OpenRewrite wildcard convention: a trailing ".x" means "any component here".
407+
// NuGet expresses the same with "*" (e.g., "10.0.*"). Normalize so downstream
408+
// NuGet.Versioning parsers accept the input.
409+
private static readonly Regex TrailingXWildcard =
410+
new(@"\.x$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
411+
412+
private static string NormalizeVersionSpec(string spec) =>
413+
TrailingXWildcard.Replace(spec, ".*");
414+
415+
// Returns true when the current version (exact or floating range) is known to
416+
// be >= the proposed target. If current is unparseable or unknown, returns
417+
// false so the upgrade proceeds.
418+
private static bool IsCurrentAtOrAbove(string? currentVersion, NuGetVersion target)
419+
{
420+
if (currentVersion == null) return false;
421+
422+
if (NuGetVersion.TryParse(currentVersion, out var current))
423+
return target <= current;
424+
425+
// Wildcard / range current version: compare against range.MinVersion as
426+
// the lower bound of what MSBuild might resolve to.
427+
var normalized = NormalizeVersionSpec(currentVersion);
428+
if (VersionRange.TryParse(normalized, out var currentRange) && currentRange.MinVersion != null)
429+
return target <= currentRange.MinVersion;
430+
431+
return false;
432+
}
433+
386434
private static List<NuGetVersion> FetchAvailableVersions(string packageName, MSBuildProject? marker)
387435
{
388436
var sources = marker?.PackageSources ?? [];

‎rewrite-csharp/csharp/OpenRewrite/CSharp/Rpc/RewriteRpcServer.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ public async Task<ParseSolutionResponse> ParseSolution(ParseSolutionRequest requ
272272
// so we parse XML directly and create the marker from project.assets.json.
273273
try
274274
{
275-
var content = File.ReadAllText(project.FilePath!);
275+
var content = ReadFilePreservingBom(project.FilePath!);
276276
var relativePath = Path.GetRelativePath(rootDir, project.FilePath!);
277277
var xmlParser = new OpenRewrite.Xml.XmlParser();
278278
var csprojDoc = xmlParser.Parse(content, relativePath);
@@ -301,6 +301,21 @@ public async Task<ParseSolutionResponse> ParseSolution(ParseSolutionRequest requ
301301
return response;
302302
}
303303

304+
/// <summary>
305+
/// Reads a text file while preserving a leading UTF-8 BOM as a `\uFEFF` character in
306+
/// the returned string. File.ReadAllText silently strips BOMs, which defeats the
307+
/// XmlParser's BOM detection and causes csproj files to round-trip without their BOM.
308+
/// </summary>
309+
private static string ReadFilePreservingBom(string filePath)
310+
{
311+
var bytes = File.ReadAllBytes(filePath);
312+
if (bytes.Length >= 3 && bytes[0] == 0xEF && bytes[1] == 0xBB && bytes[2] == 0xBF)
313+
{
314+
return "\uFEFF" + System.Text.Encoding.UTF8.GetString(bytes, 3, bytes.Length - 3);
315+
}
316+
return System.Text.Encoding.UTF8.GetString(bytes);
317+
}
318+
304319
/// <summary>
305320
/// Resolves a path to its canonical form.
306321
/// On macOS, /var and /tmp are "firmlinks" to /private/var and /private/tmp,

‎rewrite-csharp/csharp/OpenRewrite/Tests/CSharp/Recipes/ChangeDotNetTargetFrameworkTests.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,64 @@ public void DeduplicatesMultiTfmAfterReplacement()
199199
);
200200
}
201201

202+
[Fact]
203+
public void ChangeShortFormTargetFramework()
204+
{
205+
// net8 and net8.0 are equivalent TFMs — the recipe should match both forms.
206+
RewriteRun(
207+
spec => spec.SetRecipe(new ChangeDotNetTargetFramework
208+
{
209+
OldTargetFramework = "net8.0",
210+
NewTargetFramework = "net9.0"
211+
}),
212+
CsProj(
213+
"""
214+
<Project Sdk="Microsoft.NET.Sdk">
215+
<PropertyGroup>
216+
<TargetFramework>net8</TargetFramework>
217+
</PropertyGroup>
218+
</Project>
219+
""",
220+
"""
221+
<Project Sdk="Microsoft.NET.Sdk">
222+
<PropertyGroup>
223+
<TargetFramework>net9.0</TargetFramework>
224+
</PropertyGroup>
225+
</Project>
226+
"""
227+
)
228+
);
229+
}
230+
231+
[Fact]
232+
public void ChangeShortFormOldTargetFramework()
233+
{
234+
// Also works if user specifies short-form old TFM and file has dotted form.
235+
RewriteRun(
236+
spec => spec.SetRecipe(new ChangeDotNetTargetFramework
237+
{
238+
OldTargetFramework = "net8",
239+
NewTargetFramework = "net9.0"
240+
}),
241+
CsProj(
242+
"""
243+
<Project Sdk="Microsoft.NET.Sdk">
244+
<PropertyGroup>
245+
<TargetFramework>net8.0</TargetFramework>
246+
</PropertyGroup>
247+
</Project>
248+
""",
249+
"""
250+
<Project Sdk="Microsoft.NET.Sdk">
251+
<PropertyGroup>
252+
<TargetFramework>net9.0</TargetFramework>
253+
</PropertyGroup>
254+
</Project>
255+
"""
256+
)
257+
);
258+
}
259+
202260
[Fact]
203261
public void NoChangeWhenTfmNotPresent()
204262
{

‎rewrite-csharp/csharp/OpenRewrite/Tests/CSharp/Recipes/UpgradeNuGetPackageVersionTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,4 +846,41 @@ public void UpgradePropertyUsedByMultipleTargetedPackages()
846846
)
847847
);
848848
}
849+
850+
[Fact]
851+
public void UpgradeFloatingWildcardVersion()
852+
{
853+
// Source csproj uses NuGet's floating "8.0.*" syntax. The recipe should
854+
// still upgrade it to the exact target version.
855+
RewriteRun(
856+
spec => spec.SetRecipe(new UpgradeNuGetPackageVersion
857+
{
858+
PackageName = "Newtonsoft.Json",
859+
NewVersion = "14.0.1"
860+
}),
861+
CsProj(
862+
"""
863+
<Project Sdk="Microsoft.NET.Sdk">
864+
<PropertyGroup>
865+
<TargetFramework>net8.0</TargetFramework>
866+
</PropertyGroup>
867+
<ItemGroup>
868+
<PackageReference Include="Newtonsoft.Json" Version="8.0.*" />
869+
</ItemGroup>
870+
</Project>
871+
""",
872+
"""
873+
<Project Sdk="Microsoft.NET.Sdk">
874+
<PropertyGroup>
875+
<TargetFramework>net8.0</TargetFramework>
876+
</PropertyGroup>
877+
<ItemGroup>
878+
<PackageReference Include="Newtonsoft.Json" Version="14.0.1" />
879+
</ItemGroup>
880+
</Project>
881+
"""
882+
)
883+
);
884+
}
885+
849886
}

‎rewrite-csharp/csharp/OpenRewrite/Xml/XmlParser.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,11 @@ public Document Parse(string sourceStr, string sourcePath = "file.xml")
7373
var charsetName = "UTF-8";
7474
var charsetBomMarked = false;
7575

76-
// Detect UTF-8 BOM
76+
// Detect and strip UTF-8 BOM (matches Java EncodingDetectingInputStream behavior)
7777
if (sourceStr.Length > 0 && sourceStr[0] == '\uFEFF')
7878
{
7979
charsetBomMarked = true;
80+
sourceStr = sourceStr.Substring(1);
8081
}
8182

8283
var lexer = new XMLLexer(new AntlrInputStream(sourceStr));

‎rewrite-csharp/csharp/OpenRewrite/Xml/XmlPrinter.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ private static string XmlMarkerWrapper(string text) =>
2828

2929
public override Xml VisitDocument(Document document, PrintOutputCapture<P> p)
3030
{
31+
if (document.CharsetBomMarked) p.Append('\uFEFF');
3132
BeforeSyntax(document, p);
3233
if (document.Prolog != null) Visit(document.Prolog, p);
3334
Visit(document.Root, p);

‎rewrite-xml/src/main/java/org/openrewrite/xml/XmlParser.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ public class XmlParser implements Parser {
7070
"vbproj",
7171
"fsproj",
7272
"props",
73+
"targets",
74+
"slnx",
75+
"ruleset",
76+
"dotsettings",
7377
// JasperReports files
7478
"jrxml"
7579
));
@@ -121,9 +125,12 @@ public boolean accept(Path path) {
121125
return true;
122126
}
123127
}
124-
String fileName = path.getFileName().toString();
125-
return fileName.equalsIgnoreCase("nuget.config") ||
126-
fileName.equalsIgnoreCase("packages.config");
128+
String fileName = path.getFileName().toString().toLowerCase();
129+
return fileName.equals("nuget.config") ||
130+
fileName.equals("packages.config") ||
131+
// .NET Framework app/web config and their XDT transform variants
132+
// (e.g. Web.Release.config, App.Debug.config)
133+
((fileName.startsWith("app.") || fileName.startsWith("web.")) && fileName.endsWith(".config"));
127134
}
128135

129136
@Override

‎rewrite-xml/src/main/java/org/openrewrite/xml/internal/XmlPrinter.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public class XmlPrinter<P> extends XmlVisitor<PrintOutputCapture<P>> {
2727

2828
@Override
2929
public Xml visitDocument(Xml.Document document, PrintOutputCapture<P> p) {
30+
if (document.isCharsetBomMarked()) {
31+
p.append('\uFEFF');
32+
}
3033
beforeSyntax(document, p);
3134
document = (Xml.Document) super.visitDocument(document, p);
3235
afterSyntax(document, p);

‎rewrite-xml/src/test/java/org/openrewrite/xml/XmlParserTest.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,17 @@ void preserveWhitespaceOnEntities() {
694694
"Packages.config",
695695
"nuget.config",
696696
"NuGet.config",
697-
"NuGet.Config"
697+
"NuGet.Config",
698+
"Directory.Build.targets",
699+
"Some.TARGETS",
700+
"MySolution.slnx",
701+
"rules.ruleset",
702+
"Project.DotSettings",
703+
"App.config",
704+
"app.config",
705+
"Web.config",
706+
"Web.Debug.config",
707+
"App.Release.config"
698708
})
699709
void acceptWithValidPaths(String path) {
700710
assertThat(new XmlParser().accept(Path.of(path))).isTrue();
@@ -706,7 +716,10 @@ void acceptWithValidPaths(String path) {
706716
".xml",
707717
"foo.xml.",
708718
"file.cpp",
709-
"/foo/bar/baz.xml.txt"
719+
"/foo/bar/baz.xml.txt",
720+
"log4net.config",
721+
"appsettings.config",
722+
"webhook.config"
710723
})
711724
void acceptWithInvalidPaths(String path) {
712725
assertThat(new XmlParser().accept(Path.of(path))).isFalse();

0 commit comments

Comments
 (0)
⚡