Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing warning on pure collection initializer methods #5685

Open
Ilchert opened this issue Oct 28, 2021 · 2 comments
Open

Missing warning on pure collection initializer methods #5685

Ilchert opened this issue Oct 28, 2021 · 2 comments

Comments

@Ilchert
Copy link

Ilchert commented Oct 28, 2021

Missing warning when Add method with PureAttiribute used for collection initializer.

Rise warning when C# collection initializer method is marked as Pure.

public static class Ext
{
    [Pure]
    public static int Add<T>(this A<T> c, T item) => 1;

    public static void Do()
    {
        var a = new A<int>() { 1, 23, 4 }; // must warn
        a.Add(1); // warning CA1806
    }
}

public class A<T> : IEnumerable<T>
{
    public IEnumerator<T> GetEnumerator() => throw new NotImplementedException();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

This warning can help with immutable collections (marking as pure methods like this) to prevent unexpected behaviour.

var a = new ImmutableArray<int> { 1 }; // throws NulRef
@Youssef1313
Copy link
Member

@jinujoseph This is a CA1806 report. Can you move to dotnet/roslyn-analyzers?

@stephentoub Per your comment (dotnet/runtime#34098 (comment)), you were concerned about using PureAttribute for that purpose. It's currently already the case for CA1806, but the analyzer is missing implicit Add calls for collection initializer. Do you think this should be handled?

cc @mavasani as well.

@sharwell sharwell transferred this issue from dotnet/roslyn Oct 28, 2021
@stephentoub
Copy link
Member

It's currently already the case for CA1806, but the analyzer is missing implicit Add calls for collection initializer. Do you think this should be handled?

If CA1806 is already special-casing Pure and it's just buggy in how it's doing so / missing some cases, I'm ok if we want to fix those inconsistencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants