Add [NotNull] attribute on the Should() method for object assertions #2380

0xced · 2023-10-13T07:43:45Z

Compelling example:

[Theory]
[InlineData(true)]
[InlineData(false)]
public void Test1(bool currentUser)
{
    IPrincipal principal = currentUser ? new WindowsPrincipal(WindowsIdentity.GetCurrent()) : new ClaimsPrincipal();
    IIdentity? identity = principal.Identity;
    
    identity.Should().NotBeOfType<GenericIdentity>();
    identity.IsAuthenticated.Should().BeFalse();
}

Fixes #1115

github-actions · 2023-10-13T07:51:12Z

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

Register at Qodana Cloud and configure the action
Use GitHub Code Scanning with Qodana
Host Qodana report at GitHub Pages
Inspect and use qodana.sarif.json (see the Qodana SARIF format for details)

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/qodana-action@v2023.2.8
        with:
          upload-result: true

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

coveralls · 2023-10-13T08:12:45Z

Pull Request Test Coverage Report for Build 7515208120

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 97.48%

Totals
Change from base Build 7484912142:	0.0%
Covered Lines:	11762
Relevant Lines:	11944

💛 - Coveralls

dennisdoomen · 2023-10-14T07:13:33Z

Src/FluentAssertions/AssertionExtensions.cs

    /// </summary>
    [Pure]
-    public static ObjectAssertions Should(this object actualValue)
+    public static ObjectAssertions Should([System.Diagnostics.CodeAnalysis.NotNull] this object actualValue)


Why not add the attribute to all Should() methods?

That's a very good question! I think we should add the [NotNull] attribute on all Should methods that return a ReferenceTypeAssertions<,>. So I started to write a test and noticed that I could not assert parameterInfo.IsDecoratedWith<NotNullAttribute>, hence #2385. 😄

So I started to write a test and noticed that I could not assert parameterInfo.IsDecoratedWith, hence #2385. 😄

I don't think we need to have tests for that.

Well, at least it was very interesting to dive into all the assertion types, I learnt a lot. And the test is now written anyway, alongside other tests on the Should methods. But this pull request could also be merged without the tests added in AssertionExtensionsSpecs and thus without requiring #2385 to also be merged.

IT-VBFK · 2023-10-14T08:03:29Z

Isn't this something that pretends what isn't there?

In such a case we can get rid of all null checks inside in the assertions?

dennisdoomen · 2023-10-14T08:50:32Z

Isn't this something that pretends what isn't there?

In such a case we can get rid of all null checks inside in the assertions?

I'm not entirely sure what you are saying? If you mean that we're kind of pretending that the subject is never null, that's true. We're relying on the assertion method itself to take care of that.

IT-VBFK · 2023-10-14T09:09:06Z

Ok, then this is just to silence the IDE, right?

lg2de · 2023-10-14T12:10:04Z

Isn't this something that pretends what isn't there?

In such a case we can get rid of all null checks inside in the assertions?

Newer compiler version use such attributes to create warnings. Older version does not.
And: It is a warning which the user can ignore or even disable.
So, the Should method cannot ensure non-nullability with the attribute.

IT-VBFK · 2023-10-14T18:04:18Z

So, the Should method cannot ensure non-nullability with the attribute.

Ahh.. thanks for clarification :)

0xced · 2023-10-14T21:58:52Z

I have rebased this pull request on top of #2385 because the new specs (Should_methods_returning_reference_type_assertions_are_annotated_with_not_null_attribute and Should_methods_not_returning_reference_type_assertions_are_not_annotated_with_not_null_attribute) require the new assertions on ParameterInfo.

github-actions · 2024-01-06T11:20:47Z

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

0xced · 2024-01-06T11:36:15Z

I just rebased this pull request on the develop branch and make it not depend on #2385 at all.

The assertion is now written like this:

parameter.GetCustomAttribute<NotNullAttribute>().Should().NotBeNull();

instead of this (which would require #2385 to be merged first):

parameter.Should().BeDecoratedWith<NotNullAttribute>();

zhangpengchen · 2024-01-12T10:36:37Z

Hi @dennisdoomen ,

Would you consider releasing this feature soon so we can finally avoid workarounds by mixing the native assertions and fluent assertions in the tests.

Compelling example: ```csharp [Theory] [InlineData(true)] [InlineData(false)] public void Test1(bool currentUser) { IPrincipal principal = currentUser ? new WindowsPrincipal(WindowsIdentity.GetCurrent()) : new ClaimsPrincipal(); IIdentity? identity = principal.Identity; identity.Should().NotBeOfType<GenericIdentity>(); identity.IsAuthenticated.Should().BeFalse(); } ``` Fixes fluentassertions#1115

dennisdoomen

Please also update the releases.md

0xced · 2024-01-13T22:26:44Z

I just added an entry in the release notes under improvements in 6ac05e8.

dennisdoomen

Awesome

jnyrup

Noticed that [NotNull] is not added to these overloads of Should, but guess it doesn't really matter that much.

AssertionExtensions.cs:168:    public static ExecutionTimeAssertions Should(this ExecutionTime executionTime)
AssertionExtensions.cs:753:    public static TypeSelectorAssertions Should(this TypeSelector typeSelector)
AssertionExtensions.cs:788:    public static MethodInfoSelectorAssertions Should(this MethodInfoSelector methodSelector)
AssertionExtensions.cs:813:    public static PropertyInfoSelectorAssertions Should(this PropertyInfoSelector propertyInfoSelector)
AssertionExtensions.cs:865:    public static TaskCompletionSourceAssertions<T> Should<T>(this TaskCompletionSource<T> tcs)
AssertionExtensions.cs:894:    public static TaskCompletionSourceAssertions Should(this TaskCompletionSource tcs)

0xced mentioned this pull request Oct 13, 2023

Support c# 8 nullable reference #1115

Closed

0xced force-pushed the Should-NotNull branch from cd556a5 to ad09e3b Compare October 13, 2023 08:03

dennisdoomen reviewed Oct 14, 2023

View reviewed changes

0xced force-pushed the Should-NotNull branch from ad09e3b to 6b0107d Compare October 14, 2023 21:48

dennisdoomen added the enhancement label Oct 15, 2023

0xced mentioned this pull request Oct 26, 2023

Introduce new assertions on ParameterInfo #2385

Closed

7 tasks

dennisdoomen mentioned this pull request Oct 28, 2023

Workaround for ReSharper regarding ShouldNotBeNull() #1074

Closed

0xced force-pushed the Should-NotNull branch from 6b0107d to 7a80fef Compare January 6, 2024 11:07

0xced force-pushed the Should-NotNull branch from 7a80fef to 049d423 Compare January 7, 2024 08:11

0xced force-pushed the Should-NotNull branch from 049d423 to c178478 Compare January 12, 2024 10:42

dennisdoomen approved these changes Jan 13, 2024

View reviewed changes

dennisdoomen requested a review from jnyrup January 13, 2024 16:32

dennisdoomen requested changes Jan 13, 2024

View reviewed changes

Add entry in the release notes under improvements

6ac05e8

dennisdoomen approved these changes Jan 14, 2024

View reviewed changes

jnyrup approved these changes Jan 14, 2024

View reviewed changes

jnyrup merged commit ca87a81 into fluentassertions:develop Jan 14, 2024

Add [NotNull] attribute on the Should() method for object assertions #2380

Add [NotNull] attribute on the Should() method for object assertions #2380

Uh oh!

Conversation

0xced commented Oct 13, 2023

Uh oh!

github-actions bot commented Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Qodana for .NET

Uh oh!

coveralls commented Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 7515208120

💛 - Coveralls

Uh oh!

dennisdoomen Oct 14, 2023

Choose a reason for hiding this comment

Uh oh!

0xced Oct 14, 2023

Choose a reason for hiding this comment

Uh oh!

dennisdoomen Oct 15, 2023

Choose a reason for hiding this comment

Uh oh!

0xced Oct 15, 2023

Choose a reason for hiding this comment

Uh oh!

IT-VBFK commented Oct 14, 2023

Uh oh!

dennisdoomen commented Oct 14, 2023

Uh oh!

IT-VBFK commented Oct 14, 2023

Uh oh!

lg2de commented Oct 14, 2023

Uh oh!

IT-VBFK commented Oct 14, 2023

Uh oh!

0xced commented Oct 14, 2023

Uh oh!

github-actions bot commented Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Qodana for .NET

Uh oh!

0xced commented Jan 6, 2024

Uh oh!

zhangpengchen commented Jan 12, 2024

Uh oh!

dennisdoomen left a comment

Choose a reason for hiding this comment

Uh oh!

0xced commented Jan 13, 2024

Uh oh!

dennisdoomen left a comment

Choose a reason for hiding this comment

Uh oh!

jnyrup left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

github-actions bot commented Oct 13, 2023 •

edited

Loading

coveralls commented Oct 13, 2023 •

edited

Loading

github-actions bot commented Jan 6, 2024 •

edited

Loading