-
Notifications
You must be signed in to change notification settings - Fork 152
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
Turn on C# 8's null checker #458
Comments
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
If we decide to use C# 8.0 with .NET Standard 2.0 (see #780) this approach is probably not that useful. |
We decided to use C# 8.0 with .NET Standard 2.0, not for null checks, but async streams. Anyway we would be able to use null checks as well, if we introduce C# 8.0 soon. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
Update: although we still haven't enabled null checker project-wide, we've already enabled it in several files (the recently added files for the most part). For gradual progression, I suggest the @planetarium/libplanet team the methodology to adopt null checker with the minimum pain (and actually it is what we've recently done in few months indeed):
@libplanet Opinions? |
The current status: $ find Libplanet/ -name '*.cs' | wc -l
193
$ git grep '#nullable enable' Libplanet/ | wc -l
37 The easiest files to enable null checker would be exception classes: $ for f in $(find Libplanet -name '*Exception.cs'); do grep '#nullable enable' "$f" > /dev/null || echo "$f"; done
Libplanet/Blocks/InvalidBlockNonceException.cs
Libplanet/Blocks/InvalidBlockDifficultyException.cs
Libplanet/Blocks/InvalidBlockPreviousHashException.cs
Libplanet/Blocks/InvalidBlockTimestampException.cs
Libplanet/Blocks/InvalidBlockIndexException.cs
Libplanet/Blocks/InvalidGenesisBlockException.cs
Libplanet/Blocks/InvalidBlockHashException.cs
Libplanet/Blocks/InvalidBlockException.cs
Libplanet/Crypto/InvalidCiphertextException.cs
Libplanet/Net/SwarmException.cs
Libplanet/Net/DifferentAppProtocolVersionException.cs
Libplanet/Net/InvalidStateTargetException.cs
Libplanet/Net/InvalidMessageException.cs
Libplanet/Net/IceServerException.cs
Libplanet/Net/PeerNotFoundException.cs
Libplanet/Net/NoSwarmContextException.cs
Libplanet/Net/Protocols/PingTimeoutException.cs
Libplanet/Net/Protocols/PeerDiscoveryException.cs
Libplanet/KeyStore/IncorrectPassphraseException.cs
Libplanet/KeyStore/InvalidKeyJsonException.cs
Libplanet/KeyStore/KeyJsonException.cs
Libplanet/KeyStore/UnsupportedKeyJsonException.cs
Libplanet/KeyStore/NoKeyException.cs
Libplanet/KeyStore/KeyStoreException.cs
Libplanet/KeyStore/MismatchedAddressException.cs
Libplanet/Tx/InvalidTxNonceException.cs
Libplanet/Tx/InvalidTxException.cs
Libplanet/Tx/InvalidTxGenesisHashException.cs
Libplanet/Tx/InvalidTxPublicKeyException.cs
Libplanet/Tx/InvalidTxSignatureException.cs
Libplanet/Tx/InvalidTxUpdatedAddressesException.cs
Libplanet/Tx/TxViolatingBlockPolicyException.cs
Libplanet/Tx/InvalidTxIdException.cs
Libplanet/Blockchain/IncompleteBlockStatesException.cs
Libplanet/Action/MissingActionTypeException.cs
Libplanet/Action/UnexpectedlyTerminatedActionException.cs
Libplanet/Store/ChainIdNotFoundException.cs |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
As null checker is globally turned on in Libplanet project, each single file in the project no more needs to add a #null enable directive planetarium#458
As null checker is globally turned on in Libplanet project, each single file in the project no more needs to add a #null enable directive #458
As null checker is globally turned on in Libplanet project, each single file in the project no more needs to add a #null enable directive planetarium#458
As null checker is globally turned on in Libplanet project, each single file in the project no more needs to add a #null enable directive. planetarium#458
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
planetarium#458 [changelog skip]
@s2quake got this. |
The status of the entire solution before starting the task is as follows.
# pwsh script
dotnet sln list | Where-Object {
Test-Path -Path $_
} | ForEach-Object {
$directory = Split-Path -Path $_ -Parent
$isNullable = Select-Xml -Path $_ -XPath "/Project/PropertyGroup/Nullable"
if ($isNullable) {
$csFiles = Get-ChildItem -Path $directory -Recurse -Filter *.cs
$nullableFiles = @($csFiles | Where-Object { Select-String -Path $_ -Pattern "#nullable disable" } | ForEach-Object { $_ })
$progress = $nullableFiles.Length / $csFiles.Length
Write-Host "$($_)"
Write-Host " cs : $($csFiles.Length)"
Write-Host " nullable: $($nullableFiles.Length)"
Write-Host " progress: $($progress.ToString('p2'))"
}
} I will proceed with the task of removing the |
|
#3622 |
#3644 |
#3651 |
Edit: There is a way to introduce C# 8 with still targeting .NET Standard 2.0. For several reasons, we decided to use C# 8, but at the moment we use it without
<Nullable>enable</Nullable>
option. I hope we turn it on as well in the near future!Although C# 8 will have non-nullable reference types, it will requires .NET Standard 2.1 (note that we currently depend on .NET Standard 2.0), which is unlikely to supported by Unity in the near future.Instead, I suggest to introduce JetBrain'sNotNullAttribute
until Unity becomes to support .NET Standard 2.1. There's a Roslyn Analyzer implementation named NullGuard.Fody which doesnull
-checks according toNotNullAttribute
at compile time.Projects (test projects are excluded as of now):
The text was updated successfully, but these errors were encountered: