Cysharp / MemoryPack

Zero encoding extreme performance binary serializer for C# and Unity.
MIT License
3.29k stars 193 forks source link

Compiler warning regarding ToDictionary method #177

Closed vesnx closed 7 months ago

vesnx commented 1 year ago

I get a compiler warning in MemoryPack.Generator that perhaps needs consideration The class: MemoryPackGenerator.Emitter.cs

The code:

 string EmitConstructor()
    {
        // no need `;` because after using object initializer
        if (this.Constructor == null || this.Constructor.Parameters.Length == 0)
        {
            return $"new {TypeName}()";
        }
        else
        {
            var nameDict = Members.Where(x => x.Symbol != null && x.IsConstructorParameter).ToDictionary(x => x.ConstructorParameterName, x => x.Name, StringComparer.OrdinalIgnoreCase);//--> line 899
            var parameters = this.Constructor.Parameters
                .Select(x =>
                {
                    if (nameDict.TryGetValue(x.Name, out var name))
                    {
                        return $"__{name}";
                    }
                    return null; // invalid, validated.
                })
                .Where(x => x != null);

            return $"new {TypeName}({string.Join(", ", parameters)})";
        }
    }

Severity Code Description Project File Line Suppression State Warning CS8714 The type 'string?' cannot be used as type parameter 'TKey' in the generic type or method 'Enumerable.ToDictionary<TSource, TKey, TElement>(IEnumerable, Func<TSource, TKey>, Func<TSource, TElement>, IEqualityComparer?)'. Nullability of type argument 'string?' doesn't match 'notnull' constraint. MemoryPack.Generator ..\src\MemoryPack.Generator\MemoryPackGenerator.Emitter.cs 899 Active

github-actions[bot] commented 8 months ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

vesnx commented 8 months ago

why doesn't MS look at this?

olivier-spinelli commented 8 months ago

Trying to help here:

vesnx commented 8 months ago

sure, however it's not my code

olivier-spinelli commented 8 months ago

Of course. You may create a PR for this.

hadashiA commented 7 months ago

Fixed in #232.