From 5226ed80dd87ca47e3f97798e974b4235ad3ba75 Mon Sep 17 00:00:00 2001
From: Dominik Stadler <centic@apache.org>
Date: Sun, 1 Nov 2020 09:21:16 +0000
Subject: [PATCH] Improve error messages on missing Factory-implementations,
 fix Sonar issues and IDE warnings

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1883035 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/extractor/ExtractorFactory.java       |  6 ++-
 .../poi/ss/usermodel/WorkbookFactory.java     |  3 +-
 .../poifs/crypt/tests/TestHxxFEncryption.java |  2 +-
 .../poi/xssf/model/TestThemesTable.java       | 46 +++++++++----------
 .../poi/hssf/record/TestNameRecord.java       |  3 --
 .../eval/BaseTestCircularReferences.java      |  3 +-
 .../org/apache/poi/util/TestLocaleUtil.java   | 28 +++++------
 7 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/src/java/org/apache/poi/extractor/ExtractorFactory.java b/src/java/org/apache/poi/extractor/ExtractorFactory.java
index a908a73085..0833535189 100644
--- a/src/java/org/apache/poi/extractor/ExtractorFactory.java
+++ b/src/java/org/apache/poi/extractor/ExtractorFactory.java
@@ -307,8 +307,10 @@ public final class ExtractorFactory {
                 }
             }
         }
-        throw new IOException("Your InputStream was neither an OLE2 stream, nor an OOXML stream " +
-            "or you haven't provide the poi-ooxml*.jar and/or poi-scratchpad*.jar in the classpath/modulepath - FileMagic: "+fm);
+        throw new IOException(
+            "Your InputStream was neither an OLE2 stream, nor an OOXML stream " +
+            "or you haven't provide the poi-ooxml*.jar and/or poi-scratchpad*.jar in the classpath/modulepath - FileMagic: " + fm +
+            ", providers: " + Singleton.INSTANCE.provider);
     }
 
     public static void addProvider(ExtractorProvider provider){
diff --git a/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java b/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
index c0f376f65f..b730731f7c 100644
--- a/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
+++ b/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
@@ -296,7 +296,8 @@ public final class WorkbookFactory {
             }
         }
         throw new IOException("Your InputStream was neither an OLE2 stream, nor an OOXML stream " +
-            "or you haven't provide the poi-ooxml*.jar in the classpath/modulepath - FileMagic: "+fm);
+            "or you haven't provide the poi-ooxml*.jar in the classpath/modulepath - FileMagic: " + fm +
+                ", having providers: " + Singleton.INSTANCE.provider);
     }
 
     public static void addProvider(WorkbookProvider provider){
diff --git a/src/ooxml/testcases/org/apache/poi/poifs/crypt/tests/TestHxxFEncryption.java b/src/ooxml/testcases/org/apache/poi/poifs/crypt/tests/TestHxxFEncryption.java
index 393186e68d..a687045645 100644
--- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/tests/TestHxxFEncryption.java
+++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/tests/TestHxxFEncryption.java
@@ -49,7 +49,7 @@ import org.junit.runners.Parameterized.Parameters;
 
 @RunWith(Parameterized.class)
 public class TestHxxFEncryption {
-    @Parameter(value = 0)
+    @Parameter
     public POIDataSamples sampleDir;
 
     @Parameter(value = 1)
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java b/src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java
index 2642fb0074..81bb44b90e 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java
@@ -45,12 +45,14 @@ import org.junit.Test;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTColor;
 
 public class TestThemesTable {
-    private final String testFileSimple = "Themes.xlsx";
-    private final String testFileComplex = "Themes2.xlsx";
-    // TODO .xls version available too, add HSSF support then check 
+    private static final String testFileComplex = "Themes2.xlsx";
+    // TODO .xls version available too, add HSSF support then check
+
+    // For offline testing
+    private static final boolean createFiles = false;
 
     // What colours they should show up as
-    private static String[] rgbExpected = {
+    private static final String[] rgbExpected = {
             "ffffff", // Lt1
             "000000", // Dk1
             "eeece1", // Lt2
@@ -68,6 +70,7 @@ public class TestThemesTable {
     @Test
     public void testThemesTableColors() throws Exception {
         // Load our two test workbooks
+        String testFileSimple = "Themes.xlsx";
         XSSFWorkbook simple = XSSFTestDataSamples.openSampleWorkbook(testFileSimple);
         XSSFWorkbook complex = XSSFTestDataSamples.openSampleWorkbook(testFileComplex);
         // Save and re-load them, to check for stability across that
@@ -84,13 +87,10 @@ public class TestThemesTable {
         workbooks.put("Re-Saved_" + testFileSimple, simpleRS);
         workbooks.put(testFileComplex, complex);
         workbooks.put("Re-Saved_" + testFileComplex, complexRS);
-        
+
         // Sanity check
-        assertEquals(rgbExpected.length, rgbExpected.length);
-        
-        // For offline testing
-        boolean createFiles = false;
-        
+        assertEquals(12, rgbExpected.length);
+
         // Check each workbook in turn, and verify that the colours
         //  for the theme-applied cells in Column A are correct
         for (String whatWorkbook : workbooks.keySet()) {
@@ -98,7 +98,7 @@ public class TestThemesTable {
             XSSFSheet sheet = workbook.getSheetAt(0);
             int startRN = 0;
             if (whatWorkbook.endsWith(testFileComplex)) startRN++;
-            
+
             for (int rn=startRN; rn<rgbExpected.length+startRN; rn++) {
                 XSSFRow row = sheet.getRow(rn);
                 assertNotNull("Missing row " + rn + " in " + whatWorkbook, row);
@@ -119,7 +119,7 @@ public class TestThemesTable {
                 assertNotNull(ctColor);
                 assertTrue(ctColor.isSetTheme());
                 assertEquals(themeElem.idx, ctColor.getTheme());
-                
+
                 // Get the colour, via the theme
                 XSSFColor color = font.getXSSFColor();
                 // Theme colours aren't tinted
@@ -132,7 +132,7 @@ public class TestThemesTable {
                 assertEquals(
                         "Wrong theme index " + expectedThemeIdx + " on " + whatWorkbook,
                         expectedThemeIdx, themeIdx);
-                
+
                 if (createFiles) {
                     XSSFCellStyle cs = row.getSheet().getWorkbook().createCellStyle();
                     cs.setFillForegroundColor(color);
@@ -140,7 +140,7 @@ public class TestThemesTable {
                     row.createCell(1).setCellStyle(cs);
                 }
             }
-            
+
             if (createFiles) {
                 FileOutputStream fos = new FileOutputStream("Generated_"+whatWorkbook);
                 workbook.write(fos);
@@ -162,8 +162,8 @@ public class TestThemesTable {
      * Column C = Explicit Colour Foreground
      * Column E = Explicit Colour Background, Black Foreground
      * Column G = Conditional Formatting Backgrounds
-     * 
-     * Note - Grey Row has an odd way of doing the styling... 
+     *
+     * Note - Grey Row has an odd way of doing the styling...
      */
     @Test
     public void themedAndNonThemedColours() throws IOException {
@@ -239,31 +239,31 @@ public class TestThemesTable {
                 assertNull(color.getARGBHex());
             }
         }
-        
+
         // Check the CF colours
         // TODO
     }
     private static void assertCellContents(String expected, XSSFCell cell) {
         assertNotNull(cell);
-        assertEquals(expected.toLowerCase(Locale.ROOT), 
+        assertEquals(expected.toLowerCase(Locale.ROOT),
                      cell.getStringCellValue().toLowerCase(Locale.ROOT));
     }
-    
+
     @Test
     @SuppressWarnings("resource")
     public void testAddNew() {
         XSSFWorkbook wb = new XSSFWorkbook();
         wb.createSheet();
         assertNull(wb.getTheme());
-        
+
         StylesTable styles = wb.getStylesSource();
         assertNull(styles.getTheme());
-        
+
         styles.ensureThemesTable();
-        
+
         assertNotNull(styles.getTheme());
         assertNotNull(wb.getTheme());
-        
+
         wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
         styles = wb.getStylesSource();
         assertNotNull(styles.getTheme());
diff --git a/src/testcases/org/apache/poi/hssf/record/TestNameRecord.java b/src/testcases/org/apache/poi/hssf/record/TestNameRecord.java
index 6ddc194a11..7dfdf4f8ee 100644
--- a/src/testcases/org/apache/poi/hssf/record/TestNameRecord.java
+++ b/src/testcases/org/apache/poi/hssf/record/TestNameRecord.java
@@ -18,7 +18,6 @@
 package org.apache.poi.hssf.record;
 
 import static org.apache.poi.hssf.record.TestcaseRecordInputStream.confirmRecordEncoding;
-import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
@@ -115,8 +114,6 @@ public final class TestNameRecord {
 			"EQpiGoagbEQ3NMJCVEMSJiIMhTAMjxdcz2ERGmIaBqEiuqERElENSQhEGMrT+APX2vm3iyMAAA=="
 		);
 
-        assertArrayEquals(data1, data1);
-
         RecordInputStream in1 = TestcaseRecordInputStream.create(data1);
         NameRecord nr1 = new NameRecord(in1);
         assert_bug50244(nr1);
diff --git a/src/testcases/org/apache/poi/ss/formula/eval/BaseTestCircularReferences.java b/src/testcases/org/apache/poi/ss/formula/eval/BaseTestCircularReferences.java
index fc499d2277..ef9ab269e8 100644
--- a/src/testcases/org/apache/poi/ss/formula/eval/BaseTestCircularReferences.java
+++ b/src/testcases/org/apache/poi/ss/formula/eval/BaseTestCircularReferences.java
@@ -175,7 +175,7 @@ public abstract class BaseTestCircularReferences {
             fe.clearAllCachedResultValues();
             cv = fe.evaluate(cellB1);
             // Identified bug 46898
-            assertNotEquals(cv.getCellType(), ErrorEval.CIRCULAR_REF_ERROR.getErrorCode());
+            assertNotEquals(cv.getErrorValue(), ErrorEval.CIRCULAR_REF_ERROR.getErrorCode());
             assertEquals(CellType.NUMERIC, cv.getCellType());
             assertEquals(46.0, cv.getNumberValue(), 0.0);
 
@@ -184,7 +184,6 @@ public abstract class BaseTestCircularReferences {
             cv = fe.evaluate(cellE1);
             assertEquals(CellType.NUMERIC, cv.getCellType());
             assertEquals(43.0, cv.getNumberValue(), 0.0);
-
         }
     }
 }
diff --git a/src/testcases/org/apache/poi/util/TestLocaleUtil.java b/src/testcases/org/apache/poi/util/TestLocaleUtil.java
index 3a7c65a6a7..aeb79b973c 100644
--- a/src/testcases/org/apache/poi/util/TestLocaleUtil.java
+++ b/src/testcases/org/apache/poi/util/TestLocaleUtil.java
@@ -36,7 +36,7 @@ public class TestLocaleUtil {
     private static final Locale ja_JP = Locale.JAPAN;
     private static final TimeZone TOKYO = TimeZone.getTimeZone("Asia/Tokyo");
     private static final Calendar JAPAN_CALENDAR = Calendar.getInstance(TOKYO, ja_JP);
-    
+
     /**
      * Reset the Locale to the user default before the test so that it isn't influenced
      * by the LocaleUtil's state being changed by previous tests.
@@ -51,7 +51,7 @@ public class TestLocaleUtil {
         // http://www.codeaffine.com/2014/07/21/a-junit-rule-to-run-a-test-in-its-own-thread/
         LocaleUtil.setUserLocale(Locale.getDefault());
         LocaleUtil.setUserTimeZone(TimeZone.getDefault());
-        
+
         assumeFalse(ja_JP.equals(LocaleUtil.getUserLocale()));
         assumeFalse(TOKYO.equals(LocaleUtil.getUserTimeZone()));
     }
@@ -65,48 +65,48 @@ public class TestLocaleUtil {
         LocaleUtil.resetUserLocale();
         LocaleUtil.resetUserTimeZone();
     }
-    
+
     @Test
     @SuppressForbidden("implementation around default locales in POI")
     public void userLocale() {
         Locale DEFAULT_LOCALE = LocaleUtil.getUserLocale();
-        
+
         assertEquals(DEFAULT_LOCALE, LocaleUtil.getUserLocale());
         assertNotEquals(ja_JP, LocaleUtil.getUserLocale());
-        
+
         LocaleUtil.setUserLocale(ja_JP);
         assertEquals(ja_JP, LocaleUtil.getUserLocale());
-        
+
         LocaleUtil.resetUserLocale();
         assertEquals(DEFAULT_LOCALE, LocaleUtil.getUserLocale());
     }
-    
+
     @Test
     @SuppressForbidden("implementation around default locales in POI")
     public void userTimeZone() {
         TimeZone DEFAULT_TIME_ZONE = LocaleUtil.getUserTimeZone();
-        
+
         assertEquals(DEFAULT_TIME_ZONE, LocaleUtil.getUserTimeZone());
-        assertNotEquals(TOKYO, LocaleUtil.getUserLocale());
-        
+        assertNotEquals(TOKYO, LocaleUtil.getUserTimeZone());
+
         LocaleUtil.setUserTimeZone(TOKYO);
         assertEquals(TOKYO, LocaleUtil.getUserTimeZone());
     }
-    
+
     @Test
     @SuppressForbidden("implementation around default locales in POI")
     public void localeCalendar() {
         Locale DEFAULT_LOCALE = LocaleUtil.getUserLocale();
         TimeZone DEFAULT_TIME_ZONE = LocaleUtil.getUserTimeZone();
         Calendar DEFAULT_CALENDAR = LocaleUtil.getLocaleCalendar();
-        
+
         assertEquals(DEFAULT_LOCALE, LocaleUtil.getUserLocale());
         assertEquals(DEFAULT_TIME_ZONE, LocaleUtil.getUserTimeZone());
         assertCalendarEquals(DEFAULT_CALENDAR, LocaleUtil.getLocaleCalendar());
         assertNotEquals(ja_JP, LocaleUtil.getUserLocale());
         assertNotEquals(TOKYO, LocaleUtil.getUserTimeZone());
         assertCalendarNotEquals(JAPAN_CALENDAR, LocaleUtil.getLocaleCalendar());
-        
+
         LocaleUtil.setUserLocale(ja_JP);
         LocaleUtil.setUserTimeZone(TOKYO);
         assertCalendarEquals(JAPAN_CALENDAR, LocaleUtil.getLocaleCalendar());
@@ -115,7 +115,7 @@ public class TestLocaleUtil {
         //assertCalendarEquals(JAPAN_CALENDAR, LocaleUtil.getLocaleCalendar(2016, 00, 01));
         //assertCalendarEquals(JAPAN_CALENDAR, LocaleUtil.getLocaleCalendar(2016, 00, 01, 00, 00, 00));
     }
-    
+
     private static void assertCalendarNotEquals(Calendar expected, Calendar actual) {
         // FIXME: add more tests to compare calendars, ignoring whether the dates are equal
         assertNotEquals("time zone", expected.getTimeZone(), actual.getTimeZone());