diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java index f47d255c4a..fe4e41330a 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java +++ b/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); - ValueEval eval = _bookEvaluator.evaluate(evalCell); - cacheExternalWorkbookCells(evalCell); + final ValueEval eval; + try { + 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); } /** diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestFormulaEval.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestFormulaEval.java index d96fb69aa5..58c84f718c 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestFormulaEval.java +++ b/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()) { - XSSFSheet sheet = wb.createSheet(); - XSSFRow row = sheet.createRow(0); - XSSFCell cell = row.createCell(0); + try (Workbook wb = new XSSFWorkbook()) { + Sheet sheet = wb.createSheet(); + Row row = sheet.createRow(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()) { - XSSFSheet sheet = wb.createSheet(); - XSSFRow row = sheet.createRow(0); - XSSFCell cell0 = row.createCell(0); - XSSFCell cell1 = row.createCell(1); + try (Workbook wb = new XSSFWorkbook()) { + Sheet sheet = wb.createSheet(); + Row row = sheet.createRow(0); + Cell cell0 = row.createCell(0); + 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()); + } + } + } } diff --git a/poi/src/main/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java b/poi/src/main/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java index 8f8f6b8175..ea18a7c3e4 100644 --- a/poi/src/main/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java +++ b/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: (" - + ve.getClass() + ")"); + throw new IllegalStateException("Invalid ValueEval type passed for conversion: " + ve); } private static class ConsumerFactory {