ExternalAnnotations icon indicating copy to clipboard operation
ExternalAnnotations copied to clipboard

IENumerable<T>.All() and List<T>.All() methods incorrectly assumed to be pure

Open AngieEl opened this issue 6 years ago • 0 comments

The sample program below generates a pair of incorrect warnings on the All() method calls. In both cases it is assuming the invocation is a pure method and the line of code is actually a no-op; but the collection is modified.

using System.Linq;
 
namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            BlameStackoverflowForThis();
        }
 
        static void BlameStackoverflowForThis()
        {
            //https://stackoverflow.com/a/6233655/85661
            //the abuse of All() seen below is from the above answer
 
            List<TestObject> testList = new List<TestObject> { new TestObject(), new TestObject(), new TestObject() };
            IEnumerable<TestObject> testEnum = testList;
 
            //ReSharper warning:  Return value of pure method is not used
            //but text is changed from null to "foo"
            testEnum.All(c => { c.text = "foo"; return true; });
 
            //ReSharper warning:  Return value of pure method is not used
            //but text is changed from "foo" to "bar"            
            //in this case testList.ForEach(c => { c.text = "bar"; }); is a 
            //clean way to do it,  but IEnumerable doesn't have a Foreach() method
            testList.All(c => { c.text = "bar"; return true; });
 
        }
    }
 
    public class TestObject
    {
        public  string text { get; set; }
    }
}

AngieEl avatar Dec 06 '19 14:12 AngieEl