Upgrade packages pinned in Directory.Build.props + edit MSBuild files with an XML parser instead of regex - #856

Issue dotnet-outdated#561 reported that `dotnet outdated -u -t` on a solution using
central package management (CPM) added transitive packages to each
csproj *with* a `Version` attribute, which CPM rejects at restore
(NU1008). Nobody managed to reproduce it, so before changing anything
these tests pin down what actually happens today.

They run the real `dotnet` against the repo's local fake package feed,
upgrade a transitive dependency, and check the project still restores:

- Under CPM, the transitive package is added to the csproj without a
  version and its version goes into Directory.Packages.props. This
  already works, with and without `--no-restore`, so dotnet-outdated#561 appears fixed
  upstream; the test stops it coming back.
- When the parent package is referenced from Directory.Build.props, the
  transitive package is added to the project itself and the props file
  is left alone. This already works too.

Refs: dotnet-outdated#561

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The next commits stop running `dotnet add package` for packages whose
version is set in Directory.Build.props, and write the version there
directly instead. That must not catch packages `dotnet add package`
handles fine, so pin down today's behaviour first, with a shared
Directory.Build.props in place:

- A package the project declares itself, with its own `Include` and
  `Version`, is upgraded through `dotnet add package`, and the props
  file is left alone.
- A package declared nowhere (for example a transitive dependency being
  added) also goes through `dotnet add package`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ence` updates

The `GlobalPackageReference` update writes a literal version straight
into the file that declares it, keeping the file's encoding and
restoring each project once when several share the file. The next commit
needs exactly that for `PackageReference` versions in
Directory.Build.props, so split the writing out into
`UpdateLiteralVersionInFile`, which takes the element name, rather than
copying it.

No behaviour changes: `GlobalPackageReference` updates call the new
method with `"GlobalPackageReference"`, and the error message names the
element as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…graded

dotnet-outdated#301 asks for `dotnet outdated -u` to upgrade a package whose version is
set in Directory.Build.props. As of upstream master 4d722c8 it can't: it
runs `dotnet add package` for each package, which refuses with "Cannot
edit items in imported files", so the upgrade fails and both files are
left as they were.

Pin that failure down with an end-to-end characterization test against
the real `dotnet`, with and without `--no-restore`.

This new test project pins `DotNetOutdated.Fake.Logging` in its
Directory.Build.props, along with Logging's transitive dependency
`DotNetOutdated.Fake.DependencyInjection` at a version above the one
Logging asks for.

The fix in the next commit flips the test's assertions, so its diff
shows the behaviour before and after. Asserting the bug, rather than
adding a test that fails until the fix lands, avoids a commit with a
failing test, so every commit passes and `git bisect` keeps working. The
test checks the exit code and that the files are unchanged, not the
error text, which is localised and differs between SDKs.

Refs: dotnet-outdated#301

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ops (Issue dotnet-outdated#301)

A Directory.Build.props can set package versions once for every .csproj
in the folders below it, for example to pin a vulnerable transitive
dependency. dotnet-outdated already upgrades a version set in one
through a `$(Property)`, but not a literal version on a
`PackageReference`, which is what dotnet-outdated#301 asks for: "if a
Directory.Build.props file existed and provided a package version".

For those, `dotnet outdated -u` runs `dotnet add package`, which refuses
with "Cannot edit items in imported files", and the upgrade fails. The
same happens when a csproj overrides an imported version with its own
`<PackageReference Update="..." Version="...">`. Since c3fbf21 (dotnet-outdated#790)
that error is shown rather than swallowed. It comes from NuGet's
`dotnet add package` command:
https://github.com/NuGet/NuGet.Client/blob/7e935f9c72b287adc13b9f42fd7b3bdcaa1500e7/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs#L495-L502
https://github.com/NuGet/NuGet.Client/blob/7e935f9c72b287adc13b9f42fd7b3bdcaa1500e7/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx#L334-L340

In theory `dotnet add` should be able to make these edits itself; these
NuGet issues cover the area:
NuGet/Home#14317
NuGet/Home#13218
NuGet/Home#12156

dotnet-outdated#301 notes that NuKeeper handled this. NuKeeper is now archived in
favour of Dependabot and dotnet-outdated, so dotnet-outdated needs to:
NuKeeperDotNet/NuKeeper#1155

Where `dotnet add` would refuse, write the new version directly into the
file that declares it, then restore:

- The MSBuild evaluator already works out which file sets each package's
  version. It now also reports when that is a literal `PackageReference`
  version that `dotnet add` can't edit: one in an imported file, or an
  `Update` item.
- `VersionLocator` turns that into a new location type,
  `PackageReferenceVersionInFile`.
- `DotNetPackageService` writes the version there with the code split
  out of `GlobalPackageReference` updates two commits back: the file's
  encoding is kept, and a props file shared by several projects restores
  each project once.

Deciding this up front, rather than running `dotnet add` and reacting to
its failure, avoids parsing dotnet's error text, which is localised and
changes between SDKs.

The end-to-end test from the previous commit, which showed the upgrade
failing, now expects it to succeed and the project to restore.

This commit decides *which file* gets the new version. How the version
text is found inside a file is the next commit's concern.

Fixes: dotnet-outdated#301

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The next commit replaces the regular expressions that find a package's
version with an XML parser. These are the valid MSBuild files the
regexes get wrong:

- A version written as a `<Version>` child element, which MSBuild
  accepts like any other item metadata, isn't found. A
  Directory.Build.props entry written that way can't be upgraded, and
  nor can a central package with `--no-restore`; upstream's
  `CpmProjectWithNoRestore_UnmatchedEntry_ReportsFailureAndLeavesFileUnchanged`
  test already shows the latter.
- A package id containing an XML entity reference (`Include="A&dotnet-outdated#46;B"`)
  doesn't match, because the regex compares the raw text.

Pin those failures down with characterization tests. The next commit
flips their assertions, so its diff shows the behaviour before and
after. Asserting the bugs, rather than adding tests that fail until the
fix lands, avoids a commit with failing tests, so every commit passes
and `git bisect` keeps working.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Some upgrades write the new version into a file directly rather than
through `dotnet add package`: central package versions in
Directory.Packages.props, `GlobalPackageReference` items, and, since the
dotnet-outdated#301 fix, versions in Directory.Build.props. dotnet-outdated finds the
version to change in those files with regular expressions.

Regex can't reliably parse XML. dotnet-outdated#602 was one case: entries spread over
several lines weren't updated. dotnet-outdated#783 fixed that by making the regex
cleverer, but valid MSBuild still defeats it:

- A version written as a `<Version>` child element, which MSBuild
  accepts like any other item metadata, is never found. Upgrading a
  central package with `--no-restore` fails, and a Directory.Build.props
  entry can't be upgraded at all.
- A package id containing an XML entity reference (`Include="A&dotnet-outdated#46;B"`)
  doesn't match, because the regex compares the raw text.

Each such case would need another regex special case, and there will
always be another:
https://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454#1732454

Instead, find the elements with an XML parser (`XmlReader`), which can
correctly parse the `.props` xml files, and use its line and column
information to replace only the version text, so the rest of the file
stays byte-for-byte identical. (Loading and saving with `XDocument`
would reformat the whole file.) A file that isn't well-formed XML is
left unchanged and reported as not updated.

No existing library fits for editing .csproj and .props files:

- Microsoft.Build, the MSBuild API, is the "correct" editor and resolves
  import precedence, but needs Microsoft.Build.Locator plus a bound SDK
  toolset: a heavy, fragile dependency for a global tool and a departure
  from this project, which otherwise shells out to `dotnet` and reads
  NuGet libraries.
  https://www.nuget.org/packages/Microsoft.Build
  https://www.nuget.org/packages/Microsoft.Build.Locator
- FubuCsProjFile: dead since 2015, .NET Framework only.
  https://www.nuget.org/packages/FubuCsProjFile
- PackageReferenceEditor: unmaintained since 2021, and pulls in the
  ReactiveUI UI framework.
  https://www.nuget.org/packages/PackageReferenceEditor
  https://www.nuget.org/packages/ReactiveUI
- MvsSln: healthy, but depends on Microsoft.Build and is
  solution-oriented.
  https://www.nuget.org/packages/MvsSln
- SunamoCsproj: missing license text, unclear if it does what we need
  https://www.nuget.org/packages/SunamoCsproj

This covers every place that changes a version this way: central package
updates with `--no-restore`, putting back the Directory.Packages.props
layout after `dotnet add package`, `GlobalPackageReference` updates, and
the Directory.Build.props updates from the dotnet-outdated#301 fix. Restoring
`$(Variable)` references (`ReplacePackageVersion`) and editing
properties still use regex; the next two commits move them onto the XML
reader too.

The characterization tests from the previous commit now expect these
versions to be found and updated.

One upstream test asserted the old limitation: it used a `<Version>`
child element as its example of an entry that can't be updated. It now
uses an entry with no version at all.

Refs: dotnet-outdated#561
Refs: dotnet-outdated#301
Refs: dotnet-outdated#602
Refs: dotnet-outdated#316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… regexes

Two kinds of edit still find elements with regular expressions:

- Restoring a `$(Variable)` reference after an upgrade
  (`ReplacePackageVersion`).
- Editing properties (`ReplacePropertyValueAt`, `InsertProperty`,
  `InsertPropertyBefore`).

These are the valid MSBuild files they get wrong:

- A `</PropertyGroup>` inside a comment ends the group early. The
  definition or anchor after it isn't found, and `InsertProperty` puts
  the new property inside the comment.
- A package id containing an XML entity reference (`Include="A&dotnet-outdated#46;B"`)
  doesn't match, because the regex compares the raw text.
- A commented-out closing tag inside a property's value, as in
  `<AVersion>1.0.0<!-- </AVersion> --></AVersion>`, makes the edit
  change the wrong text, so the upgrade is refused with a warning.
  Upstream's
  `KnownDefinition_EditWouldChangeOtherDefinitions_DoesNothing` test
  already shows this.

Pin those failures down with characterization tests. The next commit
flips their assertions, so its diff shows the behaviour before and
after. Asserting the bugs, rather than adding tests that fail until the
fix lands, avoids a commit with failing tests, so every commit passes
and `git bisect` keeps working.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The previous commit's tests show valid MSBuild that the remaining
regexes get wrong: restoring a `$(Variable)` reference
(`ReplacePackageVersion`) and editing properties
(`ReplacePropertyValueAt`, `InsertProperty`, `InsertPropertyBefore`).

Move them onto the XML reader that `ReplaceVersion` already uses:

- `PackageXml.Elements` reads the file once and reports each element's
  name, parent and attributes, and where its start tag, content, end tag
  and text nodes sit in the original text. `FindVersionSpans` now uses
  it too.
- Each edit finds its element in that list and replaces or inserts text
  at those offsets, so the rest of the file stays byte-for-byte
  identical, as before.
- A property definition is an element directly inside a
  `<PropertyGroup>`, the same rule `MSBuildEvaluator` uses when it
  records which definition to edit, so both count definitions the same
  way.
- With a comment or other markup inside a value, only the value's text
  node is replaced, keeping the comment. With more than one text node it
  isn't clear which is the value, so nothing is changed.

The characterization tests from the previous commit now expect the edits
to be made. Upstream's
`KnownDefinition_EditWouldChangeOtherDefinitions_DoesNothing` becomes
`KnownDefinition_CommentedOutTagsInTheValue_UpdatesOnlyTheValue`: the
edit no longer touches the comment, so the upgrade goes ahead.
`MSBuildVariableWriter` still checks each property edit against the
parsed XML before writing.

`ProjectSdkReference`, which updates `Sdk` attributes, still uses regex;
the next two commits move it onto the XML reader too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

`ProjectSdkReferenceHelper` finds a project's SDKs by reading the file
as XML (`GetDeclarations`), but updates their versions by finding the
tags with regular expressions (`TryUpdate`). The two disagree on valid
MSBuild:

- An entity reference in the SDK's name or version, such as
  `Name="X&dotnet-outdated#46;Sdk"` or `Version="1&dotnet-outdated#46;0&dotnet-outdated#46;0"`, is decoded by
  discovery, so the SDK is found and reported when a newer version
  exists. The update compares the raw text, finds nothing to change and
  returns false, and `DotNetPackageService.UpdateProjectSdk` then fails
  with "Failed to update the version of SDK".
- Only an `<Sdk>` element directly under `<Project>` declares an SDK,
  and discovery ignores one anywhere else, such as an `Sdk` item in an
  `ItemGroup`. The update rewrites that item's `Version` anyway.

These files are unlikely in practice. The tests are here to show that
the regexes have edge cases they don't handle, and that an XML parser
makes dotnet-outdated more robust against XML structured in ways it
doesn't expect. XML is not a regular language, so regular expressions
can't parse it.

Pin those failures down with characterization tests. The next commit
flips their assertions, so its diff shows the behaviour before and
after. Asserting the bugs, rather than adding tests that fail until the
fix lands, avoids a commit with failing tests, so every commit passes
and `git bisect` keeps working.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The previous commit's tests show `ProjectSdkReferenceHelper.TryUpdate`,
which finds SDK declarations with regular expressions, disagreeing with
`GetDeclarations`, which reads the file as XML.

Move `TryUpdate` onto `PackageXml.Elements`, finding declarations by the
same rules as `GetDeclarations`: the root `<Project>`'s `Sdk` attribute,
`<Sdk>` elements directly under it, and `<Import Sdk="...">` elements,
none of them inside `<ProjectExtensions>`. Names and versions are
compared after entity references are decoded, and only the version text
is replaced, so the rest of the file stays byte-for-byte identical. When
an entry in the root's `Sdk` attribute changes, the attribute is
rewritten from its decoded value, escaped again.

The characterization tests from the previous commit now expect an SDK
written with entity references to be updated, and an `Sdk` item in an
`ItemGroup` to be left alone.

With this, no XML in the files dotnet-outdated edits is parsed with
regex. The regexes left in `src` parse MSBuild expressions such as
`$(Name)`, `#:` directive lines in file-based apps, and generated shell
completion scripts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…tten

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…file

A Directory.Build.props can pin a package once per target framework. Writing
every PackageReference for the package there overwrote the other target
frameworks' pins. The evaluator now reports which of the file's items for the
package is in effect, only that item is written, and each target framework of
the upgrade writes its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The root Sdk attribute was rewritten from its decoded value, which turned line
breaks into spaces and decoded entity references. Entries are still matched on
the decoded value, as MSBuild reads it, but each decoded character is mapped
back to the raw text so only the matching version's text is replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>