Fody/PropertyChanged

Nullable generic struct causes Fody exception

virzak opened this issue · 7 comments

virzak commented

Nullable generic struct causes Fody exception. Making it either not nullable or not generic fixes it.

1>------ Build started: Project: FodyStructException, Configuration: Debug Any CPU ------
1>MSBUILD : error : Fody: An unhandled exception occurred:
1>MSBUILD : error : Exception:
1>MSBUILD : error : Failed to execute weaver C:\Users\virzak\.nuget\packages\propertychanged.fody\3.2.8\build\..\weaver\PropertyChanged.Fody.dll
1>MSBUILD : error : Type:
1>MSBUILD : error : System.Exception
1>MSBUILD : error : StackTrace:
1>MSBUILD : error :    at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 202
1>MSBUILD : error :    at InnerWeaver.Execute() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 109
1>MSBUILD : error : Source:
1>MSBUILD : error : FodyIsolated
1>MSBUILD : error : TargetSite:
1>MSBUILD : error : Void ExecuteWeavers()
1>MSBUILD : error : Object reference not set to an instance of an object.
1>MSBUILD : error : Type:
1>MSBUILD : error : System.NullReferenceException
1>MSBUILD : error : StackTrace:
1>MSBUILD : error :    at Mono.Cecil.ImportGenericContext.TypeParameter(String type, Int32 position) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 94
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 648
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 492
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportTypeSpecification(TypeReference type, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 640
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportType(TypeReference type, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 492
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportMethodSpecification(MethodReference method, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 724
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportMethod(MethodReference method, ImportGenericContext context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 677
1>MSBUILD : error :    at Mono.Cecil.DefaultMetadataImporter.ImportReference(MethodReference method, IGenericParameterProvider context) in C:\Code\Fody\cecil\Mono.Cecil\Import.cs:line 744
1>MSBUILD : error :    at Mono.Cecil.ModuleDefinition.ImportReference(MethodReference method, IGenericParameterProvider context) in C:\Code\Fody\cecil\Mono.Cecil\ModuleDefinition.cs:line 915
1>MSBUILD : error :    at ModuleWeaver.GetEquality(TypeReference typeDefinition)
1>MSBUILD : error :    at ModuleWeaver.FindTypeEquality(PropertyData propertyData)
1>MSBUILD : error :    at ModuleWeaver.FindComparisonMethods(TypeNode node)
1>MSBUILD : error :    at System.Collections.Generic.List`1.ForEach(Action`1 action)
1>MSBUILD : error :    at ModuleWeaver.FindComparisonMethods()
1>MSBUILD : error :    at ModuleWeaver.Execute()
1>MSBUILD : error :    at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 174
1>MSBUILD : error : Source:
1>MSBUILD : error : Mono.Cecil
1>MSBUILD : error : TargetSite:
1>MSBUILD : error : Mono.Cecil.TypeReference TypeParameter(System.String, Int32)
1>MSBUILD : error :
1>Done building project "FodyStructException.csproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Minimal Repro

The failing unit test is in PR #624

using PropertyChanged;

[AddINotifyPropertyChangedInterface]
public class ParentOfNullableGenericStruct<T>
{
    public GenericStruct<T>? ChildProperty { get; set; }
}

public struct GenericStruct<T>
{
    public T Value { get; }
}
SimonCropp commented

what version of fody are u using?

virzak commented

I always update packages to latest. But does it even matter if the PR with unit test I added fails?

SimonCropp commented

@virzak sorry. yep good point.

@jbevain look like a cecil bug to you? want me to try for a repro in cecil ?

  • 👍1
jbevain commented

@SimonCropp I'm thinking two things:

  1. Cecil should not NRE, so that's a bug.
  2. NRE aside, the failure to Import might be a Cecil issue, or a Fody issue, depending on the method + context that is passed.

If you could repro outside of Fody for me to look at that would be absolutely fantastic.

SimonCropp commented

@jbevain i was a bug on my side

  • 👍1
jbevain commented

Thanks, I'll still look at providing something more interesting than an NRE in this method.

SimonCropp commented

have u considered using nullable ref types?