Add more information in exception when formula parsing fails

Use toString() instead of getClass() to include more information 
for some ValueEval implementations
When low-level parsing fails, the IllegalStateException does not 
contain any information, therefore wrap it and enrich it with
more information about the cell and the parsed formula

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1908608 13f79535-47bb-0310-9956-ffa450edef68

poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java

@@ -57,9 +57,27 @@ public abstract class BaseXSSFFormulaEvaluator extends BaseFormulaEvaluator {
      */
     @Override
     protected CellValue evaluateFormulaCellValue(Cell cell) {
-        EvaluationCell evalCell = toEvaluationCell(cell);
+        final ValueEval eval;
-        ValueEval eval = _bookEvaluator.evaluate(evalCell);
+        try {
-        cacheExternalWorkbookCells(evalCell);
+            EvaluationCell evalCell = toEvaluationCell(cell);
+            eval = _bookEvaluator.evaluate(evalCell);
+            cacheExternalWorkbookCells(evalCell);
+        } catch (IllegalStateException e) {
+            // enhance IllegalStateException which can be
+            // thrown somewhere deep down the evaluation
+            // and thus is often missing information necessary
+            // for troubleshooting
+            // do not enhance others to keep the exception-sub-classes
+            // in place
+            if (e.getClass() == IllegalStateException.class) {
+                throw new IllegalStateException("Failed to evaluate cell: " +
+                        new CellReference(cell.getSheet().getSheetName(), cell.getRowIndex(), cell.getColumnIndex(),
+                                false, false).formatAsString(true) + ", value: " + cell, e);
+            } else {
+                throw e;
+            }
+        }
+
         if (eval instanceof NumberEval) {
             NumberEval ne = (NumberEval) eval;
             return new CellValue(ne.getNumberValue());
@@ -75,7 +93,9 @@ public abstract class BaseXSSFFormulaEvaluator extends BaseFormulaEvaluator {
         if (eval instanceof ErrorEval) {
             return CellValue.getError(((ErrorEval)eval).getErrorCode());
         }
-        throw new IllegalStateException("Unexpected eval class (" + eval.getClass().getName() + ")");
+        throw new IllegalStateException("Unexpected eval class (" + eval.getClass().getName() + "): " + eval + ", cell: " +
+                new CellReference(cell.getSheet().getSheetName(), cell.getRowIndex(), cell.getColumnIndex(),
+                        false, false).formatAsString(true) + ", value: " + cell);
     }
 
     /**

poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestFormulaEval.java

@@ -17,22 +17,29 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellType;
+import org.apache.poi.ss.usermodel.FormulaEvaluator;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 class TestFormulaEval {
     @Test
     void testCircularRef() throws IOException {
-        try (XSSFWorkbook wb = new XSSFWorkbook()) {
+        try (Workbook wb = new XSSFWorkbook()) {
-            XSSFSheet sheet = wb.createSheet();
+            Sheet sheet = wb.createSheet();
-            XSSFRow row = sheet.createRow(0);
+            Row row = sheet.createRow(0);
-            XSSFCell cell = row.createCell(0);
+            Cell cell = row.createCell(0);
             cell.setCellFormula("A1");
-            XSSFFormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
+            FormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
             // the following assert should probably be NUMERIC not ERROR (from testing in Excel itself)
             assertEquals(CellType.ERROR, formulaEvaluator.evaluateFormulaCell(cell));
 
@@ -45,14 +52,14 @@ class TestFormulaEval {
 
     @Test
     void testCircularRef2() throws IOException {
-        try (XSSFWorkbook wb = new XSSFWorkbook()) {
+        try (Workbook wb = new XSSFWorkbook()) {
-            XSSFSheet sheet = wb.createSheet();
+            Sheet sheet = wb.createSheet();
-            XSSFRow row = sheet.createRow(0);
+            Row row = sheet.createRow(0);
-            XSSFCell cell0 = row.createCell(0);
+            Cell cell0 = row.createCell(0);
-            XSSFCell cell1 = row.createCell(1);
+            Cell cell1 = row.createCell(1);
             cell0.setCellFormula("B1");
             cell1.setCellFormula("A1");
-            XSSFFormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
+            FormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
             formulaEvaluator.evaluateAll();
 
             cell0.setCellFormula(null);
@@ -64,4 +71,56 @@ class TestFormulaEval {
             assertEquals(CellType.ERROR, cell1.getCellType());
         }
     }
+
+    @Test
+    void testExceptionForWrongFormula1() throws IOException {
+        try (Workbook wb = new XSSFWorkbook()) {
+            Sheet sheet = wb.createSheet("test-sheet");
+            Row row = sheet.createRow(0);
+            Cell cell0 = row.createCell(0);
+            Cell cell1 = row.createCell(1);
+            cell0.setCellValue(1);
+            cell1.setCellFormula("'Sheet123'!R6C13");
+
+            FormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
+            try {
+                formulaEvaluator.evaluateAll();
+                fail("Should catch exception here");
+            } catch (IllegalStateException e) {
+                assertTrue(e.getMessage().contains("test-sheet"),
+                        "Had: " + e.getMessage());
+                assertTrue(e.getMessage().contains("Sheet123"),
+                        "Had: " + e.getMessage());
+                assertTrue(e.getMessage().contains("R6C13"),
+                        "Had: " + e.getMessage());
+            }
+        }
+    }
+
+    @Test
+    void testExceptionForWrongFormula2() throws IOException {
+        try (Workbook wb = new XSSFWorkbook()) {
+            Sheet sheet = wb.createSheet("test-sheet");
+            Row row = sheet.createRow(0);
+            Cell cell0 = row.createCell(0);
+            Cell cell1 = row.createCell(1);
+            cell0.setCellValue(1);
+            cell1.setCellFormula("SUM('asldkjasldk ajd Sheet123'!R6C13)");
+
+            FormulaEvaluator formulaEvaluator = wb.getCreationHelper().createFormulaEvaluator();
+            try {
+                formulaEvaluator.evaluateAll();
+                fail("Should catch exception here");
+            } catch (IllegalStateException e) {
+                assertTrue(e.getMessage().contains("test-sheet"),
+                        "Had: " + e.getMessage());
+                assertTrue(e.getMessage().contains("Sheet123"),
+                        "Had: " + e.getMessage());
+                assertTrue(e.getMessage().contains("R6C13"),
+                        "Had: " + e.getMessage());
+                assertTrue(e.getMessage().contains("SUM"),
+                        "Had: " + e.getMessage());
+            }
+        }
+    }
 }

poi/src/main/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java

@@ -225,8 +225,7 @@ public abstract class MultiOperandNumericFunction implements Function {
             missingArgConsumer.accept((MissingArgEval) ve, temp);
             return;
         }
-        throw new IllegalStateException("Invalid ValueEval type passed for conversion: ("
+        throw new IllegalStateException("Invalid ValueEval type passed for conversion: " + ve);
-                + ve.getClass() + ")");
     }
 
     private static class ConsumerFactory {