Fix increment and decrement syntax

src/CBot/CBotInstr/CBotExprVar.cpp

@@ -44,7 +44,7 @@ CBotExprVar::~CBotExprVar()
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::ProtectionLevel privat)
+CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, bool bCheckReadOnly)
 {
 //    CBotToken*    pDebut = p;
     CBotCStack* pStk = pStack->TokenStack();
@@ -67,7 +67,7 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot
 
             if (ident > 0 && ident < 9000)
             {
-                if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, privat))
+                if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, bCheckReadOnly))
                 {
                     pStk->SetError(CBotErrPrivate, p);
                     goto err;
@@ -122,6 +122,8 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot
                         {
                             if (p->GetNext()->GetType() == ID_OPENPAR)  // a method call?
                             {
+                                if (bCheckReadOnly) goto err; // don't allow increment a method call "++"
+
                                 CBotInstr* i = CBotInstrMethode::Compile(p, pStk, var);
                                 if (!pStk->IsOk()) goto err;
                                 inst->AddNext3(i);  // added after
@@ -137,7 +139,7 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot
                                 if (var != nullptr)
                                 {
                                     i->SetUniqNum(var->GetUniqNum());
-                                    if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, privat))
+                                    if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, bCheckReadOnly))
                                     {
                                         pStk->SetError(CBotErrPrivate, pp);
                                         goto err;

src/CBot/CBotInstr/CBotExprVar.h

@@ -40,14 +40,13 @@ public:
     ~CBotExprVar();
 
     /*!
-     * \brief Compile
+     * \brief Compile an expression of a variable, possibly chained with index operators and/or dot operators
-     * \param p
+     * \param p[in, out] Pointer to first token of the expression, will be updated to point to first token after the expression
-     * \param pStack
+     * \param pStack Current compilation stack frame
-     * \param privat
+     * \param bCheckReadOnly True for operations that would modify the value of the variable
      * \return
      */
-    static CBotInstr* Compile(CBotToken*& p, CBotCStack* pStack,
+    static CBotInstr* Compile(CBotToken*& p, CBotCStack* pStack, bool bCheckReadOnly = false);
-                              CBotVar::ProtectionLevel privat = CBotVar::ProtectionLevel::Protected);
 
     /*!
      * \brief CompileMethode

src/CBot/CBotInstr/CBotFieldExpr.cpp

@@ -135,12 +135,11 @@ std::string CBotFieldExpr::GetDebugData()
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-bool CBotFieldExpr::CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar,
+bool CBotFieldExpr::CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, bool bCheckReadOnly)
-                                         CBotVar::ProtectionLevel privat)
 {
     CBotVar::ProtectionLevel varPriv = pVar->GetPrivate();
 
-    if (privat == CBotVar::ProtectionLevel::ReadOnly && varPriv == privat)
+    if (bCheckReadOnly && varPriv == CBotVar::ProtectionLevel::ReadOnly)
         return true;
 
     if (varPriv == CBotVar::ProtectionLevel::Public) return false;

src/CBot/CBotInstr/CBotFieldExpr.h

@@ -72,13 +72,12 @@ public:
      * This function doesn't set the error flag itself.
      *
      * \param pStack Current compilation stack frame
-     * \param pPrev Class instance which variable to check is part of, or nullptr if not part of a class
+     * \param pPrev Class instance which variable to check is part of, or nullptr when compiler inserts 'this.' before
      * \param pVar Variable to check
-     * \param privat CBotVar::ProtectionLevel::ReadOnly if requesting read-only access, anything else otherwise
+     * \param bCheckReadOnly True for operations that would modify the value of the variable
      * \return true if pVar is inaccessible in the current context, false if access should be allowed
      */
-    static bool CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar,
+    static bool CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, bool bCheckReadOnly = false);
-                                     CBotVar::ProtectionLevel privat = CBotVar::ProtectionLevel::Protected);
 
 protected:
     virtual const std::string GetDebugName() override { return "CBotFieldExpr"; }

src/CBot/CBotInstr/CBotLeftExpr.cpp

@@ -64,7 +64,7 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack)
             inst->m_nIdent = var->GetUniqNum();
             if (inst->m_nIdent > 0 && inst->m_nIdent < 9000)
             {
-                if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, CBotVar::ProtectionLevel::ReadOnly))
+                if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, true))
                 {
                     pStk->SetError(CBotErrPrivate, p);
                     goto err;
@@ -128,8 +128,7 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack)
                             var = var->GetItem(p->GetString());            // get item correspondent
                             if (var != nullptr)
                             {
-                                if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var,
+                                if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, true))
-                                                                        CBotVar::ProtectionLevel::ReadOnly))
                                 {
                                     pStk->SetError(CBotErrPrivate, pp);
                                     goto err;

src/CBot/CBotInstr/CBotParExpr.cpp

@@ -90,17 +90,16 @@ CBotInstr* CBotParExpr::Compile(CBotToken* &p, CBotCStack* pStack)
         // post incremented or decremented?
         if (IsOfType(p, ID_INC, ID_DEC))
         {
+            // recompile the variable for read-only
+            delete inst;
+            p = pvar;
+            inst = CBotExprVar::Compile(p, pStk, true);
             if (pStk->GetType() >= CBotTypBoolean)
             {
                 pStk->SetError(CBotErrBadType1, pp);
                 delete inst;
                 return pStack->Return(nullptr, pStk);
             }
-
-            // recompile the variable for read-only
-            delete inst;
-            p = pvar;
-            inst =  CBotExprVar::Compile(p, pStk, CBotVar::ProtectionLevel::ReadOnly);
             p = p->GetNext();
 
             CBotPostIncExpr* i = new CBotPostIncExpr();
@@ -115,24 +114,22 @@ CBotInstr* CBotParExpr::Compile(CBotToken* &p, CBotCStack* pStack)
     CBotToken* pp = p;
     if (IsOfType(p, ID_INC, ID_DEC))
     {
-        CBotPreIncExpr* i = new CBotPreIncExpr();
-        i->SetToken(pp);
-
         if (p->GetType() == TokenTypVar)
         {
-            if (nullptr != (i->m_instr =  CBotExprVar::Compile(p, pStk, CBotVar::ProtectionLevel::ReadOnly)))
+            if (nullptr != (inst = CBotExprVar::Compile(p, pStk, true)))
             {
-                if (pStk->GetType() >= CBotTypBoolean)
+                if (pStk->GetType() < CBotTypBoolean) // a number ?
                 {
-                    pStk->SetError(CBotErrBadType1, pp);
+                    CBotPreIncExpr* i = new CBotPreIncExpr();
-                    delete inst;
+                    i->SetToken(pp);
-                    return pStack->Return(nullptr, pStk);
+                    i->m_instr = inst;
+                    return pStack->Return(i, pStk);
                 }
-                return pStack->Return(i, pStk);
+                delete inst;
             }
-            delete i;
-            return pStack->Return(nullptr, pStk);
         }
+        pStk->SetError(CBotErrBadType1, pp);
+        return pStack->Return(nullptr, pStk);
     }
 
     // is it a number or DefineNum?

test/unit/CBot/CBot_test.cpp

@@ -2073,3 +2073,56 @@ TEST_F(CBotUT, ClassTestPrivateMember)
         CBotErrPrivate
     );
 }
+
+TEST_F(CBotUT, IncrementDecrementSyntax)
+{
+    auto publicProgram = ExecuteTest(
+        "public class TestClass {\n"
+        "    int GetInt() { return 1; }\n"
+        "}\n"
+        "extern void TestIncrementDecrement()\n"
+        "{\n"
+        "    int i = 1;\n"
+        "    ASSERT(2 == ++i);\n"
+        "    ASSERT(2 == i++);\n"
+        "    ASSERT(3 ==  i );\n"
+        "    ASSERT(2 == --i);\n"
+        "    ASSERT(2 == i--);\n"
+        "    ASSERT(1 ==  i );\n"
+        "}\n"
+    );
+
+    ExecuteTest(
+        "extern void PreIncrementMethodCall()\n"
+        "{\n"
+        "    TestClass tc();\n"
+        "    ++tc.GetInt();\n"
+        "}\n",
+        CBotErrBadType1
+    );
+
+    ExecuteTest(
+        "extern void PostIncrementMethodCall()\n"
+        "{\n"
+        "    TestClass tc();\n"
+        "    tc.GetInt()++;\n"
+        "}\n",
+        CBotErrBadType1
+    );
+
+    ExecuteTest(
+        "extern void BadPreIncrementEmpty()\n"
+        "{\n"
+        "    ++;\n"
+        "}\n",
+        CBotErrBadType1
+    );
+
+    ExecuteTest(
+        "extern void BadPreIncrementNotAVar()\n"
+        "{\n"
+        "    ++123;\n"
+        "}\n",
+        CBotErrBadType1
+    );
+}