Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable annotations for projects using XamlX #15796

Merged
merged 8 commits into from
Jul 29, 2024

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented May 22, 2024

What does the pull request do?

This PR enables nullable reference types in projects using XamlX:

  • Build.Tasks
  • Designer.HostApp
  • Generators
  • Markup.Xaml.Loader

Depends on kekekeks/XamlX#121

Notes

The Generators project directly uses <Nullable>enable</Nullable> instead of including NullableEnable.props. This props file disables nullable warnings for netstandard2.0 as they're usually handled by the net8.0 build, which isn't targeted by this project.

Several methods in RoslynTypeSystem now throw NotSupportedException instead of returning null, to match the other implementations. These methods are normally not called by the name generator (or they would have failed with a NullReferenceException before).

@MrJul
Copy link
Member Author

MrJul commented May 22, 2024

Apparently hitting a compiler bug regarding local functions and nullability - I can't reproduce locally and there's no warning either in XamlX pipelines. I'm updating the SDK here to see if that solves it.

Edit: it does.

@MrJul
Copy link
Member Author

MrJul commented May 23, 2024

And now hitting a bug in ILRepack! See Gillibald/il-repack#1

@maxkatz6
Copy link
Member

@MrJul could it be that ILRepack bug was fixed in the upstream? https://github.com/gluck/il-repack
It was recently revived and made active again

@MrJul
Copy link
Member Author

MrJul commented May 23, 2024

@MrJul could it be that ILRepack bug was fixed in the upstream? https://github.com/gluck/il-repack It was recently revived and made active again

Indeed, these changes are similar to mine:

gluck/il-repack@15d48ef#diff-c49a81a215a1a411a33ee869d718122208bbfe89696ef5a091322ca938ee32e9R927-R936

Thanks for letting me know! We should probably discard the fork and use the new official version.

@MrJul
Copy link
Member Author

MrJul commented Jul 27, 2024

Note: ILRepack only supported .NET Framework, so I started porting it to .NET 8 a while back. While the projects compiled, I didn't have the time to update the custom NuGet pack targets yet, which was necessary to open a valid PR.

Meanwhile, ILRepack got another PR adding dotnet tool support: gluck/il-repack#364
Contrary to my fork, it doesn't update the ILRepack.Lib to .NET 8 which I intended to use. However, I think we're completely fine with using the tool in the build tasks.

I've updated this PR to use the tool. Assemblies are now merging without errors.

@MrJul MrJul requested a review from maxkatz6 July 27, 2024 10:44
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050542-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050578-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050623-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 merged commit d1bd85e into AvaloniaUI:master Jul 29, 2024
8 of 10 checks passed
@MrJul MrJul deleted the feature/xamlx-nullable branch July 31, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants