IDE0068 False Positive

[jmolinelli]

Version Used:
VS19 Community
Steps to Reproduce:
Code example:

 if (this.TryOpenReader(sourceName, out TextReader reader))
            {
                using (reader)
                {
                var m = new Model();
                Importer.Import(m, reader);
                return m;
                }
            }

where the TryOpenReader retruns true if it is successful in opening a reader, false otherwise.

Expected Behavior:
Would NOT expect to get an IDE0068 message.
Actual Behavior:
Get an IDE0068 Message on the out variable. Tried a couple of approaches, but none seem to clear it.

[jspinella]

What specific version of VS 2019 Community are you running (Help -> About Microsoft Visual Studio -> Version 16.2.1)?

It would also help to have the full code (method definition for TryOpenReader? Where does Importer come from?)

[Grace3579]

try using var instead

if (this.TryOpenReader(sourceName, out var reader))
{
using (reader)
{
var m = new Model();
Importer.Import(m, reader);
return m;
}
}

[jmolinelli]

Version is 16.3.5 requested code is below. Please note this Message did not occur in VS17 with the exact same code -- I just upgraded to VS19 and this warning came up. Per an email from Grace3759, I tried var instead of out TextReader reader and am still getting the same false positive. Thanks jack private bool TryOpenreader(string sourceName, out TextReader reader) { TextReader qr; reader = null; switch (sourceName) { case "Default File": qr = new StreamReader(Terms.QIFFilePathName); break; case "Clipboard": var qText = Clipboard.GetText(); if (string.IsNullOrEmpty(qText)) { this.WarningDialog(this.Response(IssueId.MissingQIFData)); return false; } qr = new StringReader(qText); break; default: this.ErrorDialog(this.Response(IssueId.UnknownEntity, Resources.QIFDataSource, sourceName)); return false; } var firstToken = qr.Peek(); if (firstToken == -1 || (char)firstToken != '!') { this.WarningDialog(this.Response(IssueId.IllegalQIFFormat)); return false; } reader = qr; return true; }

…

On Tue, Oct 22, 2019 at 2:59 PM James Spinella ***@***.***> wrote: What specific version of VS 2019 Community are you running (Help -> About Microsoft Visual Studio -> *Version 16.2.1*)? It would also help to have the full code (method definition for TryOpenReader? Where does Importer come from?) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#39443?email_source=notifications&email_token=ABKMDJBQWFWDVYNR447Y4ALQP5ERFA5CNFSM4JDSYON2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB62NUA#issuecomment-545105616>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKMDJHR7EV5OIRPAEGCZZDQP5ERFANCNFSM4JDSYONQ> .

-- be well jack

[jmolinelli]

I tried var and still get the false positive Message. Also, please note that I didn't get this message on VS17 -- just upgraded to VS19 16.3.5.

…

On Tue, Oct 22, 2019 at 4:59 PM Grace3579 ***@***.***> wrote: try using var instead if (this.TryOpenReader(sourceName, out var reader)) { using (reader) { var m = new Model(); Importer.Import(m, reader); return m; } } — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#39443?email_source=notifications&email_token=ABKMDJAKRFOJK5R2C5IUK2LQP5SUJA5CNFSM4JDSYON2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB7F3QQ#issuecomment-545152450>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKMDJHAVNAIORFUOBOECMLQP5SUJANCNFSM4JDSYONQ> .

-- be well jack

[huoyaoyuan]

please note that I didn't get this message on VS17

Just because the message was recently added in VS2019. (16.2 or 16.3)

I choose to turn of it totally; the disposable analysis is totally problematic without concept of ownership in language.

[mavasani]

IDE0068 is a suggestion to refactor your code to write it in such a way that the disposable object is disposed on all paths, including exceptional code paths:

roslyn/src/Features/Core/Portable/FeaturesResources.resx

Line 1694 in e8a7d91

<value>Use recommended dispose pattern to ensure that locally scoped disposable objects are disposed on all paths. If possible, wrap the creation within a 'using' statement or a 'using' declaration. Otherwise, use a try-finally pattern, with a dedicated local variable declared before the try region and an unconditional Dispose invocation on non-null value in the 'finally' region, say 'x?.Dispose()'. If the object is explicitly disposed within the try region or the dispose ownership is transferred to another object or method, assign 'null' to the local variable just after such an operation to prevent double dispose in 'finally'</value>

         TextReader reader = null;
         try
         {
            if (this.TryOpenReader(sourceName, out reader))
            {
                var m = new Model();
                Importer.Import(m, reader);
                return m;
            }
         }
         finally
         {
              reader?.Dispose();
         }

[jmolinelli]

Thank you for your comments. It seems to me the disposable analysis and associated flag (IDE0068) could be very useful so I am trying to leave it on. I know how to write the code without the "using" approach but the code I tried seems like it should work so I am hoping to find out whether the False Positive is wrong and if it isn't', whether there is any solution using the "using" construct that is valid. Thanks again. Be well jack

…

On Wed, Oct 23, 2019 at 2:55 PM Huo Yaoyuan ***@***.***> wrote: please note that I didn't get this message on VS17 Just because the message was recently added in VS2019. (16.2 or 16.3) I choose to turn of it totally; the disposable analysis is totally problematic without concept of ownership in language. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#39443?email_source=notifications&email_token=ABKMDJAOLD5JNPAC6ND76B3QQCMYJA5CNFSM4JDSYON2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECCPZ3I#issuecomment-545586413>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKMDJAJZUALH7VHY3WNULDQQCMYJANCNFSM4JDSYONQ> .

-- be well jack

[mavasani]

It seems to me the disposable analysis and associated flag (IDE0068) could
be very useful so I am trying to leave it on.

Note that the dispose analyzers have been turned off by default in VS2019 16.4 due to some performance issues on larger solutions. You can turn them on explicitly by using the editorconfig entries mentioned here: #38984 (comment)

I know how to write the code without the "using" approach but the code I
tried seems like it should work so I am hoping to find out whether the
False Positive is wrong and if it isn't', whether there is any solution
using the "using" construct that is valid.

Your code will most likely be fine on normal program execution code paths, but the recommended implementation in #39443 (comment) will safe guard you in case of exceptions. Assume the below code for TryOpenReader:

   bool TryOpenReader(string sourceName, out TextReader reader)
   {
       reader = new ...

      // ... Some code which can throw an exception

      return true;
   }

[adam-frisby]

Is there any way the ergonomics of this could be improved in future language versions by allowing a using statement in the method call?

this.TryOpenReader(sourceName, using out TextReader reader)

[mavasani]

@pix64 - that would need to be filed as a separate compiler feature request.