openlang

The using declaration refactor moves Dispose past Publish()

Shorter syntax, longer lifetime. A C# using declaration disposes its resource at the end of the enclosing scope, and that scope is easy to miss when the refactor looks like a tidy-up.

Take a method that acquires input in a nested using block, calls Read(input), leaves the block, then calls Publish(). The editor offers to turn the block into a using declaration at method scope. Accept it and Publish() now runs before input is disposed. If Publish() throws, disposal happens as control leaves the method. In the original version it had already happened.

The editor recognized the pattern correctly. Whether this method should hold the resource across Publish() is another question, and the preview gives no hint of it. All it shows is braces going away and indentation shifting.

So what should it mark, so that someone accepting the change can see the scope moved?

The conversation continues 5

Follow the reasoning
  1. Permalink

    Highlight Publish() as a statement that now runs before disposal. Recognizing the syntax is the easy part, and it tells the editor nothing about whether this method can live with the longer lifetime.

  2. Permalink

    Highlighting it is a start. It gets more interesting if Publish() is only allowed after input has been disposed. The method-scope rewrite breaks that rule, and the text change still looks completely routine.

    Try one more candidate edit. Keep the original inner block and put the using declaration inside it, so disposal still happens at the end of that block. Offer it as its own option, with the affected statements visible in the preview. The choice that actually matters is which scope to keep, and a preview of deleted braces says very little about that.

  3. Permalink

    Careful with what that marker promises. It can say when Dispose will run. It can't say every consumer of input has finished.

    What if Read(input) stores a callback that keeps a reference to input? Then the original block has the same question, refactor or not. Flag the escaping callback when the tool can see one, and keep the refactor decision about disposal scope. A callback that can still reach a disposed object hasn't earned the right to use it.

  4. Permalink

    Here's the version that makes the preview harder. Make input an IAsyncDisposable and the block becomes await using. Disposal is now an awaited DisposeAsync at the end of the scope, so moving the scope moves an await too. In a method that also awaits Publish(), the method-scope rewrite puts Publish() before the dispose and changes where the method yields. If the code uses ConfigureAwait(false), the dispose can also end up running on a different thread than before. None of that shows in a diff of deleted braces. The preview should say "disposal moves past these awaits", in those words.

  5. Permalink

    Visual Basic never got the declaration form, so this refactor cannot even be offered there. Using input As New StreamReader(path) and End Using is the only spelling, and the scope is always written down. For someone reading the code for the first time I count that as a small kindness.

Other sides of the same question

  1. Try a dynamic let before advice-add

    Advice is the nicest thing about extending a running editor and also the thing I trust least in other people's init files. (M…