tdf#135470: Fix checks

The check should prevent error in case when the variable has
wrong type. IsObject returns false e.g. for arrays, which are
valid iterables. So proper check is using GetFullType, which
will give SbxOBJECT for any variable for which GetObject does
not set error.

"Next" should also generate an error for uninitialized loops.

Regression after 5760c94b8847164f9a7a181f031c7c86643944af

Change-Id: Ib58e1cc76ef63aa1ff8a86c9d5dc956e4780fb49
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100258
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 46 insertions, 2 deletions

diff --git a/basic/qa/basic_coverage/test_for_each.vb b/basic/qa/basic_coverage/test_for_each.vb
new file mode 100644
index 000000000000..654513e88332
--- /dev/null
+++ b/basic/qa/basic_coverage/test_for_each.vb
@@ -0,0 +1,43 @@
+'
+' This file is part of the LibreOffice project.
+'
+' This Source Code Form is subject to the terms of the Mozilla Public
+' License, v. 2.0. If a copy of the MPL was not distributed with this
+' file, You can obtain one at http://mozilla.org/MPL/2.0/.
+'
+
+Function doUnitTest as Integer
+    Dim n As Integer, i
+    Dim a(3)
+    n = 0
+    For Each i In a
+        n = n + 1
+    Next i
+    If n <> 4 Then
+        doUnitTest = "For Each over array failed"
+        Exit Function
+    End If
+
+    If TestInvalidForEachWithErrorHandler <> "13 91 14 " Then
+        doUnitTest = "For Each doesn't generate proper errors on bad arguments"
+        Exit Function
+    End If
+
+    doUnitTest = 1
+End Function
+
+Function TestInvalidForEachWithErrorHandler
+    Dim s As String
+    On Error Goto ErrHandler
+' This For Each is given a bad iterable; it must generate first error ("Data type mismatch") for b;
+    For Each a In b
+' Then proceed here (Resume Next from ErrHandler), and generate "Object variable not set" for c;
+    c.d
+' Then proceed here (Resume Next from ErrHandler), and generate "Invalid parameter" at Next.
+    Next
+    TestInvalidForEachWithErrorHandler = s
+    Exit Function
+ErrHandler:
+    s = s & Err & " "
+    Resume Next
+End Function
diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 9ac23bd2fef5..62a54b87b576 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -1144,7 +1144,7 @@ void SbiRuntime::PushForEach()
     pForStk = p;
 
     SbxVariableRef xObjVar = PopVar();
-    SbxBase* pObj = xObjVar.is() && xObjVar->IsObject() ? xObjVar->GetObject() : nullptr;
+    SbxBase* pObj = xObjVar && xObjVar->GetFullType() == SbxOBJECT ? xObjVar->GetObject() : nullptr;
 
     if (SbxDimArray* pArray = dynamic_cast<SbxDimArray*>(pObj))
     {
@@ -3118,8 +3118,9 @@ void SbiRuntime::StepTESTFOR( sal_uInt32 nOp1 )
         }
         case ForType::Error:
         {
-            // We are in Resume Next mode, and we already had one iteration
+            // We are in Resume Next mode after failed loop initialization
             bEndLoop = true;
+            Error(ERRCODE_BASIC_BAD_PARAMETER);
             break;
         }
     }