dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.05k stars 4.04k forks source link

IDE0059 is not always reported #52292

Open DaniilPalii opened 3 years ago

DaniilPalii commented 3 years ago

Version Used:

Code to Reproduce:

    public class TestUnusedVariable
    {
        public void TestUnusedVariableInTrySection()
        {
            try
            {
                var unusedVariable = new object();
            }
            catch { }
        }

        public void TestUnusedVariableWhenUsingSelect()
        {
            var unusedVariable = new object();

            Enumerable.Empty<object>().Select(x => x);
        }

        public void TestUnusedVariableWhenUsingWhere()
        {
            var unusedVariable = new object();

            Enumerable.Empty<object>().Where(_ => true);
        }
    }

Expected Behavior: Warning "IDE0059: Remove unnecessary value assignment" should be reported for each unusedVariable in the code above.

Actual Behavior: Warning "IDE0059" is not reported.

DaniilPalii commented 3 years ago

I should add that the warning is reported in those cases


    public class TestUnusedVariable
    {
        public void SimpleTestUnusedVariable()
        {
            var unusedVariable = new object(); // Warning IDE0059 Unnecessary assignment of a value to 'unusedVariable'
        }

        public void TestUnusedVariableWhenNotUsingLinq()
        {
            var unusedVariable = new object(); // Warning IDE0059 Unnecessary assignment of a value to 'unusedVariable'

            Enumerable.Empty<object>();
        }
    }
LukasGelke commented 9 months ago

this also affects foreach loops. I'm "adding" to this instead of opening a new Issue, since it seems in both examples, the use of some Linq-Stuff "disables" the Analyzer. We had a bug in production with foreach (... in Enumerable.Chunk()) which should be caught by the analyzer.

// net6.0

public class Class2
{
  private void Do1(object[] objects)
  {
    foreach (object o in objects) { } // IDE0059 - OK
  }

  private void Do2(object[] objects)
  {
    foreach (object o in objects) { } // IDE0059 - OK
    foreach (object o in objects) { } // IDE0059 - OK
    string text = "abc";
  }

  private void Do3(object[] objects)
  {
    foreach (object o in objects) { } // MISSING
    foreach (object o in objects) { } // MISSING
    object[] others = [new()];
  }

  private void Do4(object[] objects)
  {
    foreach (object o in objects) { } // MISSING
    List<int> others = [new()];
  }

  private void Do5(object[] objects)
  {
    foreach (object o in objects) { } // MISSING
    foreach (object o in objects.Where(x => x.GetHashCode() != 0)) { } // MISSING??
    // not sure because of linq and sidefects, but it would still assign `o`; and this is like our bug
  }

  private void Do6((object, object)[] objects)
  {
    foreach ((object o, object b) in objects) { } // IDE0059 *2 - OK
  }

  private void Do7((object, object)[] objects)
  {
    foreach ((object o, object b) in objects) { } // MISSING *2
    object[] others = [new()];
  }
}

and a simplified version of the bug in question:

public class Class3
{ 
  private void ProdBug(MyDto[] objects)
  {
    foreach (MyDto[] o in objects.Chunk(5)) // MISSING??
    {
      DtoWorker(objects.Select(x => x.ammount));
    }
  }

  private void DtoWorker(IEnumerable<int> chunk)
  {
    // imagine a hard limit of 5
    _ = chunk;
  }

  private readonly record struct MyDto(int ammount);
}