tdf#136404 DOCX import: ignore NaN cells in table formula

Ignore empty cells or cells with text content in data range
of AVERAGE, COUNT and PRODUCT (the new interoperability
functions in Writer), like MSO does, instead of using
NaN data as zeroes here.

Add AVERAGE, as a new function instead of alias of MEAN
to return error message instead of zero for NaN-only
arguments, like Calc does (Note: also MSO gives empty
result instead of zero).

Change-Id: I5e18f3e2b16cb0621dad2bba141ae63fb5edd241
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102012
Tested-by: Jenkins
Reviewed-by: László Németh <nemeth@numbertext.org>

5 files changed, 156 insertions, 8 deletions

diff --git a/sw/inc/calc.hxx b/sw/inc/calc.hxx
index 99c024bcac35..b7bb83af26a8 100644
--- a/sw/inc/calc.hxx
+++ b/sw/inc/calc.hxx
@@ -98,6 +98,7 @@ extern const char sCalc_Abs[];
 enum class SwCalcError
 {
     NONE=0,
+    NaN,              //  not a number (not an error, used for interoperability)
     Syntax,           //  syntax error
     DivByZero,        //  division by zero
     FaultyBrackets,   //  faulty brackets
@@ -204,6 +205,7 @@ class SwCalc
     CharClass*  m_pCharClass;
 
     sal_uInt16      m_nListPor;
+    bool        m_bHasNumber; // fix COUNT() and AVERAGE(), if all cells are NaN
     SwCalcOper  m_eCurrOper;
     SwCalcOper  m_eCurrListOper;
     SwCalcError m_eError;
@@ -240,7 +242,8 @@ public:
     CharClass* GetCharClass();
 
     void        SetCalcError( SwCalcError eErr )    { m_eError = eErr; }
-    bool        IsCalcError() const                 { return SwCalcError::NONE != m_eError; }
+    bool        IsCalcError() const                 { return SwCalcError::NONE != m_eError && SwCalcError::NaN != m_eError; }
+    bool        IsCalcNotANumber() const            { return SwCalcError::NaN == m_eError; }
 
     static bool Str2Double( const OUString& rStr, sal_Int32& rPos,
                                 double& rVal );
diff --git a/sw/qa/extras/ooxmlexport/data/tdf136404.fodt b/sw/qa/extras/ooxmlexport/data/tdf136404.fodt
new file mode 100644
index 000000000000..9ac902d45b3f
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf136404.fodt
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:officeooo="http://openoffice.org/2009/office" xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0" xmlns:ooow="http://openoffice.org/2004/writer" office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:styles>
+  <style:style style:name="Standard" style:family="paragraph" style:class="text"/>
+  <style:default-style style:family="paragraph">
+   <style:text-properties fo:language="en" fo:country="US"/>
+  </style:default-style>
+ </office:styles>
+ <office:body>
+  <office:text>
+   <table:table>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-column/>
+    <table:table-row>
+     <table:table-cell office:value-type="string">
+      <text:p>6</text:p>
+     </table:table-cell>
+     <table:table-cell office:value-type="string">
+      <text:p></text:p>
+     </table:table-cell>
+     <table:table-cell office:value-type="string">
+      <text:p></text:p>
+     </table:table-cell>
+     <table:table-cell office:value-type="string">
+      <text:p>10</text:p>
+     </table:table-cell>
+     <table:table-cell office:value-type="string">
+      <text:p>text</text:p>
+     </table:table-cell>
+     <table:table-cell office:value-type="string">
+      <text:p>text</text:p>
+     </table:table-cell>
+    </table:table-row>
+    <table:table-row>
+     <table:table-cell table:formula="ooow:COUNT(&lt;A1:F1&gt;)" office:value-type="float" office:value="2">
+      <text:p>2</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:AVERAGE(&lt;A1:F1&gt;)" office:value-type="float" office:value="0">
+      <text:p>8</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:PRODUCT(&lt;A1:F1&gt;)" office:value-type="float" office:value="60">
+      <text:p>60</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:COUNT(&lt;B1&gt;)" office:value-type="float" office:value="0">
+      <text:p>0</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:COUNT(&lt;B1:C1&gt;)" office:value-type="float" office:value="0">
+      <text:p>0</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:AVERAGE(&lt;B1&gt;)" office:value-type="float" office:value="1.79769313486232E+308">
+      <text:p text:style-name="P1">** Expression is faulty **</text:p>
+     </table:table-cell>
+     <table:table-cell table:formula="ooow:AVERAGE(&lt;B1:C1&gt;)" office:value-type="float" office:value="1.79769313486232E+308">
+      <text:p text:style-name="P1">** Expression is faulty **</text:p>
+     </table:table-cell>
+    </table:table-row>
+   </table:table>
+  </office:text>
+ </office:body>
+</office:document>
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
index 48cf77e84f0b..8d6e8228f8ba 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
@@ -173,6 +173,48 @@ DECLARE_OOXMLEXPORT_TEST(testTdf123356, "tdf123356.fodt")
     CPPUNIT_ASSERT_EQUAL(OUString("4"), xEnumerationAccess2->getPresentation(false).trim());
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf136404, "tdf136404.fodt")
+{
+    uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xFieldsAccess(xTextFieldsSupplier->getTextFields());
+    uno::Reference<container::XEnumeration> xFields(xFieldsAccess->createEnumeration());
+
+    // Ignore empty cells or cells with text content with new interoperability functions COUNT, AVERAGE and PRODUCT
+    uno::Reference<text::XTextField> xEnumerationAccess1(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<A1:F1>)"), xEnumerationAccess1->getPresentation(true).trim());
+    CPPUNIT_ASSERT_EQUAL(OUString("2"), xEnumerationAccess1->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess2(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<B1:C1>)"), xEnumerationAccess2->getPresentation(true).trim());
+    // This was 0
+    CPPUNIT_ASSERT_EQUAL(OUString("** Expression is faulty **"), xEnumerationAccess2->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess3(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<B1>)"), xEnumerationAccess3->getPresentation(true).trim());
+    // This was 0
+    CPPUNIT_ASSERT_EQUAL(OUString("** Expression is faulty **"), xEnumerationAccess3->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess4(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<B1:C1>)"), xEnumerationAccess4->getPresentation(true).trim());
+    // This was 2
+    CPPUNIT_ASSERT_EQUAL(OUString("0"), xEnumerationAccess4->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess5(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<B1>)"), xEnumerationAccess5->getPresentation(true).trim());
+    // This was 1
+    CPPUNIT_ASSERT_EQUAL(OUString("0"), xEnumerationAccess5->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess6(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("PRODUCT(<A1:F1>)"), xEnumerationAccess6->getPresentation(true).trim());
+    // This was 0
+    CPPUNIT_ASSERT_EQUAL(OUString("60"), xEnumerationAccess6->getPresentation(false).trim());
+
+    uno::Reference<text::XTextField> xEnumerationAccess7(xFields->nextElement(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<A1:F1>)"), xEnumerationAccess7->getPresentation(true).trim());
+    // This was 2
+    CPPUNIT_ASSERT_EQUAL(OUString("8"), xEnumerationAccess7->getPresentation(false).trim());
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf123390, "tdf123390.fodt")
 {
     uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
diff --git a/sw/source/core/bastyp/calc.cxx b/sw/source/core/bastyp/calc.cxx
index 17179f472352..41bf202a9ea4 100644
--- a/sw/source/core/bastyp/calc.cxx
+++ b/sw/source/core/bastyp/calc.cxx
@@ -106,7 +106,7 @@ CalcOp const aOpTable[] = {
 /* AND */     {{sCalc_And},        CALC_AND},   // log. AND
 /* ASIN */    {{sCalc_Asin},       CALC_ASIN},  // Arc sine
 /* ATAN */    {{sCalc_Atan},       CALC_ATAN},  // Arc tangent
-/* AVERAGE */ {{sCalc_Average},    CALC_MEAN},  // Mean (since LibreOffice 7.1)
+/* AVERAGE */ {{sCalc_Average},    CALC_AVERAGE}, // Average (since LibreOffice 7.1)
 /* COS */     {{sCalc_Cos},        CALC_COS},   // Cosine
 /* COUNT */   {{sCalc_Count},      CALC_COUNT}, // Count (since LibreOffice 7.1)
 /* DATE */    {{sCalc_Date},       CALC_DATE},  // Date
@@ -221,6 +221,7 @@ SwCalc::SwCalc( SwDoc& rD )
     , m_pLocaleDataWrapper( m_aSysLocale.GetLocaleDataPtr() )
     , m_pCharClass( &GetAppCharClass() )
     , m_nListPor( 0 )
+    , m_bHasNumber( false )
     , m_eCurrOper( CALC_NAME )
     , m_eCurrListOper( CALC_NAME )
     , m_eError( SwCalcError::NONE )
@@ -460,6 +461,7 @@ SwCalcExp* SwCalc::VarLook( const OUString& rStr, bool bIns )
             {
                 // Save the current values...
                 sal_uInt16 nListPor = m_nListPor;
+                bool bHasNumber = m_bHasNumber;
                 SwSbxValue nLastLeft = m_nLastLeft;
                 SwSbxValue nNumberValue = m_nNumberValue;
                 sal_Int32 nCommandPos = m_nCommandPos;
@@ -471,6 +473,7 @@ SwCalcExp* SwCalc::VarLook( const OUString& rStr, bool bIns )
 
                 // ...and write them back.
                 m_nListPor = nListPor;
+                m_bHasNumber = bHasNumber;
                 m_nLastLeft = nLastLeft;
                 m_nNumberValue = nNumberValue;
                 m_nCommandPos = nCommandPos;
@@ -674,6 +677,7 @@ SwCalcOper SwCalc::GetToken()
                 {
                 case CALC_SUM:
                 case CALC_MEAN:
+                case CALC_AVERAGE:
                 case CALC_COUNT:
                     m_eCurrListOper = CALC_PLUS;
                     break;
@@ -1120,6 +1124,7 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
         {
             SAL_INFO("sw.calc", "number: " << m_nNumberValue.GetDouble());
             SwSbxValue nErg;
+            m_bHasNumber = true;
             if( GetToken() == CALC_PHD )
             {
                 double aTmp = m_nNumberValue.GetDouble();
@@ -1196,14 +1201,19 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
             SAL_INFO("sw.calc", ")");
             break;
         case CALC_MEAN:
+        case CALC_AVERAGE:
         {
             SAL_INFO("sw.calc", "mean");
             m_nListPor = 1;
+            m_bHasNumber = CALC_MEAN == m_eCurrOper;
             GetToken();
             SwSbxValue nErg = Expr();
             double aTmp = nErg.GetDouble();
             aTmp /= m_nListPor;
-            nErg.PutDouble( aTmp );
+            if ( !m_bHasNumber )
+                m_eError = SwCalcError::DivByZero;
+            else
+                nErg.PutDouble( aTmp );
             return nErg;
             break;
         }
@@ -1211,9 +1221,10 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
         {
             SAL_INFO("sw.calc", "count");
             m_nListPor = 1;
+            m_bHasNumber = false;
             GetToken();
             SwSbxValue nErg = Expr();
-            nErg.PutDouble( m_nListPor );
+            nErg.PutDouble( m_bHasNumber ? m_nListPor : 0 );
             return nErg;
             break;
         }
diff --git a/sw/source/core/fields/cellfml.cxx b/sw/source/core/fields/cellfml.cxx
index 755d81f23fcc..47cbabe871a5 100644
--- a/sw/source/core/fields/cellfml.cxx
+++ b/sw/source/core/fields/cellfml.cxx
@@ -238,6 +238,8 @@ double SwTableBox::GetValue( SwTableCalcPara& rCalcPara ) const
 
             if( pDoc->IsNumberFormat( sText, nFormatIndex, aNum ))
                 nRet = aNum;
+            else
+                rCalcPara.m_rCalc.SetCalcError( SwCalcError::NaN ); // set for interoperability functions
         }
         // ?? otherwise it is an error
     } while( false );
@@ -354,6 +356,9 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
         SwSelBoxes aBoxes;
         GetBoxes( *pSttBox, *pEndBox, aBoxes );
 
+        // don't use empty cells or cells with text content as zeroes in interoperability functions
+        sal_Int16 nUseOnlyNumber = -1;
+
         rNewStr.append("(");
         bool bDelim = false;
         for (size_t n = 0; n < aBoxes.size() &&
@@ -362,11 +367,26 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
             const SwTableBox* pTableBox = aBoxes[n];
             if ( pTableBox->getRowSpan() >= 1 )
             {
+                double fVal = pTableBox->GetValue( *pCalcPara );
+
+                if ( pCalcPara->m_rCalc.IsCalcNotANumber() )
+                {
+                    if ( nUseOnlyNumber == -1 )
+                    {
+                        OUString sFormula = rNewStr.toString().toAsciiUpperCase();
+                        nUseOnlyNumber = sal_Int16(
+                                sFormula.lastIndexOf("AVERAGE") > -1 ||
+                                sFormula.lastIndexOf("COUNT") > -1 ||
+                                sFormula.lastIndexOf("PRODUCT") > -1 );
+                    }
+                    if ( nUseOnlyNumber > 0 )
+                        continue;
+                }
+
                 if( bDelim )
                     rNewStr.append(cListDelim);
                 bDelim = true;
-                rNewStr.append(pCalcPara->m_rCalc.GetStrResult(
-                            pTableBox->GetValue( *pCalcPara ) ));
+                rNewStr.append(pCalcPara->m_rCalc.GetStrResult( fVal ));
             }
         }
         rNewStr.append(")");
@@ -378,8 +398,15 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
         if ( pSttBox->getRowSpan() >= 1 )
         {
             rNewStr.append("(");
-            rNewStr.append(pCalcPara->m_rCalc.GetStrResult(
-                            pSttBox->GetValue( *pCalcPara ) ));
+            double fVal = pSttBox->GetValue( *pCalcPara );
+            // don't use empty cell or a cell with text content as zero in interoperability functions
+            // (except PRODUCT, where the result is correct anyway)
+            if ( !pCalcPara->m_rCalc.IsCalcNotANumber() ||
+                 ( rNewStr.toString().toAsciiUpperCase().lastIndexOf("AVERAGE") == -1 &&
+                   rNewStr.toString().toAsciiUpperCase().lastIndexOf("COUNT") == -1 ) )
+            {
+                rNewStr.append(pCalcPara->m_rCalc.GetStrResult( fVal ));
+            }
             rNewStr.append(")");
         }
     }