From: Dominik Stadler Date: Sun, 13 Jan 2019 17:14:24 +0000 (+0000) Subject: Fix some Findbugs and IDE issues, refactor some duplicated code, X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3b20c943bf1cbdc65fb0c9dac0cf48d08054f543;p=poi.git Fix some Findbugs and IDE issues, refactor some duplicated code, improve some exception texts, add comment for missing Ptg for SxName git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1851210 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/integrationtest/org/apache/poi/stress/AbstractFileHandler.java b/src/integrationtest/org/apache/poi/stress/AbstractFileHandler.java index 7f9c22ff41..fbe37a4b0b 100644 --- a/src/integrationtest/org/apache/poi/stress/AbstractFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/AbstractFileHandler.java @@ -37,6 +37,10 @@ import org.apache.poi.openxml4j.exceptions.OpenXML4JException; import org.apache.poi.util.IOUtils; import org.apache.xmlbeans.XmlException; +/** + * Base class with things that can be run for any supported file handler + * in the integration tests, mostly text-extraction related at the moment. + */ public abstract class AbstractFileHandler implements FileHandler { public static final Set EXPECTED_EXTRACTOR_FAILURES = new HashSet<>(); static { diff --git a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java index 75fd4a5e35..119175b82e 100644 --- a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java @@ -88,7 +88,7 @@ public class HSSFFileHandler extends SpreadsheetHandler { try { System.setOut(new PrintStream(new OutputStream() { @Override - public void write(int b) throws IOException { + public void write(int b) { } })); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java index 9f5461cff3..c9b3016dfa 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java @@ -28,8 +28,8 @@ import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.ss.usermodel.CellType; import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.usermodel.helpers.RowShifter; import org.apache.poi.util.Configurator; -import org.apache.poi.util.LocaleUtil; /** * High level representation of a row of a spreadsheet. @@ -725,11 +725,8 @@ public final class HSSFRow implements Row, Comparable { */ @Override public void shiftCellsRight(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { - if(step < 0) - throw new IllegalArgumentException("Shifting step may not be negative "); - if(firstShiftColumnIndex > lastShiftColumnIndex) - throw new IllegalArgumentException(String.format(LocaleUtil.getUserLocale(), - "Incorrect shifting range : %d-%d", firstShiftColumnIndex, lastShiftColumnIndex)); + RowShifter.validateShiftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step); + if(lastShiftColumnIndex + step + 1> cells.length) extend(lastShiftColumnIndex + step + 1); for (int columnIndex = lastShiftColumnIndex; columnIndex >= firstShiftColumnIndex; columnIndex--){ // process cells backwards, because of shifting @@ -746,6 +743,7 @@ public final class HSSFRow implements Row, Comparable { cells = new HSSFCell[newLenght]; System.arraycopy(temp, 0, cells, 0, temp.length); } + /** * Shifts column range [firstShiftColumnIndex-lastShiftColumnIndex] step places to the left. * @param firstShiftColumnIndex the column to start shifting @@ -754,13 +752,8 @@ public final class HSSFRow implements Row, Comparable { */ @Override public void shiftCellsLeft(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { - if(step < 0) - throw new IllegalArgumentException("Shifting step may not be negative "); - if(firstShiftColumnIndex > lastShiftColumnIndex) - throw new IllegalArgumentException(String.format(LocaleUtil.getUserLocale(), - "Incorrect shifting range : %d-%d", firstShiftColumnIndex, lastShiftColumnIndex)); - if(firstShiftColumnIndex - step < 0) - throw new IllegalStateException("Column index less than zero : " + (Integer.valueOf(firstShiftColumnIndex + step)).toString()); + RowShifter.validateShiftLeftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step); + for (int columnIndex = firstShiftColumnIndex; columnIndex <= lastShiftColumnIndex; columnIndex++){ HSSFCell cell = getCell(columnIndex); if(cell != null){ diff --git a/src/java/org/apache/poi/sl/draw/geom/PresetGeometries.java b/src/java/org/apache/poi/sl/draw/geom/PresetGeometries.java index f8b461f929..c010defc0b 100644 --- a/src/java/org/apache/poi/sl/draw/geom/PresetGeometries.java +++ b/src/java/org/apache/poi/sl/draw/geom/PresetGeometries.java @@ -26,12 +26,10 @@ import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBElement; import javax.xml.bind.JAXBException; import javax.xml.bind.Unmarshaller; -import javax.xml.stream.EventFilter; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamConstants; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; -import javax.xml.stream.events.XMLEvent; import javax.xml.transform.stream.StreamSource; import org.apache.poi.sl.draw.binding.CTCustomGeometry2D; @@ -52,14 +50,6 @@ public class PresetGeometries extends LinkedHashMap { @SuppressWarnings("unused") public void init(InputStream is) throws XMLStreamException, JAXBException { - // StAX: - EventFilter startElementFilter = new EventFilter() { - @Override - public boolean accept(XMLEvent event) { - return event.isStartElement(); - } - }; - XMLInputFactory staxFactory = StaxHelper.newXMLInputFactory(); XMLStreamReader streamReader = staxFactory.createXMLStreamReader(new StreamSource(is)); try { @@ -108,12 +98,9 @@ public class PresetGeometries extends LinkedHashMap { // in case of failure PresetGeometries lInst = new PresetGeometries(); try { - InputStream is = PresetGeometries.class. - getResourceAsStream("presetShapeDefinitions.xml"); - try { + try (InputStream is = PresetGeometries.class. + getResourceAsStream("presetShapeDefinitions.xml")) { lInst.init(is); - } finally { - is.close(); } } catch (Exception e){ throw new RuntimeException(e); diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java index bd0c9995bf..ce1af447a0 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaParser.java +++ b/src/java/org/apache/poi/ss/formula/FormulaParser.java @@ -203,7 +203,7 @@ public final class FormulaParser { final int sheetIndex = -1; //don't care? Ptg[] arr = FormulaParser.parse(tableText, workbook, FormulaType.CELL, sheetIndex, rowIndex); if (arr.length != 1 || !(arr[0] instanceof Area3DPxg) ) { - throw new IllegalStateException("Illegal structured reference"); + throw new IllegalStateException("Illegal structured reference, had length: " + arr.length); } return (Area3DPxg) arr[0]; } @@ -211,7 +211,7 @@ public final class FormulaParser { /** Read New Character From Input Stream */ private void GetChar() { // The intersection operator is a space. We track whether the run of - // whitespace preceeding "look" counts as an intersection operator. + // whitespace preceding "look" counts as an intersection operator. if (IsWhite(look)) { if (look == ' ') { _inIntersection = true; @@ -223,7 +223,8 @@ public final class FormulaParser { // Check to see if we've walked off the end of the string. if (_pointer > _formulaLength) { - throw new RuntimeException("too far"); + throw new RuntimeException("Parsed past the end of the formula, pos: " + _pointer + + ", length: " + _formulaLength + ", formula: " + _formulaString); } if (_pointer < _formulaLength) { look=_formulaString.codePointAt(_pointer); @@ -650,18 +651,24 @@ public final class FormulaParser { resetPointer(savePtr1); break; } - if (specName.equals(specAll)) { - isAllSpec = true; - } else if (specName.equals(specData)) { - isDataSpec = true; - } else if (specName.equals(specHeaders)) { - isHeadersSpec = true; - } else if (specName.equals(specThisRow)) { - isThisRowSpec = true; - } else if (specName.equals(specTotals)) { - isTotalsSpec = true; - } else { - throw new FormulaParseException("Unknown special quantifier "+ specName); + switch (specName) { + case specAll: + isAllSpec = true; + break; + case specData: + isDataSpec = true; + break; + case specHeaders: + isHeadersSpec = true; + break; + case specThisRow: + isThisRowSpec = true; + break; + case specTotals: + isTotalsSpec = true; + break; + default: + throw new FormulaParseException("Unknown special quantifier " + specName); } nSpecQuantifiers++; if (look == ','){ @@ -687,13 +694,13 @@ public final class FormulaParser { } else { nColQuantifiers++; if (look == ','){ - throw new FormulaParseException("The formula "+ _formulaString + "is illegal: you should not use ',' with column quantifiers"); + throw new FormulaParseException("The formula "+ _formulaString + " is illegal: you should not use ',' with column quantifiers"); } else if (look == ':') { GetChar(); endColumnName = parseAsColumnQuantifier(); nColQuantifiers++; if (endColumnName == null) { - throw new FormulaParseException("The formula "+ _formulaString + "is illegal: the string after ':' must be column quantifier"); + throw new FormulaParseException("The formula "+ _formulaString + " is illegal: the string after ':' must be column quantifier"); } } } @@ -708,18 +715,24 @@ public final class FormulaParser { resetPointer(savePtr0); String name = parseAsSpecialQuantifier(); if (name!=null) { - if (name.equals(specAll)) { - isAllSpec = true; - } else if (name.equals(specData)) { - isDataSpec = true; - } else if (name.equals(specHeaders)) { - isHeadersSpec = true; - } else if (name.equals(specThisRow)) { - isThisRowSpec = true; - } else if (name.equals(specTotals)) { - isTotalsSpec = true; - } else { - throw new FormulaParseException("Unknown special quantifier "+ name); + switch (name) { + case specAll: + isAllSpec = true; + break; + case specData: + isDataSpec = true; + break; + case specHeaders: + isHeadersSpec = true; + break; + case specThisRow: + isThisRowSpec = true; + break; + case specTotals: + isTotalsSpec = true; + break; + default: + throw new FormulaParseException("Unknown special quantifier " + name); } nSpecQuantifiers++; } else { @@ -1344,7 +1357,6 @@ public final class FormulaParser { /** * Adds a name (named range or user defined function) to underlying workbook's names table - * @param functionName */ private void addName(String functionName) { final Name name = _book.createName(); @@ -1878,13 +1890,12 @@ public final class FormulaParser { boolean hasUnions = false; while (true) { SkipWhite(); - switch(look) { - case ',': - GetChar(); - hasUnions = true; - ParseNode other = intersectionExpression(); - result = new ParseNode(UnionPtg.instance, result, other); - continue; + if (look == ',') { + GetChar(); + hasUnions = true; + ParseNode other = intersectionExpression(); + result = new ParseNode(UnionPtg.instance, result, other); + continue; } if (hasUnions) { return augmentWithMemPtg(result); diff --git a/src/java/org/apache/poi/ss/formula/ptg/Ptg.java b/src/java/org/apache/poi/ss/formula/ptg/Ptg.java index ed7c1d4c30..a7649482e7 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/Ptg.java +++ b/src/java/org/apache/poi/ss/formula/ptg/Ptg.java @@ -145,6 +145,7 @@ public abstract class Ptg { case MissingArgPtg.sid: return MissingArgPtg.instance; // 0x16 case StringPtg.sid: return new StringPtg(in); // 0x17 + // not implemented yet: case SxNamePtg.sid: return new SxNamePtg(in); // 0x18 case AttrPtg.sid: return new AttrPtg(in); // 0x19 case ErrPtg.sid: return ErrPtg.read(in); // 0x1c case BoolPtg.sid: return BoolPtg.read(in); // 0x1d diff --git a/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java b/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java index a27a474196..2c4b3fa507 100644 --- a/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java +++ b/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java @@ -24,6 +24,7 @@ import java.util.Set; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.util.CellRangeAddress; +import org.apache.poi.util.LocaleUtil; /** * Helper for shifting rows up or down @@ -116,4 +117,36 @@ public abstract class RowShifter extends BaseRowColShifter { // if the merged-region and the overwritten area intersect, we need to remove it return merged.intersects(overwrite); } + + /** + * Verify that the given column indices and step denote a valid range of columns to shift + * + * @param firstShiftColumnIndex the column to start shifting + * @param lastShiftColumnIndex the column to end shifting + * @param step length of the shifting step + */ + public static void validateShiftParameters(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { + if(step < 0) { + throw new IllegalArgumentException("Shifting step may not be negative, but had " + step); + } + if(firstShiftColumnIndex > lastShiftColumnIndex) { + throw new IllegalArgumentException(String.format(LocaleUtil.getUserLocale(), + "Incorrect shifting range : %d-%d", firstShiftColumnIndex, lastShiftColumnIndex)); + } + } + + /** + * Verify that the given column indices and step denote a valid range of columns to shift to the left + * + * @param firstShiftColumnIndex the column to start shifting + * @param lastShiftColumnIndex the column to end shifting + * @param step length of the shifting step + */ + public static void validateShiftLeftParameters(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { + validateShiftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step); + + if(firstShiftColumnIndex - step < 0) { + throw new IllegalStateException("Column index less than zero: " + (firstShiftColumnIndex + step)); + } + } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index e336ae458b..47ccb646bc 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -1323,4 +1323,4 @@ public final class XSSFCell extends CellBase { } } - + diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java index 363fc5bd9b..e7671c5968 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java @@ -29,10 +29,10 @@ import org.apache.poi.ss.usermodel.CellCopyPolicy; import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.ss.usermodel.CellType; import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.usermodel.helpers.RowShifter; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.util.Beta; import org.apache.poi.util.Internal; -import org.apache.poi.util.LocaleUtil; import org.apache.poi.xssf.model.StylesTable; import org.apache.poi.xssf.usermodel.helpers.XSSFRowShifter; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell; @@ -634,13 +634,8 @@ public class XSSFRow implements Row, Comparable { */ @Override public void shiftCellsRight(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { - if(step < 0) { - throw new IllegalArgumentException("Shifting step may not be negative "); - } - if(firstShiftColumnIndex > lastShiftColumnIndex) { - throw new IllegalArgumentException(String.format(LocaleUtil.getUserLocale(), - "Incorrect shifting range : %d-%d", firstShiftColumnIndex, lastShiftColumnIndex)); - } + RowShifter.validateShiftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step); + for (int columnIndex = lastShiftColumnIndex; columnIndex >= firstShiftColumnIndex; columnIndex--){ // process cells backwards, because of shifting shiftCell(columnIndex, step); } @@ -653,6 +648,7 @@ public class XSSFRow implements Row, Comparable { } } } + /** * Shifts column range [firstShiftColumnIndex-lastShiftColumnIndex] step places to the left. * @param firstShiftColumnIndex the column to start shifting @@ -661,16 +657,8 @@ public class XSSFRow implements Row, Comparable { */ @Override public void shiftCellsLeft(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) { - if(step < 0) { - throw new IllegalArgumentException("Shifting step may not be negative "); - } - if(firstShiftColumnIndex > lastShiftColumnIndex) { - throw new IllegalArgumentException(String.format(LocaleUtil.getUserLocale(), - "Incorrect shifting range : %d-%d", firstShiftColumnIndex, lastShiftColumnIndex)); - } - if(firstShiftColumnIndex - step < 0) { - throw new IllegalStateException("Column index less than zero : " + (Integer.valueOf(firstShiftColumnIndex + step)).toString()); - } + RowShifter.validateShiftLeftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step); + for (int columnIndex = firstShiftColumnIndex; columnIndex <= lastShiftColumnIndex; columnIndex++){ shiftCell(columnIndex, -step); } @@ -682,6 +670,7 @@ public class XSSFRow implements Row, Comparable { } } } + private void shiftCell(int columnIndex, int step/*pass negative value for left shift*/){ if(columnIndex + step < 0) { throw new IllegalStateException("Column index less than zero : " + (Integer.valueOf(columnIndex + step)).toString()); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestSXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestSXSSFBugs.java index 73d56c7a77..79d805b6ef 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestSXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestSXSSFBugs.java @@ -17,8 +17,9 @@ package org.apache.poi.xssf.usermodel; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.File; @@ -72,8 +73,8 @@ public final class TestSXSSFBugs extends BaseTestBugzillaIssues { s1.setRepeatingRows(cra); PrintSetup ps1 = s1.getPrintSetup(); - assertEquals(false, ps1.getValidSettings()); - assertEquals(false, ps1.getLandscape()); + assertFalse(ps1.getValidSettings()); + assertFalse(ps1.getLandscape()); // Had valid print settings before repeating @@ -81,14 +82,14 @@ public final class TestSXSSFBugs extends BaseTestBugzillaIssues { PrintSetup ps2 = s2.getPrintSetup(); ps2.setLandscape(false); - assertEquals(true, ps2.getValidSettings()); - assertEquals(false, ps2.getLandscape()); + assertTrue(ps2.getValidSettings()); + assertFalse(ps2.getLandscape()); s2.setRepeatingColumns(cra); s2.setRepeatingRows(cra); ps2 = s2.getPrintSetup(); - assertEquals(true, ps2.getValidSettings()); - assertEquals(false, ps2.getLandscape()); + assertTrue(ps2.getValidSettings()); + assertFalse(ps2.getLandscape()); wb1.close(); wb2.close(); @@ -137,7 +138,7 @@ public final class TestSXSSFBugs extends BaseTestBugzillaIssues { Cell cell = row.createCell(colIndex++); cell.setCellType(CellType.STRING); cell.setCellValue("multiple"); - cell = row.createCell(colIndex++); + cell = row.createCell(colIndex); cell.setCellType(CellType.STRING); cell.setCellValue("unique"); @@ -146,7 +147,7 @@ public final class TestSXSSFBugs extends BaseTestBugzillaIssues { writeRow(sheet, rowIndex++, 30d, "IFERROR(INDEX(A2:A7, MATCH(1, (COUNTIF(B2:B3, A2:A7) = 0) * (NOT(ISBLANK(A2:A7))), 0)), \"\")"); writeRow(sheet, rowIndex++, 2d, "IFERROR(INDEX(A2:A7, MATCH(1, (COUNTIF(B2:B4, A2:A7) = 0) * (NOT(ISBLANK(A2:A7))), 0)), \"\")"); writeRow(sheet, rowIndex++, 30d, "IFERROR(INDEX(A2:A7, MATCH(1, (COUNTIF(B2:B5, A2:A7) = 0) * (NOT(ISBLANK(A2:A7))), 0)), \"\")"); - writeRow(sheet, rowIndex++, 2d, "IFERROR(INDEX(A2:A7, MATCH(1, (COUNTIF(B2:B6, A2:A7) = 0) * (NOT(ISBLANK(A2:A7))), 0)), \"\")"); + writeRow(sheet, rowIndex, 2d, "IFERROR(INDEX(A2:A7, MATCH(1, (COUNTIF(B2:B6, A2:A7) = 0) * (NOT(ISBLANK(A2:A7))), 0)), \"\")"); /*FileOutputStream fileOut = new FileOutputStream(filename); wb.write(fileOut); @@ -211,6 +212,5 @@ public final class TestSXSSFBugs extends BaseTestBugzillaIssues { workbook.close(); out.flush(); } - // logger.info("File written!"); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java index fccfd7cf85..912ebdd49e 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaParser.java @@ -19,6 +19,7 @@ package org.apache.poi.xssf.usermodel; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -180,7 +181,7 @@ public final class TestXSSFFormulaParser { assertEquals(1, ptgs.length); assertEquals(NameXPxg.class, ptgs[0].getClass()); assertEquals(0, ((NameXPxg)ptgs[0]).getExternalWorkbookNumber()); - assertEquals(null, ((NameXPxg)ptgs[0]).getSheetName()); + assertNull(((NameXPxg) ptgs[0]).getSheetName()); assertEquals("NR_Global_B2",((NameXPxg)ptgs[0]).getNameName()); assertEquals("[0]!NR_Global_B2", ptgs[0].toFormulaString()); @@ -259,7 +260,7 @@ public final class TestXSSFFormulaParser { assertEquals(1, ptgs.length); assertEquals(NameXPxg.class, ptgs[0].getClass()); assertEquals(1, ((NameXPxg)ptgs[0]).getExternalWorkbookNumber()); - assertEquals(null, ((NameXPxg)ptgs[0]).getSheetName()); + assertNull(((NameXPxg) ptgs[0]).getSheetName()); assertEquals("NR_Global_B2",((NameXPxg)ptgs[0]).getNameName()); assertEquals("[1]!NR_Global_B2", ptgs[0].toFormulaString()); diff --git a/src/scratchpad/src/org/apache/poi/hwmf/record/HwmfWindowing.java b/src/scratchpad/src/org/apache/poi/hwmf/record/HwmfWindowing.java index 8759a5c7e6..4b1862286b 100644 --- a/src/scratchpad/src/org/apache/poi/hwmf/record/HwmfWindowing.java +++ b/src/scratchpad/src/org/apache/poi/hwmf/record/HwmfWindowing.java @@ -172,9 +172,7 @@ public class HwmfWindowing { public void draw(HwmfGraphics ctx) { final HwmfDrawProperties prop = ctx.getProperties(); final Rectangle2D old = prop.getWindow(); - double oldX = (old == null ? 0 : old.getX()); - double oldY = (old == null ? 0 : old.getY()); - if (oldX != getX() || oldY != getY()) { + if (old.getX() != getX() || old.getY() != getY()) { prop.setWindowOrg(getX(), getY()); ctx.updateWindowMapMode(); } diff --git a/src/testcases/org/apache/poi/ss/formula/eval/forked/TestForkedEvaluator.java b/src/testcases/org/apache/poi/ss/formula/eval/forked/TestForkedEvaluator.java index 3b9a53ff48..e27499ce39 100644 --- a/src/testcases/org/apache/poi/ss/formula/eval/forked/TestForkedEvaluator.java +++ b/src/testcases/org/apache/poi/ss/formula/eval/forked/TestForkedEvaluator.java @@ -18,6 +18,7 @@ package org.apache.poi.ss.formula.eval.forked; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import java.io.IOException; @@ -68,12 +69,7 @@ public class TestForkedEvaluator { Workbook wb = createWorkbook(); // The stability classifier is useful to reduce memory consumption of caching logic - IStabilityClassifier stabilityClassifier = new IStabilityClassifier() { - @Override - public boolean isCellFinal(int sheetIndex, int rowIndex, int columnIndex) { - return sheetIndex == 1; - } - }; + IStabilityClassifier stabilityClassifier = (sheetIndex, rowIndex, columnIndex) -> sheetIndex == 1; ForkedEvaluator fe1 = ForkedEvaluator.create(wb, stabilityClassifier, null); ForkedEvaluator fe2 = ForkedEvaluator.create(wb, stabilityClassifier, null); @@ -86,10 +82,16 @@ public class TestForkedEvaluator { fe2.updateCell("Inputs", 0, 0, new NumberEval(1.2)); fe2.updateCell("Inputs", 0, 1, new NumberEval(2.0)); - assertEquals(18.9, ((NumberEval) fe1.evaluate("Calculations", 0, 0)).getNumberValue(), 0.0); - assertEquals(4.0, ((NumberEval) fe2.evaluate("Calculations", 0, 0)).getNumberValue(), 0.0); + NumberEval eval1 = (NumberEval) fe1.evaluate("Calculations", 0, 0); + assertNotNull(eval1); + assertEquals(18.9, eval1.getNumberValue(), 0.0); + NumberEval eval2 = (NumberEval) fe2.evaluate("Calculations", 0, 0); + assertNotNull(eval2); + assertEquals(4.0, eval2.getNumberValue(), 0.0); fe1.updateCell("Inputs", 0, 0, new NumberEval(3.0)); - assertEquals(13.9, ((NumberEval) fe1.evaluate("Calculations", 0, 0)).getNumberValue(), 0.0); + eval1 = (NumberEval) fe1.evaluate("Calculations", 0, 0); + assertNotNull(eval1); + assertEquals(13.9, eval1.getNumberValue(), 0.0); wb.close(); } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java index 77e3ef80b5..c924a85674 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java @@ -37,6 +37,7 @@ import java.util.Map; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.ITestDataProvider; import org.apache.poi.ss.SpreadsheetVersion; +import org.apache.poi.ss.formula.FormulaParseException; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellRangeAddressList; import org.apache.poi.ss.util.PaneInformation; @@ -353,7 +354,7 @@ public abstract class BaseTestBugzillaIssues { fmla = createFunction(name, ssVersion.getMaxFunctionArgs() + 1); cell.setCellFormula(fmla); fail("Expected FormulaParseException"); - } catch (RuntimeException e){ + } catch (FormulaParseException e){ assertTrue(e.getMessage().startsWith("Too many arguments to function '"+name+"'")); } } @@ -464,8 +465,10 @@ public abstract class BaseTestBugzillaIssues { double leadingWhitespaceRatio = ((double) leadingWhitespaceColWidth)/noWhitespaceColWidth; double trailingWhitespaceRatio = ((double) leadingWhitespaceColWidth)/noWhitespaceColWidth; - assertGreaterThan("leading whitespace is longer than no whitespace", leadingWhitespaceRatio, expectedRatioThreshold); - assertGreaterThan("trailing whitespace is longer than no whitespace", trailingWhitespaceRatio, expectedRatioThreshold); + assertGreaterThan("leading whitespace is longer than no whitespace", + leadingWhitespaceRatio, expectedRatioThreshold); + assertGreaterThan("trailing whitespace is longer than no whitespace", + trailingWhitespaceRatio, expectedRatioThreshold); assertEquals("cells with equal leading and trailing whitespace have equal width", leadingWhitespaceColWidth, trailingWhitespaceColWidth); diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java index ae88bf63a9..6e663c37ee 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -258,25 +259,25 @@ public abstract class BaseTestRow { // First up, no policy given, uses default assertEquals(CellType.STRING, row.getCell(0).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(1).getCellType()); - assertEquals(null, row.getCell(2)); - assertEquals(null, row.getCell(3)); + assertNull(row.getCell(2)); + assertNull(row.getCell(3)); assertEquals(CellType.BLANK, row.getCell(4).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(5).getCellType()); // RETURN_NULL_AND_BLANK - same as default assertEquals(CellType.STRING, row.getCell(0, MissingCellPolicy.RETURN_NULL_AND_BLANK).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(1, MissingCellPolicy.RETURN_NULL_AND_BLANK).getCellType()); - assertEquals(null, row.getCell(2, MissingCellPolicy.RETURN_NULL_AND_BLANK)); - assertEquals(null, row.getCell(3, MissingCellPolicy.RETURN_NULL_AND_BLANK)); + assertNull(row.getCell(2, MissingCellPolicy.RETURN_NULL_AND_BLANK)); + assertNull(row.getCell(3, MissingCellPolicy.RETURN_NULL_AND_BLANK)); assertEquals(CellType.BLANK, row.getCell(4, MissingCellPolicy.RETURN_NULL_AND_BLANK).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(5, MissingCellPolicy.RETURN_NULL_AND_BLANK).getCellType()); // RETURN_BLANK_AS_NULL - nearly the same assertEquals(CellType.STRING, row.getCell(0, MissingCellPolicy.RETURN_BLANK_AS_NULL).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(1, MissingCellPolicy.RETURN_BLANK_AS_NULL).getCellType()); - assertEquals(null, row.getCell(2, MissingCellPolicy.RETURN_BLANK_AS_NULL)); - assertEquals(null, row.getCell(3, MissingCellPolicy.RETURN_BLANK_AS_NULL)); - assertEquals(null, row.getCell(4, MissingCellPolicy.RETURN_BLANK_AS_NULL)); + assertNull(row.getCell(2, MissingCellPolicy.RETURN_BLANK_AS_NULL)); + assertNull(row.getCell(3, MissingCellPolicy.RETURN_BLANK_AS_NULL)); + assertNull(row.getCell(4, MissingCellPolicy.RETURN_BLANK_AS_NULL)); assertEquals(CellType.NUMERIC, row.getCell(5, MissingCellPolicy.RETURN_BLANK_AS_NULL).getCellType()); // CREATE_NULL_AS_BLANK - creates as needed @@ -302,9 +303,9 @@ public abstract class BaseTestRow { assertEquals(CellType.STRING, row.getCell(0).getCellType()); assertEquals(CellType.NUMERIC, row.getCell(1).getCellType()); - assertEquals(null, row.getCell(2)); - assertEquals(null, row.getCell(3)); - assertEquals(null, row.getCell(4)); + assertNull(row.getCell(2)); + assertNull(row.getCell(3)); + assertNull(row.getCell(4)); assertEquals(CellType.NUMERIC, row.getCell(5).getCellType()); workbook.close(); @@ -376,36 +377,36 @@ public abstract class BaseTestRow { Cell cell1 = row.createCell(1); Iterator it = row.cellIterator(); assertTrue(it.hasNext()); - assertTrue(cell1 == it.next()); + assertSame(cell1, it.next()); assertFalse(it.hasNext()); // Add another cell at the end Cell cell2 = row.createCell(99); it = row.cellIterator(); assertTrue(it.hasNext()); - assertTrue(cell1 == it.next()); + assertSame(cell1, it.next()); assertTrue(it.hasNext()); - assertTrue(cell2 == it.next()); + assertSame(cell2, it.next()); // Add another cell at the beginning Cell cell3 = row.createCell(0); it = row.cellIterator(); assertTrue(it.hasNext()); - assertTrue(cell3 == it.next()); + assertSame(cell3, it.next()); assertTrue(it.hasNext()); - assertTrue(cell1 == it.next()); + assertSame(cell1, it.next()); assertTrue(it.hasNext()); - assertTrue(cell2 == it.next()); + assertSame(cell2, it.next()); // Replace cell1 Cell cell4 = row.createCell(1); it = row.cellIterator(); assertTrue(it.hasNext()); - assertTrue(cell3 == it.next()); + assertSame(cell3, it.next()); assertTrue(it.hasNext()); - assertTrue(cell4 == it.next()); + assertSame(cell4, it.next()); assertTrue(it.hasNext()); - assertTrue(cell2 == it.next()); + assertSame(cell2, it.next()); assertFalse(it.hasNext()); // Add another cell, specifying the cellType @@ -413,13 +414,13 @@ public abstract class BaseTestRow { it = row.cellIterator(); assertNotNull(cell5); assertTrue(it.hasNext()); - assertTrue(cell3 == it.next()); + assertSame(cell3, it.next()); assertTrue(it.hasNext()); - assertTrue(cell4 == it.next()); + assertSame(cell4, it.next()); assertTrue(it.hasNext()); - assertTrue(cell5 == it.next()); + assertSame(cell5, it.next()); assertTrue(it.hasNext()); - assertTrue(cell2 == it.next()); + assertSame(cell2, it.next()); assertEquals(CellType.STRING, cell5.getCellType()); wb.close(); } @@ -432,10 +433,10 @@ public abstract class BaseTestRow { Row row2 = sheet.createRow(1); // Won't be styled currently - assertEquals(false, row1.isFormatted()); - assertEquals(false, row2.isFormatted()); - assertEquals(null, row1.getRowStyle()); - assertEquals(null, row2.getRowStyle()); + assertFalse(row1.isFormatted()); + assertFalse(row2.isFormatted()); + assertNull(row1.getRowStyle()); + assertNull(row2.getRowStyle()); // Style one CellStyle style = wb1.createCellStyle(); @@ -443,9 +444,9 @@ public abstract class BaseTestRow { row2.setRowStyle(style); // Check - assertEquals(false, row1.isFormatted()); - assertEquals(true, row2.isFormatted()); - assertEquals(null, row1.getRowStyle()); + assertFalse(row1.isFormatted()); + assertTrue(row2.isFormatted()); + assertNull(row1.getRowStyle()); assertEquals(style, row2.getRowStyle()); // Save, load and re-check @@ -458,9 +459,9 @@ public abstract class BaseTestRow { row2 = sheet.getRow(1); style = wb2.getCellStyleAt(style.getIndex()); - assertEquals(false, row1.isFormatted()); - assertEquals(true, row2.isFormatted()); - assertEquals(null, row1.getRowStyle()); + assertFalse(row1.isFormatted()); + assertTrue(row2.isFormatted()); + assertNull(row1.getRowStyle()); assertEquals(style, row2.getRowStyle()); assertEquals(4, style.getDataFormat()); diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java index 9b20c6b3c6..668166380e 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java @@ -71,6 +71,7 @@ public abstract class BaseTestSheetUpdateArrayFormulas { workbook.close(); } + /** * Set single-cell array formula */ diff --git a/src/testcases/org/apache/poi/ss/util/TestCellReference.java b/src/testcases/org/apache/poi/ss/util/TestCellReference.java index 9f9f14cbe4..c2f7dfa87a 100644 --- a/src/testcases/org/apache/poi/ss/util/TestCellReference.java +++ b/src/testcases/org/apache/poi/ss/util/TestCellReference.java @@ -465,7 +465,7 @@ public final class TestCellReference { @Test public void test62828() { - Workbook wb = new HSSFWorkbook(); + final Workbook wb = new HSSFWorkbook(); final Sheet sheet = wb.createSheet("Ctor test"); final String sheetName = sheet.getSheetName(); final Row row = sheet.createRow(0);