-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Use is not null syntax
#13286
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
Use is not null syntax
#13286
Conversation
|
@iSazonov I think these changes should wait until we have updated code to use features available in C# 8, such as null operators and pattern matching. |
|
The pattern comes from C# 9.0 and is already enabled. |
|
Build failures seem to be because preview features not enabled in csharp/test_PSConfiguration.cs(100,47): error CS8652: The feature 'not pattern' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. [/Users/runner/work/1/s/test/xUnit/xUnit.tests.csproj] |
|
You can exclude xUnit files from the PR or enable C# 9.0 in the csproj. |
|
@iSazonov If you would like, you can start to review, as we are not isolating C# 9.0 changes. This is already a huge PR so it seems to be the best plan. Any more changes can be in follow-up PR. |
|
Macos failures look like |
|
@xtqqczze Please rebase to pass CIs. |
|
@iSazonov Too many merge conflicts, I will push new commits rather then resolving. |
|
First rebase on 1f904e4 |
|
Second rebase fixes up. |
|
@xtqqczze thanks for your efforts in cleaning up the code, but I don't think the |
|
@daxian-dbw This change was actually requested by @iSazonov (#13090 (comment)). Are you against |
|
@daxian-dbw What do you mean? The pattern "is not null" is for replacement |
|
Quote from dotnet/roslyn#42368:
@iSazonov That conversion makes sense. But most changes in this PR are converting Even the |
|
@daxian-dbw it's worth noting that |
|
@vexx32 Yes, there is such possibility, but I don't think it's practically a problem we need to concern about. |
|
Just to say that if |
|
Given that #13277 is reverted, I will close this PR as well. The idea is the same as in #13277 (comment), that we shouldn't force |
|
@daxian-dbw I have made |
|
I think we should re-review this again since .Net team added new Roslyn analyzer to force using new syntax "is null" and "is not". |
Yep, see dotnet/runtime#42692 |
PR Summary
-replace '!= null', 'is not null'for C# 9.0.xUnit.tests.PR Context
Follow-up #13277.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.This change is