Fix for using C# 8.0 projects (e.g. UWP projects stuck on .NET Core 3) by zhuman · Pull Request #398 · CommunityToolkit/dotnet

zhuman · 2022-08-24T00:03:39Z

Closes #376

This change fixes support for C# language version 8.0 by swapping a ImplicitObjectCreationExpression (which uses the C# 9 language feature Type myField = new();) to instead use a normal ObjectCreationExpression with the type explicitly defined (Type myField = new Type();). This enables the source analyzer to work on .NET Core 3.0 projects once again, such as UWP projects.

As part of testing this fix, I found that the source generator tests as written today don't actually validate successful compilation of any supported scenario, and that a number of scenarios that the tests claim to work on C# 7.3 don't actually work anymore. Therefore I've also changed the following:

Added "unsupported C# language version" diagnostics to generators which produce C# 8.0 language features. This should be nicer for devs rather than just producing unsupported syntax and hitting random C# compiler errors. A future exercise could be to remove these features if it's desirable to support < C# 8.0.
For each of the UnsupportedCSharpLanguageVersion_* tests, I've also added validation that the source code does compile with C# 8.0.
I've changed all test source compilation to be done using C# 8.0.
I've added a dedicated source compilation success test case for ObservableProperty/ObservableObject.

PR Checklist

Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
Based off latest main branch of toolkit
PR doesn't include merge commits (always rebase on top of our main, if needed)
Tested code with current supported SDKs
Tests for the changes have been added (for bug fixes / features) (if applicable)
Contains NO breaking changes
Code follows all style conventions

Other information

…) - fixes CommunityToolkit#376

net-foundation-cla · 2022-08-24T00:03:51Z

All CLA requirements met.

zhuman · 2022-08-24T01:07:33Z

I discovered that the unit tests for the generators actually never check for compilation errors at all (it just checks for diagnostics coming directly from the generators, it doesn't check for errors in the generated code), so there are actually other cases where newer C# features are being generated and breaking tests. I am looking at how best to solve these.

Edit: see the description, I think the best route is to make these generators explicitly give the unsupported language version diagnostic and we should add tests that validate successful compilation. I've implemented these in my change.

…egisterAllGenerator to provide a nicer "unsupported C# language version" diagnostic instead of simply generating unsupported code and failing later in compilation

…t, update tests to use the minimum supported C# language version (8.0), and also validate that unsupported version cases actually succeed with the supported version

Nirmal4G · 2022-09-04T03:47:30Z

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs

                    VariableDeclarator(Identifier(propertyName))
                    .WithInitializer(EqualsValueClause(
-                        ImplicitObjectCreationExpression()
+                        ObjectCreationExpression(IdentifierName(typeName))


We could guard it using language version but at this point, if we're going to drop .NET Native target as commented in the issue, then we could atleast warn about the minimum language version requirement.

I already did something like this in my private fork to generate language version compatible code. So, there's no need to check/warn about the version requirement. It always produces compilable code for the given target runtime.

Sergio0694 · 2022-09-06T08:19:31Z

...it.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs

            .Select(static (item, _) => Execute.GetInfo(item))
            .WithComparer(ValidationInfo.Comparer.Default);

+        context.FilterWithLanguageVersion(ref validationInfo, LanguageVersion.CSharp8, UnsupportedCSharpLanguageVersionError);


The ovservable validator doesn't use C# 8 features, no? This seems unnecessary (it's not free).

Even so, it's better to check against a min version of the language instead of leaving it up in the air!

No, it isn't. As I said, this adds too much overhead. Besides, I already plan to move this out of the generator.
I'd really like for this PR to only fix that one thing so it's nice and simple and we can merge it quickly.

EDIT: on second thought, the generator should just skip, but never cause a build error here.

CommunityToolkit.Mvvm.SourceGenerators/Messaging/IMessengerRegisterAllGenerator.cs

Sergio0694 · 2022-09-06T08:21:49Z

tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs

            }";

-        // This is explicitly allowed in C# < 8.0, as it doesn't use any new features
        VerifyGeneratedDiagnostics<ObservableValidatorValidateAllPropertiesGenerator>(


Undo this change (see previous comment), this generator doesn't require C# 8

Sergio0694 · 2022-09-06T08:23:10Z

tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs

+            new ObservableValidatorValidateAllPropertiesGenerator(),
+            new RelayCommandGenerator()
+        };
+        VerifyGeneratedDiagnostics(CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8)), generators, new string[] { });


This needs to be updated so that only the lang version required by each generator is used, not just C# 8 for all.

These two generators should never cause a build error

Sergio0694

Pushed a couple tweaks, LGTM!

Thank you!

crramirez · 2023-02-27T23:16:46Z

Hello,

One question. Have you tested with UWP projects? Because we are stuck on .Net Core 2.2 and C# 7.3, not .Net Core 3.0. We can use C# 8.0 in .net 2.2 but without all the features: https://www.reflectionit.nl/blog/2019/using-c-8-0-in-core-2-x-net-framework-and-uwp-projects

Fix for using C# 8.0 projects (e.g. UWP projects stuck on .NET Core 3…

a47ce3b

…) - fixes CommunityToolkit#376

zhuman added 2 commits August 23, 2022 19:09

Fix ObservableValidatorValidateAllPropertiesGenerator and IMessengerR…

31e712f

…egisterAllGenerator to provide a nicer "unsupported C# language version" diagnostic instead of simply generating unsupported code and failing later in compilation

Update source generator tests to actually generate the compiled outpu…

61e9e47

…t, update tests to use the minimum supported C# language version (8.0), and also validate that unsupported version cases actually succeed with the supported version

zhuman marked this pull request as ready for review August 24, 2022 02:25

Nirmal4G approved these changes Sep 4, 2022

View reviewed changes

Sergio0694 requested changes Sep 6, 2022

View reviewed changes

Sergio0694 added 2 commits September 8, 2022 23:26

Code style tweaks

7723c1e

Fix unit tests for generators

9b7d871

These two generators should never cause a build error

Sergio0694 approved these changes Sep 8, 2022

View reviewed changes

Sergio0694 merged commit 4fae4dd into CommunityToolkit:main Sep 8, 2022

Sergio0694 mentioned this pull request Oct 25, 2022

IMessenger generates incorrect code for projects targeting net462 #480

Closed

4 tasks

renovate bot mentioned this pull request Nov 27, 2024

Update nuget packages iadjadj/ArmoniK.Core#15

Open

1 task

dependabot bot mentioned this pull request Dec 27, 2025

Bump CommunityToolkit.Mvvm from 8.0.0 to 8.4.0 marcin-przywoski/SharpGallery#16

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for using C# 8.0 projects (e.g. UWP projects stuck on .NET Core 3)#398

Fix for using C# 8.0 projects (e.g. UWP projects stuck on .NET Core 3)#398
Sergio0694 merged 5 commits intoCommunityToolkit:mainfrom
zhuman:fix_cslang_80

zhuman commented Aug 24, 2022 •

edited

Loading

Uh oh!

net-foundation-cla bot commented Aug 24, 2022 •

edited

Loading

Uh oh!

zhuman commented Aug 24, 2022 •

edited

Loading

Uh oh!

Nirmal4G Sep 4, 2022

Uh oh!

Sergio0694 Sep 6, 2022

Uh oh!

Nirmal4G Sep 6, 2022

Uh oh!

Sergio0694 Sep 8, 2022 •

edited

Loading

Uh oh!

Uh oh!

Sergio0694 Sep 6, 2022

Uh oh!

Sergio0694 Sep 6, 2022

Uh oh!

Sergio0694 left a comment

Uh oh!

crramirez commented Feb 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

zhuman commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Other information

Uh oh!

net-foundation-cla bot commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhuman commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nirmal4G Sep 4, 2022

Choose a reason for hiding this comment

Uh oh!

Sergio0694 Sep 6, 2022

Choose a reason for hiding this comment

Uh oh!

Nirmal4G Sep 6, 2022

Choose a reason for hiding this comment

Uh oh!

Sergio0694 Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Sergio0694 Sep 6, 2022

Choose a reason for hiding this comment

Uh oh!

Sergio0694 Sep 6, 2022

Choose a reason for hiding this comment

Uh oh!

Sergio0694 left a comment

Choose a reason for hiding this comment

Uh oh!

crramirez commented Feb 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zhuman commented Aug 24, 2022 •

edited

Loading

net-foundation-cla bot commented Aug 24, 2022 •

edited

Loading

zhuman commented Aug 24, 2022 •

edited

Loading

Sergio0694 Sep 8, 2022 •

edited

Loading