Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional Nullable Reference Type-aware support #1004

Closed
dferretti opened this issue Nov 2, 2021 · 2 comments · Fixed by #1005
Closed

Additional Nullable Reference Type-aware support #1004

dferretti opened this issue Nov 2, 2021 · 2 comments · Fixed by #1005
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting

Comments

@dferretti
Copy link
Contributor

dferretti commented Nov 2, 2021

Description

Following #714
and borrowing from https://devblogs.microsoft.com/dotnet/try-out-nullable-reference-types/#flow-attributes-doesnotreturn-and-doesnotreturnifbool
Can DoesNotReturnIf be added to Assert.IsTrue / Assert.IsFalse ? And DoesNotReturn be added to Assert.Fail() ? There are some other null checks should be able to be skipped, such as:

Steps to reproduce

Dictionary<string, string> dictionary = ...
Assert.IsTrue(dictionary.TryGetValue(key, out var value));
Assert.AreEqual(5, value.Length); // should know that value is not null, but still warns currently

similarly:

Dictionary<string, string> dictionary = ...
var found = dictionary.TryGetValue(key, out var value);
if (!found) Assert.Fail();
Assert.AreEqual(5, value.Length); // should know that value is not null, but still warns currently

Expected behavior

No compiler warnings.

Actual behavior

Compiler warnings issued on value.Length.

Environment

Using version 2.2.7 of MSTest.TestAdapater and MSTest.TestFramework

Workaround

Similar shims can be written such as

[DoesNotReturn]
public static void AssertFail() => Assert.Fail();

public static void AssertIsTrue([DoesNotReturnIf(false)] bool condition) => Assert.IsTrue(condition);

public static void AssertIsFalse([DoesNotReturnIf(true)] bool condition) => Assert.IsFalse(condition);
@dferretti
Copy link
Contributor Author

The work is already done to add the conditionally included compiler attributes in NullableAttributes.cs, so there shouldn't be too much to add here. So if this looks like an acceptable proposal I can put in a PR for it!

@Haplois Haplois added Triaged Help-Wanted The issue is up-for-grabs, and can be claimed by commenting labels Nov 2, 2021
@Haplois
Copy link
Contributor

Haplois commented Nov 2, 2021

Thank you @dferretti for the proposal, it looks good. Please go ahead and create the PR if you wish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants