Slightly improve error message

Fix some IntelliJ/compiler warnings
Use common interfaces where possible
Cleanup after testing POIFSDump

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1738032 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2016-04-06 19:49:59 +00:00
parent 62ee0abc13
commit ec6bb2a066
5 changed files with 50 additions and 74 deletions

View File

@ -145,7 +145,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
public RowRecord getRow(int rowIndex) { public RowRecord getRow(int rowIndex) {
int maxrow = SpreadsheetVersion.EXCEL97.getLastRowIndex(); int maxrow = SpreadsheetVersion.EXCEL97.getLastRowIndex();
if (rowIndex < 0 || rowIndex > maxrow) { if (rowIndex < 0 || rowIndex > maxrow) {
throw new IllegalArgumentException("The row number must be between 0 and " + maxrow); throw new IllegalArgumentException("The row number must be between 0 and " + maxrow + ", but had: " + rowIndex);
} }
return _rowRecords.get(Integer.valueOf(rowIndex)); return _rowRecords.get(Integer.valueOf(rowIndex));
} }
@ -278,9 +278,9 @@ public final class RowRecordsAggregate extends RecordAggregate {
// Calculate Offset from the start of a DBCellRecord to the first Row // Calculate Offset from the start of a DBCellRecord to the first Row
rv.visitRecord(dbcrBuilder.build(pos)); rv.visitRecord(dbcrBuilder.build(pos));
} }
for (int i=0; i< _unknownRecords.size(); i++) { for (Record _unknownRecord : _unknownRecords) {
// Potentially breaking the file here since we don't know exactly where to write these records // Potentially breaking the file here since we don't know exactly where to write these records
rv.visitRecord(_unknownRecords.get(i)); rv.visitRecord(_unknownRecord);
} }
} }
@ -364,28 +364,24 @@ public final class RowRecordsAggregate extends RecordAggregate {
public boolean isRowGroupCollapsed(int row) { public boolean isRowGroupCollapsed(int row) {
int collapseRow = findEndOfRowOutlineGroup(row) + 1; int collapseRow = findEndOfRowOutlineGroup(row) + 1;
if (getRow(collapseRow) == null) { return getRow(collapseRow) != null && getRow(collapseRow).getColapsed();
return false;
}
return getRow( collapseRow ).getColapsed();
} }
public void expandRow(int rowNumber) { public void expandRow(int rowNumber) {
int idx = rowNumber; if (rowNumber == -1)
if (idx == -1)
return; return;
// If it is already expanded do nothing. // If it is already expanded do nothing.
if (!isRowGroupCollapsed(idx)) { if (!isRowGroupCollapsed(rowNumber)) {
return; return;
} }
// Find the start of the group. // Find the start of the group.
int startIdx = findStartOfRowOutlineGroup(idx); int startIdx = findStartOfRowOutlineGroup(rowNumber);
RowRecord row = getRow(startIdx); RowRecord row = getRow(startIdx);
// Find the end of the group. // Find the end of the group.
int endIdx = findEndOfRowOutlineGroup(idx); int endIdx = findEndOfRowOutlineGroup(rowNumber);
// expand: // expand:
// collapsed bit must be unset // collapsed bit must be unset
@ -394,7 +390,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
// to look at the start and the end of the current group to determine which // to look at the start and the end of the current group to determine which
// is the enclosing group // is the enclosing group
// hidden bit only is altered for this outline level. ie. don't un-collapse contained groups // hidden bit only is altered for this outline level. ie. don't un-collapse contained groups
if (!isRowGroupHiddenByParent(idx)) { if (!isRowGroupHiddenByParent(rowNumber)) {
for (int i = startIdx; i <= endIdx; i++) { for (int i = startIdx; i <= endIdx; i++) {
RowRecord otherRow = getRow(i); RowRecord otherRow = getRow(i);
if (row.getOutlineLevel() == otherRow.getOutlineLevel() || !isRowGroupCollapsed(i)) { if (row.getOutlineLevel() == otherRow.getOutlineLevel() || !isRowGroupCollapsed(i)) {
@ -450,6 +446,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
* @deprecated use {@link #getCellValueIterator()} instead * @deprecated use {@link #getCellValueIterator()} instead
*/ */
public CellValueRecordInterface[] getValueRecords() { public CellValueRecordInterface[] getValueRecords() {
//noinspection deprecation
return _valuesAgg.getValueRecords(); return _valuesAgg.getValueRecords();
} }

View File

@ -43,7 +43,6 @@ import org.apache.poi.util.LittleEndianOutput;
public abstract class Ptg { public abstract class Ptg {
public static final Ptg[] EMPTY_PTG_ARRAY = { }; public static final Ptg[] EMPTY_PTG_ARRAY = { };
/** /**
* Reads <tt>size</tt> bytes of the input stream, to create an array of <tt>Ptg</tt>s. * Reads <tt>size</tt> bytes of the input stream, to create an array of <tt>Ptg</tt>s.
* Extra data (beyond <tt>size</tt>) may be read if and <tt>ArrayPtg</tt>s are present. * Extra data (beyond <tt>size</tt>) may be read if and <tt>ArrayPtg</tt>s are present.
@ -174,8 +173,8 @@ public abstract class Ptg {
*/ */
public static int getEncodedSize(Ptg[] ptgs) { public static int getEncodedSize(Ptg[] ptgs) {
int result = 0; int result = 0;
for (int i = 0; i < ptgs.length; i++) { for (Ptg ptg : ptgs) {
result += ptgs[i].getSize(); result += ptg.getSize();
} }
return result; return result;
} }
@ -185,8 +184,7 @@ public abstract class Ptg {
*/ */
public static int getEncodedSizeWithoutArrayData(Ptg[] ptgs) { public static int getEncodedSizeWithoutArrayData(Ptg[] ptgs) {
int result = 0; int result = 0;
for (int i = 0; i < ptgs.length; i++) { for (Ptg ptg : ptgs) {
Ptg ptg = ptgs[i];
if (ptg instanceof ArrayPtg) { if (ptg instanceof ArrayPtg) {
result += ArrayPtg.PLAIN_TOKEN_SIZE; result += ArrayPtg.PLAIN_TOKEN_SIZE;
} else { } else {
@ -203,15 +201,11 @@ public abstract class Ptg {
* @return number of bytes written * @return number of bytes written
*/ */
public static int serializePtgs(Ptg[] ptgs, byte[] array, int offset) { public static int serializePtgs(Ptg[] ptgs, byte[] array, int offset) {
int nTokens = ptgs.length;
LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(array, offset); LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(array, offset);
List<Ptg> arrayPtgs = null; List<Ptg> arrayPtgs = null;
for (int k = 0; k < nTokens; k++) { for (Ptg ptg : ptgs) {
Ptg ptg = ptgs[k];
ptg.write(out); ptg.write(out);
if (ptg instanceof ArrayPtg) { if (ptg instanceof ArrayPtg) {
if (arrayPtgs == null) { if (arrayPtgs == null) {
@ -221,8 +215,8 @@ public abstract class Ptg {
} }
} }
if (arrayPtgs != null) { if (arrayPtgs != null) {
for (int i=0;i<arrayPtgs.size();i++) { for (Ptg arrayPtg : arrayPtgs) {
ArrayPtg p = (ArrayPtg)arrayPtgs.get(i); ArrayPtg p = (ArrayPtg) arrayPtg;
p.writeTokenValueBytes(out); p.writeTokenValueBytes(out);
} }
} }
@ -294,13 +288,14 @@ public abstract class Ptg {
public abstract boolean isBaseToken(); public abstract boolean isBaseToken();
public static boolean doesFormulaReferToDeletedCell(Ptg[] ptgs) { public static boolean doesFormulaReferToDeletedCell(Ptg[] ptgs) {
for (int i = 0; i < ptgs.length; i++) { for (Ptg ptg : ptgs) {
if (isDeletedCellRef(ptgs[i])) { if (isDeletedCellRef(ptg)) {
return true; return true;
} }
} }
return false; return false;
} }
private static boolean isDeletedCellRef(Ptg ptg) { private static boolean isDeletedCellRef(Ptg ptg) {
if (ptg == ErrPtg.REF_INVALID) { if (ptg == ErrPtg.REF_INVALID) {
return true; return true;

View File

@ -18,10 +18,6 @@
*/ */
package org.apache.poi.ss.formula; package org.apache.poi.ss.formula;
import java.io.File;
import java.io.FileOutputStream;
import java.util.Locale;
import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook; import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook;
import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.ss.formula.ptg.AbstractFunctionPtg; import org.apache.poi.ss.formula.ptg.AbstractFunctionPtg;
@ -49,6 +45,7 @@ public class TestFormulaParser extends TestCase {
fail("Expected exception"); fail("Expected exception");
} }
catch (FormulaParseException expected) { catch (FormulaParseException expected) {
// expected here
} }
} }
@ -69,6 +66,7 @@ public class TestFormulaParser extends TestCase {
fail("Expected exception"); fail("Expected exception");
} }
catch (FormulaParseException expected) { catch (FormulaParseException expected) {
// expected here
} }
} }
@ -150,8 +148,6 @@ public class TestFormulaParser extends TestCase {
} }
/** confirm formula has invalid syntax and parsing the formula results in FormulaParseException /** confirm formula has invalid syntax and parsing the formula results in FormulaParseException
* @param formula
* @param wb
*/ */
private static void parseExpectedException(String formula, FormulaParsingWorkbook wb) { private static void parseExpectedException(String formula, FormulaParsingWorkbook wb) {
try { try {
@ -162,5 +158,4 @@ public class TestFormulaParser extends TestCase {
assertNotNull(e.getMessage()); assertNotNull(e.getMessage());
} }
} }
} }

View File

@ -48,12 +48,11 @@ public class TestPOIFSDump {
"-dump-ministream", "-dump-ministream",
"-dump-mini-stream", "-dump-mini-stream",
}; };
private static final File DUMP_DIR = new File("Root Entry");
@After @After
public void tearDown() throws IOException { public void tearDown() throws IOException {
// clean up the directory that POIFSDump writes to // clean up the directory that POIFSDump writes to
deleteDirectory(DUMP_DIR); deleteDirectory(new File(new File(TEST_FILE).getName()));
} }
public static void deleteDirectory(File directory) throws IOException { public static void deleteDirectory(File directory) throws IOException {

View File

@ -17,25 +17,16 @@
package org.apache.poi.ss.formula.eval; package org.apache.poi.ss.formula.eval;
import static org.junit.Assert.assertEquals; import org.apache.poi.hssf.HSSFTestDataSamples;
import static org.junit.Assert.fail; import org.apache.poi.hssf.usermodel.*;
import org.apache.poi.ss.usermodel.*;
import org.junit.Test;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import org.apache.poi.hssf.HSSFTestDataSamples; import static org.junit.Assert.assertEquals;
import org.apache.poi.hssf.usermodel.HSSFCell; import static org.junit.Assert.fail;
import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
import org.apache.poi.hssf.usermodel.HSSFRow;
import org.apache.poi.hssf.usermodel.HSSFSheet;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellValue;
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.Test;
/** /**
* Miscellaneous tests for bugzilla entries.<p/> The test name contains the * Miscellaneous tests for bugzilla entries.<p/> The test name contains the
@ -53,21 +44,21 @@ public final class TestFormulaBugs {
InputStream is = HSSFTestDataSamples.openSampleFileStream("27349-vlookupAcrossSheets.xls"); InputStream is = HSSFTestDataSamples.openSampleFileStream("27349-vlookupAcrossSheets.xls");
// original bug may have thrown exception here, // original bug may have thrown exception here,
// or output warning to stderr // or output warning to stderr
HSSFWorkbook wb = new HSSFWorkbook(is); Workbook wb = new HSSFWorkbook(is);
HSSFSheet sheet = wb.getSheetAt(0); Sheet sheet = wb.getSheetAt(0);
HSSFRow row = sheet.getRow(1); Row row = sheet.getRow(1);
HSSFCell cell = row.getCell(0); Cell cell = row.getCell(0);
// this definitely would have failed due to 27349 // this definitely would have failed due to 27349
assertEquals("VLOOKUP(1,'DATA TABLE'!$A$8:'DATA TABLE'!$B$10,2)", cell assertEquals("VLOOKUP(1,'DATA TABLE'!$A$8:'DATA TABLE'!$B$10,2)", cell
.getCellFormula()); .getCellFormula());
// We might as well evaluate the formula // We might as well evaluate the formula
HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
CellValue cv = fe.evaluate(cell); CellValue cv = fe.evaluate(cell);
assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cv.getCellType()); assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
assertEquals(3.0, cv.getNumberValue(), 0.0); assertEquals(3.0, cv.getNumberValue(), 0.0);
wb.close(); wb.close();
@ -81,13 +72,12 @@ public final class TestFormulaBugs {
*/ */
@Test @Test
public void test27405() throws Exception { public void test27405() throws Exception {
Workbook wb = new HSSFWorkbook();
HSSFWorkbook wb = new HSSFWorkbook(); Sheet sheet = wb.createSheet("input");
HSSFSheet sheet = wb.createSheet("input");
// input row 0 // input row 0
HSSFRow row = sheet.createRow(0); Row row = sheet.createRow(0);
HSSFCell cell = row.createCell(0); /*Cell cell =*/ row.createCell(0);
cell = row.createCell(1); Cell cell = row.createCell(1);
cell.setCellValue(1); // B1 cell.setCellValue(1); // B1
// input row 1 // input row 1
row = sheet.createRow(1); row = sheet.createRow(1);
@ -113,14 +103,14 @@ public final class TestFormulaBugs {
// } // }
// use POI's evaluator as an extra sanity check // use POI's evaluator as an extra sanity check
HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
CellValue cv; CellValue cv;
cv = fe.evaluate(cell); cv = fe.evaluate(cell);
assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cv.getCellType()); assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
assertEquals(1.0, cv.getNumberValue(), 0.0); assertEquals(1.0, cv.getNumberValue(), 0.0);
cv = fe.evaluate(row.getCell(1)); cv = fe.evaluate(row.getCell(1));
assertEquals(HSSFCell.CELL_TYPE_BOOLEAN, cv.getCellType()); assertEquals(Cell.CELL_TYPE_BOOLEAN, cv.getCellType());
assertEquals(true, cv.getBooleanValue()); assertEquals(true, cv.getBooleanValue());
wb.close(); wb.close();
@ -132,14 +122,14 @@ public final class TestFormulaBugs {
*/ */
@Test @Test
public void test42448() throws IOException { public void test42448() throws IOException {
HSSFWorkbook wb = new HSSFWorkbook(); Workbook wb = new HSSFWorkbook();
HSSFSheet sheet1 = wb.createSheet("Sheet1"); Sheet sheet1 = wb.createSheet("Sheet1");
HSSFRow row = sheet1.createRow(0); Row row = sheet1.createRow(0);
HSSFCell cell = row.createCell(0); Cell cell = row.createCell(0);
// it's important to create the referenced sheet first // it's important to create the referenced sheet first
HSSFSheet sheet2 = wb.createSheet("A"); // note name 'A' Sheet sheet2 = wb.createSheet("A"); // note name 'A'
// TODO - POI crashes if the formula is added before this sheet // TODO - POI crashes if the formula is added before this sheet
// RuntimeException("Zero length string is an invalid sheet name") // RuntimeException("Zero length string is an invalid sheet name")
// Excel doesn't crash but the formula doesn't work until it is // Excel doesn't crash but the formula doesn't work until it is
@ -168,16 +158,16 @@ public final class TestFormulaBugs {
double expectedResult = (4.0 * 8.0 + 5.0 * 9.0) / 10.0; double expectedResult = (4.0 * 8.0 + 5.0 * 9.0) / 10.0;
HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
CellValue cv = fe.evaluate(cell); CellValue cv = fe.evaluate(cell);
assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cv.getCellType()); assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
assertEquals(expectedResult, cv.getNumberValue(), 0.0); assertEquals(expectedResult, cv.getNumberValue(), 0.0);
wb.close(); wb.close();
} }
private static void addCell(HSSFSheet sheet, int rowIx, int colIx, private static void addCell(Sheet sheet, int rowIx, int colIx,
double value) { double value) {
sheet.createRow(rowIx).createCell(colIx).setCellValue(value); sheet.createRow(rowIx).createCell(colIx).setCellValue(value);
} }