C#: Enforce C# 10 in .csproj files

[Repiteo]

Makes C#10 the language version explicitly in all .scproj files that didn't do so previously. The only exceptions are the projects still targeting v4.7.2, as they'd need an overall update

[raulsntos]

While it's possible to use a newer C# language version than the default one for the target framework, some language features may require new API only available in newer versions of the framework. Also, using a newer C# version with an older .NET version is not supported by Microsoft.

Because of this I'm hesitant to upgrade the C# version without changing the TFM. And if we're just setting <LangVersion> to the default then it seems redundant since it's the same as not specifying it.

Maybe you can explain the motivation behind this change?

[raulsntos]

I don't know why we had 11 here, I don't think the tests use any C# 11 features. cc @paulloz

[paulloz]

We do use raw string literals, but it shouldn't be hard to change if we want to. The syntax is just a tad cleaner.

[raulsntos]

Oh, I love raw string literals. Since we use C# 11 features then this change is wrong. Unfortunately, because we don't run the tests in CI, the checks pass even though it would break the project.

[akien-mga]

Could we enable the dotnet tests in CI?

[paulloz]

It shouldn't be too complicated. I'm not familiar with what our standards from actions / workflows are, but it's basically calling dotnet test from the test folder.

[Repiteo]

There's other projects in the repo targeting dotnet standard 2.0 with the language set to C# 10, so this isn't treading new ground. For the 6.0 projects, it is admittedly redundant, but after the weird loophole situation the build system found itself in I wanted to make sure that the expected language was at least specified across all projects

[raulsntos]

There's other projects in the repo targeting dotnet standard 2.0 with the language set to C# 10

That's true and I haven't checked but it's likely that we're using features that requires the language version upgrade which justifies it.

For the 6.0 projects, it is admittedly redundant, but after the weird loophole situation the build system found itself in

Just to be clear, specifying the C# 10 language version would not have prevented the situation¹ since the LangVer checks just don't exist² in the SDK.

I'm not opposed to being more explicit though, as long as other contributors feel the same way. So if that's the only reason for this change I'd wait to see other contributors opinions, as a result I'll move this to the 4.x milestone but let me know if I misunderstood.

Footnotes

1. Referring to https://github.com/godotengine/godot/issues/86374. ↩

2. Removed by https://github.com/dotnet/roslyn/pull/62339. ↩

[paulloz]

I like the idea of homogenizing our C# versions. It'd be nice if we could at least get down to a single explicit version for each target framework used. IDK exactly what requirements we have in our projects targeting netstandard2.0, but we shouldn't have issues for net6 and net472 (another topic would be, do we still need to target net472 for those).

[Repiteo]

I'm not opposed to being more explicit though, as long as other contributors feel the same way. So if that's the only reason for this change I'd wait to see other contributors opinions, as a result I'll move this to the 4.x milestone but let me know if I misunderstood.

Yeah, that's the main reason. I did end up realizing these changes wouldn't have an impact on the issue after setting up the PR, but the intent of safeguarding ourselves through explicit declaration remains

IDK exactly what requirements we have in our projects targeting netstandard2.0, but we shouldn't have issues for net6 and net472 (another topic would be, do we still need to target net472 for those).

Understanding why those projects are targeting netstandard2.0/net472 would be good to iron out as well. For the latter, I would assume they're largely legacy projects with niche usecases to not warrant high priority in bringing up

[raulsntos]

The only projects that target net472 are:

• GodotTools.IdeMessaging.CLI which is just a test project for the IDE integration, it could probably be upgraded but I think the IDE integration as a whole is kind of in need of a refactor so I've postponed it so far.
• GodotTools.OpenVisualStudio which uses Windows-only packages and APIs and is used to open Visual Studio as part of the IDE integration. I'm not sure if it can be upgraded or if the packages we use still require .NET Framework but it seems like it makes sense for this project to stay on net472.

As for netstandard2.0, this is the only TFM supported by source generators and analyzers so most of the projects that target it do it because of this reason. Apart from this, the other projects that target netstandard2.0 are:

• GodotTools.IdeMessaging. This is probably related to the projects mentioned above that target net472 so it probably just targets netstandard2.0 so it can be consumed by both .NET Core and .NET Framework projects.
• GodotTools.BuildLogger. I'm not sure if we really need this project to target netstandard2.0 and maybe it can be upgraded to net6.0.

[paulloz]

this is the only TFM supported by source generators and analyzers

Yes, I was more curious about the net472 ones.

For all our netstandard2.0 generators/analyzers projects, I can't think of any reason we couldn't default to 10. They basically aren't supposed to be used with anything targeting under net6.0.

As for internal test projects, I'd usually force the latest available version (not including preview). But that's a personal thing, not sure everyone would agree.

[Repiteo]

Decided to try upgrading the listed projects in a separate PR, and they all seem to work fine still? They're building without a hitch at least

[raulsntos]

• Superseded by C#: Move GodotSharp to .NET8 #92131 which would actually set the language version to 12. Thanks for the contribution nonetheless.