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

NUnit2010: Add support for detecting use of 'is' pattern inside 'Assert.That' #703

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

manfred-brands
Copy link
Member

Fixes #691

@manfred-brands
Copy link
Member Author

@mikkelbu I have added this to the EqualConstraintUsageAnalyzer because it started as is 3.
Because I had fun, it actually also converts relational constraints like Is > 1 and < 2.
So it could also fit in the ComparisonConstraintUsageAnalyzer.

One alternative is to make a separate analyzer, but that is a lot more work.

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @manfred-brands, and I think it is fine to keep it within the existing analyzer.

My only minimal concern is that the And/OrConstraints does not exactly match the semantics of the patterns. For instance, we have the note in https://docs.nunit.org/articles/nunit/writing-tests/constraints/OrConstraint.html#evaluation-order-and-precedence, but one might argue that this failure in the OrConstraint is an error and an implementation detail.

I've also tried to test some more complicated scenarios - combining multiple And and Ors - as I thought I could remember that we also had some problems with the precedence in NUnit, but I was not able to find the problem (if there is one).

So all in all I think the PR is an improvement and I'm not too worried about the differences as I don't think most people have very complicated patterns in their asserts

@manfred-brands manfred-brands merged commit 4798485 into nunit:master Mar 11, 2024
6 checks passed
@manfred-brands
Copy link
Member Author

Thanks @mikkelbu

@manfred-brands manfred-brands deleted the issues/691_IsNull branch March 11, 2024 23:27
@mikkelbu mikkelbu added this to the Release 4.1 milestone Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extent rule NUnit2010 to detect 'is null'
2 participants