louthy / language-ext

C# functional language extensions - a base class library for functional programming
MIT License
6.39k stars 414 forks source link

Collection initializers and collection expressions doesn't work with Seq<T> #1367

Open piotr-urbanski opened 2 days ago

piotr-urbanski commented 2 days ago

Creating Seq with collection initializer or collection expression doesn't work.

[Test]
public void Create_Seq_with_constructor()
{
    Seq<string> seq = new Seq<string>(["a", "b"]);
    Assert.That(seq, Has.Count.EqualTo(2));
}

[Test]
public void Create_Seq_with_collection_initializer()
{
    Seq<string> seq = new Seq<string> { "a", "b" };
    Assert.That(seq, Has.Count.EqualTo(2)); // FAIL
}

[Test]
public void Create_Seq_with_collection_expression()
{
    Seq<string> seq = ["a", "b"];
    Assert.That(seq, Has.Count.EqualTo(2)); // FAIL
}

Finding this error was tricky, because code looks ok and its hard to imagine, that variable will not have the value assigned in literally previous line of code, so I was looking everywhere before I checked if this initialization works. It's a shame they didn't replace the { } constructor route to use the CollectionBuilder if one is available, seems very short sighted to me.

louthy commented 2 days ago

Create_Seq_with_constructor and Create_Seq_with_collection_expression work for me.

Could you:

    public static void Test()
    {
        Create_Seq_with_constructor();
        Create_Seq_with_collection_expression();
    }

    public static void Create_Seq_with_constructor()
    {
        var seq = new Seq<string>(["a", "b"]);
        Debug.Assert(seq.Count == 2);
    }

    public static void Create_Seq_with_collection_expression()
    {
        Seq<string> seq = ["a", "b"];
        Debug.Assert(seq.Count == 2);
    }

Create_Seq_with_collection_initializer will never work because C# just calls .Add and ignores the result, which for immutable collections means the collection never gets any values added. I raised the issue on the dotnet/csharplang repo four years ago, which I assume triggered the new collection-initialiser support.

By the way, before CollectionBuilder, the idiomatic way to create collections is like so:

using static LanguageExt.Prelude;

var xs = Seq(1, 2, 3, 4, 5);
var xs = List(1, 2, 3, 4, 5);
var xs = Set(1, 2, 3, 4, 5);
var xs = HashSet(1, 2, 3, 4, 5);
var xs = Map(("x", 1), ("y, 2), ("z", 3));
var xs = HashMap(("x", 1), ("y, 2), ("z", 3));

It's still usually better than using [1, 2, 3, 4, 5] because it already knows the type.

piotr-urbanski commented 1 day ago

Thank you for your answer. Constructor also works for me, but collection expression and collection initializer don't. When I run the code you posted debugger stops at Debug.Assert in Create_Seq_with_collection_expression.

I'm using the latest stable version 4.4.9:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="LanguageExt.Core" Version="4.4.9" />
  </ItemGroup>
</Project>

And this program:

using LanguageExt;

Test();

void Test()
{
    Create_Seq_with_constructor();
    Create_Seq_with_collection_initializer();
    Create_Seq_with_collection_expression();
}

void Create_Seq_with_constructor()
{
    var seq = new Seq<string>(["a", "b"]);
    Console.Out.WriteLine("Constructor: " + seq.Count);
    Console.Out.WriteLine(seq);
}

void Create_Seq_with_collection_initializer()
{
    Seq<string> seq = new Seq<string> { "a", "b" };
    Console.Out.WriteLine("Collection initializer: " + seq.Count);
    Console.Out.WriteLine(seq);
}

void Create_Seq_with_collection_expression()
{
    Seq<string> seq = ["a", "b"];
    Console.Out.WriteLine("Collection expression: " + seq.Count);
    Console.Out.WriteLine(seq);
}

produces:

Constructor: 2
[a, b]
Collection initializer: 0
[]
Collection expression: 0
[]

According to ReSharper's "IL viewer" collection expression is lowered to collection initializer in High-level C# so I don't understand how it is possible, that one does work for you and the other doesn't.

In Low-level C# they both look the same:

  [CompilerGenerated]
  internal static void <<Main>$>g__Create_Seq_with_collection_initializer|0_2()
  {
    Seq<string> seq1 = new Seq<string>();
    seq1.Add("a");
    seq1.Add("b");
    Seq<string> seq2 = seq1;
    Console.Out.WriteLine(string.Concat("Collection initializer: ", seq2.Count.ToString()));
    Console.Out.WriteLine((object) seq2);
  }

  [CompilerGenerated]
  internal static void <<Main>$>g__Create_Seq_with_collection_expression|0_3()
  {
    Seq<string> seq1 = new Seq<string>();
    seq1.Add("a");
    seq1.Add("b");
    Seq<string> seq2 = seq1;
    Console.Out.WriteLine(string.Concat("Collection expression: ", seq2.Count.ToString()));
    Console.Out.WriteLine((object) seq2);
  }

So I have 2 questions:

  1. What can I do to make collection expressions work for Seq?
  2. Is there a way to disable collection expression and collection initializers for Seq?
louthy commented 1 day ago

v4.* doesn't have support for CollectionBuilder, only v5+, which is in beta atm. That's why it worked for me and not for you.

I'm surprised it compiled at all, it really shouldn't!

The only way to disable access to the C# hack that calls Add repeatedly is to build a Roslyn analyser that throws errors when it finds that pattern. There's a PR open that does this, but it needs work, and it's pretty low down in my priority list: https://github.com/louthy/language-ext/pull/868 - feel free to take it on if you feel it's important.