Fix some findbugs-issues and apply some code-cleanup and apply some smaller pull requests.

This closes #74, This closes #75, This closes #76

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

src/java/org/apache/poi/ddf/AbstractEscherOptRecord.java

@@ -17,7 +17,6 @@
 package org.apache.poi.ddf;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
@@ -135,16 +134,14 @@ public abstract class AbstractEscherOptRecord extends EscherRecord
      */
     public void sortProperties()
     {
-        Collections.sort( properties, new Comparator<EscherProperty>()
+        properties.sort(new Comparator<EscherProperty>() {
-        {
             @Override
-            public int compare( EscherProperty p1, EscherProperty p2 )
+            public int compare(EscherProperty p1, EscherProperty p2) {
-            {
                 short s1 = p1.getPropertyNumber();
                 short s2 = p2.getPropertyNumber();
                 return Short.compare(s1, s2);
             }
-        } );
+        });
     }
 
     /**

src/java/org/apache/poi/ddf/EscherDggRecord.java

@@ -20,7 +20,6 @@ package org.apache.poi.ddf;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.BitSet;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
@@ -260,7 +259,7 @@ public final class EscherDggRecord extends EscherRecord {
     }
 
     private void sortCluster() {
-        Collections.sort(field_5_fileIdClusters, new Comparator<FileIdCluster>() {
+        field_5_fileIdClusters.sort(new Comparator<FileIdCluster>() {
             @Override
             public int compare(FileIdCluster f1, FileIdCluster f2) {
                 int dgDif = f1.getDrawingGroupId() - f2.getDrawingGroupId();

src/java/org/apache/poi/hpsf/Variant.java

@@ -420,7 +420,7 @@ public class Variant
         }
         
         name += numberToName.get(vt);
-        return (name != null && !"".equals(name)) ? name : "unknown variant type";
+        return !"".equals(name) ? name : "unknown variant type";
     }
 
     /**

src/java/org/apache/poi/hssf/record/aggregates/ColumnInfoRecordsAggregate.java

@@ -18,7 +18,6 @@
 package org.apache.poi.hssf.record.aggregates;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
@@ -71,7 +70,7 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate implements
 			throw new RuntimeException("No column info records found");
 		}
 		if (!isInOrder) {
-			Collections.sort(records, CIRComparator.instance);
+			records.sort(CIRComparator.instance);
 		}
 	}
 
@@ -89,7 +88,7 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate implements
 	 */
 	public void insertColumn(ColumnInfoRecord col) {
 		records.add(col);
-		Collections.sort(records, CIRComparator.instance);
+		records.sort(CIRComparator.instance);
 	}
 
 	/**
@@ -110,8 +109,7 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate implements
 			return;
 		}
 		ColumnInfoRecord cirPrev = null;
-		for(int i=0; i<nItems; i++) {
+		for (ColumnInfoRecord cir : records) {
-			ColumnInfoRecord cir = records.get(i);
 			rv.visitRecord(cir);
 			if (cirPrev != null && CIRComparator.compareColInfos(cirPrev, cir) > 0) {
 				// Excel probably wouldn't mind, but there is much logic in this class
@@ -299,8 +297,7 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate implements
 	public void setColumn(int targetColumnIx, Short xfIndex, Integer width,
 					Integer level, Boolean hidden, Boolean collapsed) {
 		ColumnInfoRecord ci = null;
-		int k  = 0;
+		int k;
-
 		for (k = 0; k < records.size(); k++) {
 			ColumnInfoRecord tci = records.get(k);
 			if (tci.containsColumn(targetColumnIx)) {

src/java/org/apache/poi/hssf/usermodel/DVConstraint.java

@@ -66,23 +66,23 @@ public class DVConstraint implements DataValidationConstraint {
 
 	
 	private DVConstraint(int validationType, int comparisonOperator, String formulaA,
-			String formulaB, Double value1, Double value2, String[] excplicitListValues) {
+			String formulaB, Double value1, Double value2, String[] explicitListValues) {
 		_validationType = validationType;
 		_operator = comparisonOperator;
 		_formula1 = formulaA;
 		_formula2 = formulaB;
 		_value1 = value1;
 		_value2 = value2;
-		_explicitListValues = (excplicitListValues == null) ? null : excplicitListValues.clone();
+		_explicitListValues = (explicitListValues == null) ? null : explicitListValues.clone();
 	}
 	
 	
 	/**
 	 * Creates a list constraint
 	 */
-	private DVConstraint(String listFormula, String[] excplicitListValues) {
+	private DVConstraint(String listFormula, String[] explicitListValues) {
 		this(ValidationType.LIST, OperatorType.IGNORED,
-			listFormula, null, null, null, excplicitListValues);
+			listFormula, null, null, null, explicitListValues);
 	}
 
 	/**
@@ -228,7 +228,7 @@ public class DVConstraint implements DataValidationConstraint {
 			return null;
 		}
 		try {
-			return new Double(numberStr);
+			return Double.valueOf(numberStr);
 		} catch (NumberFormatException e) {
 			throw new RuntimeException("The supplied text '" + numberStr 
 					+ "' could not be parsed as a number");
@@ -242,7 +242,7 @@ public class DVConstraint implements DataValidationConstraint {
 		if (timeStr == null) {
 			return null;
 		}
-		return new Double(HSSFDateUtil.convertTime(timeStr));
+		return Double.valueOf(HSSFDateUtil.convertTime(timeStr));
 	}
 	/**
 	 * @param dateFormat pass <code>null</code> for default YYYYMMDD
@@ -263,7 +263,7 @@ public class DVConstraint implements DataValidationConstraint {
 						+ "' using specified format '" + dateFormat + "'", e);
 			}
 		}
-		return new Double(HSSFDateUtil.getExcelDate(dateVal));
+		return Double.valueOf(HSSFDateUtil.getExcelDate(dateVal));
 	}
 
 	public static DVConstraint createCustomFormulaConstraint(String formula) {
@@ -363,7 +363,7 @@ public class DVConstraint implements DataValidationConstraint {
 	 */
 	public void setValue1(double value1) {
 		_formula1 = null;
-		_value1 = new Double(value1);
+		_value1 = Double.valueOf(value1);
 	}
 
 	/**
@@ -377,7 +377,7 @@ public class DVConstraint implements DataValidationConstraint {
 	 */
 	public void setValue2(double value2) {
 		_formula2 = null;
-		_value2 = new Double(value2);
+		_value2 = Double.valueOf(value2);
 	}
 	
 	/**
@@ -407,7 +407,7 @@ public class DVConstraint implements DataValidationConstraint {
 			// Some things like union and intersection are not allowed.
 		}
 		// explicit list was provided
-		StringBuffer sb = new StringBuffer(_explicitListValues.length * 16);
+		StringBuilder sb = new StringBuilder(_explicitListValues.length * 16);
 		for (int i = 0; i < _explicitListValues.length; i++) {
 			if (i > 0) {
 				sb.append('\0'); // list delimiter is the nul char
@@ -484,7 +484,7 @@ public class DVConstraint implements DataValidationConstraint {
             if (_value == null) {
                 return null;
             }
-            return new Double(_value);
+            return Double.valueOf(_value);
         }
 
         public String string() {

src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java

@@ -62,9 +62,6 @@ import org.apache.poi.ss.util.CellReference;
  * create whatever style objects they need, caching those at the application level.
  * Thus this class only caches values needed for evaluation, not display.
  */
-/**
- *
- */
 public class EvaluationConditionalFormatRule implements Comparable<EvaluationConditionalFormatRule> {
 
     private final WorkbookEvaluator workbookEvaluator;
@@ -75,7 +72,7 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
     /* cached values */
     private final CellRangeAddress[] regions;
     /**
-     * Depending on the rule type, it may want to know about certain values in the region when evaluating {@link #matches(Cell)},
+     * Depending on the rule type, it may want to know about certain values in the region when evaluating {@link #matches(CellReference)},
      * such as top 10, unique, duplicate, average, etc.  This collection stores those if needed so they are not repeatedly calculated
      */
     private final Map<CellRangeAddress, Set<ValueAndFormat>> meaningfulRegionValues = new HashMap<>();
@@ -370,7 +367,7 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
         return comp;
     }
     /**
-     * @param cell needed for offsets from region anchor - may be null!
+     * @param ref needed for offsets from region anchor - may be null!
      * @param region for adjusting relative formulas
      * @return true/false using the same rules as Data Validation evaluations
      */
@@ -424,7 +421,7 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
                     final ConditionFilterData conf = rule.getFilterConfiguration();
                     
                     if (! conf.getBottom()) {
-                        Collections.sort(allValues, Collections.reverseOrder());
+                        allValues.sort(Collections.reverseOrder());
                     } else {
                         Collections.sort(allValues);
                     }
@@ -505,10 +502,10 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
                     }
 
                     final Set<ValueAndFormat> avgSet = new LinkedHashSet<>(1);
-                    avgSet.add(new ValueAndFormat(new Double(allValues.size() == 0 ? 0 : total / allValues.size()), null));
+                    avgSet.add(new ValueAndFormat(Double.valueOf(allValues.size() == 0 ? 0 : total / allValues.size()), null));
 
                     final double stdDev = allValues.size() <= 1 ? 0 : ((NumberEval) AggregateFunction.STDEV.evaluate(pop, 0, 0)).getNumberValue();
-                    avgSet.add(new ValueAndFormat(new Double(stdDev), null));
+                    avgSet.add(new ValueAndFormat(Double.valueOf(stdDev), null));
                     return avgSet;
                 }
             }));
@@ -527,9 +524,9 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
              * operator type
              */
             
-            Double comp = new Double(conf.getStdDev() > 0 ? (avg + (conf.getAboveAverage() ? 1 : -1) * stdDev * conf.getStdDev()) : avg) ;
+            Double comp = Double.valueOf(conf.getStdDev() > 0 ? (avg + (conf.getAboveAverage() ? 1 : -1) * stdDev * conf.getStdDev()) : avg) ;
             
-            OperatorEnum op = null;
+            final OperatorEnum op;
             if (conf.getAboveAverage()) {
                 if (conf.getEqualAverage()) {
                     op = OperatorEnum.GREATER_OR_EQUAL;
@@ -543,7 +540,7 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
                     op = OperatorEnum.LESS_THAN;
                 }
             }
-            return op != null && op.isValid(val, comp, null);
+            return op.isValid(val, comp, null);
         case CONTAINS_TEXT:
             // implemented both by a cfRule "text" attribute and a formula.  Use the formula.
             return checkFormula(ref, region);
@@ -625,7 +622,7 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
         if (cell != null) {
             final CellType type = cell.getCellType();
             if (type == CellType.NUMERIC || (type == CellType.FORMULA && cell.getCachedFormulaResultType() == CellType.NUMERIC) ) {
-                return new ValueAndFormat(new Double(cell.getNumericCellValue()), cell.getCellStyle().getDataFormatString());
+                return new ValueAndFormat(Double.valueOf(cell.getNumericCellValue()), cell.getCellStyle().getDataFormatString());
             } else if (type == CellType.STRING || (type == CellType.FORMULA && cell.getCachedFormulaResultType() == CellType.STRING) ) {
                 return new ValueAndFormat(cell.getStringCellValue(), cell.getCellStyle().getDataFormatString());
             } else if (type == CellType.BOOLEAN || (type == CellType.FORMULA && cell.getCachedFormulaResultType() == CellType.BOOLEAN) ) {

src/java/org/apache/poi/ss/formula/FormulaParser.java

@@ -1670,7 +1670,7 @@ public final class FormulaParser {
         if (!isPositive) {
             value = -value;
         }
-        return new Double(value);
+        return Double.valueOf(value);
     }
 
     private Ptg parseNumber() {

src/java/org/apache/poi/ss/formula/constant/ConstantValueParser.java

@@ -61,7 +61,7 @@ public final class ConstantValueParser {
 				in.readLong(); // 8 byte 'not used' field
 				return EMPTY_REPRESENTATION; 
 			case TYPE_NUMBER:
-				return new Double(in.readDouble());
+				return Double.valueOf(in.readDouble());
 			case TYPE_STRING:
 				return StringUtil.readUnicodeString(in);
 			case TYPE_BOOLEAN:
@@ -91,8 +91,8 @@ public final class ConstantValueParser {
 	public static int getEncodedSize(Object[] values) {
 		// start with one byte 'type' code for each value
 		int result = values.length * 1;
-		for (int i = 0; i < values.length; i++) {
+		for (Object value : values) {
-			result += getEncodedSize(values[i]);
+			result += getEncodedSize(value);
 		}
 		return result;
 	}
@@ -114,8 +114,8 @@ public final class ConstantValueParser {
 	}
 
 	public static void encode(LittleEndianOutput out, Object[] values) {
-		for (int i = 0; i < values.length; i++) {
+		for (Object value : values) {
-			encodeSingleValue(out, values[i]);
+			encodeSingleValue(out, value);
 		}
 	}
 

src/java/org/apache/poi/ss/formula/functions/Mode.java

@@ -77,8 +77,8 @@ public final class Mode implements Function {
 		double result;
 		try {
 			List<Double> temp = new ArrayList<>();
-			for (int i = 0; i < args.length; i++) {
+			for (ValueEval arg : args) {
-				collectValues(args[i], temp);
+				collectValues(arg, temp);
 			}
 			double[] values = new double[temp.size()];
 			for (int i = 0; i < values.length; i++) {
@@ -129,7 +129,7 @@ public final class Mode implements Function {
 			return;
 		}
 		if (arg instanceof NumberEval) {
-			temp.add(new Double(((NumberEval) arg).getNumberValue()));
+			temp.add(Double.valueOf(((NumberEval) arg).getNumberValue()));
 			return;
 		}
 		throw new RuntimeException("Unexpected value type (" + arg.getClass().getName() + ")");

src/java/org/apache/poi/ss/formula/functions/TextFunction.java

@@ -216,7 +216,7 @@ public abstract class TextFunction implements Function {
 				return ErrorEval.VALUE_INVALID;
 			}
 			int len = text.length();
-			if (numChars < 0 || startIx > len) {
+			if (startIx > len) {
 				return new StringEval("");
 			}
 			int endIx = Math.min(startIx + numChars, len);

src/java/org/apache/poi/ss/formula/functions/Value.java

@@ -37,7 +37,7 @@ public final class Value extends Fixed1ArgFunction {
 
 	/** "1,0000" is valid, "1,00" is not */
 	private static final int MIN_DISTANCE_BETWEEN_THOUSANDS_SEPARATOR = 4;
-	private static final Double ZERO = new Double(0.0);
+	private static final Double ZERO = Double.valueOf(0.0);
 
 	public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0) {
 		ValueEval veText;
@@ -113,7 +113,7 @@ public final class Value extends Fixed1ArgFunction {
 		boolean foundDecimalPoint = false;
 		int lastThousandsSeparatorIndex = Short.MIN_VALUE;
 
-		StringBuffer sb = new StringBuffer(len);
+		StringBuilder sb = new StringBuilder(len);
 		for (; i < len; i++) {
 			char ch = strText.charAt(i);
 			if (Character.isDigit(ch)) {

src/java/org/apache/poi/ss/usermodel/DataFormatter.java

@@ -128,7 +128,7 @@ public class DataFormatter implements Observer {
     private static final Pattern daysAsText = Pattern.compile("([d]{3,})", Pattern.CASE_INSENSITIVE);
 
     /** Pattern to find "AM/PM" marker */
-    private static final Pattern amPmPattern = Pattern.compile("((A|P)[M/P]*)", Pattern.CASE_INSENSITIVE);
+    private static final Pattern amPmPattern = Pattern.compile("(([AP])[M/P]*)", Pattern.CASE_INSENSITIVE);
     
     /** Pattern to find formats with condition ranges e.g. [>=100] */
     private static final Pattern rangeConditionalPattern = Pattern.compile(".*\\[\\s*(>|>=|<|<=|=)\\s*[0-9]*\\.*[0-9].*");
@@ -843,7 +843,7 @@ public class DataFormatter implements Observer {
         if (numberFormat == null) {
             return String.valueOf(d);
         }
-        String formatted = numberFormat.format(new Double(d));
+        String formatted = numberFormat.format(Double.valueOf(d));
         return formatted.replaceFirst("E(\\d)", "E+$1"); // to match Excel's E-notation
     }
 
@@ -894,7 +894,7 @@ public class DataFormatter implements Observer {
         String result;
         final String textValue = NumberToTextConverter.toText(value);
         if (textValue.indexOf('E') > -1) {
-            result = numberFormat.format(new Double(value));
+            result = numberFormat.format(Double.valueOf(value));
         }
         else {
             result = numberFormat.format(new BigDecimal(textValue));
@@ -1296,4 +1296,4 @@ public class DataFormatter implements Observer {
             return null; // Not supported
         }
     }
-}
+}

src/java/org/apache/poi/ss/usermodel/ExcelStyleDateFormatter.java

@@ -192,6 +192,6 @@ public class ExcelStyleDateFormatter extends SimpleDateFormat {
     
     @Override
     public int hashCode() {
-        return new Double(dateToBeFormatted).hashCode();
+        return Double.valueOf(dateToBeFormatted).hashCode();
     }
 }

src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/OOXMLSignatureFacet.java

@@ -191,7 +191,7 @@ public class OOXMLSignatureFacet extends SignatureFacet {
             }
         }
         
-        Collections.sort(manifestReferences, new Comparator<Reference>() {
+        manifestReferences.sort(new Comparator<Reference>() {
             public int compare(Reference o1, Reference o2) {
                 return o1.getURI().compareTo(o2.getURI());
             }

src/ooxml/java/org/apache/poi/xssf/binary/XSSFBHyperlinksTable.java

@@ -132,29 +132,25 @@ public class XSSFBHyperlinksTable {
                 return;
             }
             int offset = 0;
-            String relId = "";
-            String location = "";
-            String toolTip = "";
-            String display = "";
 
             hyperlinkCellRange = XSSFBCellRange.parse(data, offset, hyperlinkCellRange);
             offset += XSSFBCellRange.length;
             xlWideStringBuffer.setLength(0);
             offset += XSSFBUtils.readXLNullableWideString(data, offset, xlWideStringBuffer);
-            relId = xlWideStringBuffer.toString();
+            String relId = xlWideStringBuffer.toString();
             xlWideStringBuffer.setLength(0);
             offset += XSSFBUtils.readXLWideString(data, offset, xlWideStringBuffer);
-            location = xlWideStringBuffer.toString();
+            String location = xlWideStringBuffer.toString();
             xlWideStringBuffer.setLength(0);
             offset += XSSFBUtils.readXLWideString(data, offset, xlWideStringBuffer);
-            toolTip = xlWideStringBuffer.toString();
+            String toolTip = xlWideStringBuffer.toString();
             xlWideStringBuffer.setLength(0);
-            offset += XSSFBUtils.readXLWideString(data, offset, xlWideStringBuffer);
+            /*offset +=*/ XSSFBUtils.readXLWideString(data, offset, xlWideStringBuffer);
-            display = xlWideStringBuffer.toString();
+            String display = xlWideStringBuffer.toString();
             CellRangeAddress cellRangeAddress = new CellRangeAddress(hyperlinkCellRange.firstRow, hyperlinkCellRange.lastRow, hyperlinkCellRange.firstCol, hyperlinkCellRange.lastCol);
 
             String url = relIdToHyperlink.get(relId);
-            if (location == null || location.length() == 0) {
+            if (location.length() == 0) {
                 location = url;
             }
 

src/ooxml/java/org/apache/poi/xssf/eventusermodel/XSSFBReader.java

@@ -167,9 +167,7 @@ public class XSSFBReader extends XSSFReader {
                     PackagePart commentsPart = sheetPkg.getPackage().getPart(commentsName);
                     return new XSSFBCommentsTable(commentsPart.getInputStream());
                 }
-            } catch (InvalidFormatException e) {
+            } catch (InvalidFormatException | IOException e) {
-                return null;
-            } catch (IOException e) {
                 return null;
             }
             return null;
@@ -241,7 +239,8 @@ public class XSSFBReader extends XSSFReader {
         private void tryToAddWorksheet(byte[] data) throws XSSFBParseException {
             int offset = 0;
             //this is the sheet state #2.5.142
-            long hsShtat = LittleEndian.getUInt(data, offset); offset += LittleEndian.INT_SIZE;
+            /*long hsShtat =*/ //noinspection ResultOfMethodCallIgnored
+            LittleEndian.getUInt(data, offset); offset += LittleEndian.INT_SIZE;
 
             long iTabID = LittleEndian.getUInt(data, offset); offset += LittleEndian.INT_SIZE;
             //according to #2.4.304
@@ -251,9 +250,9 @@ public class XSSFBReader extends XSSFReader {
             StringBuilder sb = new StringBuilder();
             offset += XSSFBUtils.readXLWideString(data, offset, sb);
             String relId = sb.toString(); sb.setLength(0);
-            offset += XSSFBUtils.readXLWideString(data, offset, sb);
+            /*offset +=*/ XSSFBUtils.readXLWideString(data, offset, sb);
             String name = sb.toString();
-            if (relId != null && relId.trim().length() > 0) {
+            if (relId.trim().length() > 0) {
                 sheets.add(new XSSFSheetRef(relId, name));
             }
         }
@@ -272,7 +271,7 @@ public class XSSFBReader extends XSSFReader {
             sb.setLength(0);
             offset += XSSFBUtils.readXLWideString(data, offset, sb);
             String name = sb.toString();
-            if (relId != null && relId.trim().length() > 0) {
+            if (relId.trim().length() > 0) {
                 sheets.add(new XSSFSheetRef(relId, name));
             }
             if (offset == data.length) {
@@ -285,4 +284,4 @@ public class XSSFBReader extends XSSFReader {
             return sheets;
         }
     }
-}
+}

src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java

@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
@@ -147,7 +146,7 @@ public class XSSFExportToXml implements Comparator<String>{
             tableMappings.put(commonXPath, table);
         }
 
-        Collections.sort(xpaths,this);
+        xpaths.sort(this);
 
         for(String xpath : xpaths) {
 

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFComment.java

@@ -256,6 +256,7 @@ public class XSSFComment implements Comment {
         // There is a very odd xmlbeans bug when changing the row
         //  arrays which can lead to corrupt pointer
         // This call seems to fix them again... See bug #50795
+        //noinspection ResultOfMethodCallIgnored
         vmlShape.getClientDataList().toString();
     }
 }

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java

@@ -74,7 +74,7 @@ public class XSSFRow implements Row, Comparable<XSSFRow> {
         for (CTCell c : row.getCArray()) {
             XSSFCell cell = new XSSFCell(this, c);
             // Performance optimization for bug 57840: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-            final Integer colI = new Integer(cell.getColumnIndex()); // NOSONAR
+            final Integer colI = Integer.valueOf(cell.getColumnIndex()); // NOSONAR
             _cells.put(colI, cell);
             sheet.onReadCell(cell);
         }
@@ -230,7 +230,7 @@ public class XSSFRow implements Row, Comparable<XSSFRow> {
     @Override
     public XSSFCell createCell(int columnIndex, CellType type) {
         // Performance optimization for bug 57840: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-        final Integer colI = new Integer(columnIndex); // NOSONAR
+        final Integer colI = Integer.valueOf(columnIndex); // NOSONAR
         CTCell ctCell;
         XSSFCell prev = _cells.get(colI);
         if(prev != null){
@@ -270,7 +270,7 @@ public class XSSFRow implements Row, Comparable<XSSFRow> {
     	if(cellnum < 0) throw new IllegalArgumentException("Cell index must be >= 0");
 
         // Performance optimization for bug 57840: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-    	final Integer colI = new Integer(cellnum); // NOSONAR
+    	final Integer colI = Integer.valueOf(cellnum); // NOSONAR
         XSSFCell cell = _cells.get(colI);
         switch (policy) {
             case RETURN_NULL_AND_BLANK:
@@ -500,7 +500,7 @@ public class XSSFRow implements Row, Comparable<XSSFRow> {
            _sheet.getWorkbook().onDeleteFormula(xcell);
         }
         // Performance optimization for bug 57840: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-        final Integer colI = new Integer(cell.getColumnIndex()); // NOSONAR
+        final Integer colI = Integer.valueOf(cell.getColumnIndex()); // NOSONAR
         _cells.remove(colI);
     }
 

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -266,7 +266,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
         for (CTRow row : worksheetParam.getSheetData().getRowArray()) {
             XSSFRow r = new XSSFRow(row, this);
             // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-            final Integer rownumI = new Integer(r.getRowNum()); // NOSONAR
+            final Integer rownumI = Integer.valueOf(r.getRowNum()); // NOSONAR
             _rows.put(rownumI, r);
         }
     }
@@ -751,7 +751,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
     @Override
     public XSSFRow createRow(int rownum) {
         // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-        final Integer rownumI = new Integer(rownum); // NOSONAR
+        final Integer rownumI = Integer.valueOf(rownum); // NOSONAR
         CTRow ctRow;
         XSSFRow prev = _rows.get(rownumI);
         if(prev != null){
@@ -1448,7 +1448,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
     @Override
     public XSSFRow getRow(int rownum) {
         // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-        final Integer rownumI = new Integer(rownum); // NOSONAR
+        final Integer rownumI = Integer.valueOf(rownum); // NOSONAR
         return _rows.get(rownumI);
     }
     
@@ -1479,8 +1479,8 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
         }
         else {
             // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-            final Integer startI = new Integer(startRowNum); // NOSONAR
+            final Integer startI = Integer.valueOf(startRowNum); // NOSONAR
-            final Integer endI = new Integer(endRowNum+1); // NOSONAR
+            final Integer endI = Integer.valueOf(endRowNum+1); // NOSONAR
             final Collection<XSSFRow> inclusive = _rows.subMap(startI, endI).values();
             rows.addAll(inclusive);
         }
@@ -1982,7 +1982,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
 
         // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
         final int rowNum = row.getRowNum();
-        final Integer rowNumI = new Integer(rowNum); // NOSONAR
+        final Integer rowNumI = Integer.valueOf(rowNum); // NOSONAR
         // this is not the physical row number!
         final int idx = _rows.headMap(rowNumI).size();
         _rows.remove(rowNumI);
@@ -2994,7 +2994,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
             if (shouldRemoveRow(startRow, endRow, n, rownum)) {
                 // remove row from worksheet.getSheetData row array
                 // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-                final Integer rownumI = new Integer(row.getRowNum()); // NOSONAR
+                final Integer rownumI = Integer.valueOf(row.getRowNum()); // NOSONAR
                 int idx = _rows.headMap(rownumI).size();
                 worksheet.getSheetData().removeRow(idx);
 
@@ -3118,7 +3118,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
         Map<Integer, XSSFRow> map = new HashMap<>();
         for(XSSFRow r : _rows.values()) {
             // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory
-            final Integer rownumI = new Integer(r.getRowNum()); // NOSONAR
+            final Integer rownumI = Integer.valueOf(r.getRowNum()); // NOSONAR
             map.put(rownumI, r);
         }
         _rows.clear();

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFTable.java

@@ -146,7 +146,7 @@ public class XWPFTable implements IBodyElement, ISDTContents {
          * CTTblGrid tblgrid=table.addNewTblGrid();
          * tblgrid.addNewGridCol().setW(new BigInteger("2000"));
          */
-        getRows();
+        //getRows();
     }
 
     /**

src/resources/devtools/findbugs-filters.xml

@@ -130,5 +130,12 @@
         <Bug pattern="DM_NUMBER_CTOR" />
     </Match>
 
+	<!-- On purpose -->
+    <Match>
+        <Or>
+            <Class name="org.apache.poi.xssf.usermodel.XSSFComment"/>
+        </Or>
+        <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+    </Match>
 
 </FindBugsFilter>

src/scratchpad/src/org/apache/poi/hdgf/chunks/Chunk.java

@@ -137,7 +137,7 @@ public final class Chunk {
 				command = new Command(cdef);
 			}
 
-			// Bizarely, many of the offsets are from the start of the
+			// Bizarrely, many of the offsets are from the start of the
 			//  header, not from the start of the chunk body
 			switch(type) {
 			case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7:
@@ -172,7 +172,7 @@ public final class Chunk {
 					command.value = Byte.valueOf(contents[offset]);
 					break;
 				case 9:
-					command.value = new Double(
+					command.value = Double.valueOf(
 							LittleEndian.getDouble(contents, offset)
 					);
 					break;

src/scratchpad/src/org/apache/poi/hsmf/dev/TypesLister.java

@@ -19,7 +19,6 @@ package org.apache.poi.hsmf.dev;
 
 import java.io.PrintStream;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 
 import org.apache.poi.hsmf.datatypes.MAPIProperty;
@@ -32,19 +31,19 @@ public class TypesLister {
    
    public void listByName(PrintStream out) {
       ArrayList<MAPIProperty> all = new ArrayList<>(MAPIProperty.getAll());
-      Collections.sort(all, new Comparator<MAPIProperty>() {
+      all.sort(new Comparator<MAPIProperty>() {
-         public int compare(MAPIProperty a, MAPIProperty b) {
+          public int compare(MAPIProperty a, MAPIProperty b) {
-            return a.name.compareTo(b.name);
+              return a.name.compareTo(b.name);
-         }
+          }
       });
       list(all, out);
    }
    public void listById(PrintStream out) {
       ArrayList<MAPIProperty> all = new ArrayList<>(MAPIProperty.getAll());
-      Collections.sort(all, new Comparator<MAPIProperty>() {
+      all.sort(new Comparator<MAPIProperty>() {
-         public int compare(MAPIProperty a, MAPIProperty b) {
+          public int compare(MAPIProperty a, MAPIProperty b) {
-             return Integer.compare(a.id, b.id);
+              return Integer.compare(a.id, b.id);
-         }
+          }
       });
       list(all, out);
    }

src/scratchpad/src/org/apache/poi/hwpf/model/CHPBinTable.java

@@ -172,8 +172,7 @@ public class CHPBinTable
         }
 
         List<CHPX> oldChpxSortedByStartPos = new ArrayList<>(_textRuns);
-        Collections.sort( oldChpxSortedByStartPos,
+        oldChpxSortedByStartPos.sort(PropertyNode.StartComparator.instance);
-                PropertyNode.StartComparator.instance );
 
         logger.log( POILogger.DEBUG, "CHPX sorted by start position in ",
                 Long.valueOf( System.currentTimeMillis() - start ), " ms" );
@@ -278,7 +277,7 @@ public class CHPBinTable
                 }
             }
 
-            Collections.sort( chpxs, chpxFileOrderComparator );
+            chpxs.sort(chpxFileOrderComparator);
 
             SprmBuffer sprmBuffer = new SprmBuffer( 0 );
             for ( CHPX chpx : chpxs )

src/scratchpad/src/org/apache/poi/hwpf/model/OldCHPBinTable.java

@@ -17,8 +17,6 @@
 
 package org.apache.poi.hwpf.model;
 
-import java.util.Collections;
-
 import org.apache.poi.poifs.common.POIFSConstants;
 import org.apache.poi.util.Internal;
 import org.apache.poi.util.LittleEndian;
@@ -65,6 +63,6 @@ public final class OldCHPBinTable extends CHPBinTable
                     _textRuns.add( chpx );
             }
     }
-    Collections.sort( _textRuns, PropertyNode.StartComparator.instance );
+    _textRuns.sort(PropertyNode.StartComparator.instance);
   }
 }

src/scratchpad/src/org/apache/poi/hwpf/model/OldSectionTable.java

@@ -17,8 +17,6 @@
 
 package org.apache.poi.hwpf.model;
 
-import java.util.Collections;
-
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.Internal;
 import org.apache.poi.util.LittleEndian;
@@ -81,6 +79,6 @@ public final class OldSectionTable extends SectionTable
 
             _sections.add( sepx );
     }
-    Collections.sort( _sections, PropertyNode.StartComparator.instance );
+    _sections.sort(PropertyNode.StartComparator.instance);
   }
 }

src/scratchpad/src/org/apache/poi/hwpf/model/OldTextPieceTable.java

@@ -23,16 +23,9 @@ import java.util.Collections;
 import org.apache.poi.util.CodePageUtil;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.Internal;
-import org.apache.poi.util.POILogFactory;
-import org.apache.poi.util.POILogger;
-
 
 @Internal
 public class OldTextPieceTable extends TextPieceTable {
-
-    private static final POILogger logger = POILogFactory
-            .getLogger(OldTextPieceTable.class);
-
     //arbitrarily selected; may need to increase
     private static final int MAX_RECORD_LENGTH = 100_000_000;
 
@@ -103,7 +96,7 @@ public class OldTextPieceTable extends TextPieceTable {
         // into order, if they're not already
         Collections.sort(_textPieces);
         _textPiecesFCOrder = new ArrayList<>(_textPieces);
-        Collections.sort(_textPiecesFCOrder, new FCComparator());
+        _textPiecesFCOrder.sort(new FCComparator());
 
     }
 

src/scratchpad/src/org/apache/poi/hwpf/model/PAPBinTable.java

@@ -20,7 +20,6 @@ package org.apache.poi.hwpf.model;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.IdentityHashMap;
 import java.util.LinkedList;
@@ -159,8 +158,7 @@ public class PAPBinTable
         }
 
         List<PAPX> oldPapxSortedByEndPos = new ArrayList<>(paragraphs);
-        Collections.sort( oldPapxSortedByEndPos,
+        oldPapxSortedByEndPos.sort(PropertyNode.EndComparator.instance);
-                PropertyNode.EndComparator.instance );
 
         logger.log( POILogger.DEBUG, "PAPX sorted by end position in ",
                 Long.valueOf( System.currentTimeMillis() - start ), " ms" );
@@ -255,7 +253,7 @@ public class PAPBinTable
             }
 
             // restore file order of PAPX
-            Collections.sort( papxs, papxFileOrderComparator );
+            papxs.sort(papxFileOrderComparator);
 
             SprmBuffer sprmBuffer = null;
             for ( PAPX papx : papxs )
@@ -281,7 +279,6 @@ public class PAPBinTable
         logger.log( POILogger.DEBUG, "PAPX rebuilded from document text in ",
                 Long.valueOf( System.currentTimeMillis() - start ), " ms (",
                 Integer.valueOf( paragraphs.size() ), " elements)" );
-        start = System.currentTimeMillis();
     }
 
     public void insert(int listIndex, int cpStart, SprmBuffer buf)

src/scratchpad/src/org/apache/poi/hwpf/model/SectionTable.java

@@ -20,7 +20,6 @@ package org.apache.poi.hwpf.model;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import org.apache.poi.hwpf.model.io.HWPFFileSystem;
@@ -96,11 +95,10 @@ public class SectionTable
         //    is getting on for black magic...
         boolean matchAt = false;
         boolean matchHalf = false;
-        for (int i=0; i<_sections.size(); i++) {
+        for (SEPX s : _sections) {
-            SEPX s = _sections.get(i);
             if (s.getEnd() == mainLength) {
                 matchAt = true;
-            } else if(s.getEnd() == mainLength || s.getEnd() == mainLength -1) {
+            } else if (s.getEnd() == mainLength || s.getEnd() == mainLength - 1) {
                 matchHalf = true;
             }
         }
@@ -119,7 +117,7 @@ public class SectionTable
             }
         }
 
-        Collections.sort( _sections, PropertyNode.StartComparator.instance );
+        _sections.sort(PropertyNode.StartComparator.instance);
     }
 
     public void adjustForInsert(int listIndex, int length)
@@ -187,15 +185,13 @@ public class SectionTable
         int len = _sections.size();
         PlexOfCps plex = new PlexOfCps(SED_SIZE);
 
-        for (int x = 0; x < len; x++)
+        for (SEPX sepx : _sections) {
-        {
-            SEPX sepx = _sections.get(x);
             byte[] grpprl = sepx.getGrpprl();
 
             // write the sepx to the document stream. starts with a 2 byte size
             // followed by the grpprl
             byte[] shortBuf = new byte[2];
-            LittleEndian.putShort(shortBuf, 0, (short)grpprl.length);
+            LittleEndian.putShort(shortBuf, 0, (short) grpprl.length);
 
             wordDocumentStream.write(shortBuf);
             wordDocumentStream.write(grpprl);
@@ -208,7 +204,7 @@ public class SectionTable
 
             /* original line */
             GenericPropertyNode property = new GenericPropertyNode(
-                            sepx.getStart(), sepx.getEnd(), sed.toByteArray() );
+                    sepx.getStart(), sepx.getEnd(), sed.toByteArray());
             /*
              * Line using Ryan's FCtoCP() conversion method - unable to observe
              * any effect on our testcases when using this code - piers

src/scratchpad/src/org/apache/poi/hwpf/model/TextPieceTable.java

@@ -115,7 +115,7 @@ public class TextPieceTable implements CharIndexTranslator {
         // into order, if they're not already
         Collections.sort(_textPieces);
         _textPiecesFCOrder = new ArrayList<>(_textPieces);
-        Collections.sort(_textPiecesFCOrder, new FCComparator());
+        _textPiecesFCOrder.sort(new FCComparator());
     }
 
     protected TextPiece newTextPiece(int nodeStartChars, int nodeEndChars, byte[] buf, PieceDescriptor pd) {
@@ -126,7 +126,7 @@ public class TextPieceTable implements CharIndexTranslator {
         _textPieces.add(piece);
         _textPiecesFCOrder.add(piece);
         Collections.sort(_textPieces);
-        Collections.sort(_textPiecesFCOrder, new FCComparator());
+        _textPiecesFCOrder.sort(new FCComparator());
     }
 
     /**

src/scratchpad/src/org/apache/poi/hwpf/usermodel/BookmarksImpl.java

@@ -248,7 +248,7 @@ public class BookmarksImpl implements Bookmarks
             indices[counter++] = entry.getKey().intValue();
             List<GenericPropertyNode> updated = new ArrayList<>(
                     entry.getValue());
-            Collections.sort( updated, PropertyNode.EndComparator.instance );
+            updated.sort(PropertyNode.EndComparator.instance);
             entry.setValue( updated );
         }
         Arrays.sort( indices );

src/scratchpad/src/org/apache/poi/hwpf/usermodel/FieldsImpl.java

@@ -43,7 +43,7 @@ public class FieldsImpl implements Fields
      * This is port and adaptation of Arrays.binarySearch from Java 6 (Apache
      * Harmony).
      */
-    private static <T> int binarySearch( List<PlexOfField> list,
+    private static int binarySearch( List<PlexOfField> list,
             int startIndex, int endIndex, int requiredStartOffset )
     {
         checkIndexForBinarySearch( list.size(), startIndex, endIndex );
@@ -136,7 +136,7 @@ public class FieldsImpl implements Fields
         if ( plexOfFields == null || plexOfFields.isEmpty() )
             return new HashMap<>();
 
-        Collections.sort( plexOfFields, comparator );
+        plexOfFields.sort(comparator);
         List<FieldImpl> fields = new ArrayList<>(
                 plexOfFields.size() / 3 + 1);
         parseFieldStructureImpl( plexOfFields, 0, plexOfFields.size(), fields );