dotnet / roslyn-analyzers

MIT License
1.58k stars 464 forks source link

False non-copyable report #7389

Open 333fred opened 2 weeks ago

333fred commented 2 weeks ago

Analyzer

Diagnostic ID: RS0042: Unsupported use of non-copyable type 'Microsoft.AspNetCore.Razor.PooledObjects.PooledArrayBuilder' in 'LocalReference' operation

Analyzer source

NuGet Package: Roslyn.Diagnostics.Analyzers

Version: 3.11.0-beta1.24170.2

Describe the bug

When using a non-copyable struct in a list pattern, RS0042 erroneously fires.

Steps To Reproduce

  1. Create a collection type that supports list patterns, and mark it non-copyable.
  2. Use that collection in a list pattern.

Expected behavior

No errors

Actual behavior

RS0042: Unsupported use of non-copyable type 'Microsoft.AspNetCore.Razor.PooledObjects.PooledArrayBuilder' in 'LocalReference' operation

Additional context

333fred commented 2 weeks ago

image As an example.

sharwell commented 2 weeks ago

In reviewing this code on SharpLab, it looks like there are some cases where a shadow copy of the pattern variable is made. The first step of resolving this will be identifying cases where the value is definitely known to not be copied.

using System;
using System.Collections;
using System.Collections.Generic;

public class C {
    C2 _value;

    public void M() {
        using var value = default(C2);

        // no copy here
        if (value is [.., string s])
        {
            return;
        }
    }

    public void M2() {
        // copy here, even though _value is not marked readonly
        if (_value is [.., string s2])
        {
            return;
        }
    }
}

public struct C2 : IEnumerable<object>, IDisposable
{
    public int Count => 1;

    public object this[int index] => throw new NotImplementedException();

    public void Dispose()
    {
        throw new NotImplementedException();
    }

    public IEnumerator<object> GetEnumerator()
    {
        throw new NotImplementedException();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}
333fred commented 2 weeks ago

I would allow it in all cases. A list pattern is simply an enumeration: if that affects the state of the struct, then it's already broken.