Fix argument null exception when updating assembly references with no public key token

[rhyskoedijk]

What are you trying to accomplish?

Fix #10549.
See #10549 (comment) for context.

When a .NET Framework project contains an assembly reference that does not have "PublicKeyToken" specified, e.g.

<Reference Include="CSharpFunctionalExtensions, Version=2.40.3.0, Culture=neutral, processorArchitecture=MSIL">
  <HintPath>..\packages\CSharpFunctionalExtensions.2.40.3\lib\net461\CSharpFunctionalExtensions.dll</HintPath>
</Reference>

The update will fail when updating assembly binding redirects in app.config/web.config, e.g.

Unhandled exception: System.ArgumentNullException: Value cannot be null. (Parameter 'value')
   at System.ArgumentNullException.Throw(String paramName)
   at System.Xml.Linq.XAttribute..ctor(XName name, Object value)
   at NuGet.Runtime.AssemblyBinding.ToXElement()
   at NuGetUpdater.Core.BindingRedirectManager.AddBindingRedirects(ConfigurationFile configFile, IEnumerable`1 bindingRedirects) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs:line 202
   at NuGetUpdater.Core.BindingRedirectManager.UpdateBindingRedirectsAsync(ProjectBuildFile projectBuildFile) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs:line 40
   at NuGetUpdater.Core.PackagesConfigUpdater.UpdateDependencyAsync(String repoRootPath, String projectPath, String dependencyName, String previousDependencyVersion, String newDependencyVersion, String packagesConfigPath, Logger logger) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs:line 89
   at NuGetUpdater.Core.UpdaterWorker.RunUpdaterAsync(String repoRootPath, String projectPath, String dependencyName, String previousDependencyVersion, String newDependencyVersion, Boolean isTransitive) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs:line 187
   at NuGetUpdater.Core.UpdaterWorker.RunForProjectAsync(String repoRootPath, String projectPath, String dependencyName, String previousDependencyVersion, String newDependencyVersion, Boolean isTransitive) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs:line 163
   at NuGetUpdater.Core.UpdaterWorker.RunAsync(String repoRootPath, String workspacePath, String dependencyName, String previousDependencyVersion, String newDependencyVersion, Boolean isTransitive, String resultOutputPath) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs:line 56
   at NuGetUpdater.Cli.Commands.UpdateCommand.<>c__DisplayClass8_0.<<GetCommand>b__0>d.MoveNext() in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs:line 37

From my [limited] understanding, the issue appears to be caused by a bug in the NuGet.Core v2 package.

The issue happens because when parsing assembly bindings, "PublicKeyToken" is optional and can be null. However, when writing assembly bindings to config, "PublicKeyToken" is assumed to be non-null, which causes the ArgumentNullException.

This issue has been fixed in NuGet.Client (v3+) by adding extra null checks, but NuGetUpdater cannot update to this version without considerable rework.

The change proposed here makes "PublicKeyToken" required and treats null and the string literal "null" as the same thing during comparisons. When parsing assembly bindings, the token will default to "null" if not specified.

The same fix was also applied to the "Culture" property, which appears to also be affected by this issue (must be non-null). It defaults to "neutral" if not specified.

Anything you want to highlight for special attention from reviewers?

If there is an easy way to remove NuGet.Core v2 and instead use NuGet.Client v6.x, that is probably the better way to fix this issue. I ran in to a lot of errors attempting to do this, but might not be doing it right.

How will you know you've accomplished your goal?

Updates to NuGet projects containing assembly references with no public key token complete without errors.
This can be tested with:

bin/dry-run.rb nuget rhyskoedijk/dependabot-tests --dir="/WebApplication1-BindingRedirectNRE" --dep="System.Text.Json"

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[abdulapopoola]

Thanks @rhyskoedijk !