Uno.CodeGen icon indicating copy to clipboard operation
Uno.CodeGen copied to clipboard

Compiler error CS0453 with nullable ref types + GeneratedImmutable

Open bddckr opened this issue 5 years ago • 2 comments

I'm submitting a...

  • Bug report (I searched for similar issues and did not find one)

Current behavior

When using nullable reference types and [GeneratedImmutable] the generated methods have wrong parameters.

For a public string? TestProperty { get; } we get

public Builder WithTestProperty(global::System.Nullable<string> value)
{
    TestProperty = value;
    return this;
}

for example.

The value parameter here gives a compile error CS0453.

Expected behavior

We should be able to use GeneratedImmutable on (partial) classes that use nullable reference types without any errors.

Minimal reproduction of the problem with instructions

using Uno;

#nullable enable

[GeneratedImmutable]
public sealed partial class Test
{
    [EqualityKey]
    public string? TestProperty { get; }
}

#nullable restore 

Environment

Nuget Package: Uno.CodeGen

Package Version(s): 1.33.0-dev.143

Affected platform(s):
- [ ] iOS
- [ ] Android
- [ ] WebAssembly
- [x] Windows
- [ ] Build tasks

(This is what I'm testing on, but I assume all platforms that support C# 8's nullable reference types are affected.)

Visual Studio
- [x] 2019 (version: 16.3.9)
- [ ] 2017 Preview (version: )
- [ ] for Mac (version: )

Relevant plugins
- [ ] Resharper (version: )

bddckr avatar Nov 16 '19 20:11 bddckr

Oooh that's a funny bug! Thanks for reporting.

carldebilly avatar Nov 17 '19 21:11 carldebilly

I'd like to take a stab at this, but I'll need some help. Apologies if there's something I'm missing - I looked into this repo's contrib guidelines (and other docs) as well as in Uno.SourceGeneration.

Adding a test

The tests project targets net461, which doesn't support C# 8.0. There is Uno.CodeGen.Tests.ExternalClasses, though, which I can edit to include netstandard2.1 in the TargetFrameworks. It does however look like only the net461 framework is used when building this project... How can I utilize this for debugging and testing purposes?

Additionally I can't see how Uno.CodeGen.Tests.ExternalClasses is actually used. Any suggestions here?

I also removed Given_ClassLifecycle.cs, as I get errors when building:

5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_Constructors_With_Parameters_Then_ParametersAggregatedOnInitialize.g.cs(16,8,16,419): error CS1029: #error: 'Constructor '.ctor' on line 226 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). '
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_SimulateLifetimeOfObject_Then_AllMethodInvoked_Subject.g.cs(16,8,16,523): error CS1029: #error: 'Constructor '.ctor' on line 50 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). Tips: The easiet way is to invoke the parameterless constructor (i.e. add ': this()' to your constructor)'
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_RealConstructor_InvokesInitialize_Subject.g.cs(16,8,16,524): error CS1029: #error: 'Constructor '.ctor' on line 116 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). Tips: The easiet way is to invoke the parameterless constructor (i.e. add ': this()' to your constructor)'
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_InheritFromAnotherLifecycleObject_Then_AllMethodInvoked_Subject.g.cs(16,8,16,418): error CS1029: #error: 'Constructor '.ctor' on line 95 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). '
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_InheritFromAnotherLifecycleObject_Then_AllMethodInvoked_Base.g.cs(16,8,16,523): error CS1029: #error: 'Constructor '.ctor' on line 82 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). Tips: The easiet way is to invoke the parameterless constructor (i.e. add ': this()' to your constructor)'
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_RealConstructor_InvokesParameterLessContructor_Subject.g.cs(16,8,16,524): error CS1029: #error: 'Constructor '.ctor' on line 164 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). Tips: The easiet way is to invoke the parameterless constructor (i.e. add ': this()' to your constructor)'
5>obj\Debug\net461\g\ClassLifecycleGenerator\Tests\Given_ClassLifecycle\When_RealConstructor_InvokesParameterLessContructor_Subject.g.cs(17,8,17,524): error CS1029: #error: 'Constructor '.ctor' on line 165 of file 'C:\Projects\Uno.CodeGen\src\Uno.CodeGen.Tests\Given_ClassLifecycle.cs' does not invoke the 'Initialize' method. As you marked (or used some builders that marked) some method with the '[ConstructorMethod]' and since you have defined some constructors in your code, you must invoke this method in all your constructor (either directly, or by invoking another contructor). Tips: The easiet way is to invoke the parameterless constructor (i.e. add ': this()' to your constructor)'
5>Done building project "Uno.CodeGen.Tests.csproj" -- FAILED.

Adding support for nullable ref types

Any hints on where to look for adding support for nullable reference types? It's definitely the immutable part of generators, not the equality one.

Looks like this is a good starting point https://github.com/unoplatform/Uno.CodeGen/blob/5f5bb490506ad1bb75a03da00b906b8445dd918d/src/Uno.CodeGen/ImmutableGenerator.cs#L527-L531 which leads me to https://github.com/unoplatform/Uno.CodeGen/blob/5f5bb490506ad1bb75a03da00b906b8445dd918d/src/Uno.CodeGen/ImmutableGenerator.cs#L813-L818 and from there on it seems like we may be hitting an issue with (this particular version of the package that provides the) Roslyn API here not knowing about the C# 8.0 feature? I'm not familiar enough to know yet - which is why I'd like to write the test first and debug into this call chain a bit 😄

bddckr avatar Nov 18 '19 12:11 bddckr