Resolves: tdf#96426 significant whitespace as intersection in Excel syntax

Also when reading/writing OOXML, so change SC_OPCODE_INTERSECT of
RID_STRLIST_FUNCTION_NAMES_ENGLISH_OOXML accordingly to " ", where
previously "!" was expected and written, which was plain wrong.

(cherry picked from commit 26adceb098134d918f6d57c8687ab057e24adc39)

simplify the ReplaceToken() offset logic to absolute offsets

(cherry picked from commit f41257dc9913cd6020a3a37bf425c20b51e18ece)

fully check for adjacent RPN end, tdf#96426 follow-up

(cherry picked from commit b0992e11905e36a64edeb92a13acfde5837c1878)

narrow down where a space could be an intersection, tdf#96426 follow-up

(cherry picked from commit e0875f8e348a3aca036bc0cc629fb038fabc8062)
Adapted to backport.

more differentiated significant whitespace recognition, tdf#96426 follow-up

(cherry picked from commit 0f8a8332a52cd03b43aaab86e0c232e0964d7111)

first range can be anywhere before second at RPN end, tdf#96426 follow-up

... not just adjacent to the one at the end. So we actually can handle
INDIRECT("A2:C2") INDIRECT("B1:B3")

(cherry picked from commit 0c5663cfb13f4f55e246d42ac464d5e2c2f23099)

check presence of token, tdf#96426 follow-up

(cherry picked from commit edd4370f5ba49a26a526995b6a28f623d68041ce)

4c368dfd113b02d208013b4ba79dff606769a150
8d02fb63bc0c5cb48aabaf7a8800f5f9ac95cbf5
886e559c6f6041bf4889fdd6d89c12a10be70e5f
c53a4a0d19a11298895efb28e2786e48a071e72b
081409a82a9ff64f163115bf4597afbb9b2f5fa6
e8030ebc13bb1ae2246611f5722da97970b8c544

Change-Id: Ic0cfd7afc657f07bfd8e37de61b3621cc68685ff
Reviewed-on: https://gerrit.libreoffice.org/24374
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

7 files changed, 187 insertions, 23 deletions

diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index cf44e924d98c..367387faa294 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -266,6 +266,99 @@ struct OpCodeMapData
 };
 
 
+bool isPotentialRangeLeftOp( OpCode eOp )
+{
+    switch (eOp)
+    {
+        case ocClose:
+            return true;
+        default:
+            return false;
+    }
+}
+
+bool isRangeResultFunction( OpCode eOp )
+{
+    switch (eOp)
+    {
+        case ocIndirect:
+        case ocOffset:
+            return true;
+        default:
+            return false;
+    }
+}
+
+bool isRangeResultOpCode( OpCode eOp )
+{
+    switch (eOp)
+    {
+        case ocRange:
+        case ocUnion:
+        case ocIntersect:
+        case ocIndirect:
+        case ocOffset:
+            return true;
+        default:
+            return false;
+    }
+}
+
+/**
+    @param  pToken
+            MUST be a valid token, caller has to ensure.
+
+    @param  bRight
+            If bRPN==false, bRight==false means opcodes for left side are
+            checked, bRight==true means opcodes for right side. If bRPN==true
+            it doesn't matter.
+ */
+bool isPotentialRangeType( FormulaToken* pToken, bool bRPN, bool bRight )
+{
+    switch (pToken->GetType())
+    {
+        case svByte:                // could be range result, but only a few
+            if (bRPN)
+                return isRangeResultOpCode( pToken->GetOpCode());
+            else if (bRight)
+                return isRangeResultFunction( pToken->GetOpCode());
+            else
+                return isPotentialRangeLeftOp( pToken->GetOpCode());
+        case svSingleRef:
+        case svDoubleRef:
+        case svIndex:               // could be range
+        //case svRefList:           // um..what?
+        case svExternalSingleRef:
+        case svExternalDoubleRef:
+        case svExternalName:        // could be range
+            return true;
+        default:
+            // Separators are not part of RPN and right opcodes need to be
+            // other StackVarEnum types or functions and thus svByte.
+            return !bRPN && !bRight && isPotentialRangeLeftOp( pToken->GetOpCode());
+    }
+}
+
+bool isIntersectable( FormulaToken** pCode1, FormulaToken** pCode2 )
+{
+    FormulaToken* pToken1 = *pCode1;
+    FormulaToken* pToken2 = *pCode2;
+    if (pToken1 && pToken2)
+        return isPotentialRangeType( pToken1, true, false) && isPotentialRangeType( pToken2, true, true);
+    return false;
+}
+
+bool isAdjacentOrGapRpnEnd( sal_uInt16 nPC,
+        FormulaToken const * const * const pCode,
+        FormulaToken const * const * const pCode1,
+        FormulaToken const * const * const pCode2 )
+{
+    return nPC >= 2 && pCode1 && pCode2 &&
+            (pCode2 > pCode1) && (pCode - pCode2 == 1) &&
+            (*pCode1 != nullptr) && (*pCode2 != nullptr);
+}
+
+
 } // namespace
 
 
@@ -1058,6 +1151,8 @@ bool FormulaCompiler::GetToken()
         bStop = true;
     else
     {
+        FormulaTokenRef pCurrToken = mpToken;
+        FormulaTokenRef pSpacesToken;
         short nWasColRowName;
         if ( pArr->nIndex
           && pArr->pCode[ pArr->nIndex-1 ]->GetOpCode() == ocColRowName )
@@ -1067,6 +1162,9 @@ bool FormulaCompiler::GetToken()
         mpToken = pArr->Next();
         while( mpToken && mpToken->GetOpCode() == ocSpaces )
         {
+            // For significant whitespace remember last ocSpaces token. Usually
+            // there's only one even for multiple spaces.
+            pSpacesToken = mpToken;
             if ( nWasColRowName )
                 nWasColRowName++;
             if ( bAutoCorrect && !pStack )
@@ -1092,6 +1190,17 @@ bool FormulaCompiler::GetToken()
                 mpToken = new FormulaByteToken( ocIntersect );
                 pArr->nIndex--;     // we advanced to the second ocColRowName, step back
             }
+            else if (pSpacesToken && FormulaGrammar::isExcelSyntax( meGrammar) &&
+                    pCurrToken && mpToken &&
+                    isPotentialRangeType( pCurrToken.get(), false, false) &&
+                    isPotentialRangeType( mpToken.get(), false, true))
+            {
+                // Let IntersectionLine() <- Factor() decide how to treat this,
+                // once the actual arguments are determined in RPN.
+                mpToken = pSpacesToken;
+                pArr->nIndex--;     // step back from next non-spaces token
+                return true;
+            }
         }
     }
     if( bStop )
@@ -1562,12 +1671,33 @@ void FormulaCompiler::RangeLine()
 void FormulaCompiler::IntersectionLine()
 {
     RangeLine();
-    while (mpToken->GetOpCode() == ocIntersect)
+    while (mpToken->GetOpCode() == ocIntersect || mpToken->GetOpCode() == ocSpaces)
     {
+        sal_uInt16 nCodeIndex = pArr->nIndex - 1;
+        FormulaToken** pCode1 = pCode - 1;
         FormulaTokenRef p = mpToken;
         NextToken();
         RangeLine();
-        PutCode(p);
+        FormulaToken** pCode2 = pCode - 1;
+        if (p->GetOpCode() == ocSpaces)
+        {
+            // Convert to intersection if both left and right are references or
+            // functions (potentially returning references, if not then a space
+            // or no space would be a syntax error anyway), not other operators
+            // or operands. Else discard.
+            if (isAdjacentOrGapRpnEnd( pc, pCode, pCode1, pCode2) && isIntersectable( pCode1, pCode2))
+            {
+                FormulaTokenRef pIntersect( new FormulaByteToken( ocIntersect));
+                // Replace ocSpaces with ocIntersect so that when switching
+                // formula syntax the correct operator string is created.
+                pArr->ReplaceToken( nCodeIndex, pIntersect.get(), FormulaTokenArray::ReplaceMode::CODE_ONLY);
+                PutCode( pIntersect);
+            }
+        }
+        else
+        {
+            PutCode(p);
+        }
     }
 }
 
@@ -1918,6 +2048,14 @@ const FormulaToken* FormulaCompiler::CreateStringFromToken( OUStringBuffer& rBuf
     }
     else if( eOp >= ocInternalBegin && eOp <= ocInternalEnd )
         rBuffer.appendAscii( pInternal[ eOp - ocInternalBegin ] );
+    else if (eOp == ocIntersect)
+    {
+        // Nasty, ugly, horrific, terrifying..
+        if (FormulaGrammar::isExcelSyntax( meGrammar))
+            rBuffer.append(' ');
+        else
+            rBuffer.append( mxSymbols->getSymbol( eOp));
+    }
     else if( (sal_uInt16) eOp < mxSymbols->getSymbolCount())        // Keyword:
         rBuffer.append( mxSymbols->getSymbol( eOp));
     else
@@ -2206,7 +2344,25 @@ OpCode FormulaCompiler::NextToken()
                 }
             }
         }
-        eLastOp = eOp;
+        // Nasty, ugly, horrific, terrifying.. significant whitespace..
+        if (eOp == ocSpaces && FormulaGrammar::isExcelSyntax( meGrammar))
+        {
+            // Fake an intersection op as last op for the next round, but at
+            // least roughly check if it could make sense at all.
+            FormulaToken* pPrev = pArr->PeekPrevNoSpaces();
+            if (pPrev && isPotentialRangeType( pPrev, false, false))
+            {
+                FormulaToken* pNext = pArr->PeekNextNoSpaces();
+                if (pNext && isPotentialRangeType( pNext, false, true))
+                    eLastOp = ocIntersect;
+                else
+                    eLastOp = eOp;
+            }
+            else
+                eLastOp = eOp;
+        }
+        else
+            eLastOp = eOp;
     }
     return eOp;
 }
diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index c16f3cad4332..7511c34a8720 100644
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -868,16 +868,13 @@ FormulaToken* FormulaTokenArray::MergeArray( )
 FormulaToken* FormulaTokenArray::ReplaceToken( sal_uInt16 nOffset, FormulaToken* t,
         FormulaTokenArray::ReplaceMode eMode )
 {
-    if (eMode == BACKWARD_CODE_ONLY)
-        nOffset = nLen - nOffset - 1;
-
     if (nOffset < nLen)
     {
         CheckToken(*t);
         t->IncRef();
         FormulaToken* p = pCode[nOffset];
         pCode[nOffset] = t;
-        if (eMode == FORWARD_CODE_AND_RPN && p->GetRef() > 1)
+        if (eMode == CODE_AND_RPN && p->GetRef() > 1)
         {
             for (sal_uInt16 i=0; i < nRPN; ++i)
             {
diff --git a/formula/source/core/resource/core_resource.src b/formula/source/core/resource/core_resource.src
index d5db65ec0d7b..af22f1b6bc65 100644
--- a/formula/source/core/resource/core_resource.src
+++ b/formula/source/core/resource/core_resource.src
@@ -477,7 +477,7 @@ Resource RID_STRLIST_FUNCTION_NAMES_ENGLISH_OOXML
     String SC_OPCODE_AND { Text = "AND" ; };
     String SC_OPCODE_OR { Text = "OR" ; };
     String SC_OPCODE_XOR { Text = "_xlfn.XOR" ; };
-    String SC_OPCODE_INTERSECT { Text = "!" ; };
+    String SC_OPCODE_INTERSECT { Text = " " ; };
     String SC_OPCODE_UNION { Text = "~" ; };
     String SC_OPCODE_RANGE { Text = ":" ; };
     String SC_OPCODE_NOT { Text = "NOT" ; };
diff --git a/include/formula/grammar.hxx b/include/formula/grammar.hxx
index 91ca7ae1171a..6500d68df898 100644
--- a/include/formula/grammar.hxx
+++ b/include/formula/grammar.hxx
@@ -203,6 +203,21 @@ public:
             css::sheet::FormulaLanguage::OOXML;
     }
 
+    /// If grammar has an Excel syntax, determined by address convention.
+    static inline bool isExcelSyntax( const Grammar eGrammar )
+    {
+        AddressConvention eConv = extractRefConvention( eGrammar );
+        switch (eConv)
+        {
+            case FormulaGrammar::AddressConvention::CONV_XL_A1:
+            case FormulaGrammar::AddressConvention::CONV_XL_R1C1:
+            case FormulaGrammar::AddressConvention::CONV_XL_OOX:
+                return true;
+            default:
+                return false;
+        }
+    }
+
 };
 
 } // formula
diff --git a/include/formula/tokenarray.hxx b/include/formula/tokenarray.hxx
index 292b3182cb8b..1735d0022a85 100644
--- a/include/formula/tokenarray.hxx
+++ b/include/formula/tokenarray.hxx
@@ -127,23 +127,19 @@ protected:
 public:
     enum ReplaceMode
     {
-        BACKWARD_CODE_ONLY,     ///< offset goes backward, replacement only in pCode
-        FORWARD_CODE_AND_RPN    ///< offset goes forward, replacement in pCode and RPN
+        CODE_ONLY,      ///< replacement only in pCode
+        CODE_AND_RPN    ///< replacement in pCode and pRPN
     };
 
 protected:
     /** Also used by the compiler. The token MUST had been allocated with new!
         @param  nOffset
-                If eMode==BACKWARD_CODE_ONLY negative offset of token, 0==last,
-                1==previous, ...
-                If eMode==FORWARD_CODE_AND_RPN positive offset of token, 0==first,
-                1==second, ...
+                Absolute offset in pCode of the token to be replaced.
         @param  eMode
-                If BACKWARD_CODE_ONLY only the token in pCode at nLen-nOffset-1
-                is replaced.
-                If FORWARD_CODE_AND_RPN the token in pCode at nOffset is
-                replaced; if the original token was also referenced in the RPN
-                array then that reference is replaced with a reference to the new
+                If CODE_ONLY only the token in pCode at nOffset is replaced.
+                If CODE_AND_RPN the token in pCode at nOffset is replaced;
+                if the original token was also referenced in the pRPN array
+                then that reference is replaced with a reference to the new
                 token as well.
      */
     FormulaToken*           ReplaceToken( sal_uInt16 nOffset, FormulaToken*, ReplaceMode eMode );
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index 5f258dfcc0d6..9d272197c06d 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -4281,8 +4281,8 @@ ScTokenArray* ScCompiler::CompileString( const OUString& rFormula )
                 FormulaToken* pTableRefToken = new ScTableRefToken( pPrev->GetIndex(), ScTableRefToken::TABLE);
                 maTableRefs.push_back( TableRefEntry( pTableRefToken));
                 // pPrev may be dead hereafter.
-                static_cast<ScTokenArray*>(pArr)->ReplaceToken( 1, pTableRefToken,
-                        FormulaTokenArray::ReplaceMode::BACKWARD_CODE_ONLY);
+                static_cast<ScTokenArray*>(pArr)->ReplaceToken( nIdx, pTableRefToken,
+                        FormulaTokenArray::ReplaceMode::CODE_ONLY);
             }
         }
         switch (eOp)
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index d73d8d71da1c..1a8305153524 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -2402,7 +2402,7 @@ void ScTokenArray::ReadjustAbsolute3DReferences( const ScDocument* pOldDoc, cons
                     OUString aTabName;
                     sal_uInt16 nFileId;
                     GetExternalTableData(pOldDoc, pNewDoc, rRef1.Tab(), aTabName, nFileId);
-                    ReplaceToken( j, new ScExternalDoubleRefToken(nFileId, aTabName, rRef), FORWARD_CODE_AND_RPN);
+                    ReplaceToken( j, new ScExternalDoubleRefToken(nFileId, aTabName, rRef), CODE_AND_RPN);
                     // ATTENTION: rRef can't be used after this point
                 }
             }
@@ -2419,7 +2419,7 @@ void ScTokenArray::ReadjustAbsolute3DReferences( const ScDocument* pOldDoc, cons
                     OUString aTabName;
                     sal_uInt16 nFileId;
                     GetExternalTableData(pOldDoc, pNewDoc, rRef.Tab(), aTabName, nFileId);
-                    ReplaceToken( j, new ScExternalSingleRefToken(nFileId, aTabName, rRef), FORWARD_CODE_AND_RPN);
+                    ReplaceToken( j, new ScExternalSingleRefToken(nFileId, aTabName, rRef), CODE_AND_RPN);
                     // ATTENTION: rRef can't be used after this point
                 }
             }