Analyze throw expression and adjust analysis of null-coalescing operator (#32991)

src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

@@ -15574,6 +15574,24 @@ internal class CSharpResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Possible null value..
+        /// </summary>
+        internal static string WRN_PossibleNull {
+            get {
+                return ResourceManager.GetString("WRN_PossibleNull", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Possible null value..
+        /// </summary>
+        internal static string WRN_PossibleNull_Title {
+            get {
+                return ResourceManager.GetString("WRN_PossibleNull_Title", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to &apos;{0}&apos;: new protected member declared in sealed class.
         /// </summary>

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -5319,6 +5319,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="WRN_NullReferenceArgument_Title" xml:space="preserve">
     <value>Possible null reference argument.</value>
   </data>
+  <data name="WRN_PossibleNull" xml:space="preserve">
+    <value>Possible null value.</value>
+  </data>
+  <data name="WRN_PossibleNull_Title" xml:space="preserve">
+    <value>Possible null value.</value>
+  </data>
   <data name="HDN_NullCheckIsProbablyAlwaysTrue" xml:space="preserve">
     <value>Result of the comparison is possibly always true.</value>
   </data>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1625,6 +1625,7 @@ internal enum ErrorCode
         ERR_DiscardPatternInSwitchStatement = 8523,
         #endregion diagnostics introduced for recursive patterns
 
+        WRN_PossibleNull = 8597,
         ERR_IllegalSuppression = 8598,
         WRN_IllegalPPWarningSafeOnly = 8599,
         WRN_ConvertingNullableToNonNullable = 8600,

src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs

@@ -34,6 +34,7 @@ static ErrorFacts()
             builder.Add(getId(ErrorCode.WRN_NullableValueTypeMayBeNull));
             builder.Add(getId(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint));
             builder.Add(getId(ErrorCode.WRN_NullabilityMismatchInTypeParameterReferenceTypeConstraint));
+            builder.Add(getId(ErrorCode.WRN_PossibleNull));
 
             NullableFlowAnalysisSafetyWarnings = builder.ToImmutable();
 
@@ -395,6 +396,7 @@ internal static int GetWarningLevel(ErrorCode code)
                 case ErrorCode.WRN_GivenExpressionNeverMatchesPattern:
                 case ErrorCode.WRN_GivenExpressionAlwaysMatchesConstant:
                 case ErrorCode.WRN_CaseConstantNamedUnderscore:
+                case ErrorCode.WRN_PossibleNull:
                     return 1;
                 default:
                     return 0;

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -1844,7 +1844,7 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder<i
 
                             // We need to continue the walk regardless of whether the receiver should be updated.
                             var receiverType = conditional.Receiver.Type;
-                            if (shouldUpdateType(receiverType))
+                            if (PossiblyNullableType(receiverType))
                             {
                                 slotBuilder.Add(slot);
                             }
@@ -1878,7 +1878,7 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder<i
                         // we need more special handling here
 
                         slot = MakeSlot(operand);
-                        if (slot > 0 && shouldUpdateType(operand.Type))
+                        if (slot > 0 && PossiblyNullableType(operand.Type))
                         {
                             // If we got a slot then all previous BoundCondtionalReceivers must have been handled.
                             Debug.Assert(_lastConditionalAccessSlot == -1);
@@ -1895,11 +1895,11 @@ private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder<i
 
                 return;
             }
-
-            bool shouldUpdateType(TypeSymbol operandType)
-                => !(operandType is null) && (!operandType.IsValueType || operandType.IsNullableType());
         }
 
+        private static bool PossiblyNullableType(TypeSymbol operandType)
+            => !(operandType is null) && (!operandType.IsValueType || operandType.IsNullableType());
+
         private static void MarkSlotsAsNotNullable(ArrayBuilder<int> slots, ref LocalState stateToUpdate)
         {
             if (slots is null)
@@ -1913,6 +1913,14 @@ private static void MarkSlotsAsNotNullable(ArrayBuilder<int> slots, ref LocalSta
             }
         }
 
+        private void LearnFromNonNullTest(BoundExpression expression, ref LocalState state)
+        {
+            var slotBuilder = ArrayBuilder<int>.GetInstance();
+            GetSlotsToMarkAsNotNullable(expression, slotBuilder);
+            MarkSlotsAsNotNullable(slotBuilder, ref state);
+            slotBuilder.Free();
+        }
+
         private static BoundExpression SkipReferenceConversions(BoundExpression possiblyConversion)
         {
             while (possiblyConversion.Kind == BoundKind.Conversion)
@@ -1934,7 +1942,6 @@ private static BoundExpression SkipReferenceConversions(BoundExpression possibly
 
         public override BoundNode VisitNullCoalescingAssignmentOperator(BoundNullCoalescingAssignmentOperator node)
         {
-
             BoundExpression leftOperand = node.LeftOperand;
             BoundExpression rightOperand = node.RightOperand;
             int leftSlot = MakeSlot(leftOperand);
@@ -1986,7 +1993,12 @@ public override BoundNode VisitNullCoalescingOperator(BoundNullCoalescingOperato
                 return null;
             }
 
-            var leftState = this.State.Clone();
+            var whenNotNull = this.State.Clone();
+            LearnFromNonNullTest(leftOperand, ref whenNotNull);
+
+            // Consider learning in whenNull branch as well
+            // https://github.com/dotnet/roslyn/issues/30297
+
             if (leftResult.ValueCanBeNull() == false)
             {
                 ReportNonSafetyDiagnostic(ErrorCode.HDN_ExpressionIsProbablyNeverNull, leftOperand.Syntax);
@@ -2001,7 +2013,7 @@ public override BoundNode VisitNullCoalescingOperator(BoundNullCoalescingOperato
             // https://github.com/dotnet/roslyn/issues/29955 For cases where the left operand determines
             // the type, we should unwrap the right conversion and re-apply.
             rightResult = VisitRvalueWithResult(rightOperand);
-            Join(ref this.State, ref leftState);
+            Join(ref this.State, ref whenNotNull);
             TypeSymbol resultType;
             var leftResultType = leftResult.TypeSymbol;
             var rightResultType = rightResult.TypeSymbol;
@@ -2778,7 +2790,7 @@ private void VisitArgumentEvaluate(ImmutableArray<BoundExpression> arguments, Im
 
                 var argument = arguments[i];
                 var argumentType = argument.Type;
-                if ((object)argumentType == null || (argumentType.IsValueType && !argumentType.IsNullableType()))
+                if (!PossiblyNullableType(argumentType))
                 {
                     continue;
                 }
@@ -4726,7 +4738,7 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt
                 {
                     // We are supposed to track information for the node. Use whatever we managed to
                     // accumulate so far.
-                    if (!resultType.IsValueType || resultType.IsNullableType())
+                    if (PossiblyNullableType(resultType.TypeSymbol))
                     {
                         int slot = MakeMemberSlot(receiverOpt, member);
                         if (slot > 0 && slot < this.State.Capacity)
@@ -5113,7 +5125,7 @@ public override BoundNode VisitAsOperator(BoundAsOperator node)
                 NullableAnnotation nullableAnnotation = NullableAnnotation.Unknown;
                 var type = node.Type;
 
-                if (!type.IsValueType || type.IsNullableType())
+                if (PossiblyNullableType(type))
                 {
                     var operandType = _resultType;
                     switch (node.Conversion.Kind)
@@ -5445,10 +5457,7 @@ private void CheckPossibleNullReceiver(BoundExpression receiverOpt, bool checkNu
                     ReportSafetyDiagnostic(isValueType ? ErrorCode.WRN_NullableValueTypeMayBeNull : ErrorCode.WRN_NullReferenceReceiver, syntaxOpt ?? receiverOpt.Syntax);
                 }
 
-                var slotBuilder = ArrayBuilder<int>.GetInstance();
-                GetSlotsToMarkAsNotNullable(receiverOpt, slotBuilder);
-                MarkSlotsAsNotNullable(slotBuilder, ref State);
-                slotBuilder.Free();
+                LearnFromNonNullTest(receiverOpt, ref this.State);
             }
         }
 
@@ -5556,9 +5565,36 @@ public override BoundNode VisitDiscardExpression(BoundDiscardExpression node)
 
         public override BoundNode VisitThrowExpression(BoundThrowExpression node)
         {
-            var result = base.VisitThrowExpression(node);
-            SetUnknownResultNullability();
-            return result;
+            VisitThrow(node.Expression);
+            _resultType = default;
+            return null;
+        }
+
+        public override BoundNode VisitThrowStatement(BoundThrowStatement node)
+        {
+            VisitThrow(node.ExpressionOpt);
+            return null;
+        }
+
+        private void VisitThrow(BoundExpression expr)
+        {
+            if (expr != null)
+            {
+                var result = VisitRvalueWithResult(expr);
+                // Cases:
+                // null
+                // null!
+                // Other (typed) expression, including suppressed ones
+                if (expr.ConstantValue?.IsNull == true ||
+                    result.GetValueNullableAnnotation().IsAnyNullable())
+                {
+                    if (!expr.IsSuppressed)
+                    {
+                        ReportSafetyDiagnostic(ErrorCode.WRN_PossibleNull, expr.Syntax);
+                    }
+                }
+            }
+            SetUnreachable();
         }
 
         public override BoundNode VisitYieldReturnStatement(BoundYieldReturnStatement node)

src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs

@@ -186,6 +186,7 @@ public static bool IsWarning(ErrorCode code)
                 case ErrorCode.WRN_IsTypeNamedUnderscore:
                 case ErrorCode.WRN_GivenExpressionNeverMatchesPattern:
                 case ErrorCode.WRN_GivenExpressionAlwaysMatchesConstant:
+                case ErrorCode.WRN_PossibleNull:
                 case ErrorCode.WRN_IllegalPPWarningSafeOnly:
                 case ErrorCode.WRN_ConvertingNullableToNonNullable:
                 case ErrorCode.WRN_NullReferenceAssignment:

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs

@@ -91,6 +91,11 @@ public static bool IsNonNullableValueType(this TypeSymbol typeArgument)
 
         public static bool IsNullableTypeOrTypeParameter(this TypeSymbol type)
         {
+            if (type is null)
+            {
+                return false;
+            }
+
             if (type.TypeKind == TypeKind.TypeParameter)
             {
                 var constraintTypes = ((TypeParameterSymbol)type).ConstraintTypesNoUseSiteDiagnostics;

src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Typ hodnoty, která připouští hodnotu null, nemůže být null.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Výraz switch nezpracovává všechny možné vstupy (není vyčerpávající).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Ein Werttyp, der NULL zulässt, kann NULL sein.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Der switch-Ausdruck verarbeitet nicht alle möglichen Eingaben (nicht umfassend).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Un tipo que acepta valores NULL puede ser nulo.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">La expresión switch no controla todas las entradas posibles (no es exhaustiva).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Le type valeur Nullable peut avoir une valeur null.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">L'expression switch ne prend pas en charge toutes les entrées possibles (elle n'est pas exhaustive).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Il tipo valore nullable non può essere Null.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">L'espressione switch non gestisce tutti gli input possibili (non è esaustiva).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Null 許容値型は Null になる場合があります。</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">switch 式がすべての可能な入力を処理しません (すべてを網羅していません)。</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Nullable 값 형식이 null일 수 있습니다.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">switch 식은 가능한 입력을 모두 처리하지는 않습니다(전체 아님).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Typ wartości dopuszczający wartość null może być równy null.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Wyrażenie switch nie obsługuje wszystkich możliwych danych wejściowych (nie jest kompletne).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">O tipo de valor de nulidade pode ser nulo.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">A expressão switch não manipula todas as entradas possíveis (não é finita).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Тип значения, допускающего NULL, может быть NULL.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Выражение switch обрабатывает не все возможные входные данные (оно не полное).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">Boş değer atanabilir değer türü null olabilir.</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Switch ifadesi tüm olası girişleri işlemiyor (tam kapsamlı değil).</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf

@@ -1237,6 +1237,16 @@
         <target state="translated">可为 null 的值类型可为 null。</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">Switch 表达式不会处理所有可能的输入（它并非详尽无遗）。</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf

@@ -1532,6 +1532,16 @@
         <target state="translated">可為 Null 的實值型別可為 Null。</target>
         <note />
       </trans-unit>
+      <trans-unit id="WRN_PossibleNull">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="WRN_PossibleNull_Title">
+        <source>Possible null value.</source>
+        <target state="new">Possible null value.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="WRN_SwitchExpressionNotExhaustive">
         <source>The switch expression does not handle all possible inputs (it is not exhaustive).</source>
         <target state="translated">switch 運算式未處理所有可能的輸入 (其並不徹底)。</target>

src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs

@@ -295,6 +295,7 @@ public void WarningLevel_2()
                         case ErrorCode.WRN_NullabilityMismatchInTypeParameterReferenceTypeConstraint:
                         case ErrorCode.WRN_CaseConstantNamedUnderscore:
                         case ErrorCode.ERR_FeatureInPreview:
+                        case ErrorCode.WRN_PossibleNull:
                             Assert.Equal(1, ErrorFacts.GetWarningLevel(errorCode));
                             break;
                         case ErrorCode.WRN_InvalidVersionFormat: