BADF00D / DisposableFixer

This is a Visual Studio Extension and NuGet package that should identify and fix problems as memleaks while using IDisposables.
Other
35 stars 7 forks source link

When informing about undisposed local variables, their name should be part of the message #133

Closed Cassandra-d closed 4 years ago

Cassandra-d commented 4 years ago

Prerequisites

Description

Actual: warning about undisposed object within Using statement on disposed object. Expected: warning on the object that actually isn't disposed.

Source Code

class Program
{
    static async Task Main(string[] args)
    {
        HttpResponseMessage resp;

        using (var client = new HttpClient())
        {
            using (var stringContent = new StringContent("data"))
            {
                resp = await client.PostAsync("Url", stringContent);
            }
        }
    }
}

Screenshot

dispose3
BADF00D commented 4 years ago

Hi, thanks for reporting. Unfortunately I do not understand the problem. I'm using VS 2017 (15.9.15) with DisposableFixer 2.1.3. An undisposed local variable is reported. this sounds right to me, since resp is an IDisposable wich is not disposed. If you are adding an resp?.Dispose() at the end of main, the Diagnostics disappears as expected. Did I misunderstand you?

Cassandra-d commented 4 years ago

@BADF00D Hello. Right, there is correct message as there is undisposed variable.

It's a little bit confusing, because I'm looking at the part with underlining and see only one variable there — client. And client is disposed with using. So i'm thinking “why there is an error if I have using clause?”. And it took some time to understand that curly underlining doesn't draw my attention to a variable “client”, but to the expressions that instantiates another variable.

Can we do something about it? It's a little bit misleading as for me. For example, can we have actual undisposed variable marked? Or change warning text to a statement that says that “this expression creates undisposed instances”?

BADF00D commented 4 years ago

Ok, I understand your problem. Marking the variable/property/field is not an option, because at the place where you are assigning the variable, the declaration might not be visible at all (because it might be declared in code outside the screen). But we can change the message to a better one. Do you think it would be useful to add the variable name in the message? Something like 'Expression assigned an undisposed instance to local variable resp'?

Cassandra-d commented 4 years ago

Yes, that would be awesome, if that is possible, as there could be several variables on the same line and who knows what else on the lines above or below, so context of the problem could be even more confusing than in my example.