Incorrect suggestion to "Convert to 'using' declaration

Created July 27, 2020 09:22

(Using Resharper version 2020.1.4)

Consider the following console application:

using System;
using System.Runtime.Serialization;
using System.Text;
using System.Xml;

namespace ConsoleApp1
{
    class Program
    {
        static void Main()
        {
            Console.WriteLine(Serialise("test"));
        }

        public static string Serialise(string data)
        {
            var serialiser = new DataContractSerializer(typeof(string));
            var result = new StringBuilder();

            using (var writer = XmlWriter.Create(result))
            {
                serialiser.WriteObject(writer, data);
            }

            return result.ToString();
        }
    }
}

The output of this is:

<?xml version="1.0" encoding="utf-16"?><string xmlns="http://schemas.microsoft.com/2003/10/Serialization/">test</string>

Resharper suggests converting the `using` to a using declaration, like so:

public static string Serialise(string data)
{
    var serialiser = new DataContractSerializer(typeof(string));
    var result = new StringBuilder();

    using var writer = XmlWriter.Create(result);
    serialiser.WriteObject(writer, data);

    return result.ToString();
}

If you follow this advice then the code is broken - the return value from `Serialize()` will now be an empty string.

Is this a bug? Or just something to watch out for? (If the latter, then it makes the "Convert to 'using' declaration in file|project|solution" options too dangerous to use effectively.)

2 comments

Tor Gustad

Created May 16, 2023 12:48

Upvote on this.

I see the context menu item in Rider now (2023.1.1) mentions that the suggested change will extend the lifetime of a resource, but I think it should be possible to disable the "using declaration" style suggestion completely.

Here is another example of how this change will fail:

using System;
using System.IO;
using System.Text;

void UsingStatement()
{
    var theFile = Path.GetTempFileName();
    
    // inspection will suggest a change to the using declaration
    using (var testA = File.Open(theFile, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None))
    {
        testA.Write(Encoding.UTF8.GetBytes("We are testing the using statement."));
    }
    
    Console.WriteLine(File.ReadAllText(theFile));
}

void UsingDeclaration()
{
    var theFile = Path.GetTempFileName();
    
    using var testA = File.Open(theFile, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
    testA.Write(Encoding.UTF8.GetBytes("We are testing the using declaration."));
    
    Console.WriteLine(File.ReadAllText(theFile));
}

try
{
    UsingStatement(); // this works
}
catch (Exception ex)
{
    Console.WriteLine(ex.Message);
}

try
{ 
    UsingDeclaration(); // this fails
}
catch (Exception ex)
{
    Console.WriteLine(ex.Message);
}

I have personally decided to never use the syntax as I think it has far more drawbacks than benefits.

Olga Rodygina

Created May 23, 2023 15:47

Hello,
thank you for the report.
I've created an issue in YouTrack: https://youtrack.jetbrains.com/issue/RSRP-492396/Incorrect-suggestion-to-Convert-to-using-declaration, you are welcome to comment on it and follow.
Thank you!

Please sign in to leave a comment.