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. |
dahlia
changed the title
|
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
37The 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. |
|
The current status: Progress: 79.4%. Now we have more null-checked files than unchecked files, we can enable it project-wide (in Libplanet/Libplanet.csproj file), and opt out only exceptional files. |
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'sNotNullAttributeuntil Unity becomes to support .NET Standard 2.1. There's a Roslyn Analyzer implementation named NullGuard.Fody which doesnull-checks according toNotNullAttributeat compile time.Projects (test projects are excluded as of now):
The text was updated successfully, but these errors were encountered: