activate javadoc lint and fix errors

add signing and checksum generation
add slf4j-bridge for tests
add dependencies to ooxml-lite
fix complex enum classes
add override annotations
move gradle logic to root build
generate javadoc in its own dist directory, because JPMS complains about duplicate modules otherwise

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

27 files changed, 236 insertions, 312 deletions

diff --git a/poi-scratchpad/build.gradle b/poi-scratchpad/build.gradle
index 4669dc98dc..c88cfeae14 100644
--- a/poi-scratchpad/build.gradle
+++ b/poi-scratchpad/build.gradle
@@ -56,13 +56,6 @@ final Pattern MODULE_REGEX = ~'\\.jar$'
 final List MAIN_MODULE_PATH = sourceSets.main.runtimeClasspath.findAll{ it.path =~ MODULE_REGEX }.collect{ it.parent }.unique()
 final List TEST_MODULE_PATH = sourceSets.test.runtimeClasspath.findAll{ it.path =~ MODULE_REGEX && !(it.path =~ MODULE_NOT_REGEX) }.collect{ it.parent }.unique()
 
-java {
-    sourceCompatibility = JavaVersion.VERSION_1_8
-    targetCompatibility = JavaVersion.VERSION_1_8
-    withJavadocJar()
-    withSourcesJar()
-}
-
 task compileJava9(type: JavaCompile) {
     dependsOn 'compileJava', ':poi:jar'
 
@@ -142,23 +135,9 @@ task testJar(type: Jar, dependsOn: testClasses) {
     }
 }
 
-sourcesJar {
-    destinationDirectory = file("../build/dist/maven/${project.archivesBaseName}")
-    exclude 'META-INF/services/**'
-}
-
 javadoc {
-	failOnError = true
 	doFirst {
 		options {
-			if (JavaVersion.current().isJava9Compatible()) {
-				addBooleanOption('html5', true)
-			}
-			links 'https://poi.apache.org/apidocs/dev/'
-			links 'https://docs.oracle.com/javase/8/docs/api/'
-			use = true
-			splitIndex = true
-			source = "1.8"
 			classpath += configurations.javadocs.files
 		}
 	}
@@ -171,39 +150,10 @@ artifacts {
 test {
     dependsOn { testJar }
 
-    useJUnitPlatform()
-
    doFirst {
-        jvmArgs = [
-            '-Djava.io.tmpdir=build',
-            '-DPOI.testdata.path=../test-data',
-            '-Djava.awt.headless=true',
-            '-Djava.locale.providers=JRE,CLDR',
-            '-Duser.language=en',
-            '-Duser.country=US',
-            '-Djavax.xml.stream.XMLInputFactory=com.sun.xml.internal.stream.XMLInputFactoryImpl',
-            "-Dversion.id=${project.version}",
-            '-ea',
-            '-Djunit.jupiter.execution.parallel.enabled=true',
-            '-Djunit.jupiter.execution.parallel.config.strategy=fixed',
-            '-Djunit.jupiter.execution.parallel.config.fixed.parallelism=3'
-            // -Xjit:verbose={compileStart|compileEnd},vlog=build/jit.log${no.jit.sherlock}   ... if ${isIBMVM}
-        ]
         if (JavaVersion.current() != JavaVersion.VERSION_1_8) {
             jvmArgs += [
-                '-Dsun.reflect.debugModuleAccessChecks=true',
-                '-Dcom.sun.xml.bind.v2.bytecode.ClassTailor.noOptimize=true',
-                '--illegal-access=warn',
-
                 '--add-modules', MODULE_NAME,
-
-                // see https://github.com/java9-modularity/gradle-modules-plugin/issues/97
-                // opposed to the recommendation there, it doesn't work to add ... to the dependencies
-                // testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.7.1'
-                // gradles gradle-worker.jar is still not a JPMS module and thus runs as unnamed module
-                '--add-exports','org.junit.platform.commons/org.junit.platform.commons.util=ALL-UNNAMED',
-                '--add-exports','org.junit.platform.commons/org.junit.platform.commons.logging=ALL-UNNAMED',
-
                 '--module-path', '../build/dist/maven/poi-scratchpad-tests:' + files(TEST_MODULE_PATH).asPath,
             ]
         }
@@ -213,10 +163,6 @@ test {
 publishing {
     publications {
         POI(MavenPublication) {
-            artifactId project.archivesBaseName
-
-            from components.java
-
             pom {
                 name = 'Apache POI'
                 description = 'Apache POI - Java API To Access Microsoft Format Files (Scratchpad)'
@@ -224,5 +170,3 @@ publishing {
         }
     }
 }
-
-generatePomFileForPOIPublication.destination = "../build/dist/maven/${project.archivesBaseName}/${project.archivesBaseName}-${project.version}.pom"
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/extractor/QuickButCruddyTextExtractor.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/extractor/QuickButCruddyTextExtractor.java
index 1c2a0e6d79..ec17e6d525 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/extractor/QuickButCruddyTextExtractor.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/extractor/QuickButCruddyTextExtractor.java
@@ -55,7 +55,7 @@ import org.apache.poi.util.LittleEndian;
 public final class QuickButCruddyTextExtractor {
     private POIFSFileSystem fs;
     private InputStream is;
-    private byte[] pptContents;
+    private final byte[] pptContents;
 
     /**
      * Really basic text extractor, that will also return lots of crud text.
@@ -78,7 +78,6 @@ public final class QuickButCruddyTextExtractor {
 
     /**
      * Creates an extractor from a given file name
-     * @param fileName
      */
     @SuppressWarnings("resource")
     public QuickButCruddyTextExtractor(String fileName) throws IOException {
@@ -87,7 +86,6 @@ public final class QuickButCruddyTextExtractor {
 
     /**
      * Creates an extractor from a given input stream
-     * @param iStream
      */
     @SuppressWarnings("resource")
     public QuickButCruddyTextExtractor(InputStream iStream) throws IOException {
@@ -97,7 +95,6 @@ public final class QuickButCruddyTextExtractor {
 
     /**
      * Creates an extractor from a POIFS Filesystem
-     * @param poifs
      */
     public QuickButCruddyTextExtractor(POIFSFileSystem poifs) throws IOException {
         fs = poifs;
@@ -192,9 +189,7 @@ public final class QuickButCruddyTextExtractor {
             String text = cs.getText();
 
             // Ignore the ones we know to be rubbish
-            if(text.equals("___PPT10")) {
-            } else if(text.equals("Default Design")) {
-            } else {
+            if (!"___PPT10".equals(text) && !"Default Design".equals(text)) {
                 textV.add(text);
             }
         }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/ActiveXShape.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/ActiveXShape.java
index f8c02deac7..26af08bd80 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/ActiveXShape.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/ActiveXShape.java
@@ -46,7 +46,7 @@ public final class ActiveXShape extends HSLFPictureShape {
     public static final int DEFAULT_ACTIVEX_THUMBNAIL = -1;
 
     /**
-     * Create a new <code>Picture</code>
+     * Create a new {@code Picture}
      *
     * @param pictureData the picture data
      */
@@ -56,10 +56,10 @@ public final class ActiveXShape extends HSLFPictureShape {
     }
 
     /**
-      * Create a <code>Picture</code> object
+      * Create a {@code Picture} object
       *
-      * @param escherRecord the <code>EscherSpContainer</code> record which holds information about
-      *        this picture in the <code>Slide</code>
+      * @param escherRecord the {@code EscherSpContainer} record which holds information about
+      *        this picture in the {@code Slide}
       * @param parent the parent shape of this picture
       */
      protected ActiveXShape(EscherContainerRecord escherRecord, ShapeContainer<HSLFShape,HSLFTextParagraph> parent){
@@ -69,13 +69,14 @@ public final class ActiveXShape extends HSLFPictureShape {
     /**
      * Create a new Placeholder and initialize internal structures
      *
-     * @return the created <code>EscherContainerRecord</code> which holds shape data
+     * @return the created {@code EscherContainerRecord} which holds shape data
      */
     @Override
     protected EscherContainerRecord createSpContainer(int idx, boolean isChild) {
         EscherContainerRecord ecr = super.createSpContainer(idx, isChild);
 
         EscherSpRecord spRecord = ecr.getChildById(EscherSpRecord.RECORD_ID);
+        assert(spRecord != null);
         spRecord.setFlags(EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HASSHAPETYPE | EscherSpRecord.FLAG_OLESHAPE);
 
         setShapeType(ShapeType.HOST_CONTROL);
@@ -114,8 +115,6 @@ public final class ActiveXShape extends HSLFPictureShape {
 
     /**
      * Set a property of this ActiveX control
-     * @param key
-     * @param value
      */
     public void setProperty(String key, String value){
 
@@ -133,7 +132,7 @@ public final class ActiveXShape extends HSLFPictureShape {
         if (lst == null) {
             return null;
         }
-        
+
         for (Record ch : lst.getChildRecords()) {
             if(ch instanceof ExControl){
                 ExControl c = (ExControl)ch;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/Polygon.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/Polygon.java
index 3c4e8b04cf..2f3aa0941f 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/Polygon.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/Polygon.java
@@ -40,7 +40,7 @@ public final class Polygon extends HSLFAutoShape {
     /**
      * Create a Polygon object and initialize it from the supplied Record container.
      *
-     * @param escherRecord       <code>EscherSpContainer</code> container which holds information about this shape
+     * @param escherRecord       {@code EscherSpContainer} container which holds information about this shape
      * @param parent    the parent of the shape
      */
    protected Polygon(EscherContainerRecord escherRecord, ShapeContainer<HSLFShape,HSLFTextParagraph> parent){
@@ -69,9 +69,6 @@ public final class Polygon extends HSLFAutoShape {
 
     /**
      * Set the polygon vertices
-     *
-     * @param xPoints
-     * @param yPoints
      */
     public void setPoints(float[] xPoints, float[] yPoints)
     {
@@ -147,10 +144,9 @@ public final class Polygon extends HSLFAutoShape {
     private float findBiggest( float[] values )
     {
         float result = Float.MIN_VALUE;
-        for ( int i = 0; i < values.length; i++ )
-        {
-            if (values[i] > result)
-                result = values[i];
+        for (float value : values) {
+            if (value > result)
+                result = value;
         }
         return result;
     }
@@ -158,10 +154,9 @@ public final class Polygon extends HSLFAutoShape {
     private float findSmallest( float[] values )
     {
         float result = Float.MAX_VALUE;
-        for ( int i = 0; i < values.length; i++ )
-        {
-            if (values[i] < result)
-                result = values[i];
+        for (float value : values) {
+            if (value < result)
+                result = value;
         }
         return result;
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/textproperties/BitMaskTextProp.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/textproperties/BitMaskTextProp.java
index 17d5ea5610..92ab8c6b0a 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/textproperties/BitMaskTextProp.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/model/textproperties/BitMaskTextProp.java
@@ -37,9 +37,9 @@ import org.apache.poi.util.GenericRecordUtil;
 public abstract class BitMaskTextProp extends TextProp {
     protected static final Logger LOG = LogManager.getLogger(BitMaskTextProp.class);
 
-	private String[] subPropNames;
-	private int[] subPropMasks;
-	private boolean[] subPropMatches;
+	private final String[] subPropNames;
+	private final int[] subPropMasks;
+	private final boolean[] subPropMatches;
 
 	/** Fetch the list of the names of the sub properties */
 	public String[] getSubPropNames() { return subPropNames; }
@@ -141,9 +141,6 @@ public abstract class BitMaskTextProp extends TextProp {
 
 	/**
 	 * Convenience method to set a value with mask, without splitting it into the subvalues
-	 *
-	 * @param val
-	 * @param writeMask
 	 */
 	public void setValueWithMask(int val, int writeMask) {
 	    setWriteMask(writeMask);
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SlideListWithText.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SlideListWithText.java
index b57207e41d..685e42b4ab 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SlideListWithText.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SlideListWithText.java
@@ -64,7 +64,7 @@ public final class SlideListWithText extends RecordContainer {
 	 */
 	public static final int NOTES = 2;
 
-	private byte[] _header;
+	private final byte[] _header;
 
 	private SlideAtomsSet[] slideAtomsSets;
 
@@ -124,7 +124,6 @@ public final class SlideListWithText extends RecordContainer {
 	/**
 	 * Add a new SlidePersistAtom, to the end of the current list,
 	 *  and update the internal list of SlidePersistAtoms
-	 * @param spa
 	 */
 	public void addSlidePersistAtom(SlidePersistAtom spa) {
 		// Add the new SlidePersistAtom at the end
@@ -164,12 +163,14 @@ public final class SlideListWithText extends RecordContainer {
 	/**
 	 * Return the value we were given at creation
 	 */
+	@Override
 	public long getRecordType() { return _type; }
 
 	/**
 	 * Write the contents of the record back, so it can be written
 	 *  to disk
 	 */
+	@Override
 	public void writeOut(OutputStream out) throws IOException {
 		writeOut(_header[0],_header[1],_type,_children,out);
 	}
@@ -182,8 +183,8 @@ public final class SlideListWithText extends RecordContainer {
 	 *  along with some others.
 	 */
 	public static class SlideAtomsSet {
-		private SlidePersistAtom slidePersistAtom;
-		private org.apache.poi.hslf.record.Record[] slideRecords;
+		private final SlidePersistAtom slidePersistAtom;
+		private final org.apache.poi.hslf.record.Record[] slideRecords;
 
 		/** Get the SlidePersistAtom, which gives details on the Slide this text is associated with */
 		public SlidePersistAtom getSlidePersistAtom() { return slidePersistAtom; }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFBackground.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFBackground.java
index eac3d846fb..c734ce28fb 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFBackground.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFBackground.java
@@ -30,6 +30,7 @@ public final class HSLFBackground extends HSLFShape implements Background<HSLFSh
         super(escherRecord, parent);
     }
 
+    @Override
     protected EscherContainerRecord createSpContainer(boolean isChild) {
         return null;
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFComment.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFComment.java
index 0548fca452..c5618f0c59 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFComment.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFComment.java
@@ -25,7 +25,7 @@ import java.awt.geom.Point2D;
 import java.util.Date;
 
 public final class HSLFComment implements Comment {
-    private Comment2000 _comment2000;
+    private final Comment2000 _comment2000;
 
     public HSLFComment(Comment2000 comment2000) {
         _comment2000 = comment2000;
@@ -38,6 +38,7 @@ public final class HSLFComment implements Comment {
     /**
      * Get the Author of this comment
      */
+    @Override
     public String getAuthor() {
         return _comment2000.getAuthor();
     }
@@ -45,6 +46,7 @@ public final class HSLFComment implements Comment {
     /**
      * Set the Author of this comment
      */
+    @Override
     public void setAuthor(String author) {
         _comment2000.setAuthor(author);
     }
@@ -52,6 +54,7 @@ public final class HSLFComment implements Comment {
     /**
      * Get the Author's Initials of this comment
      */
+    @Override
     public String getAuthorInitials() {
         return _comment2000.getAuthorInitials();
     }
@@ -59,6 +62,7 @@ public final class HSLFComment implements Comment {
     /**
      * Set the Author's Initials of this comment
      */
+    @Override
     public void setAuthorInitials(String initials) {
         _comment2000.setAuthorInitials(initials);
     }
@@ -66,6 +70,7 @@ public final class HSLFComment implements Comment {
     /**
      * Get the text of this comment
      */
+    @Override
     public String getText() {
         return _comment2000.getText();
     }
@@ -73,6 +78,7 @@ public final class HSLFComment implements Comment {
     /**
      * Set the text of this comment
      */
+    @Override
     public void setText(String text) {
         _comment2000.setText(text);
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShape.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShape.java
index 8fed381c38..f06230a4ce 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShape.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShape.java
@@ -55,18 +55,14 @@ import org.apache.poi.util.StringUtil;
 import org.apache.poi.util.Units;
 
 /**
- *  <p>
-  * Represents a Shape which is the elemental object that composes a drawing.
+ * Represents a Shape which is the elemental object that composes a drawing.
  *  This class is a wrapper around EscherSpContainer which holds all information
  *  about a shape in PowerPoint document.
- *  </p>
  *  <p>
  *  When you add a shape, you usually specify the dimensions of the shape and the position
  *  of the upper'left corner of the bounding box for the shape relative to the upper'left
  *  corner of the page, worksheet, or slide. Distances in the drawing layer are measured
  *  in points (72 points = 1 inch).
- *  </p>
- * <p>
  */
 public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     private static final Logger LOG = LogManager.getLogger(HSLFShape.class);
@@ -79,12 +75,12 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
 
     /**
      * Parent of this shape.
-     * <code>null</code> for the topmost shapes.
+     * {@code null} for the topmost shapes.
      */
-    private ShapeContainer<HSLFShape,HSLFTextParagraph> _parent;
+    private final ShapeContainer<HSLFShape,HSLFTextParagraph> _parent;
 
     /**
-     * The <code>Sheet</code> this shape belongs to
+     * The {@code Sheet} this shape belongs to
      */
     private HSLFSheet _sheet;
 
@@ -92,11 +88,11 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
      * Fill
      */
     private HSLFFill _fill;
-    
+
     /**
      * Create a Shape object. This constructor is used when an existing Shape is read from from a PowerPoint document.
      *
-     * @param escherRecord       <code>EscherSpContainer</code> container which holds information about this shape
+     * @param escherRecord       {@code EscherSpContainer} container which holds information about this shape
      * @param parent             the parent of this Shape
      */
       protected HSLFShape(EscherContainerRecord escherRecord, ShapeContainer<HSLFShape,HSLFTextParagraph> parent){
@@ -233,12 +229,12 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     /**
      * Helper method to return escher child by record ID
      *
-     * @return escher record or <code>null</code> if not found.
+     * @return escher record or {@code null} if not found.
      */
     public static <T extends EscherRecord> T getEscherChild(EscherContainerRecord owner, int recordId){
         return owner.getChildById((short)recordId);
     }
-    
+
     /**
      * @since POI 3.14-Beta2
      */
@@ -249,18 +245,18 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     public <T extends EscherRecord> T getEscherChild(int recordId){
         return _escherContainer.getChildById((short)recordId);
     }
-    
+
     /**
      * @since POI 3.14-Beta2
      */
     public <T extends EscherRecord> T getEscherChild(EscherRecordTypes recordId){
         return getEscherChild(recordId.typeID);
     }
-    
+
     /**
      * Returns  escher property by id.
      *
-     * @return escher property or <code>null</code> if not found.
+     * @return escher property or {@code null} if not found.
      *
      * @deprecated use {@link #getEscherProperty(EscherPropertyTypes)} instead
      */
@@ -273,7 +269,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     /**
      * Returns  escher property by type.
      *
-     * @return escher property or <code>null</code> if not found.
+     * @return escher property or {@code null} if not found.
      */
     public static <T extends EscherProperty> T getEscherProperty(AbstractEscherOptRecord opt, EscherPropertyTypes type){
         return (opt == null) ? null : opt.lookup(type);
@@ -428,7 +424,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     }
 
     /**
-     *  @return the <code>SlideShow</code> this shape belongs to
+     *  @return the {@code SlideShow} this shape belongs to
      */
     @Override
     public HSLFSheet getSheet(){
@@ -436,7 +432,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     }
 
     /**
-     * Assign the <code>SlideShow</code> this shape belongs to
+     * Assign the {@code SlideShow} this shape belongs to
      *
      * @param sheet owner of this shape
      */
@@ -491,10 +487,10 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
             col = applySysIndexProcedure(ecr, col);
             return col;
         }
-        
+
         return new Color(rgb[0], rgb[1], rgb[2]);
     }
-    
+
     private Color getSysIndexColor(EscherColorRef ecr) {
         SysIndexSource sis = ecr.getSysIndexSource();
         if (sis == null) {
@@ -502,7 +498,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
             PresetColor pc = PresetColor.valueOfNativeId(sysIdx);
             return (pc != null) ? pc.color : null;
         }
-        
+
         // TODO: check for recursive loops, when color getter also reference
         // a different color type
         switch (sis) {
@@ -554,17 +550,17 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
             default:
                 break;
         }
-            
+
         return null;
     }
-        
+
     private Color applySysIndexProcedure(EscherColorRef ecr, Color col) {
-        
+
         final SysIndexProcedure sip = ecr.getSysIndexProcedure();
         if (col == null || sip == null) {
             return col;
         }
-        
+
         switch (sip) {
             case DARKEN_COLOR: {
                 // see java.awt.Color#darken()
@@ -572,29 +568,29 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
                 int r = (int)Math.rint(col.getRed()*FACTOR);
                 int g = (int)Math.rint(col.getGreen()*FACTOR);
                 int b = (int)Math.rint(col.getBlue()*FACTOR);
-                return new Color(r,g,b);                
+                return new Color(r,g,b);
             }
             case LIGHTEN_COLOR: {
                 double FACTOR = (0xFF-ecr.getRGB()[2])/255.;
-                               
+
                 int r = col.getRed();
                 int g = col.getGreen();
                 int b = col.getBlue();
-                
+
                 r += Math.rint((0xFF-r)*FACTOR);
                 g += Math.rint((0xFF-g)*FACTOR);
                 b += Math.rint((0xFF-b)*FACTOR);
-                
+
                 return new Color(r,g,b);
             }
             default:
                 // TODO ...
                 break;
         }
-        
+
         return col;
     }
-    
+
     double getAlpha(EscherPropertyTypes opacityProperty) {
         AbstractEscherOptRecord opt = getEscherOptRecord();
         EscherSimpleProperty op = getEscherProperty(opt, opacityProperty);
@@ -602,7 +598,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
         int opacity = (op == null) ? defaultOpacity : op.getPropertyValue();
         return Units.fixedPointToDouble(opacity);
     }
-    
+
     Color toRGB(int val){
         int a = (val >> 24) & 0xFF;
         int b = (val >> 16) & 0xFF;
@@ -669,12 +665,12 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
         }
         return opt;
     }
-    
+
     public boolean getFlipHorizontal(){
         EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
         return (spRecord.getFlags()& EscherSpRecord.FLAG_FLIPHORIZ) != 0;
     }
-     
+
     public void setFlipHorizontal(boolean flip) {
         EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
         int flag = spRecord.getFlags() | EscherSpRecord.FLAG_FLIPHORIZ;
@@ -685,7 +681,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
         EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
         return (spRecord.getFlags()& EscherSpRecord.FLAG_FLIPVERT) != 0;
     }
-    
+
     public void setFlipVertical(boolean flip) {
         EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
         int flag = spRecord.getFlags() | EscherSpRecord.FLAG_FLIPVERT;
@@ -696,7 +692,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
         int rot = getEscherProperty(EscherPropertyTypes.TRANSFORM__ROTATION);
         return Units.fixedPointToDouble(rot);
     }
-    
+
     public void setRotation(double theta){
         int rot = Units.doubleToFixedPoint(theta % 360.0);
         setEscherProperty(EscherPropertyTypes.TRANSFORM__ROTATION, rot);
@@ -726,7 +722,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     /**
      * Search for EscherClientDataRecord, if found, convert its contents into an array of HSLF records
      *
-     * @return an array of HSLF records contained in the shape's EscherClientDataRecord or <code>null</code>
+     * @return an array of HSLF records contained in the shape's EscherClientDataRecord or {@code null}
      */
     protected List<? extends Record> getClientRecords() {
         HSLFEscherClientDataRecord clientData = getClientData(false);
@@ -736,7 +732,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     /**
      * Create a new HSLF-specific EscherClientDataRecord
      *
-     * @param create if true, create the missing record 
+     * @param create if true, create the missing record
      * @return the client record or null if it was missing and create wasn't activated
      */
     protected HSLFEscherClientDataRecord getClientData(boolean create) {
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
index 3f1e48c6af..318b0ffded 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java
@@ -86,14 +86,6 @@ import org.apache.poi.util.Units;
 public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFTextParagraph,HSLFTextRun> {
     private static final Logger LOG = LogManager.getLogger(HSLFTextParagraph.class);
 
-    /**
-     * How to align the text
-     */
-    /* package */static final int AlignLeft = 0;
-    /* package */static final int AlignCenter = 1;
-    /* package */static final int AlignRight = 2;
-    /* package */static final int AlignJustify = 3;
-
     // Note: These fields are protected to help with unit testing
     // Other classes shouldn't really go playing with them!
     private final TextHeaderAtom _headerAtom;
@@ -120,19 +112,23 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
             this.tabStop = tabStop;
         }
 
+        @Override
         public double getPositionInPoints() {
             return tabStop.getPositionInPoints();
         }
 
+        @Override
         public void setPositionInPoints(double position) {
             tabStop.setPositionInPoints(position);
             setDirty();
         }
 
+        @Override
         public TabStopType getType() {
             return tabStop.getType();
         }
 
+        @Override
         public void setType(TabStopType type) {
             tabStop.setType(type);
             setDirty();
@@ -165,18 +161,6 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
         setParagraphStyle(new TextPropCollection(1, TextPropType.paragraph));
     }
 
-    /* package */HSLFTextParagraph(HSLFTextParagraph other) {
-        _headerAtom = other._headerAtom;
-        _byteAtom = other._byteAtom;
-        _charAtom = other._charAtom;
-        _parentShape = other._parentShape;
-        _sheet = other._sheet;
-        _ruler = other._ruler;
-        shapeId = other.shapeId;
-        parentList = other.parentList;
-        setParagraphStyle(other._paragraphStyle);
-    }
-
     public void addTextRun(HSLFTextRun run) {
         _runs.add(run);
     }
@@ -248,8 +232,6 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
 
     /**
      * Sets the index of the paragraph in the SLWT container
-     *
-     * @param index
      */
     protected void setIndex(int index) {
         if (_headerAtom != null) {
@@ -636,7 +618,7 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
      * Sets the bullet character
      */
     public void setBulletChar(Character c) {
-        Integer val = (c == null) ? null : (int)c.charValue();
+        Integer val = (c == null) ? null : (int) c;
         setParagraphTextPropVal("bullet.char", val);
     }
 
@@ -765,23 +747,21 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
     @Override
     public List<? extends TabStop> getTabStops() {
         final List<HSLFTabStop> tabStops;
-        final TextRulerAtom textRuler;
         if (getSheet() instanceof HSLFSlideMaster) {
             final HSLFTabStopPropCollection tpc = getMasterPropVal(_paragraphStyle, HSLFTabStopPropCollection.NAME);
             if (tpc == null) {
                 return null;
             }
             tabStops = tpc.getTabStops();
-            textRuler = null;
         } else {
-            textRuler = (TextRulerAtom)_headerAtom.getParentRecord().findFirstOfType(RecordTypes.TextRulerAtom.typeID);
+            final TextRulerAtom textRuler = (TextRulerAtom) _headerAtom.getParentRecord().findFirstOfType(RecordTypes.TextRulerAtom.typeID);
             if (textRuler == null) {
                 return null;
             }
             tabStops = textRuler.getTabStops();
         }
 
-        return tabStops.stream().map((tabStop) -> new HSLFTabStopDecorator(tabStop)).collect(Collectors.toList());
+        return tabStops.stream().map(HSLFTabStopDecorator::new).collect(Collectors.toList());
     }
 
     @Override
@@ -836,11 +816,11 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
 
     private boolean getFlag(int index) {
         BitMaskTextProp tp = getPropVal(_paragraphStyle, ParagraphFlagsTextProp.NAME);
-        return (tp == null) ? false : tp.getSubValue(index);
+        return tp != null && tp.getSubValue(index);
     }
 
     private void setFlag(int index, boolean value) {
-        BitMaskTextProp tp = (BitMaskTextProp)_paragraphStyle.addWithName(ParagraphFlagsTextProp.NAME);
+        BitMaskTextProp tp = _paragraphStyle.addWithName(ParagraphFlagsTextProp.NAME);
         tp.setSubValue(value, index);
         setDirty();
     }
@@ -1047,7 +1027,6 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
             StringUtil.putCompressedUnicode(rawText, byteText, 0);
             byteAtom.setText(byteText);
         }
-        assert (newRecord != null);
 
         RecordContainer _txtbox = headerAtom.getParentRecord();
         Record[] cr = _txtbox.getChildRecords();
@@ -1528,7 +1507,7 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
                     break;
                 }
                 List<HSLFTextRun> runs = p.getTextRuns();
-                for (int rlen=0,rIdx=0; rIdx < runs.size(); csIdx+=rlen, rIdx++) {
+                for (int rlen,rIdx=0; rIdx < runs.size(); csIdx+=rlen, rIdx++) {
                     HSLFTextRun run = runs.get(rIdx);
                     rlen = run.getLength();
                     if (csIdx < h.getEndIndex() && h.getStartIndex() < csIdx+rlen) {
@@ -1722,8 +1701,6 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
     }
 
     /**
-     * {@inheritDoc}
-     *
      * @see RoundTripHFPlaceholder12
      */
     @Override
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/draw/HwmfDrawProperties.java b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/draw/HwmfDrawProperties.java
index f0993b24da..ebb3a0234a 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/draw/HwmfDrawProperties.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/draw/HwmfDrawProperties.java
@@ -322,8 +322,6 @@ public class HwmfDrawProperties {
     /**
      * Sets the current palette.
      * It's the callers duty to set a modifiable copy of the palette.
-     *
-     * @param palette
      */
     public void setPalette(List<PaletteEntry> palette) {
         this.palette = palette;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfEscape.java b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfEscape.java
index c56ae3f17f..39586fdd7e 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfEscape.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfEscape.java
@@ -223,6 +223,7 @@ public class HwmfEscape implements HwmfRecord {
         return escapeFunction;
     }
 
+    @SuppressWarnings("unchecked")
     public <T extends HwmfEscapeData> T getEscapeData() {
         return (T)escapeData;
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfFill.java b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfFill.java
index 90fe33881e..4dda258df2 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfFill.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfFill.java
@@ -171,10 +171,12 @@ public class HwmfFill {
          */
         int regionIndex;
 
+        @Override
         public HwmfRecordType getWmfRecordType() {
             return HwmfRecordType.paintRegion;
         }
 
+        @Override
         public int init(LittleEndianInputStream leis, long recordSize, int recordFunction) throws IOException {
             regionIndex = leis.readUShort();
             return LittleEndianConsts.SHORT_SIZE;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfRecord.java b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfRecord.java
index b68af18460..17a89185c7 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfRecord.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwmf/record/HwmfRecord.java
@@ -31,10 +31,9 @@ public interface HwmfRecord extends GenericRecord {
      *
      * @param leis the little endian input stream
      * @return count of processed bytes
-     * @throws IOException
      */
     int init(LittleEndianInputStream leis, long recordSize, int recordFunction) throws IOException;
-    
+
     /**
      * Apply the record settings to the graphics context
      *
@@ -43,7 +42,7 @@ public interface HwmfRecord extends GenericRecord {
     void draw(HwmfGraphics ctx);
 
     @Override
-    default Enum getGenericRecordType() {
+    default Enum<?> getGenericRecordType() {
         return getWmfRecordType();
     }
 }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/CharIndexTranslator.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/CharIndexTranslator.java
index da2e913b1c..f8665be8d4 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/CharIndexTranslator.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/CharIndexTranslator.java
@@ -44,7 +44,6 @@ public interface CharIndexTranslator {
     /**
      * Check if index is in table
      *
-     * @param bytePos
      * @return true if index in table, false if not
      */
     boolean isIndexInTable(int bytePos);
@@ -52,7 +51,6 @@ public interface CharIndexTranslator {
     /**
      * Return first index &gt;= bytePos that is in table
      *
-     * @param bytePos
      * @return  first index greater or equal to bytePos that is in table
      */
     int lookIndexForward(int bytePos);
@@ -60,7 +58,6 @@ public interface CharIndexTranslator {
     /**
      * Return last index &lt;= bytePos that is in table
      *
-     * @param bytePos
      * @return last index less of equal to bytePos that is in table
      */
     int lookIndexBackward(int bytePos);
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/FileInformationBlock.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/FileInformationBlock.java
index 0fb355cc4a..ecb125b6e8 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/FileInformationBlock.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/FileInformationBlock.java
@@ -38,7 +38,7 @@ import static org.apache.logging.log4j.util.Unbox.box;
 /**
  * <p>The File Information Block (FIB). Holds pointers
  *  to various bits of the file, and lots of flags which
- *  specify properties of the document.<p>
+ *  specify properties of the document.
  * <ul>
  * <li>The {@link FibBase} class, holds the first 32 bytes.</li>
  * <li>The next part, the fibRgW / FibRgW97, is handled by the {@link FibRgW97}.</li>
@@ -244,12 +244,12 @@ public final class FileInformationBlock {
 
         // field info
         for ( FieldsDocumentPart part : FieldsDocumentPart.values() )
-            knownFieldSet.add( Integer.valueOf( part.getFibFieldsField() ) );
+            knownFieldSet.add(part.getFibFieldsField());
 
         // bookmarks
-        knownFieldSet.add( Integer.valueOf( FIBFieldHandler.PLCFBKF ) );
-        knownFieldSet.add( Integer.valueOf( FIBFieldHandler.PLCFBKL ) );
-        knownFieldSet.add( Integer.valueOf( FIBFieldHandler.STTBFBKMK ) );
+        knownFieldSet.add(FIBFieldHandler.PLCFBKF);
+        knownFieldSet.add(FIBFieldHandler.PLCFBKL);
+        knownFieldSet.add(FIBFieldHandler.STTBFBKMK);
 
         // notes
         for ( NoteType noteType : NoteType.values() ) {
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldCHPBinTable.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldCHPBinTable.java
index 04fdd322a2..24c2a95610 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldCHPBinTable.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldCHPBinTable.java
@@ -33,13 +33,7 @@ import org.apache.poi.util.LittleEndian;
 public final class OldCHPBinTable extends CHPBinTable
 {
   /**
-   * Constructor used to read an old-style binTable
-   *  in from a Word document.
-   *
-   * @param documentStream
-   * @param offset
-   * @param size
-   * @param fcMin
+   * Constructor used to read an old-style binTable in from a Word document.
    */
   public OldCHPBinTable(byte[] documentStream, int offset,
                      int size, int fcMin, OldTextPieceTable tpt)
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPiece.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPiece.java
index b14c38223d..22289ae954 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPiece.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPiece.java
@@ -52,7 +52,6 @@ public class OldTextPiece extends TextPiece {
 
     /**
      * @return nothing, ever. Always throws an UnsupportedOperationException
-     * @throws UnsupportedOperationException
      */
     @NotImplemented
     @Override
@@ -77,8 +76,8 @@ public class OldTextPiece extends TextPiece {
      *
      * @param start Local start position, in characters
      * @param end   Local end position, in characters
-     * @throws UnsupportedOperationException
      */
+    @Override
     @Deprecated
     @NotImplemented
     public String substring(int start, int end) {
@@ -89,6 +88,7 @@ public class OldTextPiece extends TextPiece {
      * Not implemented for OldTextPiece.
      * Always throws UnsupportedOperationException
      */
+    @Override
     @Deprecated
     @NotImplemented
     public void adjustForDelete(int start, int length) {
@@ -98,6 +98,7 @@ public class OldTextPiece extends TextPiece {
     /**
      * Returns the length, in bytes
      */
+    @Override
     public int bytesLength() {
         return rawBytes.length;
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPieceTable.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPieceTable.java
index e6a0887f32..478af6a272 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPieceTable.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OldTextPieceTable.java
@@ -94,7 +94,7 @@ public class OldTextPieceTable extends TextPieceTable {
         // into order, if they're not already
         Collections.sort(_textPieces);
         _textPiecesFCOrder = new ArrayList<>(_textPieces);
-        _textPiecesFCOrder.sort(new FCComparator());
+        _textPiecesFCOrder.sort(byFilePosition());
 
     }
 
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java
index 658cb60c9e..6c8ee4dd46 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java
@@ -39,8 +39,8 @@ public final class PlexOfCps {
     private static final int MAX_RECORD_LENGTH = 10_485_760;
 
     private int _iMac;
-    private int _cbStruct;
-    private List<GenericPropertyNode> _props;
+    private final int _cbStruct;
+    private final List<GenericPropertyNode> _props;
 
     public PlexOfCps(int sizeOfStruct) {
         _props = new ArrayList<>();
@@ -69,18 +69,10 @@ public final class PlexOfCps {
     void adjust(int startCp, int shift) {
         for (GenericPropertyNode node : _props) {
             if (node.getStart() > startCp) {
-                if (node.getStart() + shift < startCp) {
-                    node.setStart(startCp);
-                } else {
-                    node.setStart(node.getStart() + shift);
-                }
+                node.setStart(Math.max(node.getStart() + shift, startCp));
             }
             if (node.getEnd() >= startCp) {
-                if (node.getEnd() + shift < startCp) {
-                    node.setEnd(startCp);
-                } else {
-                    node.setEnd(node.getEnd() + shift);
-                }
+                node.setEnd(Math.max(node.getEnd() + shift, startCp));
             }
         }
     }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlfLfo.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlfLfo.java
index 79b8587bae..b92c2d41b2 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlfLfo.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlfLfo.java
@@ -162,7 +162,6 @@ public class PlfLfo
     /**
      * @param ilfo 1-based index
      * @return The {@link LFO} stored at the given index
-     * @throws NoSuchElementException
      */
     public LFO getLfo( int ilfo ) throws NoSuchElementException
     {
@@ -177,7 +176,6 @@ public class PlfLfo
     /**
      * @param ilfo 1-based index
      * @return The {@link LFOData} stored at the given index
-     * @throws NoSuchElementException
      */
     public LFOData getLfoData( int ilfo ) throws NoSuchElementException
     {
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPiece.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPiece.java
index 0c606cbf03..bdb9ba492a 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPiece.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPiece.java
@@ -32,9 +32,9 @@ import org.apache.poi.util.StringUtil;
  */
 @Internal
 public class TextPiece extends PropertyNode<TextPiece> {
-    private boolean _usesUnicode;
+    private final boolean _usesUnicode;
 
-    private PieceDescriptor _pd;
+    private final PieceDescriptor _pd;
 
     public TextPiece(TextPiece other) {
         super(other);
@@ -142,6 +142,7 @@ public class TextPiece extends PropertyNode<TextPiece> {
      * @param start  The start position for the delete, in characters
      * @param length The number of characters to delete
      */
+    @Override
     @Deprecated
     public void adjustForDelete(int start, int length) {
         int myStart = getStart();
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPieceTable.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPieceTable.java
index 22434de9a2..5f8c4da5ee 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPieceTable.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/TextPieceTable.java
@@ -18,7 +18,6 @@ package org.apache.poi.hwpf.model;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -114,7 +113,7 @@ public class TextPieceTable implements CharIndexTranslator {
         // into order, if they're not already
         Collections.sort(_textPieces);
         _textPiecesFCOrder = new ArrayList<>(_textPieces);
-        _textPiecesFCOrder.sort(new FCComparator());
+        _textPiecesFCOrder.sort(byFilePosition());
     }
 
     protected TextPiece newTextPiece(int nodeStartChars, int nodeEndChars, byte[] buf, PieceDescriptor pd) {
@@ -125,7 +124,7 @@ public class TextPieceTable implements CharIndexTranslator {
         _textPieces.add(piece);
         _textPiecesFCOrder.add(piece);
         Collections.sort(_textPieces);
-        _textPiecesFCOrder.sort(new FCComparator());
+        _textPiecesFCOrder.sort(byFilePosition());
     }
 
     /**
@@ -169,6 +168,7 @@ public class TextPieceTable implements CharIndexTranslator {
         return false;
     }
 
+    @Override
     public int getByteIndex(int charPos) {
         int byteCount = 0;
         for (TextPiece tp : _textPieces) {
@@ -308,6 +308,7 @@ public class TextPieceTable implements CharIndexTranslator {
         return _textPieces.hashCode();
     }
 
+    @Override
     public boolean isIndexInTable(int bytePos) {
         for (TextPiece tp : _textPiecesFCOrder) {
             int pieceStart = tp.getPieceDescriptor().getFilePosition();
@@ -339,6 +340,7 @@ public class TextPieceTable implements CharIndexTranslator {
         return false;
     }
 
+    @Override
     public int lookIndexBackward(final int startBytePos) {
         int bytePos = startBytePos;
         int lastEnd = 0;
@@ -361,6 +363,7 @@ public class TextPieceTable implements CharIndexTranslator {
         return bytePos;
     }
 
+    @Override
     public int lookIndexForward(final int startBytePos) {
         if (_textPiecesFCOrder.isEmpty())
             throw new IllegalStateException("Text pieces table is empty");
@@ -433,10 +436,7 @@ public class TextPieceTable implements CharIndexTranslator {
         return textPlex.toByteArray();
     }
 
-    protected static class FCComparator implements Comparator<TextPiece>, Serializable {
-        public int compare(TextPiece textPiece, TextPiece textPiece1) {
-            return Integer.compare(textPiece.getPieceDescriptor().fc, textPiece1
-                    .getPieceDescriptor().fc);
-        }
+    static Comparator<TextPiece> byFilePosition() {
+        return Comparator.comparing(t -> t.getPieceDescriptor().getFilePosition());
     }
 }
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/HeaderStories.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/HeaderStories.java
index b15757ae03..8df6f545c7 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/HeaderStories.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/HeaderStories.java
@@ -34,7 +34,7 @@ import org.apache.poi.hwpf.model.SubdocumentType;
  *  as offsets are not yet updated!
  */
 public final class HeaderStories {
-	private Range headerStories;
+	private final Range headerStories;
 	private PlexOfCps plcfHdd;
 
 	private boolean stripFields;
@@ -50,7 +50,7 @@ public final class HeaderStories {
 
         if (fib.getSubdocumentTextStreamLength( SubdocumentType.HEADER ) == 0)
 		    return;
-		
+
 		if(fib.getPlcfHddSize() == 0) {
 			return;
 		}
@@ -58,7 +58,7 @@ public final class HeaderStories {
         // Handle the PlcfHdd
         /*
          * Page 88:
-         * 
+         *
          * "The plcfhdd, a table whose location and length within the file is
          * stored in fib.fcPlcfhdd and fib.cbPlcfhdd, describes where the text
          * of each header/footer begins. If there are n headers/footers stored
@@ -69,11 +69,11 @@ public final class HeaderStories {
          * plcfhdd. Note: at the limit CP - 1, Word always places a chEop as a
          * placeholder which is never displayed as part of the header/footer.
          * This allows Word to change an existing header/footer to be empty.
-         * 
+         *
          * If there are n header/footers, the n+2nd CP entry value is always 1
          * greater than the n+1st CP entry value. A paragraph end (ASCII 13) is
          * always stored at the file position marked by the n+1st CP value.
-         * 
+         *
          * The transformation in a full saved file from a header/footer CP to an
          * offset from the beginning of a file (fc) is
          * fc=fib.fcMin+ccpText+ccpFtn+cp."
@@ -83,7 +83,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getFootnoteSeparator()
@@ -92,7 +92,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getFootnoteContSeparator()
@@ -101,7 +101,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getFootnoteContNote()
@@ -110,7 +110,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getEndnoteSeparator()
@@ -119,7 +119,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getEndnoteContSeparator()
@@ -128,7 +128,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getEndnoteContNote()
@@ -167,27 +167,27 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
 	@Deprecated
 	public String getEvenHeader() {
 		return getAt(6+0);
 	}
 	/**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getOddHeader() {
 		return getAt(6+1);
 	}
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getFirstHeader() {
 		return getAt(6+4);
 	}
-	
+
 
     public Range getEvenHeaderSubrange() {
         return getSubrangeAt(6+0);
@@ -198,7 +198,7 @@ public final class HeaderStories {
     public Range getFirstHeaderSubrange() {
         return getSubrangeAt(6+4);
     }
-    
+
 	/**
 	 * Returns the correct, defined header for the given
 	 *  one based page
@@ -226,7 +226,7 @@ public final class HeaderStories {
 	}
 
 	/**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getEvenFooter()
@@ -235,7 +235,7 @@ public final class HeaderStories {
     }
 
 	/**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getOddFooter()
@@ -244,7 +244,7 @@ public final class HeaderStories {
     }
 
     /**
-     * @deprecated 3.8 beta 4 
+     * @deprecated 3.8 beta 4
      */
     @Deprecated
     public String getFirstFooter()
@@ -370,7 +370,7 @@ public final class HeaderStories {
 
 	/**
 	 * Are fields currently being stripped from
-	 *  the text that this {@link HeaderStories} returns?
+	 *  the text that this HeaderStories returns?
 	 *  Default is false, but can be changed
 	 */
 	public boolean areFieldsStripped() {
@@ -380,7 +380,6 @@ public final class HeaderStories {
 	 * Should fields (eg macros) be stripped from
 	 *  the text that this class returns?
 	 * Default is not to strip.
-	 * @param stripFields
 	 */
 	public void setAreFieldsStripped(boolean stripFields) {
 		this.stripFields = stripFields;
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/Picture.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/Picture.java
index d4283a612b..26bfc0546a 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/Picture.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/usermodel/Picture.java
@@ -30,15 +30,19 @@ import org.apache.logging.log4j.Logger;
 import org.apache.poi.ddf.EscherBSERecord;
 import org.apache.poi.ddf.EscherBlipRecord;
 import org.apache.poi.ddf.EscherComplexProperty;
+import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherOptRecord;
 import org.apache.poi.ddf.EscherProperty;
 import org.apache.poi.ddf.EscherPropertyTypes;
 import org.apache.poi.ddf.EscherRecord;
+import org.apache.poi.ddf.EscherRecordTypes;
+import org.apache.poi.ddf.EscherSimpleProperty;
 import org.apache.poi.hwpf.model.PICF;
 import org.apache.poi.hwpf.model.PICFAndOfficeArtData;
 import org.apache.poi.sl.image.ImageHeaderPNG;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.StringUtil;
+import org.apache.poi.util.Units;
 
 /**
  * Represents embedded picture extracted from Word Document
@@ -264,43 +268,43 @@ public final class Picture {
 
     /**
      * @return The amount the picture has been cropped on the left in twips
-     * @deprecated POI 3.8 beta 4.
      */
-    @Deprecated
-    public int getDxaCropLeft()
-    {
-        // TODO: use new properties
-        // if (_picfAndOfficeArtData == null || _picfAndOfficeArtData.getShape()
-        // == null)
-        // return 0;
-        //
-        // final EscherContainerRecord shape = _picfAndOfficeArtData.getShape();
-        // EscherOptRecord optRecord = shape.getChildById( (short) 0xF00B );
-        // if (optRecord == null)
-        // return 0;
-        //
-        // EscherProperty property = optRecord.lookup( 0x0102 );
-        // if (property == null || !(property instanceof EscherSimpleProperty))
-        // return 0;
-        //
-        // EscherSimpleProperty simpleProperty = (EscherSimpleProperty)
-        // property;
-        // return simpleProperty.getPropertyValue();
-
+    public int getDxaCropLeft() {
         return _picf.getDxaReserved1();
     }
 
     /**
+     * The location, expressed as a fraction of the image width, of the left side of
+     * the crop rectangle. A value of 0 specifies that the left side of the image is uncropped.
+     * Positive values specify cropping into the image. Negative values specify cropping out from the
+     * image. The default value for this property is 0.
+     *
+     * @return the left crop percent
+     */
+    public double getCropLeft() {
+        return getCrop(EscherPropertyTypes.BLIP__CROPFROMLEFT);
+    }
+
+    /**
      * @return The amount the picture has been cropped on the right in twips
-     * @deprecated POI 3.8 beta 4.
      */
-    @Deprecated
-    public int getDxaCropRight()
-    {
+    public int getDxaCropRight() {
         return _picf.getDxaReserved2();
     }
 
     /**
+     * the location of the right side, expressed as a fraction of the image width, of
+     * the crop rectangle. A value of 0 specifies that the right side of the image is uncropped.
+     * Positive values specify cropping into the image. Negative values specify cropping out from the
+     * image. The default value for this property is 0.
+     *
+     * @return the right crop percent
+     */
+    public double getCropRight() {
+        return getCrop(EscherPropertyTypes.BLIP__CROPFROMRIGHT);
+    }
+
+    /**
      * Gets the initial width of the picture, in twips, prior to cropping or
      * scaling.
      *
@@ -313,25 +317,58 @@ public final class Picture {
 
     /**
      * @return The amount the picture has been cropped on the bottom in twips
-     * @deprecated POI 3.8 beta 5.
      */
-    @Deprecated
-    public int getDyaCropBottom()
-    {
+    public int getDyaCropBottom() {
         return _picf.getDyaReserved2();
     }
 
     /**
+     * the location, expressed as a fraction of the image height, of the bottom of
+     * the crop rectangle. A value of 0 specifies that the bottom of the image is uncropped.
+     * Positive values specify cropping into the image. Negative values specify cropping out from the
+     * image. The default value for this property is 0
+     *
+     * @return the bottom crop percent
+     */
+    public double getCropBottom() {
+        return getCrop(EscherPropertyTypes.BLIP__CROPFROMBOTTOM);
+    }
+
+    /**
      * @return The amount the picture has been cropped on the top in twips
-     * @deprecated POI 3.8 beta 5.
      */
-    @Deprecated
-    public int getDyaCropTop()
-    {
+    public int getDyaCropTop() {
         return _picf.getDyaReserved1();
     }
 
     /**
+     * the location, expressed as a fraction of the image height, of the top of the crop
+     * rectangle. A value of 0 specifies that the top of the image is uncropped. Positive values
+     * specify cropping into the image. Negative values specify cropping out from the image. The default
+     * value for this property is 0.
+     *
+     * @return the top crop percent
+     */
+    public double getCropTop() {
+        return getCrop(EscherPropertyTypes.BLIP__CROPFROMTOP);
+    }
+
+    private double getCrop(EscherPropertyTypes propType) {
+        EscherContainerRecord shape;
+        if (_picfAndOfficeArtData != null && (shape = _picfAndOfficeArtData.getShape()) != null) {
+            EscherOptRecord optRecord = shape.getChildById(EscherRecordTypes.OPT.typeID);
+            if (optRecord != null) {
+                EscherProperty property = optRecord.lookup(propType);
+                if (property instanceof EscherSimpleProperty) {
+                    EscherSimpleProperty simpleProperty = (EscherSimpleProperty) property;
+                    return Units.fixedPointToDouble(simpleProperty.getPropertyValue());
+                }
+            }
+        }
+        return 0;
+    }
+
+    /**
      * Gets the initial height of the picture, in twips, prior to cropping or
      * scaling.
      *
@@ -490,62 +527,59 @@ public final class Picture {
                 + ( fileExt.length() > 0 ? "." + fileExt : "" );
     }
 
-    public PictureType suggestPictureType()
-    {
+    public PictureType suggestPictureType() {
         if (_blipRecords.size() != 1 ) {
             return PictureType.UNKNOWN;
         }
 
         EscherRecord escherRecord = _blipRecords.get( 0 );
-        switch ( escherRecord.getRecordId() )
-        {
-        case (short) 0xF007:
-        {
+        if (escherRecord instanceof EscherBSERecord) {
             EscherBSERecord bseRecord = (EscherBSERecord) escherRecord;
-            switch ( bseRecord.getBlipTypeWin32() )
-            {
-            case 0x00:
-                return PictureType.UNKNOWN;
-            case 0x01:
-                return PictureType.UNKNOWN;
-            case 0x02:
+            switch ( bseRecord.getBlipTypeWin32() ) {
+                case 0x00:
+                    return PictureType.UNKNOWN;
+                case 0x01:
+                    return PictureType.UNKNOWN;
+                case 0x02:
+                    return PictureType.EMF;
+                case 0x03:
+                    return PictureType.WMF;
+                case 0x04:
+                    return PictureType.PICT;
+                case 0x05:
+                    return PictureType.JPEG;
+                case 0x06:
+                    return PictureType.PNG;
+                case 0x07:
+                    return PictureType.BMP;
+                case 0x11:
+                    return PictureType.TIFF;
+                case 0x12:
+                    return PictureType.JPEG;
+                default:
+                    return PictureType.UNKNOWN;
+            }
+        }
+
+        Enum<?> recordType = escherRecord.getGenericRecordType();
+        assert (recordType instanceof EscherRecordTypes);
+        switch ((EscherRecordTypes)recordType) {
+            case BLIP_EMF:
                 return PictureType.EMF;
-            case 0x03:
+            case BLIP_WMF:
                 return PictureType.WMF;
-            case 0x04:
+            case BLIP_PICT:
                 return PictureType.PICT;
-            case 0x05:
+            case BLIP_JPEG:
                 return PictureType.JPEG;
-            case 0x06:
+            case BLIP_PNG:
                 return PictureType.PNG;
-            case 0x07:
+            case BLIP_DIB:
                 return PictureType.BMP;
-            case 0x11:
+            case BLIP_TIFF:
                 return PictureType.TIFF;
-            case 0x12:
-                return PictureType.JPEG;
             default:
                 return PictureType.UNKNOWN;
-            }
-        }
-        case (short) 0xF01A:
-            return PictureType.EMF;
-        case (short) 0xF01B:
-            return PictureType.WMF;
-        case (short) 0xF01C:
-            return PictureType.PICT;
-        case (short) 0xF01D:
-            return PictureType.JPEG;
-        case (short) 0xF01E:
-            return PictureType.PNG;
-        case (short) 0xF01F:
-            return PictureType.BMP;
-        case (short) 0xF029:
-            return PictureType.TIFF;
-        case (short) 0xF02A:
-            return PictureType.JPEG;
-        default:
-            return PictureType.UNKNOWN;
         }
     }
 
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/util/DoubleByteUtil.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/util/DoubleByteUtil.java
index 5d55711ed9..6c8ef1214b 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/util/DoubleByteUtil.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/util/DoubleByteUtil.java
@@ -40,9 +40,6 @@ public class DoubleByteUtil
      * We know MS zero-padded ascii, and we drop those.
      * There may be areas for improvement in this.
      *
-     * @param data
-     * @param offset
-     * @param lengthInBytes
      * @return Decoded String
      */
     public static String cp950ToString(byte[] data, int offset, int lengthInBytes) {
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToHtmlConverter.java b/poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToHtmlConverter.java
index a7e8b1fec7..ed42294333 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToHtmlConverter.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToHtmlConverter.java
@@ -100,7 +100,7 @@ public class TestWordToHtmlConverter {
 
     })
     void testFile(String file, String contains) throws Exception {
-        boolean emulatePictureStorage = file.contains("picture");
+        boolean emulatePictureStorage = !file.contains("equation");
 
         String result = getHtmlText(file, emulatePictureStorage);
         assertNotNull(result);