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

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<String> EXPECTED_EXTRACTOR_FAILURES = new HashSet<>();
     static {

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) {
 				}
 			}));
 

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<HSSFRow> {
      */
     @Override
     public void shiftCellsRight(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) {
-        if(step < 0)
+        RowShifter.validateShiftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step);
-            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(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<HSSFRow> {
         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<HSSFRow> {
      */
     @Override
     public void shiftCellsLeft(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) {
-        if(step < 0)
+        RowShifter.validateShiftLeftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step);
-            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());
         for (int columnIndex = firstShiftColumnIndex; columnIndex <= lastShiftColumnIndex; columnIndex++){ 
             HSSFCell cell = getCell(columnIndex);
             if(cell != null){

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<String, CustomGeometry> {
 
     @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<String, CustomGeometry> {
             // in case of failure
             PresetGeometries lInst = new PresetGeometries();
             try {
-                InputStream is = PresetGeometries.class.
+                try (InputStream is = PresetGeometries.class.
-                    getResourceAsStream("presetShapeDefinitions.xml");
+                        getResourceAsStream("presetShapeDefinitions.xml")) {
-                try {
                     lInst.init(is);
-                } finally {
-                    is.close();
                 }
             } catch (Exception e){
                 throw new RuntimeException(e);

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)) {
+            switch (specName) {
+                case specAll:
                     isAllSpec = true;
-            } else if (specName.equals(specData)) {
+                    break;
+                case specData:
                     isDataSpec = true;
-            } else if (specName.equals(specHeaders)) {
+                    break;
+                case specHeaders:
                     isHeadersSpec = true;
-            } else if (specName.equals(specThisRow)) {
+                    break;
+                case specThisRow:
                     isThisRowSpec = true;
-            } else if (specName.equals(specTotals)) {
+                    break;
+                case specTotals:
                     isTotalsSpec = true;
-            } else {
+                    break;
-                throw new FormulaParseException("Unknown special quantifier "+ specName);
+                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)) {
+                    switch (name) {
+                        case specAll:
                             isAllSpec = true;
-                    } else if (name.equals(specData)) {
+                            break;
+                        case specData:
                             isDataSpec = true;
-                    } else if (name.equals(specHeaders)) {
+                            break;
+                        case specHeaders:
                             isHeadersSpec = true;
-                    } else if (name.equals(specThisRow)) {
+                            break;
+                        case specThisRow:
                             isThisRowSpec = true;
-                    } else if (name.equals(specTotals)) {
+                            break;
+                        case specTotals:
                             isTotalsSpec = true;
-                    } else {
+                            break;
-                        throw new FormulaParseException("Unknown special quantifier "+ name);
+                        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,8 +1890,7 @@ public final class FormulaParser {
         boolean hasUnions = false;
         while (true) {
             SkipWhite();
-            switch(look) {
+            if (look == ',') {
-                case ',':
                 GetChar();
                 hasUnions = true;
                 ParseNode other = intersectionExpression();

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

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));
+        }
+    }
 }

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<XSSFRow> {
      */
     @Override
     public void shiftCellsRight(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) {
-        if(step < 0) {
+        RowShifter.validateShiftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step);
-            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));
-        }
         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<XSSFRow> {
             }
         }
     }
+
     /**
      * 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<XSSFRow> {
      */
     @Override
     public void shiftCellsLeft(int firstShiftColumnIndex, int lastShiftColumnIndex, int step) {
-        if(step < 0) {
+        RowShifter.validateShiftLeftParameters(firstShiftColumnIndex, lastShiftColumnIndex, step);
-            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());
-        }
         for (int columnIndex = firstShiftColumnIndex; columnIndex <= lastShiftColumnIndex; columnIndex++){
             shiftCell(columnIndex, -step);
         }
@@ -682,6 +670,7 @@ public class XSSFRow implements Row, Comparable<XSSFRow> {
             }
         }
     }
+
     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());

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());
+        assertFalse(ps1.getValidSettings());
-        assertEquals(false, ps1.getLandscape());
+        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());
+        assertTrue(ps2.getValidSettings());
-        assertEquals(false, ps2.getLandscape());
+        assertFalse(ps2.getLandscape());
         s2.setRepeatingColumns(cra);
         s2.setRepeatingRows(cra);
 
         ps2 = s2.getPrintSetup();
-        assertEquals(true, ps2.getValidSettings());
+        assertTrue(ps2.getValidSettings());
-        assertEquals(false, ps2.getLandscape());
+        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!");
     }
 }

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());
 

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());
+            if (old.getX() != getX() || old.getY() != getY()) {
-            double oldY = (old == null ? 0 : old.getY());
-            if (oldX != getX() || oldY != getY()) {
                 prop.setWindowOrg(getX(), getY());
                 ctx.updateWindowMapMode();
             }

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() {
+		IStabilityClassifier stabilityClassifier = (sheetIndex, rowIndex, columnIndex) -> sheetIndex == 1;
-			@Override
-            public boolean isCellFinal(int sheetIndex, int rowIndex, int columnIndex) {
-				return 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);
+		NumberEval eval1 = (NumberEval) fe1.evaluate("Calculations", 0, 0);
-		assertEquals(4.0, ((NumberEval) fe2.evaluate("Calculations", 0, 0)).getNumberValue(), 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();
 	}

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("leading whitespace is longer than no whitespace",
-        assertGreaterThan("trailing whitespace is longer than no whitespace", trailingWhitespaceRatio, expectedRatioThreshold);
+                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);
         

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));
+        assertNull(row.getCell(2));
-        assertEquals(null, row.getCell(3));
+        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));
+        assertNull(row.getCell(2, MissingCellPolicy.RETURN_NULL_AND_BLANK));
-        assertEquals(null, row.getCell(3, 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));
+        assertNull(row.getCell(2, MissingCellPolicy.RETURN_BLANK_AS_NULL));
-        assertEquals(null, row.getCell(3, MissingCellPolicy.RETURN_BLANK_AS_NULL));
+        assertNull(row.getCell(3, MissingCellPolicy.RETURN_BLANK_AS_NULL));
-        assertEquals(null, row.getCell(4, 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));
+        assertNull(row.getCell(2));
-        assertEquals(null, row.getCell(3));
+        assertNull(row.getCell(3));
-        assertEquals(null, row.getCell(4));
+        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<Cell> 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());
+        assertFalse(row1.isFormatted());
-       assertEquals(false, row2.isFormatted());
+        assertFalse(row2.isFormatted());
-       assertEquals(null, row1.getRowStyle());
+        assertNull(row1.getRowStyle());
-       assertEquals(null, row2.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());
+        assertFalse(row1.isFormatted());
-       assertEquals(true, row2.isFormatted());
+        assertTrue(row2.isFormatted());
-       assertEquals(null, row1.getRowStyle());
+        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());
+        assertFalse(row1.isFormatted());
-       assertEquals(true, row2.isFormatted());
+        assertTrue(row2.isFormatted());
-       assertEquals(null, row1.getRowStyle());
+        assertNull(row1.getRowStyle());
        assertEquals(style, row2.getRowStyle());
        assertEquals(4, style.getDataFormat());
        

src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java

@@ -71,6 +71,7 @@ public abstract class BaseTestSheetUpdateArrayFormulas {
         
         workbook.close();
     }
+
     /**
      *  Set single-cell array formula
      */

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);