Fix crash in Condition property for VB For-To loop

This time it is caused by BontrolVariable bounf to BoundBadExpression.
Also improved the robustness in the Condition property.

src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs

@@ -1739,8 +1739,11 @@ public sealed override void Initialize(AnalysisContext context)
                          IForLoopStatement forLoop = (IForLoopStatement)loop;
                          var forCondition = forLoop.Condition;
 
-                         // Generate a warning to prove we didn't crash
-                         operationContext.ReportDiagnostic(Diagnostic.Create(ForLoopConditionCrashDescriptor, forLoop.Condition.Syntax.GetLocation()));
+                         if (forCondition.IsInvalid)
+                         {
+                             // Generate a warning to prove we didn't crash
+                             operationContext.ReportDiagnostic(Diagnostic.Create(ForLoopConditionCrashDescriptor, forLoop.Condition.Syntax.GetLocation()));
+                         }
                      }
                  },
                  OperationKind.LoopStatement);

src/Compilers/Core/Portable/Compilation/Expression.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.Linq;
 
 namespace Microsoft.CodeAnalysis.Semantics
@@ -104,7 +105,7 @@ public ConditionalChoice(IOperation condition, IOperation ifTrue, IOperation ifF
 
         public OperationKind Kind => OperationKind.ConditionalChoiceExpression;
 
-        public bool IsInvalid => Condition == null || Condition.IsInvalid || IfTrueValue == null || IfTrueValue.IsInvalid || IfFalseValue == null || IfFalseValue.IsInvalid;
+        public bool IsInvalid => Condition == null || Condition.IsInvalid || IfTrueValue == null || IfTrueValue.IsInvalid || IfFalseValue == null || IfFalseValue.IsInvalid || Type == null;
 
         public Optional<object> ConstantValue => default(Optional<object>);
 
@@ -301,6 +302,7 @@ internal class Literal : ILiteralExpression
 
         public Literal(ConstantValue value, ITypeSymbol resultType, SyntaxNode syntax)
         {
+            Debug.Assert(value != null, "value can't be null");
             _value = value;
             this.Type = resultType;
             this.Syntax = syntax;
@@ -312,7 +314,7 @@ public Literal(ConstantValue value, ITypeSymbol resultType, SyntaxNode syntax)
 
         public OperationKind Kind => OperationKind.LiteralExpression;
 
-        public bool IsInvalid => false;
+        public bool IsInvalid => _value.IsBad;
 
         public Optional<object> ConstantValue => new Optional<object>(_value.Value);
 
@@ -354,7 +356,12 @@ public Binary(BinaryOperationKind binaryOperationKind, IOperation left, IOperati
 
         public OperationKind Kind => OperationKind.BinaryOperatorExpression;
 
-        public bool IsInvalid => LeftOperand == null || LeftOperand.IsInvalid || RightOperand == null || RightOperand.IsInvalid;
+        public bool IsInvalid => LeftOperand == null 
+                                || LeftOperand.IsInvalid 
+                                || RightOperand == null
+                                || RightOperand.IsInvalid 
+                                || BinaryOperationKind == BinaryOperationKind.Invalid 
+                                || Type == null;
 
         public Optional<object> ConstantValue => default(Optional<object>);
 

src/Compilers/VisualBasic/Portable/BoundTree/Statement.vb

@@ -628,7 +628,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                         Dim limitValue As IOperation = If(BoundFor.LimitValue.IsConstant, DirectCast(BoundFor.LimitValue, IOperation), New Temporary(SyntheticLocalKind.ForLoopLimitValue, BoundFor, BoundFor.LimitValue))
                         Dim controlVariable As BoundExpression = BoundFor.ControlVariable
 
-                        Dim booleanType As ITypeSymbol = controlVariable.ExpressionSymbol.DeclaringCompilation.GetSpecialType(SpecialType.System_Boolean)
+                        ' controlVariable can be a BoundBadExpression in case of error
+                        Dim booleanType As ITypeSymbol = controlVariable.ExpressionSymbol?.DeclaringCompilation.GetSpecialType(SpecialType.System_Boolean)
 
                         Dim operators As BoundForToUserDefinedOperators = Me.OperatorsOpt
                         If operators IsNot Nothing Then
@@ -645,7 +646,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
                                 Dim stepValue As IOperation = New Temporary(SyntheticLocalKind.ForLoopStepValue, BoundFor, BoundFor.StepValue)
                                 Dim stepRelationalCode As BinaryOperationKind = DeriveBinaryOperationKind(BinaryOperatorKind.GreaterThanOrEqual, BoundFor.StepValue)
-                                Dim stepCondition As IOperation = New Binary(stepRelationalCode, stepValue, New BoundLiteral(BoundFor.StepValue.Syntax, Semantics.Expression.SynthesizeNumeric(stepValue.Type, 0), BoundFor.StepValue.Type), booleanType, BoundFor.StepValue.Syntax)
+                                Dim stepCondition As IOperation = New Binary(stepRelationalCode,
+                                                                             stepValue,
+                                                                             New Literal(Semantics.Expression.SynthesizeNumeric(stepValue.Type, 0), BoundFor.StepValue.Type, BoundFor.StepValue.Syntax),
+                                                                             booleanType,
+                                                                             BoundFor.StepValue.Syntax)
 
                                 Dim positiveStepRelationalCode As BinaryOperationKind = DeriveBinaryOperationKind(BinaryOperatorKind.LessThanOrEqual, controlVariable)
                                 Dim positiveStepCondition As IOperation = New Binary(positiveStepRelationalCode, controlVariable, limitValue, booleanType, limitValue.Syntax)

src/Compilers/VisualBasic/Test/Semantic/Diagnostics/OperationAnalyzerTests.vb

@@ -1661,6 +1661,8 @@ End Class
             Dim source = <compilation>
                              <file name="c.vb">
                                  <![CDATA[
+Imports System   
+
 Module M1
     Class C1(Of t)
         Shared Widening Operator CType(ByVal p1 As C1(Of t)) As Integer
@@ -1681,16 +1683,46 @@ Module M1
         Next
     End Sub
 End Module
+     
+Module M2
+    ReadOnly Property Moo As Integer
+        Get
+            Return 1
+        End Get
+    End Property
+
+    WriteOnly Property Boo As integer
+        Set(value As integer)
+
+        End Set
+    End Property
+
+    Sub Main()
+        For Moo = 1 to Moo step Moo
+        Next
+
+        For Boo = 1 to Boo step Boo
+        Next
+    End Sub
+End Module
 ]]>
                              </file>
                          </compilation>
 
             Dim comp = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime(source)
-            comp.VerifyDiagnostics(Diagnostic(ERRID.ERR_UnacceptableForLoopOperator2, "For i As C1(Of Integer) = 1 To 10").WithArguments("Public Shared Operator -(p1 As M1.C1(Of Integer), p2 As M1.C1(Of Integer)) As M1.C1(Of Short)", "M1.C1(Of Integer)").WithLocation(17, 9),
-                                   Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", "<=").WithLocation(17, 9),
-                                   Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", ">=").WithLocation(17, 9))
+            comp.VerifyDiagnostics(
+                Diagnostic(ERRID.ERR_LoopControlMustNotBeProperty, "Moo").WithLocation(38, 13),
+                Diagnostic(ERRID.ERR_LoopControlMustNotBeProperty, "Boo").WithLocation(41, 13),
+                Diagnostic(ERRID.ERR_NoGetProperty1, "Boo").WithArguments("Boo").WithLocation(41, 24),
+                Diagnostic(ERRID.ERR_NoGetProperty1, "Boo").WithArguments("Boo").WithLocation(41, 33),
+                Diagnostic(ERRID.ERR_UnacceptableForLoopOperator2, "For i As C1(Of Integer) = 1 To 10").WithArguments("Public Shared Operator -(p1 As M1.C1(Of Integer), p2 As M1.C1(Of Integer)) As M1.C1(Of Short)", "M1.C1(Of Integer)").WithLocation(19, 9),
+                Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", "<=").WithLocation(19, 9),
+                Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", ">=").WithLocation(19, 9),
+                Diagnostic(ERRID.HDN_UnusedImportStatement, "Imports System").WithLocation(1, 1))
             comp.VerifyAnalyzerDiagnostics({New ForLoopConditionCrashVBTestAnalyzer}, Nothing, Nothing, False,
-                                           Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "10").WithLocation(17, 40))
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "Moo").WithLocation(38, 24),
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "Boo").WithLocation(41, 24),
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "10").WithLocation(19, 40))
         End Sub
     End Class
 End Namespace