Update to .NET 9 and other updates #2
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1 @@ | ||
| /artifacts/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| namespace Dispownership.Test; | ||
|
|
||
| public sealed class AsyncDisposableTest | ||
| { | ||
| [Fact] | ||
| public async Task DisposesInnerValueIfOwned() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (AsyncDisposable.Owned(value)) | ||
| { | ||
| } | ||
|
|
||
| Assert.True(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotDisposeInnerValueIfBorrowed() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (AsyncDisposable.Borrowed(value)) | ||
| { | ||
| } | ||
|
|
||
| Assert.False(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotDisposeOwnedValueAfterMove() | ||
| { | ||
| var value = new AsyncDisposableStub(); | ||
|
|
||
| await using (var disposable = AsyncDisposable.Owned(value)) | ||
| { | ||
| _ = disposable.Take(); | ||
| } | ||
|
|
||
| Assert.False(value.Disposed); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task ThrowsWhenMovingAValueTwice() | ||
| { | ||
| await using var disposable = AsyncDisposable.Owned(new AsyncDisposableStub()); | ||
| _ = disposable.Take(); | ||
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task ThrowsWhenMovingABorrowedValue() | ||
| { | ||
| await using var disposable = AsyncDisposable.Borrowed(new AsyncDisposableStub()); | ||
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| [Theory] | ||
| [MemberData(nameof(Disposables))] | ||
| public async Task ThrowsWhenDisposingTwice(AsyncDisposable<AsyncDisposableStub> disposable) | ||
| { | ||
| #pragma warning disable IDISP007 | ||
| await disposable.DisposeAsync(); | ||
| #pragma warning restore IDISP007 | ||
| await Assert.ThrowsAsync<ObjectDisposedException>(async () => await disposable.DisposeAsync()); | ||
| } | ||
|
|
||
| public static TheoryData<AsyncDisposable<AsyncDisposableStub>> Disposables() | ||
| => [ | ||
| AsyncDisposable.Owned(new AsyncDisposableStub()), | ||
| AsyncDisposable.Borrowed(new AsyncDisposableStub()), | ||
| ]; | ||
|
|
||
| public sealed class AsyncDisposableStub : IAsyncDisposable | ||
| { | ||
| public bool Disposed { get; private set; } | ||
|
|
||
| public ValueTask DisposeAsync() | ||
| { | ||
| Disposed = true; | ||
| return ValueTask.CompletedTask; | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,3 @@ | ||
| namespace Dispownership.Test; | ||
|
|
||
| public sealed class DisposableTest | ||
|
|
@@ -67,7 +64,25 @@ public void ThrowsWhenMovingABorrowedValue() | |
| Assert.Throws<InvalidOperationException>(() => disposable.Take()); | ||
| } | ||
|
|
||
| [Theory] | ||
| [MemberData(nameof(Disposables))] | ||
| public void ThrowsWhenDisposingTwice(Disposable<DisposableStub> disposable) | ||
| { | ||
| #pragma warning disable IDISP007 | ||
| disposable.Dispose(); | ||
| #pragma warning restore IDISP007 | ||
| Assert.Throws<ObjectDisposedException>(disposable.Dispose); | ||
| } | ||
|
|
||
| #pragma warning disable IDISP004 | ||
| public static TheoryData<Disposable<DisposableStub>> Disposables() | ||
| => [ | ||
| Disposable.Owned(new DisposableStub()), | ||
| Disposable.Borrowed(new DisposableStub()), | ||
|
There was a problem hiding this comment. I don't see why this test case should throw on double dispose, isn't this the point, that it is not disposed when borrowed? Maybe I just don't understand. There was a problem hiding this comment. Yes, the inner disposable is not disposed when borrowed. What I changed is that the wrapper tracks that it got disposed and throws in that case. I thought it would be a good thing for consistency since that's what other disposables do too. There was a problem hiding this comment. Oh, never mind I misunderstood the dispose pattern 😆 Dispose should do nothing when called multiple times, but anything that accesses the resource should throw. There was a problem hiding this comment. I'm not sure we should implement this for our consumers. I know this is correct according the dispose pattern as per Microsoft, but if we track dispose, the contained value can't decide if it throws when used after dispose. This is a huge change and not necessarily in the interest of consumers, I fear? What do you think? There might be existing code that doesn't care if dispose is called, and then take after. This would be a breaking change for them, at least theoretically. There was a problem hiding this comment. I agree it might be a bit of a change, but I think it's a good one :) Consumers of the disposable wrapper currently see two different behaviours when the wrapper is disposed:
One of the use cases of the disposable is to treat owned and borrowed objects the same way. Always throwing if disposed might help catch bugs :) |
||
| ]; | ||
| #pragma warning restore IDISP004 | ||
|
|
||
| public sealed class DisposableStub : IDisposable | ||
| { | ||
| public bool Disposed { get; private set; } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,21 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFramework>net9.0</TargetFramework> | ||
| <IsPackable>false</IsPackable> | ||
| <OutputType>Exe</OutputType> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <Using Include="Xunit" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="..\Dispownership\Dispownership.csproj" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="xunit.v3" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" /> | ||
| </ItemGroup> | ||
| </Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "sdk": { | ||
| "version": "9.0.100", | ||
|
||
| "rollForward": "feature" | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.