{"id":404055,"date":"2024-06-29T17:29:11","date_gmt":"2024-06-29T17:29:11","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=404055"},"modified":"-0001-11-30T00:00:00","modified_gmt":"2024-06-29T17:29:11","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=404055","title":{"rendered":"<span>Nullable Reference will not protect you, and here is the proof<\/span>"},"content":{"rendered":"<div><!--[--><!--]--><\/div>\n<div id=\"post-content-body\">\n<div>\n<div class=\"article-formatted-body article-formatted-body article-formatted-body_version-1\">\n<div xmlns=\"http:\/\/www.w3.org\/1999\/xhtml\">\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/30d\/8bd\/3c1\/30d8bd3c1040f014485bc84ba2d169c6.png\" alt=\"image1.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/30d\/8bd\/3c1\/30d8bd3c1040f014485bc84ba2d169c6.png\"\/><\/div>\n<p>  Have you ever wanted to get rid of the problem with dereferencing null references? If so, using Nullable Reference types is not your choice. Do you want to know why? This will be our topic today.<br \/>  <a name=\"habracut\"><\/a><br \/>  We warned you, and it happened. About a year ago, my colleagues wrote an <a href=\"https:\/\/viva64.com\/en\/b\/0631\/\">article<\/a> in which they warned that the introduction of Nullable Reference types will not protect against dereferencing null references. Now we have an indisputable proof of what we were saying found in the depths of Roslyn.<\/p>\n<h2>Nullable Reference types<\/h2>\n<p>  The idea itself of adding<i> Nullable Reference<\/i> (further as NR) types seems noteworthy to me, since the problem related to dereferencing null references is still relevant to this day. Nevertheless, the implementation of protection against dereferencing turned out to be extremely unreliable. According to the idea of creators, only those variables whose type is marked with the &#171;?&#187; symbol can accept the <i>null<\/i> value. For example, a variable of the <i>string?<\/i> type indicates that it might contain <i>null<\/i>, and a variable of the <i>string<\/i> type might imply the opposite <\/p>\n<p>  However, nobody&#8217;s stopping us from passing <i>null<\/i> to <i>non-nullable reference<\/i> variables (further as \u2014 NNR) of types, because they are not implemented at the IL code level. The compiler&#8217;s built-in static analyzer is responsible for this limitation. Therefore, this new feature is more of a recommendation. Here is a simple example showing how it works:<\/p>\n<pre><code class=\"cpp\">#nullable enable object? nullable = null; object nonNullable = nullable; var deref = nonNullable.ToString();<\/code><\/pre>\n<p>  As we can see, the <i>nonNullable<\/i> type is specified as NNR, but we can safely pass <i>null<\/i> there. Of course, we will get a warning about converting &#171;Converting null literal or possible null value to non-nullable type&#187;. However, we can get round it a bit more aggressively:<\/p>\n<pre><code class=\"cpp\">#nullable enable object? nullable = null; object nonNullable = nullable!; \/\/ &lt;= var deref = nonNullable.ToString();<\/code><\/pre>\n<p>  One exclamation mark and there are no warnings. If you&#8217;re a nitpicker, the following option is also available:<\/p>\n<pre><code class=\"cpp\">#nullable enable object nonNullable = null!; var deref = nonNullable.ToString();<\/code><\/pre>\n<p>  Here&#8217;s another example. Let&#8217;s create two simple console projects. In the first we write:<\/p>\n<pre><code class=\"cpp\">namespace NullableTests {     public static class Tester     {         public static string RetNull() => null;     } }<\/code><\/pre>\n<p>  In the second one we write:<\/p>\n<pre><code class=\"cpp\">#nullable enable   namespace ConsoleApp1 {     class Program     {         static void Main(string[] args)         {             string? nullOrNotNull = NullableTests.Tester.RetNull();             System.Console.WriteLine(nullOrNotNull.Length);         }     } }<\/code><\/pre>\n<p>  Hover the cursor over <i>nullOrNotNull<\/i> and see this message:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/e65\/240\/f59\/e65240f5971a4a68c98c1bacf0ba79a1.png\" alt=\"image2.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/e65\/240\/f59\/e65240f5971a4a68c98c1bacf0ba79a1.png\"\/><\/div>\n<p>  It&#8217;s a hint that the string here can&#8217;t be <i>null<\/i>. But we already know that it will be <i>null<\/i> right here. Run the project and get the exception:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/150\/484\/882\/1504848827b6d5a243562d910da2f201.png\" alt=\"image3.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/150\/484\/882\/1504848827b6d5a243562d910da2f201.png\"\/><\/div>\n<p>  Sure, these are just synthetic examples that demonstrate that this feature doesn&#8217;t guarantee you protection from dereferencing a null reference. If you consider synthetic examples to be boring and you&#8217;re wondering where real examples are, don&#8217;t worry \u2014 they will be further in the article.<\/p>\n<p>  NR types also have another problem \u2014 it&#8217;s not clear whether they are enabled or not. For example, the solution has two projects. One is marked up using this syntax, and the other is not. When you go to the project with NR types, you can decide that if one is marked up, then all are marked up. However, this will not be the case. It turns out that you need to check every time whether nullable context is enabled in a project or file. Otherwise, you might mistakenly assume that the normal reference type is NNR.<\/p>\n<h2>How we found proofs <\/h2>\n<p>  When developing new diagnostics in the PVS-Studio analyzer, we always test them on our database of real projects. This helps for several reasons. For example, we can: <\/p>\n<ul>\n<li>watch &#171;live&#187; the quality of received warnings;<\/li>\n<li>get rid of some false positives;<\/li>\n<li>find interesting fragments in the code which you can tell someone about;<\/li>\n<li>etc.<\/li>\n<\/ul>\n<p>  One of the new diagnostics \u2014 V3156 found places where exceptions can occur due to potential <i>null<\/i>. The diagnostic message is as follows: &#171;The argument of the method is not expected to be null&#187;. Its main point is that a <i>null<\/i> value can be passed as an argument to a method that does not expect <i>null<\/i>. This can lead, for example, to an exception or incorrect execution of the called method. You can read more about this diagnostic rule <a href=\"https:\/\/www.viva64.com\/en\/w\/v3156\/\">here<\/a>.<\/p>\n<h2>Proofs are here<\/h2>\n<p>  So here we are in the main part of this article. get ready to see real code fragments from the Roslyn project which the diagnostic issued warnings for. Their underlying idea is that either the NNR type is passed <i>null<\/i>, or there is no checking of the NR type value. All of this can result in an exception.<\/p>\n<p>  <b>Example 1<\/b><\/p>\n<pre><code class=\"cpp\">private static Dictionary&lt;object, SourceLabelSymbol> BuildLabelsByValue(ImmutableArray&lt;LabelSymbol> labels) {   ....   object key;   var constantValue = label.SwitchCaseLabelConstant;   if ((object)constantValue != null &amp;&amp; !constantValue.IsBad)   {     key = KeyForConstant(constantValue);   }   else if (labelKind == SyntaxKind.DefaultSwitchLabel)   {     key = s_defaultKey;   }   else   {     key = label.IdentifierNodeOrToken.AsNode();   }    if (!map.ContainsKey(key))                \/\/ &lt;=   {     map.Add(key, label);   }    .... }<\/code><\/pre>\n<p>  V3156 The first argument of the &#8216;ContainsKey&#8217; method is not expected to be null. Potential null value: key. SwitchBinder.cs 121<\/p>\n<p>  The message states that <i>key<\/i> is potential <i>null<\/i>. Let&#8217;s see where this variable can get this value. Let&#8217;s check the <i>KeyForConstant<\/i> method first:<\/p>\n<pre><code class=\"cpp\">protected static object KeyForConstant(ConstantValue constantValue) {   Debug.Assert((object)constantValue != null);   return constantValue.IsNull ? s_nullKey : constantValue.Value; } private static readonly object s_nullKey = new object();<\/code><\/pre>\n<p>  Since <i>s_nullKey<\/i> is not <i>null<\/i>, see what constantValue.Value returns:<\/p>\n<pre><code class=\"cpp\">public object? Value {   get   {     switch (this.Discriminator)     {       case ConstantValueTypeDiscriminator.Bad: return null;  \/\/ &lt;=       case ConstantValueTypeDiscriminator.Null: return null; \/\/ &lt;=       case ConstantValueTypeDiscriminator.SByte: return Boxes.Box(SByteValue);       case ConstantValueTypeDiscriminator.Byte: return Boxes.Box(ByteValue);       case ConstantValueTypeDiscriminator.Int16: return Boxes.Box(Int16Value);       ....       default: throw ExceptionUtilities.UnexpectedValue(this.Discriminator);     }   } }<\/code><\/pre>\n<p>  There are two null literals here, but in this case, we won&#8217;t go into any <i>case<\/i> with them. This is due to <i>IsBad<\/i> and <i>IsNull<\/i> checks. However, I would like to draw your attention to the return type of this property. It is an NR type, but the <i>KeyForConstant<\/i> method already returns the NNR type. It turns out that normally the <i>KeyForConstant<\/i> method can return <i>null<\/i>.<\/p>\n<p>  Another source that can return <i>null<\/i> is the <i>AsNode<\/i> method:<\/p>\n<pre><code class=\"cpp\">public SyntaxNode? AsNode() {   if (_token != null)   {     return null;   }    return _nodeOrParent; }<\/code><\/pre>\n<p>  Again, please note the return type of the method \u2014 it is NR. It turns out that when we say that a method can return <i>null<\/i>, it doesn&#8217;t affect anything. What&#8217;s interesting here is the fact that the compiler here does not complain about the conversion from NR to NNR:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/cb7\/7fc\/3a9\/cb77fc3a9b527b6a3ccbb3d9e923e1e5.png\" alt=\"image4.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/cb7\/7fc\/3a9\/cb77fc3a9b527b6a3ccbb3d9e923e1e5.png\"\/><\/div>\n<p>  <b>Example 2<\/b><\/p>\n<pre><code class=\"cpp\">private SyntaxNode CopyAnnotationsTo(SyntaxNode sourceTreeRoot,                                       SyntaxNode destTreeRoot) {     var nodeOrTokenMap = new Dictionary&lt;SyntaxNodeOrToken,                                        SyntaxNodeOrToken>();   ....   if (sourceTreeNodeOrTokenEnumerator.Current.IsNode)   {     var oldNode = destTreeNodeOrTokenEnumerator.Current.AsNode();     var newNode = sourceTreeNodeOrTokenEnumerator.Current.AsNode()                                        .CopyAnnotationsTo(oldNode);              nodeOrTokenMap.Add(oldNode, newNode); \/\/ &lt;=   }   .... }<\/code><\/pre>\n<p>  V3156 The first argument of the &#8216;Add&#8217; method is not expected to be null. Potential null value: oldNode. SyntaxAnnotationTests.cs 439<\/p>\n<p>  Another example with the <i>AsNode<\/i> function, which was described above. Only this time <i>oldNode<\/i> will have the NR type. While the <i>key<\/i> described above had the NNR type.<\/p>\n<p>  By the way, I can&#8217;t help but share an interesting finding with you. As I described above, when developing diagnostics, we check them on different projects. When checking the warnings of this rule, I noticed a curious thing. About 70% of all warnings were issued for methods of the <i>Dictionary<\/i> class. In which most of them fell on the <i>TryGetValue<\/i> method. This may be because we subconsciously do not expect exceptions from a method that contains the word <i>try<\/i>. So, check your code for this pattern, you might find something similar.<\/p>\n<p>  <b>Example 3<\/b><\/p>\n<pre><code class=\"cpp\">private static SymbolTreeInfo TryReadSymbolTreeInfo(     ObjectReader reader,     Checksum checksum,     Func&lt;string, ImmutableArray&lt;Node>,      Task&lt;SpellChecker>> createSpellCheckerTask) {   ....   var typeName = reader.ReadString();   var valueCount = reader.ReadInt32();    for (var j = 0; j &lt; valueCount; j++)   {     var containerName = reader.ReadString();     var name = reader.ReadString();      simpleTypeNameToExtensionMethodMap.Add(typeName, \/\/ &lt;=                             new ExtensionMethodInfo(containerName, name));    }   .... }<\/code><\/pre>\n<p>  V3156 The first argument of the &#8216;Add&#8217; method is passed as an argument to the &#8216;TryGetValue&#8217; method and is not expected to be null. Potential null value: typeName. SymbolTreeInfo_Serialization.cs 255<\/p>\n<p>  The analyzer says that the problem is in <i>typeName<\/i>. Let&#8217;s first make sure that this argument is indeed a potential <i>null<\/i>. Now look at <i>ReadString<\/i>:<\/p>\n<pre><code class=\"cpp\">public string ReadString() => ReadStringValue();<\/code><\/pre>\n<p>  <i>Ok, check out ReadStringValue<\/i>:<\/p>\n<pre><code class=\"cpp\"> private string ReadStringValue() {   var kind = (EncodingKind)_reader.ReadByte();   return kind == EncodingKind.Null ? null : ReadStringValue(kind); }<\/code><\/pre>\n<p>  Great, now let&#8217;s recall where our variable was passed to:<\/p>\n<pre><code class=\"cpp\">simpleTypeNameToExtensionMethodMap.Add(typeName, \/\/ &lt;=                               new ExtensionMethodInfo(containerName,                                                       name));<\/code><\/pre>\n<p>  I think it&#8217;s high time we took a peek inside the <i>Add<\/i> method:<\/p>\n<pre><code class=\"cpp\">public bool Add(K k, V v) {   ValueSet updated;    if (_dictionary.TryGetValue(k, out ValueSet set)) \/\/ &lt;=   {     ....   }   .... }<\/code><\/pre>\n<p>  Indeed, if we pass <i>null<\/i> as the first argument to the <i>Add<\/i> method, we will get the <i>ArgumentNullException<\/i>. <\/p>\n<p>  By the way, here&#8217;s what&#8217;s interesting \u2014 what if we hover the cursor over <i>typeName<\/i> in <i>Visual Studio<\/i>, will we see that its type is <i>string?<\/i>:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/32a\/99b\/3d3\/32a99b3d3637912932c4618ffdac614f.png\" alt=\"image5.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/32a\/99b\/3d3\/32a99b3d3637912932c4618ffdac614f.png\"\/><\/div>\n<p>  The return type of the method is simply <i>string<\/i>:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/310\/495\/2b8\/3104952b85859f80edc50aa929d1dd7b.png\" alt=\"image6.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/310\/495\/2b8\/3104952b85859f80edc50aa929d1dd7b.png\"\/><\/div>\n<p>  In addition, if we create an NNR variable and assign it <i>typeName<\/i>, no error will be output.<\/p>\n<h2>Let&#8217;s crash Roslyn<\/h2>\n<p>  Doing this not out of spite, but for fun, I suggest trying to reproduce one of the examples shown.<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/588\/bf2\/b67\/588bf2b675f69b9e7146cf5f3f6c57d8.png\" alt=\"image7.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/588\/bf2\/b67\/588bf2b675f69b9e7146cf5f3f6c57d8.png\"\/><\/div>\n<p>  <b>Test 1<\/b><\/p>\n<p>  Let&#8217;s take the example described under number 3:<\/p>\n<pre><code class=\"cpp\">private static SymbolTreeInfo TryReadSymbolTreeInfo(     ObjectReader reader,     Checksum checksum,     Func&lt;string, ImmutableArray&lt;Node>,      Task&lt;SpellChecker>> createSpellCheckerTask) {   ....   var typeName = reader.ReadString();   var valueCount = reader.ReadInt32();    for (var j = 0; j &lt; valueCount; j++)   {     var containerName = reader.ReadString();     var name = reader.ReadString();      simpleTypeNameToExtensionMethodMap.Add(typeName, \/\/ &lt;=                             new ExtensionMethodInfo(containerName, name));    }   .... }<\/code><\/pre>\n<p>  To reproduce it, we will need to call the <i>TryReadSymbolTreeInfo<\/i> method, but it is <i>private<\/i>. The good thing is that the <a href=\"https:\/\/github.com\/dotnet\/roslyn\/blob\/18ede13943b0bfae1b44ef078b2f3923159bcd32\/src\/Workspaces\/Core\/Portable\/FindSymbols\/SymbolTree\/SymbolTreeInfo_Serialization.cs\">class<\/a> with it has the <i>ReadSymbolTreeInfo_ForTestingPurposesOnly<\/i> method, which is already <i>internal<\/i>:<\/p>\n<pre><code class=\"cpp\">internal static SymbolTreeInfo ReadSymbolTreeInfo_ForTestingPurposesOnly(     ObjectReader reader,      Checksum checksum) {   return TryReadSymbolTreeInfo(reader, checksum,           (names, nodes) => Task.FromResult(             new SpellChecker(checksum,                               nodes.Select(n => new StringSlice(names,                                                                 n.NameSpan))))); }<\/code><\/pre>\n<p>  It is very nice that we are simply offered to test the <i>TryReadSymbolTreeInfo<\/i> method. So, let&#8217;s create our own class right here and write the following code:<\/p>\n<pre><code class=\"cpp\">public class CheckNNR {   public static void Start()   {     using var stream = new MemoryStream();     using var writer = new BinaryWriter(stream);     writer.Write((byte)170);     writer.Write((byte)9);     writer.Write((byte)0);     writer.Write(0);     writer.Write(0);     writer.Write(1);     writer.Write((byte)0);     writer.Write(1);     writer.Write((byte)0);     writer.Write((byte)0);     stream.Position = 0;      using var reader = ObjectReader.TryGetReader(stream);     var checksum = Checksum.Create(\"val\");      SymbolTreeInfo.ReadSymbolTreeInfo_ForTestingPurposesOnly(reader, checksum);   } }<\/code><\/pre>\n<p>  Now we build <i>Roslyn<\/i>, create a simple console application, include all the necessary dll files, and write this code:<\/p>\n<pre><code class=\"cpp\">static void Main(string[] args) {   CheckNNR.Start(); }<\/code><\/pre>\n<p>  Run, reach the desired point and see:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/885\/057\/962\/8850579624fcc5b0c3605f39b72d8192.png\" alt=\"image8.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/885\/057\/962\/8850579624fcc5b0c3605f39b72d8192.png\"\/><\/div>\n<p>  Next, go to the <i>Add<\/i> method and get the expected exception:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/833\/b27\/774\/833b2777413cd75a1f650ad62d542f75.png\" alt=\"image9.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/833\/b27\/774\/833b2777413cd75a1f650ad62d542f75.png\"\/><\/div>\n<p>  Let me remind you that the <i>ReadString<\/i> method returns an NNR type that cannot contain <i>null<\/i> as intended. This example once again confirms the relevance of the PVS-Studio diagnostic rules for searching for dereferencing null links.<\/p>\n<p>  <b>Test 2<\/b><\/p>\n<p>  Well, since we have already started reproducing examples, why not reproduce another one. This example will not relate to NR types. However, the same V3156 diagnostic found it, and I wanted to tell you about it. Here&#8217;s the code:<\/p>\n<pre><code class=\"cpp\">public SyntaxToken GenerateUniqueName(SemanticModel semanticModel,                                        SyntaxNode location,                                        SyntaxNode containerOpt,                                        string baseName,                                        CancellationToken cancellationToken) {   return GenerateUniqueName(semanticModel,                              location,                              containerOpt,                              baseName,                              filter: null,                              usedNames: null,    \/\/ &lt;=                             cancellationToken); }<\/code><\/pre>\n<p>  V3156 The sixth argument of the &#8216;GenerateUniqueName&#8217; method is passed as an argument to the &#8216;Concat&#8217; method and is not expected to be null. Potential null value: null. AbstractSemanticFactsService.cs 24<\/p>\n<p>  I&#8217;ll be honest: when making this diagnostic, I didn&#8217;t really expect triggering warnings for simple <i>null<\/i>. After all, it is quite strange to pass <i>null<\/i> to a method that throws an exception because of it. Although, I have seen places where this was justified (for example, with the <i>Expression<\/i> class), but that&#8217;s not the point now. <\/p>\n<p>  So, I was very intrigued when I saw this warning. Let&#8217;s see what is happening in the <i>GenerateUniqueName<\/i> method.<\/p>\n<pre><code class=\"cpp\">public SyntaxToken GenerateUniqueName(SemanticModel semanticModel,                                       SyntaxNode location,                                        SyntaxNode containerOpt,                                       string baseName,                                        Func&lt;ISymbol, bool> filter,                                       IEnumerable&lt;string> usedNames,                                        CancellationToken cancellationToken) {   var container = containerOpt ?? location                        .AncestorsAndSelf()                        .FirstOrDefault(a => SyntaxFacts.IsExecutableBlock(a)                                           || SyntaxFacts.IsMethodBody(a));    var candidates = GetCollidableSymbols(semanticModel,                                          location,                                          container,                                          cancellationToken);    var filteredCandidates = filter != null ? candidates.Where(filter)                                            : candidates;    return GenerateUniqueName(baseName,                              filteredCandidates.Select(s => s.Name)                                               .Concat(usedNames));     \/\/ &lt;= }<\/code><\/pre>\n<p>  As we can see, there is only one exit point in the method, no exceptions are thrown and there is no <i>goto<\/i>. In other words, nothing prevents us from passing <i>usedNames<\/i> to the <i>Concat<\/i> method and getting the <i>ArgumentNullException<\/i>. <\/p>\n<p>  But talk is cheap, so let&#8217;s just do it. First, we have to find out where we can call this method from. The method itself is in the <i><a href=\"https:\/\/github.com\/dotnet\/roslyn\/blob\/18ede13943b0bfae1b44ef078b2f3923159bcd32\/src\/Workspaces\/SharedUtilitiesAndExtensions\/Workspace\/Core\/LanguageServices\/SemanticsFactsService\/AbstractSemanticFactsService.cs\">AbstractSemanticFactsService<\/a><\/i> class. The class is abstract, so for convenience, let&#8217;s take the <i><a href=\"https:\/\/github.com\/dotnet\/roslyn\/blob\/18ede13943b0bfae1b44ef078b2f3923159bcd32\/src\/Workspaces\/SharedUtilitiesAndExtensions\/Workspace\/CSharp\/LanguageServices\/CSharpSemanticFactsService.cs\">CSharpSemanticFactsService<\/a><\/i> class, which is inherited from it. In the file of this class, we&#8217;ll create our own one, which will call the <i>GenerateUniqueName<\/i> method. It looks like this:<\/p>\n<pre><code class=\"cpp\">public class DropRoslyn {   private const string ProgramText =      @\"using System;     using System.Collections.Generic;     using System.Text     namespace HelloWorld     {       class Program       {         static void Main(string[] args)         {           Console.WriteLine(\"\"Hello, World!\"\");         }       }     }\";      public void Drop()   {     var tree = CSharpSyntaxTree.ParseText(ProgramText);     var instance = CSharpSemanticFactsService.Instance;     var compilation = CSharpCompilation                       .Create(\"Hello World\")                       .AddReferences(MetadataReference                                      .CreateFromFile(typeof(string)                                                      .Assembly                                                      .Location))                       .AddSyntaxTrees(tree);          var semanticModel = compilation.GetSemanticModel(tree);     var syntaxNode1 = tree.GetRoot();     var syntaxNode2 = tree.GetRoot();          var baseName = \"baseName\";     var cancellationToken = new CancellationToken();          instance.GenerateUniqueName(semanticModel,                                  syntaxNode1,                                  syntaxNode2,                                  baseName,                                  cancellationToken);   } }<\/code><\/pre>\n<p>  Now we build Roslyn, create a simple console application, include all the necessary dll files, and write this code:<\/p>\n<pre><code class=\"cpp\">class Program {   static void Main(string[] args)   {     DropRoslyn dropRoslyn = new DropRoslyn();     dropRoslyn.Drop();   } }<\/code><\/pre>\n<p>  Run the app and get the following:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/83a\/e19\/741\/83ae1974191c1732dcbd2ed44c7f2cc8.png\" alt=\"image10.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/83a\/e19\/741\/83ae1974191c1732dcbd2ed44c7f2cc8.png\"\/><\/div>\n<p>  <\/p>\n<h2>This is confusing<\/h2>\n<p>  Let&#8217;s say we agree with the nullable concept. It turns out that if we see the NR type, we assume that it may contain a potential <i>null<\/i>. However, sometimes we can stumble upon cases when the compiler tells us the opposite. Therefore, we&#8217;ll walk through several cases where the use of this concept is not intuitive.<\/p>\n<p>  <b>Case 1<\/b><\/p>\n<pre><code class=\"cpp\">internal override IEnumerable&lt;SyntaxToken>? TryGetActiveTokens(SyntaxNode node) {   ....   var bodyTokens = SyntaxUtilities                    .TryGetMethodDeclarationBody(node)                    ?.DescendantTokens();    if (node.IsKind(SyntaxKind.ConstructorDeclaration,                    out ConstructorDeclarationSyntax? ctor))   {     if (ctor.Initializer != null)     {       bodyTokens = ctor.Initializer                        .DescendantTokens()                        .Concat(bodyTokens); \/\/ &lt;=     }   }   return bodyTokens; }<\/code><\/pre>\n<p>  V3156 The first argument of the &#8216;Concat&#8217; method is not expected to be null. Potential null value: bodyTokens. CSharpEditAndContinueAnalyzer.cs 219<\/p>\n<p>  First of all, we check out why <i>bodyTokens<\/i> is a potential <i>null<\/i> and notice the <i>null conditional<\/i> statement:<\/p>\n<pre><code class=\"cpp\">var bodyTokens = SyntaxUtilities                  .TryGetMethodDeclarationBody(node)                  ?.DescendantTokens();              \/\/ &lt;=<\/code><\/pre>\n<p>  If we go inside the <i>TryGetMethodDeclarationBody<\/i> method, we will see that it can return <i>null<\/i>. However, it is relatively large, so I&#8217;m giving a <a href=\"https:\/\/github.com\/dotnet\/roslyn\/blob\/18ede13943b0bfae1b44ef078b2f3923159bcd32\/src\/Features\/CSharp\/Portable\/EditAndContinue\/SyntaxUtilities.cs\">link<\/a> for you to see it for yourself. So, it&#8217;s all clear with <i>bodyTokens<\/i>, but I&#8217;d like to point out the <i>ctor<\/i> argument:<\/p>\n<pre><code class=\"cpp\">if (node.IsKind(SyntaxKind.ConstructorDeclaration,                  out ConstructorDeclarationSyntax? ctor))<\/code><\/pre>\n<p>  As we can see, its type is set as NR. At the same time, here&#8217;s dereference in the line below:<\/p>\n<pre><code class=\"cpp\">if (ctor.Initializer != null)<\/code><\/pre>\n<p>  This combination is a bit ominous. Nonetheless, you will say that, most likely, if <i>IsKind<\/i> returns <i>true<\/i>, then <i>ctor<\/i> is definitely not <i>null<\/i>. So it is:<\/p>\n<pre><code class=\"cpp\">public static bool IsKind&lt;TNode>(     [NotNullWhen(returnValue: true)] this SyntaxNode? node, \/\/ &lt;=     SyntaxKind kind,     [NotNullWhen(returnValue: true)] out TNode? result)     \/\/ &lt;=     where TNode : SyntaxNode  {   if (node.IsKind(kind))   {     result = (TNode)node;     return true;   }    result = null;   return false; }<\/code><\/pre>\n<p>  Special attributes used here indicate at which output value the parameters will not be <i>null<\/i>. We can make sure of it by looking at the logic of the <i>IsKind<\/i> method. It turns out that the <i>ctor<\/i> type must be NNR inside the condition. The compiler is aware of it and says that <i>ctor<\/i> inside the condition will not be <i>null<\/i>. But if we want to get it ourselves, we have to go inside the <i>IsKind<\/i> method and notice the attribute there. Otherwise, it looks like dereferencing the NR variable without checking for <i>null<\/i>. We can try making this a bit more visible as follows:<\/p>\n<pre><code class=\"cpp\">if (node.IsKind(SyntaxKind.ConstructorDeclaration,                  out ConstructorDeclarationSyntax? ctor)) {     if (ctor!.Initializer != null) \/\/ &lt;=     {       ....     } }<\/code><\/pre>\n<p>  <b>Case 2<\/b><\/p>\n<pre><code class=\"cpp\">public TextSpan GetReferenceEditSpan(InlineRenameLocation location,                                       string triggerText,                                       CancellationToken cancellationToken) {   var searchName = this.RenameSymbol.Name;   if (_isRenamingAttributePrefix)   {     searchName = GetWithoutAttributeSuffix(this.RenameSymbol.Name);   }    var index = triggerText.LastIndexOf(searchName,            \/\/ &lt;=                                       StringComparison.Ordinal);   .... }<\/code><\/pre>\n<p>  V3156 The first argument of the &#8216;LastIndexOf&#8217; method is not expected to be null. Potential null value: searchName. AbstractEditorInlineRenameService.SymbolRenameInfo.cs 126<\/p>\n<p>  We are interested in the <i>searchName<\/i> variable. <i>null<\/i> can be written into it after calling the <i>GetWithoutAttributeSuffix<\/i> method, but it&#8217;s not that simple. Let&#8217;s see what happens in it:<\/p>\n<pre><code class=\"cpp\">private string GetWithoutAttributeSuffix(string value)     => value.GetWithoutAttributeSuffix(isCaseSensitive:                 _document.GetRequiredLanguageService&lt;ISyntaxFactsService>()                          .IsCaseSensitive)!;<\/code><\/pre>\n<p>  Let&#8217;s dig a bit deeper:<\/p>\n<pre><code class=\"cpp\">internal static string? GetWithoutAttributeSuffix(             this string name,             bool isCaseSensitive) {   return TryGetWithoutAttributeSuffix(name, isCaseSensitive, out var result)           ? result : null; }<\/code><\/pre>\n<p>  It turns out that the <i>TryGetWithoutAttributeSuffix<\/i> method will return either <i>result<\/i> or <i>null<\/i>. And the method returns the NR type. However, when we go back a step, we notice that the method type has suddenly changed to NNR. This is due to the hidden sign &#171;!&#187;:<\/p>\n<pre><code class=\"cpp\">_document.GetRequiredLanguageService&lt;ISyntaxFactsService>()          .IsCaseSensitive)!; \/\/ &lt;=<\/code><\/pre>\n<p>  By the way, it is quite tricky to notice it in Visual Studio:<\/p>\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/5f2\/47a\/aad\/5f247aaad74e18a31497b6c54f898365.png\" alt=\"image11.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/5f2\/47a\/aad\/5f247aaad74e18a31497b6c54f898365.png\"\/><\/div>\n<p>  By setting it, the developer tells us that the method will never return <i>null<\/i>. Although, looking at the previous examples and going into the <i>TryGetWithoutAttributeSuffix<\/i> method, I personally can&#8217;t be sure:<\/p>\n<pre><code class=\"cpp\">internal static bool TryGetWithoutAttributeSuffix(             this string name,             bool isCaseSensitive,             [NotNullWhen(returnValue: true)] out string? result) {   if (name.HasAttributeSuffix(isCaseSensitive))   {     result = name.Substring(0, name.Length - AttributeSuffix.Length);     return true;   }    result = null;   return false; }<\/code><\/pre>\n<p>  <\/p>\n<h2>Conclusion<\/h2>\n<p>  In conclusion, I would like to note that the attempt to save us from unnecessary <i>null<\/i> checks is a great idea. However, NR types are rather advisory in nature, because no one strictly forbids us to pass <i>null<\/i> to the NNR type. Therefore, the corresponding PVS-Studio rules remain relevant. For example, such as <a href=\"https:\/\/www.viva64.com\/en\/w\/v3080\/\">V3080<\/a> or <a href=\"https:\/\/www.viva64.com\/en\/w\/v3156\/\">V3156<\/a>.<\/p>\n<p>  All the best to you and thank you for your attention.<\/p><\/div>\n<\/div>\n<\/div>\n<p><!----><!----><\/div>\n<p><!----><!----><br \/> \u0441\u0441\u044b\u043b\u043a\u0430 \u043d\u0430 \u043e\u0440\u0438\u0433\u0438\u043d\u0430\u043b \u0441\u0442\u0430\u0442\u044c\u0438 <a href=\"https:\/\/habr.com\/ru\/articles\/522226\/\"> https:\/\/habr.com\/ru\/articles\/522226\/<\/a><\/p>\n","protected":false},"excerpt":{"rendered":"<div><!--[--><!--]--><\/div>\n<div id=\"post-content-body\">\n<div>\n<div class=\"article-formatted-body article-formatted-body article-formatted-body_version-1\">\n<div xmlns=\"http:\/\/www.w3.org\/1999\/xhtml\">\n<div style=\"text-align:center;\"><img decoding=\"async\" src=\"https:\/\/habrastorage.org\/r\/w1560\/getpro\/habr\/post_images\/30d\/8bd\/3c1\/30d8bd3c1040f014485bc84ba2d169c6.png\" alt=\"image1.png\" data-src=\"https:\/\/habrastorage.org\/getpro\/habr\/post_images\/30d\/8bd\/3c1\/30d8bd3c1040f014485bc84ba2d169c6.png\"\/><\/div>\n<p>  Have you ever wanted to get rid of the problem with dereferencing null references? If so, using Nullable Reference types is not your choice. Do you want to know why? This will be our topic today.  <\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[],"tags":[],"class_list":["post-404055","post","type-post","status-publish","format-standard","hentry"],"_links":{"self":[{"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=\/wp\/v2\/posts\/404055","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=404055"}],"version-history":[{"count":0,"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=\/wp\/v2\/posts\/404055\/revisions"}],"wp:attachment":[{"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=404055"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=404055"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/savepearlharbor.com\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=404055"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}